open-telemetry / open-telemetry/opentelemetry-python-contrib
HTTPXClientInstrumentor does not instrument subclasses of httpx.Client created before instrumentation
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.1k
- Forks
- 1.1k
- Avg merge
- 4d 15h
- Merged PRs (30d)
- 16
Description
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:
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/8fd2105ceae604cf39a67f1c4dd154753b43fcd1/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py#L569-L570
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:
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/8fd2105ceae604cf39a67f1c4dd154753b43fcd1/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py#L491-L526
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?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/init.py, especially HTTPXClientInstrumentor._instrument and the _InstrumentedClient and _InstrumentedAsyncClient definitions. Reproduce the two import-order examples from the issue, including the generic httpx.Client subclass case. Done means subclasses created before instrumentation are instrumented without breaking the existing client behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100