Add proper length zero padding to hex strings sent on the wire#908
Add proper length zero padding to hex strings sent on the wire#908lzchen merged 2 commits intoopen-telemetry:masterfrom
Conversation
|
|
toumorokoshi
left a comment
toumorokoshi
left a comment
There was a problem hiding this comment.
once CI is fixed and a specific regression test is added, LGTM!
Also can you add entries to the appropriate changelogs?
| span_names = ("test1", "test2", "test3", "test4") | ||
| trace_id = 0x6E0C63257DE34C926F9EFCD03927272E | ||
| span_id = 0x34BF92DEEFC58C92 | ||
| trace_id = 0x0E6C63257DE34C926F9EFCD03927272E |
There was a problem hiding this comment.
IMO it would be better if we added a specific test to ensure that zero-padding works as intended. The current test makes no indicator that the zero-padded traceid is intentional to catch bugs.
A good test with a docstring per regression to educate and ensure future refactors are aware of the rationale for the test, is great.
There was a problem hiding this comment.
Thanks for the review, will do your suggestions tomorrow.
I hope I can get the CLA signed by my company though.
It's weird, I had the approved CLA for CNCF last month but this got switched to EasyCLA which requires a new signature.
There was a problem hiding this comment.
Thank you! I believe if you signed at least one of the CLAs, that's ok for us to merge. But I'll let the maintainers weigh in there.
ecourreges-orange
force-pushed
the
master
branch
from
5baee80 to
5c2a704
Compare
ecourreges-orange
force-pushed
the
master
branch
from
1b4fbcb to
d42fa63
Compare
lzchen
left a comment
lzchen
left a comment
There was a problem hiding this comment.
Please update the CHANGELOG
ecourreges-orange
force-pushed
the
master
branch
2 times, most recently
from
c0f7044 to
cc5df61
Compare
ecourreges-orange
force-pushed
the
master
branch
from
cc5df61 to
fc58032
Compare
…entId sent on the wire For compatibility with jaeger-collector Fixes open-telemetry#907
…emetry#910) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
3 participants
traceId length 32, parentId and spanId length 16
For compatibility with jaeger-collector
Fixes #907