HTTPXClientInstrumentor does not instrument subclasses of httpx.Client created before instrumentation

[palvarezcordoba]

When you call HTTPXClientInstrumentor.instrument, what happens in HTTPXClientInstrumentor._instrument, is that it creates subclasses of httpx.Clientand httpx.AsyncClient, and replaces httpx clients with those subclasses:

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

Lines 569 to 570 in 8fd2105

	httpx.Client = _InstrumentedClient
	httpx.AsyncClient = _InstrumentedAsyncClient

The problem with this approach is that subclasses of httpx clients created before calling HTTPXClientInstrumentor.instrument will not be instrumented, because they are already subclasses of the original client class.

When I updated openai I found that it broke telemetry. The reason is that it now uses subclasses of httpx clients.
So, if openai is imported before instrumenting httpx, telemetry is not going to work.
You can check this issue: openai/openai-python#1144.
To be clear, this is not a bug on openai. It is a problem in this package, which maybe I would not label as bug, but it is a surprising and unexpected behavior from a user's point of view, and as far as I know, it's not documented.

Steps to reproduce

Running this, openai httpx requests are instrumented:

from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor, _InstrumentedClient

HTTPXClientInstrumentor().instrument()
from openai._base_client import SyncHttpxClientWrapper
assert issubclass(SyncHttpxClientWrapper, _InstrumentedClient)

But this version is not instrumented, and the assertion fails:

from openai._base_client import SyncHttpxClientWrapper
from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor, _InstrumentedClient

HTTPXClientInstrumentor().instrument()
assert issubclass(SyncHttpxClientWrapper, _InstrumentedClient)

Again, not openai specific, it is the same with any subclass:

from opentelemetry.instrumentation.httpx import (
    HTTPXClientInstrumentor,
    _InstrumentedClient,
)
import httpx

class Client(httpx.Client):
    pass

HTTPXClientInstrumentor().instrument()
assert issubclass(Client, _InstrumentedClient)

What is the expected behavior?
Subclasses of httpx's clients are instrumented even if they are created before instrumenting.

What is the actual behavior?
Subclasses of httpx's clients are not instrumented if they are created before instrumenting.

Solution proposal

The code of _InstrumentedClient and _InstrumentedAsyncClient is this:

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

Lines 491 to 526 in 8fd2105

	class _InstrumentedClient(httpx.Client):
	_tracer_provider = None
	_request_hook = None
	_response_hook = None
	def __init__(self, *args, **kwargs):
	super().__init__(*args, **kwargs)
	self._original_transport = self._transport
	self._is_instrumented_by_opentelemetry = True
	self._transport = SyncOpenTelemetryTransport(
	self._transport,
	tracer_provider=_InstrumentedClient._tracer_provider,
	request_hook=_InstrumentedClient._request_hook,
	response_hook=_InstrumentedClient._response_hook,
	)
	class _InstrumentedAsyncClient(httpx.AsyncClient):
	_tracer_provider = None
	_request_hook = None
	_response_hook = None
	def __init__(self, *args, **kwargs):
	super().__init__(*args, **kwargs)
	self._original_transport = self._transport
	self._is_instrumented_by_opentelemetry = True
	self._transport = AsyncOpenTelemetryTransport(
	self._transport,
	tracer_provider=_InstrumentedAsyncClient._tracer_provider,
	request_hook=_InstrumentedAsyncClient._request_hook,
	response_hook=_InstrumentedAsyncClient._response_hook,
	)

It is subclassing to add some properties, and wrapping _transport in a SyncOpenTelemetryTransport or it's async version.
One way to avoid this behavior would be to patch directly the client classes, instead of patching the httpx package to replace the original clients with the instrumented ones.

However, I'm not sure about what unintentional consecuences this could have.
What do you think about this?

[palvarezcordoba]

And any other instrumentor doing it's job this way is probably behaving the same way as HTTPXClientInstrumentor.

[jagerzhang]

is here any update? i have the same question.

[lzchen]

@palvarezcordoba

This indeed is expected behavior and seems to be more of a documentation issue.

[palvarezcordoba]

@lzchen really? This behavior is a bit weird but as long as it’s documented it’s fine 👍