fix(httpx): check if mount transport is None before wrap (open-telemetry#3022)

Author: emdneto

CHANGELOG.md

@@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `opentelemetry-instrumentation-httpx`: instrument_client is a static method again
   ([#3003](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3003))
+- `opentelemetry-instrumentation-httpx`: Check if mount transport is none before wrap it
+  ([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022))
 
 ### Breaking changes
 

instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py

@@ -1005,17 +1005,18 @@ def instrument_client(
                 ),
             )
             for transport in client._mounts.values():
-                wrap_function_wrapper(
-                    transport,
-                    "handle_request",
-                    partial(
-                        cls._handle_request_wrapper,
-                        tracer=tracer,
-                        sem_conv_opt_in_mode=sem_conv_opt_in_mode,
-                        request_hook=request_hook,
-                        response_hook=response_hook,
-                    ),
-                )
+                if hasattr(transport, "handle_request"):
+                    wrap_function_wrapper(
+                        transport,
+                        "handle_request",
+                        partial(
+                            cls._handle_request_wrapper,
+                            tracer=tracer,
+                            sem_conv_opt_in_mode=sem_conv_opt_in_mode,
+                            request_hook=request_hook,
+                            response_hook=response_hook,
+                        ),
+                    )
             client._is_instrumented_by_opentelemetry = True
         if hasattr(client._transport, "handle_async_request"):
             wrap_function_wrapper(
@@ -1030,17 +1031,18 @@ def instrument_client(
                 ),
             )
             for transport in client._mounts.values():
-                wrap_function_wrapper(
-                    transport,
-                    "handle_async_request",
-                    partial(
-                        cls._handle_async_request_wrapper,
-                        tracer=tracer,
-                        sem_conv_opt_in_mode=sem_conv_opt_in_mode,
-                        async_request_hook=async_request_hook,
-                        async_response_hook=async_response_hook,
-                    ),
-                )
+                if hasattr(transport, "handle_async_request"):
+                    wrap_function_wrapper(
+                        transport,
+                        "handle_async_request",
+                        partial(
+                            cls._handle_async_request_wrapper,
+                            tracer=tracer,
+                            sem_conv_opt_in_mode=sem_conv_opt_in_mode,
+                            async_request_hook=async_request_hook,
+                            async_response_hook=async_response_hook,
+                        ),
+                    )
             client._is_instrumented_by_opentelemetry = True
 
     @staticmethod

instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py

@@ -741,6 +741,10 @@ def create_client(
         def create_proxy_transport(self, url: str):
             pass
 
+        @abc.abstractmethod
+        def get_transport_handler(self, transport):
+            pass
+
         def setUp(self):
             super().setUp()
             self.client = self.create_client()
@@ -763,17 +767,15 @@ def assert_proxy_mounts(self, mounts, num_mounts, transport_type=None):
             self.assertEqual(len(mounts), num_mounts)
             for transport in mounts:
                 with self.subTest(transport):
+                    if transport is None:
+                        continue
                     if transport_type:
                         self.assertIsInstance(
                             transport,
                             transport_type,
                         )
                     else:
-                        handler = getattr(transport, "handle_request", None)
-                        if not handler:
-                            handler = getattr(
-                                transport, "handle_async_request"
-                            )
+                        handler = self.get_transport_handler(transport)
                         self.assertTrue(
                             isinstance(handler, ObjectProxy)
                             and getattr(handler, "__wrapped__")
@@ -983,6 +985,21 @@ def test_uninstrument_new_client(self):
             self.assertEqual(result.text, "Hello!")
             self.assert_span()
 
+        @mock.patch.dict(
+            "os.environ", {"NO_PROXY": "http://mock/status/200"}, clear=True
+        )
+        def test_instrument_with_no_proxy(self):
+            proxy_mounts = self.create_proxy_mounts()
+            HTTPXClientInstrumentor().instrument()
+            client = self.create_client(mounts=proxy_mounts)
+            result = self.perform_request(self.URL, client=client)
+            self.assert_span(num_spans=1)
+            self.assertEqual(result.text, "Hello!")
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+            )
+
         def test_instrument_proxy(self):
             proxy_mounts = self.create_proxy_mounts()
             HTTPXClientInstrumentor().instrument()
@@ -994,6 +1011,27 @@ def test_instrument_proxy(self):
                 2,
             )
 
+        @mock.patch.dict(
+            "os.environ", {"NO_PROXY": "http://mock/status/200"}, clear=True
+        )
+        def test_instrument_client_with_no_proxy(self):
+            proxy_mounts = self.create_proxy_mounts()
+            client = self.create_client(mounts=proxy_mounts)
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+                (httpx.HTTPTransport, httpx.AsyncHTTPTransport),
+            )
+            HTTPXClientInstrumentor.instrument_client(client)
+            result = self.perform_request(self.URL, client=client)
+            self.assertEqual(result.text, "Hello!")
+            self.assert_span(num_spans=1)
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+            )
+            HTTPXClientInstrumentor.uninstrument_client(client)
+
         def test_instrument_client_with_proxy(self):
             proxy_mounts = self.create_proxy_mounts()
             client = self.create_client(mounts=proxy_mounts)
@@ -1188,6 +1226,9 @@ def perform_request(
     def create_proxy_transport(self, url):
         return httpx.HTTPTransport(proxy=httpx.Proxy(url))
 
+    def get_transport_handler(self, transport):
+        return getattr(transport, "handle_request", None)
+
     def test_can_instrument_subclassed_client(self):
         class CustomClient(httpx.Client):
             pass
@@ -1241,6 +1282,9 @@ async def _perform_request():
     def create_proxy_transport(self, url):
         return httpx.AsyncHTTPTransport(proxy=httpx.Proxy(url))
 
+    def get_transport_handler(self, transport):
+        return getattr(transport, "handle_async_request", None)
+
     def test_basic_multiple(self):
         # We need to create separate clients because in httpx >= 0.19,
         # closing the client after "with" means the second http call fails