Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPXClientInstrumentor.instrument_client is no longer a staticmethod, and documentation is out of date #2998

Closed
robotadam opened this issue Nov 13, 2024 · 2 comments · Fixed by #3003
Labels
bug Something isn't working

Comments

@robotadam
Copy link

Describe your environment

OS: Debian bookworm, docker
Python version: 3.11
Package version: 0.49

What happened?

In PR #2909 HTTPXClientInstrumentor.instrument_client was changed from a staticmethod to an instance method. This breaks existing usage, as described in the docs:

import httpx
from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor

url = "https://some.url/get"

with httpx.Client(transport=telemetry_transport) as client:
    HTTPXClientInstrumentor.instrument_client(client)
    response = client.get(url)

Since this change, the code must use

with httpx.Client(transport=telemetry_transport) as client:
    HTTPXClientInstrumentor().instrument_client(client)

Steps to Reproduce

import httpx
from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor

url = "http://example.com/get"

with httpx.Client() as client:
    HTTPXClientInstrumentor.instrument_client(client)
    response = client.get(url)

Expected Result

Expected that the client is instrumented with no errors.

Actual Result

TypeError: HTTPXClientInstrumentor.instrument_client() missing 1 required positional argument: 'client'

Additional context

This was called out as a breaking change in the changelog, but without a description of what to change.

I'll also note that uninstrument_client is still a staticmethod, so there might be some confusion now that these are not symmetric.

Would you like to implement a fix?

None

@robotadam robotadam added the bug Something isn't working label Nov 13, 2024
@xrmx
Copy link
Contributor

xrmx commented Nov 14, 2024

Yeah, we didn't have a single test that used it as a static method so I missed it 😓

If you look at the code I think it's easy to make it a staticmethod again because AFAICS it's using self only for calling another staticmethod of the same class.

@robotadam
Copy link
Author

@xrmx Thanks for the quick look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants