diff --git a/CHANGELOG.md b/CHANGELOG.md index b339f8a21e..7924d9211e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726)) - `opentelemetry-instrumentation-fastapi` Add dependency support for fastapi-slim ([#2702](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2702)) +- `opentelemetry-instrumentation-urllib3` improve request_hook, replacing `headers` and `body` parameters with a single `request_info: RequestInfo` parameter that now contains the `method` and `url` ([#2711](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2711)) ### Fixed diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index a05084725d..4bcd0816fd 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -47,14 +47,19 @@ def strip_query_params(url: str) -> str: .. code:: python - # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool - def request_hook(span, request): - pass - - # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool - # `response` is an instance of urllib3.response.HTTPResponse - def response_hook(span, request, response): - pass + def request_hook( + span: Span, + pool: urllib3.connectionpool.HTTPConnectionPool, + request_info: RequestInfo, + ) -> Any: + ... + + def response_hook( + span: Span, + pool: urllib3.connectionpool.HTTPConnectionPool, + response: urllib3.response.HTTPResponse, + ) -> Any: + ... URLLib3Instrumentor().instrument( request_hook=request_hook, response_hook=response_hook @@ -81,6 +86,7 @@ def response_hook(span, request, response): import collections.abc import io import typing +from dataclasses import dataclass from timeit import default_timer from typing import Collection @@ -135,14 +141,29 @@ def response_hook(span, request, response): _excluded_urls_from_env = get_excluded_urls("URLLIB3") + +@dataclass +class RequestInfo: + """Arguments that were passed to the ``urlopen()`` call.""" + + __slots__ = ("method", "url", "headers", "body") + + # The type annotations here come from ``HTTPConnectionPool.urlopen()``. + method: str + url: str + headers: typing.Optional[typing.Mapping[str, str]] + body: typing.Union[ + bytes, typing.IO[typing.Any], typing.Iterable[bytes], str, None + ] + + _UrlFilterT = typing.Optional[typing.Callable[[str], str]] _RequestHookT = typing.Optional[ typing.Callable[ [ Span, urllib3.connectionpool.HTTPConnectionPool, - typing.Dict, - typing.Optional[str], + RequestInfo, ], None, ] @@ -317,7 +338,16 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): span_name, kind=SpanKind.CLIENT, attributes=span_attributes ) as span, set_ip_on_next_http_connection(span): if callable(request_hook): - request_hook(span, instance, headers, body) + request_hook( + span, + instance, + RequestInfo( + method=method, + url=url, + headers=headers, + body=body, + ), + ) inject(headers) # TODO: add error handling to also set exception `error.type` in new semconv with suppress_http_instrumentation(): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 5ad4e40ca3..0e6037ed11 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -27,7 +27,10 @@ _HTTPStabilityMode, _OpenTelemetrySemanticConventionStability, ) -from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor +from opentelemetry.instrumentation.urllib3 import ( + RequestInfo, + URLLib3Instrumentor, +) from opentelemetry.instrumentation.utils import ( suppress_http_instrumentation, suppress_instrumentation, @@ -42,6 +45,7 @@ from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase +from opentelemetry.trace import Span from opentelemetry.util.http import get_excluded_urls # pylint: disable=too-many-public-methods @@ -521,10 +525,10 @@ def test_credential_removal(self): self.assert_success_span(response, self.HTTP_URL) def test_hooks(self): - def request_hook(span, request, body, headers): + def request_hook(span, pool, request_info): span.update_name("name set from hook") - def response_hook(span, request, response): + def response_hook(span, pool, response): span.set_attribute("response_hook_attr", "value") URLLib3Instrumentor().uninstrument() @@ -541,11 +545,17 @@ def response_hook(span, request, response): self.assertEqual(span.attributes["response_hook_attr"], "value") def test_request_hook_params(self): - def request_hook(span, request, headers, body): + def request_hook( + span: Span, + _pool: urllib3.connectionpool.ConnectionPool, + request_info: RequestInfo, + ) -> None: + span.set_attribute("request_hook_method", request_info.method) + span.set_attribute("request_hook_url", request_info.url) span.set_attribute( - "request_hook_headers", json.dumps(dict(headers)) + "request_hook_headers", json.dumps(dict(request_info.headers)) ) - span.set_attribute("request_hook_body", body) + span.set_attribute("request_hook_body", request_info.body) URLLib3Instrumentor().uninstrument() URLLib3Instrumentor().instrument( @@ -564,6 +574,10 @@ def request_hook(span, request, headers, body): span = self.assert_span() + self.assertEqual(span.attributes["request_hook_method"], "POST") + self.assertEqual( + span.attributes["request_hook_url"], "http://mock/status/200" + ) self.assertIn("request_hook_headers", span.attributes) self.assertEqual( span.attributes["request_hook_headers"], json.dumps(headers) @@ -572,8 +586,12 @@ def request_hook(span, request, headers, body): self.assertEqual(span.attributes["request_hook_body"], body) def test_request_positional_body(self): - def request_hook(span, request, headers, body): - span.set_attribute("request_hook_body", body) + def request_hook( + span: Span, + _pool: urllib3.connectionpool.ConnectionPool, + request_info: RequestInfo, + ) -> None: + span.set_attribute("request_hook_body", request_info.body) URLLib3Instrumentor().uninstrument() URLLib3Instrumentor().instrument(