Skip to content

Commit

Permalink
fix(httpx): check if mount transport is None before wrap (#3022)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emdneto
emdneto authored Nov 19, 2024
1 parent 1c820ea commit 19a59e4
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
46 changes: 24 additions & 22 deletions ...n/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
54 changes: 49 additions & 5 deletions instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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__")
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 19a59e4

Please sign in to comment.