requests: Allow controlling whether propagation headers are sent

[kamalmarhubi]

Problem

We'd like to use opentelemetry to instrument requests for internal HTTP requests, but we also send requests to external parties. If we put user IDs in baggage, we'd prefer not to send those outside of our systems, but there is no way to do that with the current approach.

Desired solution

Provide configuration for which origins have propagation enabled. This could take the form of allow- and deny-lists, or a user-provided callback that returns a boolean based on the request.

Alternatives

Let the user instrument individual Session objects so that they can use an instrumented Session for internal calls, and an uninstrumented one for external calls. The API could be mirrored off of the flask instrumentation, which provides an instrument_app method.

So, perhaps an interface like:

class RequestsInstrumentor(BaseInstrumentor):

    # ... existing contents

    def instrument_session(session: requests.Session) -> None:
        ...

    def uninstrument_session(session: requests.Session) -> None:
        ...

This has a downside of not creating spans for those calls, when all we actually want is to prevent context propagation. It also forces cookie saving and connection pooling on users, making it hard to get the same behaviour as the bare requests.{get, post, ...} methods.

[owais]

Allowing to instrument individual sessions sounds like a useful feature.

Another way you can do this today without any changes in Otel is by implementing a custom propagator and using that instead of baggage. You can easily sub-class the baggage propagator and skip inserting PII data into headers. The propagators just receive carriers (usually headers) instead of request/response objects so in addition to having a custom propagator, you'd also want to inject an identifying header into the carrier.

For example you can inject something like _INTERNAL_STRIP_PII=true header into requests to external services. Your custom baggage propagator should then be able to detect the presence of this header and not inject PII data in that case.

[owais]

@open-telemetry/python-approvers It might be useful having something similar to "suppress_instrumentation" feature for propagation. Then users could prevent context from being injected into specific outgoing requests as:

with context.attach(context.set_value("suppress_propagation")):
     requests.post("http://untrusted_url", ...)

Each instrumentation would decide for itself if it wants to respect the presence of this context var or not.

[github-actions]

This issue was marked stale due to lack of activity. It will be closed in 30 days.

[kamalmarhubi]

@owais ah thanks for the custom propagator suggestion. For whatever reason I didn't think of that.

I do like the idea of something supported within the libraries themselves. The suppress_instrumentation thing looks like an internal detail though, and not something a library consumer should use. Is that right?

[owais]

I do like the idea of something supported within the libraries themselves. The suppress_instrumentation thing looks like an internal detail though, and not something a library consumer should use. Is that right?

Right, it's meant to be used only by the SDK and instrumentations.

[owais]

I think custom propagator is actually a pretty good solution for this use case. I don't think we need to address this in stock propagators or SDKs since it is very easy to implement in each project. Someone could even publish a opentelemetry-baggae-with-privacy-respecting-feature-and-other-goodness to pypi for others to use :D

As long as this project makes it possible for the community to build reliable solutions to such problems, I don't think we need to address them here.

Closing this but feel free to re-open if anyone thinks this is not viable for all cases.