Skip to content

Adding new optional post_injection_hook to RequestsInstrumentor #3657

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Added
- `opentelemetry-instrumentation-requests` Added support for post-injection-hook.
([#3657](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3657))

## Version 1.36.0/0.57b0 (2025-07-29)

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,31 @@ def response_hook(span, request_obj, response):

will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``.

Post Injection Hook
*******************
To customize headers after the current context is injected, you can provide a function that will be called with
both the headers and request object.

For example, to avoid propagating `tracestate`/`baggage` to non-allowed hosts while still creating a span:

.. code:: python

allowed_hosts = frozenset(["my-company.com", "my-cloud-provider.com"])

def post_injection_hook(headers, request):
# Parse the domain from the request URL
parsed_url = urlparse(request.url)
request_host = parsed_url.hostname

# Remove headers if the host is NOT in the allowed list
if request_host not in allowed_hosts:
headers.pop("tracestate", None)
headers.pop("baggage", None)

RequestsInstrumentor().instrument(
post_injection_hook=post_injection_hook
)

API
---
"""
Expand Down Expand Up @@ -156,6 +181,9 @@ def response_hook(span, request_obj, response):

_RequestHookT = Optional[Callable[[Span, PreparedRequest], None]]
_ResponseHookT = Optional[Callable[[Span, PreparedRequest, Response], None]]
_PostInjectionHookT = Optional[
Callable[[CaseInsensitiveDict[str], PreparedRequest], None]
]


def _set_http_status_code_attribute(
Expand Down Expand Up @@ -194,6 +222,7 @@ def _instrument(
response_hook: _ResponseHookT = None,
excluded_urls: ExcludeList | None = None,
sem_conv_opt_in_mode: _StabilityMode = _StabilityMode.DEFAULT,
post_injection_hook: _PostInjectionHookT = None,
):
"""Enables tracing of all requests calls that go through
:code:`requests.session.Session.request` (this includes
Expand Down Expand Up @@ -300,6 +329,9 @@ def get_or_create_headers():
headers = get_or_create_headers()
inject(headers)

if callable(post_injection_hook):
post_injection_hook(headers, request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it work for your use case to -- instead of passing in a general-purpose post-injection hook -- pass in an allowlist and inject or not based on that? Seems like the allowlist use case would be common enough to justify baking in explicit support (even at the api/spec level).

Copy link
Author

@eddawley eddawley Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that but I think users will differ in what and when to propagate these headers. For example, this user wants to avoid all headers for one host while only baggage for external domains.

I suspect it will be difficult to have a truly one-size fits all. I could even see a case where you base the decision on values in the baggage itself.

edit: also i noticed the request/response hooks started as separate single purpose options that were changed to hooks and I didn't want to repeat that


with suppress_http_instrumentation():
start_time = default_timer()
try:
Expand Down Expand Up @@ -450,6 +482,7 @@ def _instrument(self, **kwargs: Any):
duration_histogram_boundaries = kwargs.get(
"duration_histogram_boundaries"
)
post_injection_hook = kwargs.get("post_injection_hook")
meter = get_meter(
__name__,
__version__,
Expand Down Expand Up @@ -486,6 +519,7 @@ def _instrument(self, **kwargs: Any):
else parse_excluded_urls(excluded_urls)
),
sem_conv_opt_in_mode=semconv_opt_in_mode,
post_injection_hook=post_injection_hook,
)

def _uninstrument(self, **kwargs: Any):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,21 @@ def response_hook(span, request_obj, response):
self.assertEqual(span.name, "name set from hook")
self.assertEqual(span.attributes["response_hook_attr"], "value")

def test_post_injection_hook(self):
def post_injection_hook(headers, request):
headers.pop("traceparent", None)

RequestsInstrumentor().uninstrument()
RequestsInstrumentor().instrument(
post_injection_hook=post_injection_hook
)

self.perform_request(self.URL)

span = self.assert_span()
self.assertEqual(span.name, "GET")
self.assertNotIn("traceparent", httpretty.last_request().headers)

def test_excluded_urls_explicit(self):
url_404 = "http://mock/status/404"
httpretty.register_uri(
Expand Down