Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate the usages of SpanContext.create with invalid ids, but useful flags and trace state. #2751

Closed
jkwatson opened this issue Feb 8, 2021 · 2 comments · Fixed by #2803
Assignees
Labels
priority:p1 Critical issues and bugs. Highest priority; breaking API changes.
Milestone

Comments

@jkwatson
Copy link
Contributor

jkwatson commented Feb 8, 2021

When implementing #2728 , we uncovered a strange use of the SpanContext.create where tests were passing in invalid span/trace ids, but useful trace flags and trace state. This may just be a testing usage at the moment, but before removing it, we need to research whether it is a valid use-case, or just a testing shortcut.

The outcome of this story should either be
a) document this use and why, potentially adding an additional creational method to support it
b) remove this usage from the tests and make the SpanContext.create return a fully INVALID SpanContext if the span id and trace id are both invalid.

@jkwatson jkwatson added the priority:p1 Critical issues and bugs. Highest priority; breaking API changes. label Feb 8, 2021
@jkwatson jkwatson added this to the 1.0 milestone Feb 8, 2021
@Oberon00
Copy link
Member

Oberon00 commented Feb 8, 2021

I can say that we use this at Dynatrace where we have a custom extractor that works over channels that only allow for annotation of messages with a few bytes (not enough for full trace + span ID). In these cases, we extract a trace+span ID of zero but a trace state with this shorter ID.

There is a corresponding spec issue: open-telemetry/opentelemetry-specification#753

To make this actually useful, we have a patch applied to the SDK span builder to not discard parents with invalid trace and/or span ID but to still use every bit (trace ID only, span ID only, tracestate, trace flags) that is valid in the parent.

@jkwatson
Copy link
Contributor Author

So, it sounds like there definitely is a use-case for this. I think we should just document that if you pass in invalid trace id or span id, that they'll be replaced with purely invalid (i.e. all 0s) ids. I think that will make clear what we do today and preserve the current behavior.

If, in the future, we need a way to request that you get a different result (i.e. not only all zeroes, but the pure getInvalid() version, we can add a method to support that use-case.

Sound ok?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p1 Critical issues and bugs. Highest priority; breaking API changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants