HTTPXClientInstrumentor.instrument_client fails when NO_PROXY is set

robotadam commented 3 days ago

Describe your environment

OS: Ubuntu in docker Python version: 3.11 Package version: 0.49b2

What happened?

If the environment variable NO_PROXY is set HTTPXClientInstrumentor.instrument_client(client) will fail when wrapping the mounted transport with wrap_function_wrapper because the transport is None for that mount.

Steps to Reproduce

export NO_PROXY=http://example.com

import httpx
from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor
with httpx.Client() as client:
    HTTPXClientInstrumentor.instrument_client(client)

Expected Result

No exception, client is instrumented.

Actual Result

Error in wrapt/patches.py", line 46, in lookup_attribute

AttributeError: 'NoneType' object has no attribute 'handle_request'

Additional context

I had tests that failed only in CircleCI and I traced it to the presence of the environment variable NO_PROXY. HTTPX makes a mount for each entry in NO_PROXY, but with transport set to None. https://www.python-httpx.org/environment_variables/#no_proxy

CircleCI is where I found this, because CircleCI sets this env var in its base cimg images, probably to ensure curl works. They set it to 127.0.0.1,localhost,circleci-internal-outer-build-agent.

The affected code is https://github.com/open-telemetry/opentelemetry-python-contrib/blob/54e684aa6985536c8807567c592ea60751d62162/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py#L1008 and https://github.com/open-telemetry/opentelemetry-python-contrib/blob/54e684aa6985536c8807567c592ea60751d62162/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py#L1033

The easiest fix appears to be to skip a mount if transport is None.

There are two workarounds:

Use HTTPXClientInstrumentor().instrument() instead of instrument_client.
Set trust_env=False when creating the httpx.Client or httpx.AsyncClient. httpx won't use the env variables at all, which affects proxies and ssl key definitions.

Would you like to implement a fix?

None

xrmx commented 3 days ago

Note to self: never touch the httpx instrumentation again :sweat_smile:

robotadam commented 3 days ago

Note to self: never touch the httpx instrumentation again 😅

@xrmx 😂 Sorry! Hope this one is easy, at least.

emdneto commented 3 days ago

@xrmx @robotadam I'm opening a PR 😅

open-telemetry / opentelemetry-python-contrib