OpenTracing Shim: Allow invalid but sampled SpanContext to be returned. #3471
Conversation
This is done to support the jaeger-debug-id functionality, which allows invalid SpanContext with debug information to be propagated.
|
Ping @yurishkuro |
There was a problem hiding this comment.
I would like to see a reference implementation first. This could mean much larger changes to the SDK implementation, eg handling parent context that is sampled but doesn't have a trace id. Also, the current SDK span context does not have a place to store debug-id. The ticket mentions trace state, but it would cause the value to propagate.
|
I do think we should implement & support this. This was quite a common debugging workflow at Uber with Jaeger SDK. But my objection was to starting with changing the spec without having an implementation and full understanding what else needs to change in the SDK. |
|
@yurishkuro So https://github.com/open-telemetry/opentelemetry-java/pull/5380/files has the changes we would need for Java, plus some simulation of how this could be done. I'm guessing you want a little more 'complete'? cc @ChenX1993 |
|
Left some comments in the PR - would like it to be a bit cleaner, but looks promising. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
@yurishkuro While we iterate on the details of the related PR, should we get this PR merged? Anything else you'd like me to clean up? |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
|
@MrAlias does the current language ( |
|
Merging as we use the suggested "null or empty" statement as recommended. |
…d. (open-telemetry#3471) This is done to support the `jaeger-debug-id` functionality, which allows invalid SpanContext with debug information to be propagated. This came up through an issue in Java: open-telemetry/opentelemetry-java#5339
This is done to support the
jaeger-debug-idfunctionality, which allows invalid SpanContext with debug information to be propagated.This came up through an issue in Java: open-telemetry/opentelemetry-java#5339
@yurishkuro please review ;)