lepture / authlib

The ultimate Python library in building OAuth, OpenID Connect clients and servers. JWS,JWE,JWK,JWA,JWT included.
https://authlib.org/
BSD 3-Clause "New" or "Revised" License
4.49k stars 448 forks source link

Make clients easier to get instrumented #528

Closed sam-mosleh closed 1 year ago

sam-mosleh commented 1 year ago

Is your feature request related to a problem? Please describe.

OpenTelemetry introduced an automatic way to monkey-patch popular libraries to trace them, but Authlib can't take advantage of it. Currently, the only way for instrumenting Authlib's Httpx client is by instrumenting each client instance:

from authlib.integrations.httpx_client import AsyncOAuth2Client
from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor

client = AsyncOAuth2Client(
    client_id=client_id,
    client_secret=client_secret,
    scope="tweet.read",
    redirect_uri=redirect_uri,
    code_challenge_method="S256",
    token_endpoint="https://api.twitter.com/2/oauth2/token",
    authorization_endpoint="https://twitter.com/i/oauth2/authorize",
    grant_type="authorization_code",
)
HTTPXClientInstrumentor().instrument_client(
    client, tracer_provider=tracer_provider
)

Which for large code bases is a tedious job

Describe the solution you'd like

Being able to instrument clients without changes to the code base which is achieved for AsyncOAuth2Client by changing inheritance from AsyncClient to httpx.AsyncClient

Additional context

I would be happy to try to open a PR for this. Thanks!

lepture commented 1 year ago

The AsyncClient is imported from httpx.AsyncClient, they are the same thing.

If you can't use

from httpx import AsyncClient

for instruments, even though I can change Authlib's implementation, there will always be other libraries don't use httpx.AsyncClient directly.

In my opinion, it is the instrument tool's job to take care of these things. It can monkey patch httpx.AsyncClient directly on class level.

sam-mosleh commented 1 year ago

Since Opentelemetry replaces the whole class with the new one, they won't be the same because AsyncClient will refer to the old, unpatched class. You are right about other libraries, but since your Authlib is widely used, fixing it here will save a lot of time for larger projects.

lepture commented 1 year ago

@sam-mosleh I'm happy to receive a patch then. Thanks for the feedback.