search

open-telemetry / opentelemetry-java

OpenTelemetry Java SDK
https://opentelemetry.io
Apache License 2.0
1.9k stars 782 forks source link

CorrelationsContext does not work from the client #1530

Closed rvowles closed 3 years ago

rvowles commented 3 years ago

Is your feature request related to a problem? Please describe. I have come from OpenTracing and I would like to use OpenTelemetry in a complete replacement. I am advocating it for use in passing feature flag state from clients to servers so they propagate through the infrastructure (which we do for OpenTracing). However, with the Java implementation (instrumentation) this simply doesn't work.

This for example with the Jaeger libraries and OpenTracing works fine, baggage appears:

curl -v -H "jaeger-debug-id: 11" -H "uberctx-fhub.FEATURE_TITLE_TO_UPPERCASE: true" -H 'content-type: application/json' http://localhost:8099/todo/list

With OpenTelemetry replacing it, this doesn't work:

curl -v -H 'traceparent: 00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01' -H 'otcorrelations: FEATURE_TITLE_TO_UPPERCASE=true' -H 'content-type: application/json' http://localhost:8099/todo/list

I can see the span and parent and so forth all turning up, but the CorrelationContext is always empty.

OpenTelemetry.getCorrelationContextManager().getCurrentContext().getEntries()

Describe the solution you'd like Either this to work or some indication as to why it isn't working or I have it completely wrong and it will never work.

Describe alternatives you've considered I'm not aware apart from sticking with OpenTracing what other options there are.

Additional context Add any other context or screenshots about the feature request here.

jkwatson commented 3 years ago

Hi @rvowles . This definitely hasn't been implemented yet. I think it's currently held up by this spec issue: https://github.com/open-telemetry/opentelemetry-specification/issues/536

It definitely needs to be addressed before GA, so I will tag it as such.

Thanks for highlighting this! I had forgotten this was still not done.

rvowles commented 3 years ago

Thanks - the implication in that issue is they have settled on the header "Baggage" and are just waiting on the process to move its inexorable way through the w3c process.

jkwatson commented 3 years ago

Thanks - the implication in that issue is they have settled on the header "Baggage" and are just waiting on the process to move its inexorable way through the w3c process.

I think that's the case, but I haven't really been following it all that closely, TBH.

anuraaga commented 3 years ago

Hi @rvowles - are you using the OpenTelemetry agent? If so, I'm guessing this is a duplicate of https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/973 - indeed we have no support for correlation context in the agent yet.

jkwatson commented 3 years ago

This is now unblocked with updates to the spec: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/baggage/api.md

rvowles commented 3 years ago

That spec is a bit odd? The description says it MUST use otcorrelations as the header and then the example uses otbaggages ?

jkwatson commented 3 years ago

That spec is a bit odd? The description says it MUST use otcorrelations as the header and then the example uses otbaggages ?

I think there are still spec updates pending on this. If not, then logging an issue in the spec repo would be very helpful! Thanks!