fix(httpx): check if mount transport is None before wrap (#3022)
This commit is contained in:
parent
1c820ea96e
commit
19a59e4be7
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -1005,17 +1005,18 @@ class HTTPXClientInstrumentor(BaseInstrumentor):
|
|||
),
|
||||
)
|
||||
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 @@ class HTTPXClientInstrumentor(BaseInstrumentor):
|
|||
),
|
||||
)
|
||||
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
|
||||
|
|
|
|||
|
|
@ -741,6 +741,10 @@ class BaseTestCases:
|
|||
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 @@ class BaseTestCases:
|
|||
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 @@ class BaseTestCases:
|
|||
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 @@ class BaseTestCases:
|
|||
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 @@ class TestSyncInstrumentationIntegration(BaseTestCases.BaseInstrumentorTest):
|
|||
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 @@ class TestAsyncInstrumentationIntegration(BaseTestCases.BaseInstrumentorTest):
|
|||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue