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

B3 Propagator doesn't handle 64-bit TraceIDs #686

Closed
Aneurysm9 opened this issue Apr 30, 2020 · 5 comments · Fixed by #698
Closed

B3 Propagator doesn't handle 64-bit TraceIDs #686

Aneurysm9 opened this issue Apr 30, 2020 · 5 comments · Fixed by #698
Assignees

Comments

@Aneurysm9
Copy link
Member

The B3 spec allows for both 64-bit and 128-bit TraceIDs, but does not specify how to handle interop between systems that produce/expect different length IDs.

The Otel-Go B3 propagator uses core.TraceIDFromHex() which requires a hex-encoded TraceID be 32 hex digits.

Should we left-pad incoming TraceID values in the B3 propagator prior to decoding, or should we take another approach?

@jmacd
Copy link
Contributor

jmacd commented Apr 30, 2020

@MikeGoldsmith @codeboten would you comment? I believe you've studied this.

@lizthegrey
Copy link
Member

Please escalate this to the Spec SIG, as we need to consistently handle this across all of OTel.

@Aneurysm9
Copy link
Member Author

I actually think that the spec provides enough to consistently handle the issue, so I'll not open an issue against the spec repo, but I will add it to the agenda for the next Spec SIG meeting so we can discuss.

I think that between the trace API spec declaring that "[t]he OpenTelemetry SpanContext representation conforms to the w3c TraceContext specification" and the w3c interop guidance stating that "[s]horter identifiers SHOULD be left padded with zeros when converted to 16 bytes trace-id", the correct approach is clear.

After surveying the other OTel implementations, I found the following that exhibit the same issue when receiving 64-bit B3 TraceID values:

The following implementations left-pad the TraceID:

The following implementations do not validate the TraceID length and parse as received from hex to integers:

The opentelemetry-erlang implementation converts from hex to integer after validating the incoming hex is 16 or 32 digits.

The remaining implementations (PHP, Ruby, C++) do not appear to provide B3 propagators.

@SergeyKanzhelev
Copy link
Member

Some relevant discussion: w3c/trace-context#349 I agree, we need consistency on whether short Trace-ID should be accepted

@tedsuo
Copy link

tedsuo commented May 5, 2020

+1 for left padding with zeros

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants