Skip to content
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

Control baggage propagation in automatic instrumentation libraries #3957

Open
robotadam opened this issue Mar 22, 2024 · 6 comments
Open

Control baggage propagation in automatic instrumentation libraries #3957

robotadam opened this issue Mar 22, 2024 · 6 comments
Labels
spec:baggage Related to the specification/baggage directory triage:accepted:needs-sponsor Ready to be implemented, but does not yet have a specification sponsor

Comments

@robotadam
Copy link

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 open-telemetry/opentelemetry.io#3530 and there was also discussion in #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.

@robotadam robotadam added the spec:baggage Related to the specification/baggage directory label Mar 22, 2024
@trask trask unassigned jmacd Mar 26, 2024
@trask trask added triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted triage:deciding:tc-inbox Needs attention from the TC in order to move forward labels Mar 26, 2024
@dyladan
Copy link
Member

dyladan commented Mar 26, 2024

@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
Copy link
Contributor

jmacd commented Apr 5, 2024

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

@jmacd jmacd removed the triage:deciding:tc-inbox Needs attention from the TC in order to move forward label Apr 5, 2024
@jack-berg
Copy link
Member

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
Copy link
Member

pellared commented Apr 9, 2024

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

@carlosalberto carlosalberto added triage:accepted:needs-sponsor Ready to be implemented, but does not yet have a specification sponsor and removed triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted labels Apr 9, 2024
@carlosalberto
Copy link
Contributor

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

@danielgblanco
Copy link
Contributor

Would #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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:baggage Related to the specification/baggage directory triage:accepted:needs-sponsor Ready to be implemented, but does not yet have a specification sponsor
Projects
Status: Spec - Accepted
Development

No branches or pull requests

8 participants