-
Notifications
You must be signed in to change notification settings - Fork 288
TraceID.String prefix with zeroes #533
TraceID.String prefix with zeroes #533
Conversation
The wire encoding of the TraceID uses zero prefixes (jaegertracing#472). The JaegerUI also uses zero prefixes (since 1.16). So it makes sense that the Stringer of the TraceID also does this. Resolves jaegertracing#532 Signed-off-by: Lukas Vogel <[email protected]>
@@ -344,9 +344,9 @@ func (c *SpanContext) isDebugIDContainerOnly() bool { | |||
|
|||
func (t TraceID) String() string { | |||
if t.High == 0 { | |||
return fmt.Sprintf("%x", t.Low) | |||
return fmt.Sprintf("%016x", t.Low) |
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.
I would also do it in L382 for span ID
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.
Done.
Should I squash? Or will you do that on pre-merge?
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.
we squash on merge
Also test parsing back works. Signed-off-by: Lukas Vogel <[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!
please run all the tests, there are CI failures. |
how do I run this thing locally? Signed-off-by: Lukas Vogel <[email protected]>
Sorry somehow couldn't get this to run locally. a go.mod would really make this easier :P |
Signed-off-by: Lukas Vogel <[email protected]>
Alright, seems to pass CI now. I just had to check the project out in the correct local folder, then I could get it to work locally. Thanks. |
Codecov Report
@@ Coverage Diff @@
## master #533 +/- ##
=======================================
Coverage 89.28% 89.28%
=======================================
Files 61 61
Lines 3919 3919
=======================================
Hits 3499 3499
Misses 294 294
Partials 126 126
Continue to review full report at Codecov.
|
…gator Based on the openzipkin/b3-propagation specification, the SpanID and ParentSpanID are to be encoded as 16 lower-hex characters. jaegertracing#533 fixed this for TraceID, and fixed the SpanID.String() method, but did not update the Zipkin Propagator SpanID encoding. This fix uses SpanID.String() to ensure proper encoding of the SpanID and ParentSpandID within the Zipkin Propagator. Signed-off-by: Nathan Bowler <[email protected]>
Based on the openzipkin/b3-propagation specification, the SpanID and ParentSpanID are to be encoded as 16 lower-hex characters. #533 fixed this for TraceID, and fixed the SpanID.String() method, but did not update the Zipkin Propagator SpanID encoding. This fix uses SpanID.String() to ensure proper encoding of the SpanID and ParentSpandID within the Zipkin Propagator. Signed-off-by: Nathan Bowler <[email protected]>
The wire encoding of the TraceID uses zero prefixes (#472).
The JaegerUI also uses zero prefixes (since 1.16).
So it makes sense that the Stringer of the TraceID also does this.
Resolves #532
Signed-off-by: Lukas Vogel [email protected]