From 7f552db38ccfead1b40f7823f67a4bdbdd0285bc Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 18:25:53 +0100 Subject: [PATCH 01/15] httpx: instrument transport instead of client --- .../tests/test_httpx_integration.py | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 148fe27893..76a333fca3 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -14,6 +14,8 @@ # pylint: disable=too-many-lines +from __future__ import annotations + import abc import asyncio import typing @@ -593,10 +595,10 @@ def create_transport( @abc.abstractmethod def create_client( self, - transport: typing.Union[ - SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport, None - ] = None, - **kwargs, + transport: SyncOpenTelemetryTransport + | AsyncOpenTelemetryTransport + | None = None, + **kwargs: typing.Any, ): pass @@ -730,9 +732,9 @@ class BaseInstrumentorTest(BaseTest, metaclass=abc.ABCMeta): @abc.abstractmethod def create_client( self, - transport: typing.Union[ - SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport, None - ] = None, + transport: SyncOpenTelemetryTransport + | AsyncOpenTelemetryTransport + | None = None, **kwargs, ): pass @@ -926,6 +928,17 @@ def test_instrument_client_called_on_the_class(self): self.assertEqual(result.text, "Hello!") self.assert_span(num_spans=1) + def test_instrument_multiple_clients_with_the_same_transport(self): + client1 = typing.cast(httpx.Client, self.create_client()) + client2 = self.create_client(client1._transport) + + HTTPXClientInstrumentor().instrument_client(client1) + HTTPXClientInstrumentor().instrument_client(client2) + + result = self.perform_request(self.URL, client=client1) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=1) + def test_instrumentation_without_client(self): HTTPXClientInstrumentor().instrument() results = [ From 3902028669ec3559482c7cf82b59c8aa1e62d3c4 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 18:38:09 +0100 Subject: [PATCH 02/15] And now? --- .../tests/test_httpx_integration.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 76a333fca3..4e56330e73 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -929,8 +929,9 @@ def test_instrument_client_called_on_the_class(self): self.assert_span(num_spans=1) def test_instrument_multiple_clients_with_the_same_transport(self): - client1 = typing.cast(httpx.Client, self.create_client()) - client2 = self.create_client(client1._transport) + transport = self.create_transport() + client1 = self.create_client(transport=transport) + client2 = self.create_client(trasnport=transport) HTTPXClientInstrumentor().instrument_client(client1) HTTPXClientInstrumentor().instrument_client(client2) From 70e8cab6f3fc1ff3e5b399e0acb38ab78ceca1c0 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 18:40:49 +0100 Subject: [PATCH 03/15] fix typo --- .../tests/test_httpx_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 4e56330e73..7ff08715ab 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -931,7 +931,7 @@ def test_instrument_client_called_on_the_class(self): def test_instrument_multiple_clients_with_the_same_transport(self): transport = self.create_transport() client1 = self.create_client(transport=transport) - client2 = self.create_client(trasnport=transport) + client2 = self.create_client(transport=transport) HTTPXClientInstrumentor().instrument_client(client1) HTTPXClientInstrumentor().instrument_client(client2) From de40117b6e686e4a1853c60fbded09a3a3d631fa Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 18:50:47 +0100 Subject: [PATCH 04/15] Move the test around --- .../tests/test_httpx_integration.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 7ff08715ab..a25e4d1a2b 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -928,18 +928,6 @@ def test_instrument_client_called_on_the_class(self): self.assertEqual(result.text, "Hello!") self.assert_span(num_spans=1) - def test_instrument_multiple_clients_with_the_same_transport(self): - transport = self.create_transport() - client1 = self.create_client(transport=transport) - client2 = self.create_client(transport=transport) - - HTTPXClientInstrumentor().instrument_client(client1) - HTTPXClientInstrumentor().instrument_client(client2) - - result = self.perform_request(self.URL, client=client1) - self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=1) - def test_instrumentation_without_client(self): HTTPXClientInstrumentor().instrument() results = [ @@ -1148,6 +1136,18 @@ def test_credential_removal(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) + def test_instrument_multiple_clients_with_the_same_transport(self): + transport = self.create_transport() + client1 = self.create_client(transport=transport) + client2 = self.create_client(transport=transport) + + HTTPXClientInstrumentor().instrument_client(client1) + HTTPXClientInstrumentor().instrument_client(client2) + + result = self.perform_request(self.URL, client=client1) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=1) + class TestAsyncIntegration(BaseTestCases.BaseManualTest): response_hook = staticmethod(_async_response_hook) From 4f3b83d0eed6fb6b7a4daa2245b6d7fce39b36ab Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 19:00:17 +0100 Subject: [PATCH 05/15] try to fix the issue --- .../opentelemetry/instrumentation/httpx/__init__.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 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 195c784408..5aedf50c6e 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -890,7 +890,9 @@ def instrument_client( that is called right before the span ends """ - if getattr(client, "_is_instrumented_by_opentelemetry", False): + if getattr( + client._transport, "_is_instrumented_by_opentelemetry", False + ): _logger.warning( "Attempting to instrument Httpx client while already instrumented" ) @@ -946,7 +948,7 @@ def instrument_client( response_hook=response_hook, ), ) - client._is_instrumented_by_opentelemetry = True + client._transport._is_instrumented_by_opentelemetry = True if hasattr(client._transport, "handle_async_request"): wrap_function_wrapper( client._transport, @@ -972,7 +974,7 @@ def instrument_client( async_response_hook=async_response_hook, ), ) - client._is_instrumented_by_opentelemetry = True + client._transport._is_instrumented_by_opentelemetry = True @staticmethod def uninstrument_client( @@ -987,9 +989,9 @@ def uninstrument_client( unwrap(client._transport, "handle_request") for transport in client._mounts.values(): unwrap(transport, "handle_request") - client._is_instrumented_by_opentelemetry = False + client._transport._is_instrumented_by_opentelemetry = False elif hasattr(client._transport, "handle_async_request"): unwrap(client._transport, "handle_async_request") for transport in client._mounts.values(): unwrap(transport, "handle_async_request") - client._is_instrumented_by_opentelemetry = False + client._transport._is_instrumented_by_opentelemetry = False From e5f15799c0fd558a434d2ddd479b799f636f5942 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 19:27:33 +0100 Subject: [PATCH 06/15] fix issue --- .../tests/test_httpx_integration.py | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index a25e4d1a2b..742c8eee98 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -928,6 +928,17 @@ def test_instrument_client_called_on_the_class(self): self.assertEqual(result.text, "Hello!") self.assert_span(num_spans=1) + def test_instrument_multiple_clients_with_the_same_transport(self): + client1 = self.create_client() + client2 = self.create_client(transport=client1._transport) + + HTTPXClientInstrumentor().instrument_client(client1) + HTTPXClientInstrumentor().instrument_client(client2) + + result = self.perform_request(self.URL, client=client1) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=1) + def test_instrumentation_without_client(self): HTTPXClientInstrumentor().instrument() results = [ @@ -1136,18 +1147,6 @@ def test_credential_removal(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], self.URL) - def test_instrument_multiple_clients_with_the_same_transport(self): - transport = self.create_transport() - client1 = self.create_client(transport=transport) - client2 = self.create_client(transport=transport) - - HTTPXClientInstrumentor().instrument_client(client1) - HTTPXClientInstrumentor().instrument_client(client2) - - result = self.perform_request(self.URL, client=client1) - self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=1) - class TestAsyncIntegration(BaseTestCases.BaseManualTest): response_hook = staticmethod(_async_response_hook) From f42c678136f4748e1b5dad2743e328ac47e14b63 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 19:46:28 +0100 Subject: [PATCH 07/15] make sure that test fails --- .../src/opentelemetry/instrumentation/httpx/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 5aedf50c6e..18ac6fb6ad 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -948,7 +948,7 @@ def instrument_client( response_hook=response_hook, ), ) - client._transport._is_instrumented_by_opentelemetry = True + client._is_instrumented_by_opentelemetry = True if hasattr(client._transport, "handle_async_request"): wrap_function_wrapper( client._transport, @@ -974,7 +974,7 @@ def instrument_client( async_response_hook=async_response_hook, ), ) - client._transport._is_instrumented_by_opentelemetry = True + client._is_instrumented_by_opentelemetry = True @staticmethod def uninstrument_client( @@ -989,9 +989,9 @@ def uninstrument_client( unwrap(client._transport, "handle_request") for transport in client._mounts.values(): unwrap(transport, "handle_request") - client._transport._is_instrumented_by_opentelemetry = False + client._is_instrumented_by_opentelemetry = False elif hasattr(client._transport, "handle_async_request"): unwrap(client._transport, "handle_async_request") for transport in client._mounts.values(): unwrap(transport, "handle_async_request") - client._transport._is_instrumented_by_opentelemetry = False + client._is_instrumented_by_opentelemetry = False From 52471f97f05d1b3379d9e3eb7d17e0419063649c Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 19:46:55 +0100 Subject: [PATCH 08/15] make sure that test fails --- .../src/opentelemetry/instrumentation/httpx/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 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 18ac6fb6ad..195c784408 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -890,9 +890,7 @@ def instrument_client( that is called right before the span ends """ - if getattr( - client._transport, "_is_instrumented_by_opentelemetry", False - ): + if getattr(client, "_is_instrumented_by_opentelemetry", False): _logger.warning( "Attempting to instrument Httpx client while already instrumented" ) From f20905981fa7273eac186508b5fa61c4beae7a27 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 20:06:28 +0100 Subject: [PATCH 09/15] Transport was not being passed... --- .../tests/test_httpx_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 742c8eee98..9383bd269a 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -1223,7 +1223,7 @@ def create_client( transport: typing.Optional[SyncOpenTelemetryTransport] = None, **kwargs, ): - return httpx.Client(**kwargs) + return httpx.Client(transport=transport, **kwargs) def perform_request( self, @@ -1273,7 +1273,7 @@ def create_client( transport: typing.Optional[AsyncOpenTelemetryTransport] = None, **kwargs, ): - return httpx.AsyncClient(**kwargs) + return httpx.AsyncClient(transport=transport, **kwargs) def perform_request( self, From ed97732e6dec8b75ec4e47c49be5b6c83730cd0f Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 20:13:42 +0100 Subject: [PATCH 10/15] Fix test --- .../opentelemetry/instrumentation/httpx/__init__.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 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 195c784408..5aedf50c6e 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -890,7 +890,9 @@ def instrument_client( that is called right before the span ends """ - if getattr(client, "_is_instrumented_by_opentelemetry", False): + if getattr( + client._transport, "_is_instrumented_by_opentelemetry", False + ): _logger.warning( "Attempting to instrument Httpx client while already instrumented" ) @@ -946,7 +948,7 @@ def instrument_client( response_hook=response_hook, ), ) - client._is_instrumented_by_opentelemetry = True + client._transport._is_instrumented_by_opentelemetry = True if hasattr(client._transport, "handle_async_request"): wrap_function_wrapper( client._transport, @@ -972,7 +974,7 @@ def instrument_client( async_response_hook=async_response_hook, ), ) - client._is_instrumented_by_opentelemetry = True + client._transport._is_instrumented_by_opentelemetry = True @staticmethod def uninstrument_client( @@ -987,9 +989,9 @@ def uninstrument_client( unwrap(client._transport, "handle_request") for transport in client._mounts.values(): unwrap(transport, "handle_request") - client._is_instrumented_by_opentelemetry = False + client._transport._is_instrumented_by_opentelemetry = False elif hasattr(client._transport, "handle_async_request"): unwrap(client._transport, "handle_async_request") for transport in client._mounts.values(): unwrap(transport, "handle_async_request") - client._is_instrumented_by_opentelemetry = False + client._transport._is_instrumented_by_opentelemetry = False From b89ee8ffa763d7c4ebf6cff2438101d7b90871c7 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 20:36:23 +0100 Subject: [PATCH 11/15] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b8d39ff0e..b955089e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-httpx` Instrument transport instead of HTTPX client + ([#3106](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3106)) + ## Version 1.29.0/0.50b0 (2024-12-11) ### Added From 7660a00ca4acee10e1981e44547dbf9003a14a16 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Dec 2024 20:44:27 +0100 Subject: [PATCH 12/15] Do nothing on transport already instrumented --- .../opentelemetry/instrumentation/httpx/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 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 5aedf50c6e..82dc23838d 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -890,13 +890,15 @@ def instrument_client( that is called right before the span ends """ - if getattr( - client._transport, "_is_instrumented_by_opentelemetry", False - ): + if getattr(client, "_is_instrumented_by_opentelemetry", False): _logger.warning( "Attempting to instrument Httpx client while already instrumented" ) return + if getattr( + client._transport, "is_instrumented_by_opentelemetry", False + ): + return _OpenTelemetrySemanticConventionStability._initialize() sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( @@ -949,6 +951,7 @@ def instrument_client( ), ) client._transport._is_instrumented_by_opentelemetry = True + client._is_instrumented_by_opentelemetry = True if hasattr(client._transport, "handle_async_request"): wrap_function_wrapper( client._transport, @@ -975,6 +978,7 @@ def instrument_client( ), ) client._transport._is_instrumented_by_opentelemetry = True + client._is_instrumented_by_opentelemetry = True @staticmethod def uninstrument_client( @@ -990,8 +994,10 @@ def uninstrument_client( for transport in client._mounts.values(): unwrap(transport, "handle_request") client._transport._is_instrumented_by_opentelemetry = False + client._is_instrumented_by_opentelemetry = False elif hasattr(client._transport, "handle_async_request"): unwrap(client._transport, "handle_async_request") for transport in client._mounts.values(): unwrap(transport, "handle_async_request") client._transport._is_instrumented_by_opentelemetry = False + client._is_instrumented_by_opentelemetry = False From cd925e710a9cdfc969302e762b53322b61a858b0 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 16 Dec 2024 09:34:19 +0100 Subject: [PATCH 13/15] Empty-Commit From 9378dfdc429601611c16b3e633aceee628503100 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 16 Dec 2024 11:32:05 +0100 Subject: [PATCH 14/15] Add _is_instrumented_by_opentelemetry to the transport --- .../src/opentelemetry/instrumentation/httpx/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) 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 82dc23838d..f0d119081b 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -402,6 +402,8 @@ class SyncOpenTelemetryTransport(httpx.BaseTransport): that is called right before the span ends """ + _is_instrumented_by_opentelemetry = True + def __init__( self, transport: httpx.BaseTransport, @@ -527,6 +529,8 @@ class AsyncOpenTelemetryTransport(httpx.AsyncBaseTransport): that is called right before the span ends """ + _is_instrumented_by_opentelemetry = True + def __init__( self, transport: httpx.AsyncBaseTransport, From 7095cd04af466f13a6445450f42105f0e9c4c5a4 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 16 Dec 2024 11:34:23 +0100 Subject: [PATCH 15/15] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b955089e67..1707cc8bc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -- `opentelemetry-instrumentation-httpx` Instrument transport instead of HTTPX client +- `opentelemetry-instrumentation-httpx` Add the `is_instrumented` flag to the transport ([#3106](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3106)) ## Version 1.29.0/0.50b0 (2024-12-11)