From 62d673726b9100e91616ed352820668d7f96ae7b Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sun, 25 Aug 2024 12:39:59 +0200 Subject: [PATCH 1/5] httpx: fix handling of async hooks Don't default to sync hooks for async hooks, then check that they are actually async. Fixes #2734 --- .../instrumentation/httpx/__init__.py | 9 ++-- .../tests/test_httpx_integration.py | 45 ++++++++++++++++++- 2 files changed, 48 insertions(+), 6 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 43db6c3a82..3a7ad47c5c 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -192,6 +192,7 @@ async def async_response_hook(span, request, response): """ import logging import typing +from asyncio import iscoroutinefunction from types import TracebackType import httpx @@ -731,15 +732,15 @@ def _instrument(self, **kwargs): 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) + async_request_hook = kwargs.get("async_request_hook") + async_response_hook = kwargs.get("async_response_hook") if callable(request_hook): _InstrumentedClient._request_hook = request_hook - if callable(async_request_hook): + if callable(async_request_hook) and iscoroutinefunction(async_request_hook): _InstrumentedAsyncClient._request_hook = async_request_hook if callable(response_hook): _InstrumentedClient._response_hook = response_hook - if callable(async_response_hook): + if callable(async_response_hook) and iscoroutinefunction(async_response_hook): _InstrumentedAsyncClient._response_hook = async_response_hook tracer_provider = kwargs.get("tracer_provider") _InstrumentedClient._tracer_provider = tracer_provider diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 011b5e57d2..7aa74467b2 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -780,9 +780,13 @@ def test_custom_tracer_provider(self): HTTPXClientInstrumentor().uninstrument() def test_response_hook(self): + response_hook_key = "async_response_hook" if asyncio.iscoroutinefunction(self.response_hook) else "response_hook" + response_hook_kwargs = { + response_hook_key: self.response_hook + } HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, - response_hook=self.response_hook, + **response_hook_kwargs ) client = self.create_client() result = self.perform_request(self.URL, client=client) @@ -823,9 +827,13 @@ def test_response_hook_sync_async_kwargs(self): HTTPXClientInstrumentor().uninstrument() def test_request_hook(self): + request_hook_key = "async_request_hook" if asyncio.iscoroutinefunction(self.request_hook) else "request_hook" + request_hook_kwargs = { + request_hook_key: self.request_hook + } HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, - request_hook=self.request_hook, + **request_hook_kwargs, ) client = self.create_client() result = self.perform_request(self.URL, client=client) @@ -1214,3 +1222,36 @@ def test_basic_multiple(self): self.perform_request(self.URL, client=self.client) self.perform_request(self.URL, client=self.client2) self.assert_span(num_spans=2) + + def test_async_response_hook_does_nothing_if_not_coroutine(self): + HTTPXClientInstrumentor().instrument( + tracer_provider=self.tracer_provider, + async_response_hook=_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( + dict(span.attributes), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + ) + HTTPXClientInstrumentor().uninstrument() + + def test_async_request_hook_does_nothing_if_not_coroutine(self): + HTTPXClientInstrumentor().instrument( + tracer_provider=self.tracer_provider, + async_request_hook=_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") + HTTPXClientInstrumentor().uninstrument() From e9f6cafb38517a78a38a7dd80308e0d7a79ab099 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sun, 25 Aug 2024 12:44:44 +0200 Subject: [PATCH 2/5] Add CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5ce56423a..c375b01c96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Fixed +- `opentelemetry-instrumentation-httpx` fix handling of async hooks + ([2823](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2823)) - `opentelemetry-instrumentation-system-metrics` fix `process.runtime.cpu.utilization` values to be shown in range of 0 to 1 ([2812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2812)) - `opentelemetry-instrumentation-fastapi` fix `fastapi` auto-instrumentation by removing `fastapi-slim` support, `fastapi-slim` itself is discontinued from maintainers From e4b0ea4cfd6b92ce6593872b384247cf2c4a7a8f Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sun, 25 Aug 2024 15:37:24 +0200 Subject: [PATCH 3/5] fixup lint --- .../instrumentation/httpx/__init__.py | 8 +++++-- .../tests/test_httpx_integration.py | 22 +++++++++++-------- 2 files changed, 19 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 3a7ad47c5c..15ee59a183 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -736,11 +736,15 @@ def _instrument(self, **kwargs): async_response_hook = kwargs.get("async_response_hook") if callable(request_hook): _InstrumentedClient._request_hook = request_hook - if callable(async_request_hook) and iscoroutinefunction(async_request_hook): + if callable(async_request_hook) and iscoroutinefunction( + async_request_hook + ): _InstrumentedAsyncClient._request_hook = async_request_hook if callable(response_hook): _InstrumentedClient._response_hook = response_hook - if callable(async_response_hook) and iscoroutinefunction(async_response_hook): + if callable(async_response_hook) and iscoroutinefunction( + async_response_hook + ): _InstrumentedAsyncClient._response_hook = async_response_hook tracer_provider = kwargs.get("tracer_provider") _InstrumentedClient._tracer_provider = tracer_provider diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 7aa74467b2..78938eb337 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -780,13 +780,15 @@ def test_custom_tracer_provider(self): HTTPXClientInstrumentor().uninstrument() def test_response_hook(self): - response_hook_key = "async_response_hook" if asyncio.iscoroutinefunction(self.response_hook) else "response_hook" - response_hook_kwargs = { - response_hook_key: self.response_hook - } + response_hook_key = ( + "async_response_hook" + if asyncio.iscoroutinefunction(self.response_hook) + else "response_hook" + ) + response_hook_kwargs = {response_hook_key: self.response_hook} HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, - **response_hook_kwargs + **response_hook_kwargs, ) client = self.create_client() result = self.perform_request(self.URL, client=client) @@ -827,10 +829,12 @@ def test_response_hook_sync_async_kwargs(self): HTTPXClientInstrumentor().uninstrument() def test_request_hook(self): - request_hook_key = "async_request_hook" if asyncio.iscoroutinefunction(self.request_hook) else "request_hook" - request_hook_kwargs = { - request_hook_key: self.request_hook - } + request_hook_key = ( + "async_request_hook" + if asyncio.iscoroutinefunction(self.request_hook) + else "request_hook" + ) + request_hook_kwargs = {request_hook_key: self.request_hook} HTTPXClientInstrumentor().instrument( tracer_provider=self.tracer_provider, **request_hook_kwargs, From 9f3da93c276c8e2fd26d41bc3cb52605b7d22f54 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 26 Aug 2024 13:58:51 +0200 Subject: [PATCH 4/5] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c375b01c96..b98fdc565d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Fixed - `opentelemetry-instrumentation-httpx` fix handling of async hooks - ([2823](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2823)) + ([#2823](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2823)) - `opentelemetry-instrumentation-system-metrics` fix `process.runtime.cpu.utilization` values to be shown in range of 0 to 1 ([2812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2812)) - `opentelemetry-instrumentation-fastapi` fix `fastapi` auto-instrumentation by removing `fastapi-slim` support, `fastapi-slim` itself is discontinued from maintainers From 8022e23a73b2f23043f810222dea94e40844d8a8 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 26 Aug 2024 16:33:37 +0200 Subject: [PATCH 5/5] Fix changelog --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b98fdc565d..3dd53582ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,25 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -## Added +### Added - `opentelemetry-instrumentation-kafka-python` Instrument temporary fork, kafka-python-ng inside kafka-python's instrumentation ([#2537](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2537)) -## Breaking changes +### Breaking changes - `opentelemetry-bootstrap` Remove `opentelemetry-instrumentation-aws-lambda` from the defaults instrumentations ([#2786](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2786)) -## Fixed +### Fixed - `opentelemetry-instrumentation-httpx` fix handling of async hooks ([#2823](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2823)) - `opentelemetry-instrumentation-system-metrics` fix `process.runtime.cpu.utilization` values to be shown in range of 0 to 1 - ([2812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2812)) + ([#2812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2812)) - `opentelemetry-instrumentation-fastapi` fix `fastapi` auto-instrumentation by removing `fastapi-slim` support, `fastapi-slim` itself is discontinued from maintainers - ([2783](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2783)) + ([#2783](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2783)) - `opentelemetry-instrumentation-aws-lambda` Avoid exception when a handler is not present. ([#2750](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2750)) - `opentelemetry-instrumentation-django` Fix regression - `http.target` re-added back to old semconv duration metrics