[distributed_headers] Accept signed integers as IDs. #530
Conversation
This is what dd-trace-js sends over the wire: https://github.com/DataDog/dd-trace-js/blob/aa607860ea6bb57f2f69fa7ee72ed3ce25abfbf6/src/opentracing/propagation/text_map.js#L14-L15 It’s also what has been patched in e.g. dd-trace-go: DataDog/dd-trace-go#326
|
|
||
| def id(header) | ||
| value = header(header).to_i | ||
| return if value == 0 || value >= Span::MAX_ID |
There was a problem hiding this comment.
I assumed that 0 is truly invalid as an ID, but definitely don’t know for sure.
There was a problem hiding this comment.
Btw, is there a trace specification available?
There was a problem hiding this comment.
At this moment documentation is not available. But since this issue surfaced its something on our radar.
|
Thanks for the contribution! Looks good overall, however I'm worried if the process of converting to unsigned is compatible with how Go is performing it. Would it be possible to add another test for the same number as Go implementation tests - https://github.com/DataDog/dd-trace-go/pull/326/files#diff-58eb3484bc934d38b7496e8a7363200fR47 ? BTW The tests are not passing because of CircleCI security where Caches cannot be persisted by forks - our CI is not working properly for some PRs. We have PR in progress to fix that #526 |
pawelchcki
added
core
alloy
force-pushed
the
accept-negative-distributed-tracing-ids
branch
from
198704f
to
142192f
Compare
|
Sure thing 👍 Updated with the test from the Go PR. |
alloy
force-pushed
the
accept-negative-distributed-tracing-ids
branch
from
142192f
to
49b882d
Compare
|
@alloy There's a CI failure related to a Rubocop violation; if we can fix that and see the tests pass, I think we can merge this for 0.15.0. |
|
@delner Done. Thanks for the poke 👍 |
This is what dd-trace-js sends over the wire:
https://github.com/DataDog/dd-trace-js/blob/aa607860ea6bb57f2f69fa7ee72ed3ce25abfbf6/src/opentracing/propagation/text_map.js#L14-L15
It’s also what has been patched in e.g. dd-trace-go:
DataDog/dd-trace-go#326