-
Notifications
You must be signed in to change notification settings - Fork 288
Update zipkin propagation logic to support 128bit traceIDs #373
Conversation
Signed-off-by: Douglas Reid <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR! Could you please add a unit test that validates the 128bit logic?
Codecov Report
@@ Coverage Diff @@
## master #373 +/- ##
==========================================
+ Coverage 87.22% 87.29% +0.06%
==========================================
Files 54 54
Lines 3006 3006
==========================================
+ Hits 2622 2624 +2
+ Misses 272 271 -1
+ Partials 112 111 -1
Continue to review full report at Codecov.
|
Signed-off-by: Douglas Reid <[email protected]>
@yurishkuro Sure. Added some tests. Please let me know if more is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great, thank you!
oh, did you miss the sign-off on the last commit? |
zipkin/propagation_test.go
Outdated
assert.EqualValues(t, jaeger.TraceID{High: high, Low: low}, spanCtx.TraceID()) | ||
|
||
hdr := opentracing.TextMapCarrier{} | ||
propagator.Inject(spanCtx, hdr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of propagator.Inject
is not checked (from errcheck
)
Signed-off-by: Douglas Reid <[email protected]>
5ea0b01
to
527f301
Compare
@yurishkuro probably. updated commit and force pushed. |
🎉 |
Signed-off-by: Douglas Reid [email protected]
Which problem is this PR solving?
Short description of the changes