Remove separation of TextMap and HttpHeader propagation in opentracingshim

[anuraaga]

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 propagators

https://github.com/open-telemetry/opentelemetry-java/blob/main/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/OpenTracingPropagators.java

[anuraaga]

@carlosalberto @malafeev Any thoughts on this? :)

[carlosalberto]

I think this means we can get rid of the distinction in shims and always use the same propagator for both formats, right?

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:

TEXT_MAP: JaegerPropagator with no encoding
HTTP_HEADERS: JaegerPropagator with encoding

(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 ;) )

[anuraaga]

I see - but JaegerPropagator with no encoding won't be compliant with OTel spec then. Is it an exception we'd make for Jaeger propagator?

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 OpenTracingPropagators concept to the user?

[reyang]

@carlosalberto could you take this issue and drive it forward? thanks!

[anuraaga]

@carlosalberto Just curious if you have any new thoughts on this

[yurishkuro]

hmm... https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#textmap-propagator says:

In order to increase compatibility, the key/value pairs MUST only consist of US-ASCII characters that make up valid HTTP header fields as per RFC 7230.

is % a valid character for HTTP header? I don't think so, but it is meant to be valid for the values. Specifically, W3C Trace Context says that tracestate is (https://www.w3.org/TR/trace-context/#value):

The value is an opaque string containing up to 256 printable ASCII [RFC0020] characters (i.e., the range 0x20 to 0x7E) except comma (,) and (=). Note that this also excludes tabs, newlines, carriage returns, etc.

So it seems OTEL's TextMap format cannot represent W3C tracestate.

[carlosalberto]

Ping @anuraaga

[anuraaga]

@carlosalberto Is there any AI for me?

[anuraaga]

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 JaegerPropagator with encoding one way or another. So I was waiting for you to do a deep dive on "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 OpenTracingPropagators concept to the user?" :P