-
Notifications
You must be signed in to change notification settings - Fork 1
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
support zipkin b3 propagation #5
Conversation
Box::new(opentelemetry_sdk::propagation::TraceContextPropagator::new()), | ||
Box::new(opentelemetry_zipkin::Propagator::new()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the Zipkin one go first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, Zipkin should go last. The composite propagator gets the latest propagation value.
https://github.com/open-telemetry/opentelemetry-rust/blob/2286378632d498ce4b2da109e5aa131b34ae1a8f/opentelemetry/src/propagation/composite.rs#L106
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that is confusing. Thanks for clarifying.
### What We'd like to accept B3 trace headers because Google Cloud sometimes messes with W3C headers. ### How The change has already been made in hasura/ndc-sdk-rs#5.
Connectors should support both W3C and Zipkin B3 propagation. B3 propagation headers priority should be higher because W3C traceparent header is intercepted by Cloud Run service.