search

open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.68k stars 884 forks source link

Control baggage propagation in automatic instrumentation libraries #3957

Open robotadam opened 5 months ago

robotadam commented 5 months ago

This is a followup to #3799 ; in that thread it was decided that context should always be propagated, but baggage is potentially a problem.

What are you trying to achieve?

Given an application that uses an instrumentation for an HTTP or RPC client library (e.g. for the Python requests library), any baggage set by the primary application will also be sent to a third party, even if it is only intended for internal use. This has the potential to leak internal private data. Opentelemetry may want to adopt a pattern that allows baggage to only be sent for some urls, similar to the OTEL_PYTHON_REQUESTS_EXCLUDED_URLS environment variable.

Additional context.

That this happens was added to the documentation in https://github.com/open-telemetry/opentelemetry.io/pull/3530 and there was also discussion in https://github.com/open-telemetry/opentelemetry-specification/issues/1633 .

In the system I'm working on we wouldn't be able both baggage and instrumentation libraries at the same time, because we need the client spans to properly debug and trace outgoing API calls, but we cannot allow baggage to be sent to these external APIs.

dyladan commented 5 months ago

@jsuereth this seems related to what we talked about in the past that the propagation API is not sufficient (believe lambda propagation was the context of that conversation). There are definitely some cases in which the instrumentation has knowledge required in order to make effective propagation decisions.

jmacd commented 4 months ago

I have added this to the Tuesday, April 9 Specification SIG agenda.

jack-berg commented 4 months ago

We should see if we can accomplish this purely through the W3C Baggage Propagator. The issue with having this be an instrumentation level configuration is that it will be difficult / impossible to enforce standard configuration for instrumentation which lives outside the OpenTelemetry org, which is something we expect more of as time goes by.

In theory, the W3C baggage propagator should be able to access the current client span from the context, and enable / disable propagation based on the contents, including the URL.

pellared commented 4 months ago

Is this not an issue for all propagators (not only W3C Baggage)?

carlosalberto commented 4 months ago

During the Spec call of April 9th there was initial agreement on the need to support this, hence marking this as valid.

danielgblanco commented 1 month ago

Would https://github.com/open-telemetry/opentelemetry-specification/issues/1633 be a more general issue to discuss this as something needed to be supported on the Propagators API to stop all context being propagated, not only baggage?