From 24f4b7e46694795e42ae7da67d6ee1c76c0a2b1d Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 24 Aug 2023 20:40:14 -0400 Subject: [PATCH 1/4] fix: sync response hooks being used on httpx.AsyncClient --- .../instrumentation/httpx/__init__.py | 41 ++++++++++++++----- .../tests/test_httpx_integration.py | 36 ++++++++++++++++ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index bb40adbc26..1565588abc 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -131,7 +131,16 @@ def response_hook(span, request, response): # status_code, headers, stream, extensions = response pass - HTTPXClientInstrumentor().instrument(request_hook=request_hook, response_hook=response_hook) + async def async_request_hook(span, request): + # method, url, headers, stream, extensions = request + pass + + async def async_response_hook(span, request, response): + # method, url, headers, stream, extensions = request + # status_code, headers, stream, extensions = response + pass + + HTTPXClientInstrumentor().instrument(request_hook=request_hook, response_hook=response_hook, async_request_hook=async_request_hook, async_response_hook=async_response_hook) Or if you are using the transport classes directly: @@ -376,8 +385,8 @@ def __init__( self, transport: httpx.AsyncBaseTransport, tracer_provider: typing.Optional[TracerProvider] = None, - request_hook: typing.Optional[RequestHook] = None, - response_hook: typing.Optional[ResponseHook] = None, + request_hook: typing.Optional[AsyncRequestHook] = None, + response_hook: typing.Optional[AsyncResponseHook] = None, ): self._transport = transport self._tracer = get_tracer( @@ -509,21 +518,27 @@ def _instrument(self, **kwargs): Args: **kwargs: Optional arguments ``tracer_provider``: a TracerProvider, defaults to global - ``request_hook``: A hook that receives the span and request that is called - right after the span is created - ``response_hook``: A hook that receives the span, request, and response - that is called right before the span ends + ``request_hook``: A ``httpx.Client`` hook that receives the span and request + that is called right after the span is created + ``response_hook``: A ``httpx.Client`` hook that receives the span, request, + and response that is called right before the span ends + ``async_request_hook``: Async ``request_hook`` for ``httpx.AsyncClient`` + ``async_response_hook``: Async``response_hook`` for ``httpx.AsyncClient`` """ self._original_client = httpx.Client self._original_async_client = httpx.AsyncClient request_hook = kwargs.get("request_hook") response_hook = kwargs.get("response_hook") + async_request_hook = kwargs.get("async_request_hook", request_hook) + async_response_hook = kwargs.get("async_response_hook", response_hook) if callable(request_hook): _InstrumentedClient._request_hook = request_hook - _InstrumentedAsyncClient._request_hook = request_hook + if callable(async_request_hook): + _InstrumentedAsyncClient._request_hook = async_request_hook if callable(response_hook): _InstrumentedClient._response_hook = response_hook - _InstrumentedAsyncClient._response_hook = response_hook + if callable(async_response_hook): + _InstrumentedAsyncClient._response_hook = async_response_hook tracer_provider = kwargs.get("tracer_provider") _InstrumentedClient._tracer_provider = tracer_provider _InstrumentedAsyncClient._tracer_provider = tracer_provider @@ -544,8 +559,12 @@ def _uninstrument(self, **kwargs): def instrument_client( client: typing.Union[httpx.Client, httpx.AsyncClient], tracer_provider: TracerProvider = None, - request_hook: typing.Optional[RequestHook] = None, - response_hook: typing.Optional[ResponseHook] = None, + request_hook: typing.Union[ + typing.Optional[RequestHook], typing.Optional[AsyncRequestHook] + ] = None, + response_hook: typing.Union[ + typing.Optional[ResponseHook], typing.Optional[AsyncResponseHook] + ] = None, ) -> None: """Instrument httpx Client or AsyncClient diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index daddaad306..db35ab2639 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -421,6 +421,28 @@ def test_response_hook(self): ) HTTPXClientInstrumentor().uninstrument() + def test_response_hook_sync_async_kwargs(self): + HTTPXClientInstrumentor().instrument( + tracer_provider=self.tracer_provider, + response_hook=_response_hook, + async_response_hook=_async_response_hook, + ) + client = self.create_client() + result = self.perform_request(self.URL, client=client) + + self.assertEqual(result.text, "Hello!") + span = self.assert_span() + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 200, + HTTP_RESPONSE_BODY: "Hello!", + }, + ) + HTTPXClientInstrumentor().uninstrument() + def test_request_hook(self): HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, @@ -434,6 +456,20 @@ def test_request_hook(self): self.assertEqual(span.name, "GET" + self.URL) HTTPXClientInstrumentor().uninstrument() + def test_request_hook_sync_async_kwargs(self): + HTTPXClientInstrumentor().instrument( + tracer_provider=self.tracer_provider, + request_hook=_request_hook, + async_request_hook=_async_request_hook, + ) + client = self.create_client() + result = self.perform_request(self.URL, client=client) + + self.assertEqual(result.text, "Hello!") + span = self.assert_span() + self.assertEqual(span.name, "GET" + self.URL) + HTTPXClientInstrumentor().uninstrument() + def test_request_hook_no_span_update(self): HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, From 44d2c7634baedea0b7e7f074b4eb06a9f642259e Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 24 Aug 2023 20:52:31 -0400 Subject: [PATCH 2/4] docs: add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d56bcc43b..af35dfa0ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1889](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1889)) - Fixed union typing error not compatible with Python 3.7 introduced in `opentelemetry-util-http`, fix tests introduced by patch related to sanitize method for wsgi ([#1913](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1913)) +- `opentelemetry-instrumentation-httpx` Fix mixing async and non async hooks + ([#1920](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1920)) ### Added From 0b16fe7d0dacb16c93f7e22f69d19d64f58c7189 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 24 Aug 2023 21:09:15 -0400 Subject: [PATCH 3/4] docs: improved docs a bit more --- .../README.rst | 34 +++++++++++++++++-- .../instrumentation/httpx/__init__.py | 25 ++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/README.rst b/instrumentation/opentelemetry-instrumentation-httpx/README.rst index 1e03eb128e..7916c185cf 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/README.rst +++ b/instrumentation/opentelemetry-instrumentation-httpx/README.rst @@ -136,7 +136,21 @@ The hooks can be configured as follows: # status_code, headers, stream, extensions = response pass - HTTPXClientInstrumentor().instrument(request_hook=request_hook, response_hook=response_hook) + async def async_request_hook(span, request): + # method, url, headers, stream, extensions = request + pass + + async def async_response_hook(span, request, response): + # method, url, headers, stream, extensions = request + # status_code, headers, stream, extensions = response + pass + + HTTPXClientInstrumentor().instrument( + request_hook=request_hook, + response_hook=response_hook, + async_request_hook=async_request_hook, + async_response_hook=async_response_hook + ) Or if you are using the transport classes directly: @@ -144,7 +158,7 @@ Or if you are using the transport classes directly: .. code-block:: python - from opentelemetry.instrumentation.httpx import SyncOpenTelemetryTransport + from opentelemetry.instrumentation.httpx import SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport def request_hook(span, request): # method, url, headers, stream, extensions = request @@ -155,6 +169,15 @@ Or if you are using the transport classes directly: # status_code, headers, stream, extensions = response pass + async def async_request_hook(span, request): + # method, url, headers, stream, extensions = request + pass + + async def async_response_hook(span, request, response): + # method, url, headers, stream, extensions = request + # status_code, headers, stream, extensions = response + pass + transport = httpx.HTTPTransport() telemetry_transport = SyncOpenTelemetryTransport( transport, @@ -162,6 +185,13 @@ Or if you are using the transport classes directly: response_hook=response_hook ) + async_transport = httpx.AsyncHTTPTransport() + telemetry_transport = AsyncOpenTelemetryTransport( + async_transport, + request_hook=async_request_hook, + response_hook=async_response_hook + ) + References ---------- diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 1565588abc..2057db89fb 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -140,7 +140,12 @@ async def async_response_hook(span, request, response): # status_code, headers, stream, extensions = response pass - HTTPXClientInstrumentor().instrument(request_hook=request_hook, response_hook=response_hook, async_request_hook=async_request_hook, async_response_hook=async_response_hook) + HTTPXClientInstrumentor().instrument( + request_hook=request_hook, + response_hook=response_hook, + async_request_hook=async_request_hook, + async_response_hook=async_response_hook + ) Or if you are using the transport classes directly: @@ -148,7 +153,7 @@ async def async_response_hook(span, request, response): .. code-block:: python - from opentelemetry.instrumentation.httpx import SyncOpenTelemetryTransport + from opentelemetry.instrumentation.httpx import SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport def request_hook(span, request): # method, url, headers, stream, extensions = request @@ -159,6 +164,15 @@ def response_hook(span, request, response): # status_code, headers, stream, extensions = response pass + async def async_request_hook(span, request): + # method, url, headers, stream, extensions = request + pass + + async def async_response_hook(span, request, response): + # method, url, headers, stream, extensions = request + # status_code, headers, stream, extensions = response + pass + transport = httpx.HTTPTransport() telemetry_transport = SyncOpenTelemetryTransport( transport, @@ -166,6 +180,13 @@ def response_hook(span, request, response): response_hook=response_hook ) + async_transport = httpx.AsyncHTTPTransport() + telemetry_transport = AsyncOpenTelemetryTransport( + async_transport, + request_hook=async_request_hook, + response_hook=async_response_hook + ) + API --- """ From 0c6cc9139818513506ea1b59e8b3964958f869ef Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Thu, 24 Aug 2023 21:10:16 -0400 Subject: [PATCH 4/4] docs: fix variable name --- instrumentation/opentelemetry-instrumentation-httpx/README.rst | 2 +- .../src/opentelemetry/instrumentation/httpx/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/README.rst b/instrumentation/opentelemetry-instrumentation-httpx/README.rst index 7916c185cf..cc465dd615 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/README.rst +++ b/instrumentation/opentelemetry-instrumentation-httpx/README.rst @@ -186,7 +186,7 @@ Or if you are using the transport classes directly: ) async_transport = httpx.AsyncHTTPTransport() - telemetry_transport = AsyncOpenTelemetryTransport( + async_telemetry_transport = AsyncOpenTelemetryTransport( async_transport, request_hook=async_request_hook, response_hook=async_response_hook diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 2057db89fb..9a8c931bdb 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -181,7 +181,7 @@ async def async_response_hook(span, request, response): ) async_transport = httpx.AsyncHTTPTransport() - telemetry_transport = AsyncOpenTelemetryTransport( + async_telemetry_transport = AsyncOpenTelemetryTransport( async_transport, request_hook=async_request_hook, response_hook=async_response_hook