pydantic / logfire

Uncomplicated Observability for Python and beyond! 🪵🔥
https://docs.pydantic.dev/logfire/
MIT License
1.62k stars 46 forks source link

Specifying `http_client` in openai client options causing `TypeError` with `instrument_httpx` #264

Open CNSeniorious000 opened 1 week ago

CNSeniorious000 commented 1 week ago

Description

If I use instrument_httpx(), and then construct an OpenAI client with a custom httpx client, openai will raise an TypeError:

TypeError: Invalid `http_client` argument; Expected an instance of `httpx.AsyncClient` but got <class 'httpx.AsyncClient'>

Python, Logfire & OS Versions, related packages (not required)

logfire="0.42.0"
platform="Windows-10-10.0.22631-SP0"
python="3.11.9 (tags/v3.11.9:de54cf5, Apr  2 2024, 10:12:12) [MSC v.1938 64 bit (AMD64)]"
[related_packages]
requests="2.32.3"
pydantic="2.7.4"
openai="1.34.0"
protobuf="4.25.3"
rich="13.7.1"
executing="2.0.1"
opentelemetry-api="1.25.0"
opentelemetry-exporter-otlp-proto-common="1.25.0"
opentelemetry-exporter-otlp-proto-http="1.25.0"
opentelemetry-instrumentation="0.46b0"
opentelemetry-instrumentation-asgi="0.46b0"
opentelemetry-instrumentation-fastapi="0.46b0"
opentelemetry-instrumentation-httpx="0.46b0"
opentelemetry-instrumentation-system-metrics="0.46b0"
opentelemetry-proto="1.25.0"
opentelemetry-sdk="1.25.0"
opentelemetry-semantic-conventions="0.46b0"
opentelemetry-util-http="0.46b0"
alexmojaki commented 1 week ago

I can reproduce with this code:

import httpx
import openai

import logfire

logfire.configure()

client = httpx.AsyncClient()
logfire.instrument_httpx()
openai.AsyncClient(http_client=client, api_key='...')

The error doesn't happen if client = httpx.AsyncClient() is done after logfire.instrument_httpx(). What's happening is that logfire.instrument_httpx replaces the class httpx.AsyncClient with its own subclass, so when openai checks isinstance(http_client, httpx.AsyncClient) it fails because now http_client is an instance of the original class while httpx.AsyncClient is a subclass of the original.

This should be reported to OTEL.

CNSeniorious000 commented 1 week ago

I see. Thanks for your clear explanation. I should instrument_httpx after any httpx instance initialized. But maybe I think OTEL can't instrument httpx without subclassing it?

alexmojaki commented 1 week ago

No, you should call instrument_httpx before constructing the client. In fact you should do that anyway if you want the client to be instrumented.

OTEL could instrument httpx without subclassing, e.g. by patching the __init__ method on the original class.

CNSeniorious000 commented 1 week ago

Yeah thanks. I made a typos mistake.

I've refactored my code and openai works well with httpx now. I am going to post an issue to OTEL now. Thanks for your reproduction example