-
Notifications
You must be signed in to change notification settings - Fork 888
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
Remove separation of TextMap and HttpHeader propagation in opentracingshim #1608
Comments
@carlosalberto @malafeev Any thoughts on this? :) |
We can't ;) i.e. I know that Jaeger used to use a Jaeger propagator with encoding and one format, and one for the other one - something like:
(We still need to add support for such encoding flag in the current Jaeger propagator, btw). I do agree that most of the time we will be using the same Propagator though (unless you are using Jaeger ;) ) |
I see - but Also, is it something we can leave as an implementation detail? If OpenTelemetry propagator is configured to JaegerPropagator then use that behavior, without having to introduce the |
@carlosalberto could you take this issue and drive it forward? thanks! |
@carlosalberto Just curious if you have any new thoughts on this |
is
So it seems OTEL's TextMap format cannot represent W3C tracestate. |
Ping @anuraaga |
@carlosalberto Is there any AI for me? |
To clarify, @yurishkuro brings up a good point that our TextMap may have issues other than Jaeger. But anyways, I was assuming that we will need support for a |
Looking at the opentracing definition of TextMap vs HttpHeader, the latter is a subset of TextMap where key/value pairs are all compatible with HTTP. Lucky for us, OpenTelemetry propagators all have that restriction
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#textmap-propagator
I think this means we can get rid of the distinction in shims and always use the same propagator for both formats, right? It would drastically simplify shims I think, we've currently had to introduce an entirely new concept,
OpenTracingPropagators
to support the separation of propagatorshttps://github.com/open-telemetry/opentelemetry-java/blob/main/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/OpenTracingPropagators.java
The text was updated successfully, but these errors were encountered: