Starlette instrumentation can have ordering issues

[anuraaga]

Currently, starlette instrumentation uses a class override pattern which can have ordering issues that cause autoinstrumentation to not apply.

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py#L287

In practice, I see most users manually instrument the app because of this, for example here

https://github.com/YunosukeY/otel-and-a2a-sample/blob/main/server.py#L47

I think it should be possible to switch to wrapping the __init__ method without replacement which can usually solve such ordering issues.

/cc @xrmx was interested in this (I hope :) )

[codefromthecrypt]

Basically, we can't magically drop in "opentelemetry-instrumentation-starlette" and have opentelemetry-instrument take care of instrumenting A2A due to this glitch in how starlette instrumentation works.

cc @whitlockjc even though the code rag referenced from @YunosukeY used the former samples, still applies to the official SDK that's brewing here https://github.com/google/a2a-python

cc @Kludex likely applies to anything else that uses starlette, like MCP or FastA2A, right?

[Kludex]

cc @Kludex likely applies to anything else that uses starlette, like MCP or FastA2A, right?

Yes, it does.

---

I didn't get the "ordering issues" part tho, when does it occur?

[xrmx]

I didn't get the "ordering issues" part tho, when does it occur?

@Kludex the httpx instrumentor had a similar issue where loading the openai library in the openai instrumentation before the httpx instrumentor was loaded made the httpx client substitution not patch the openai subclasses and so no http spans for openai client calls. We solved that doing lazy loading of both openai library in openai instrumentation and lazy patching of the httpx library in the httpx instrumentor (f6b68d0) to avoid top level import of patched libraries.
This will also help making instrumentors loadable when the instrumented library is not available.

[anuraaga]

Sorry for the noise, I realized I was misunderstanding things. The main problem was just that the instrumentation targets a very old version of starlette

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-starlette/pyproject.toml#L37

Not sure if there are actual technical issues with running on the newer versions, but I think in this case while there is potential for ordering issues due to the subclass pattern, before that it's more important to loosen that to allow modern versions of starlette. Will close this issue since it is probably not actually an issue in the wild.

[Kludex]

Given the constraints, I'll take the opportunity and rework the instrumentation. No one uses those versions, and if they do, they should bump it anyway...

Anyway, reference:

https://pepy.tech/projects/starlette?timeRange=threeMonths&category=version&includeCIDownloads=true&granularity=daily&viewType=line&versions=0.46.2%2C0.46.1%2C0.46.0%2C0.14.2

[SrdjanLL]

Hi @Kludex, sorry for bad timing, but I already had this in the works and just submitted the PR when I saw your comment. Hope that's all right with you and we avoid duplicating efforts. Perhaps you can give me feedback on the refactor.

Thanks!

[anuraaga]

Thanks @SrdjanLL ! While not as critical as I initially thought, I agree that using wrapt is still a worthwhile improvement for reducing issues in the future. Likely just a reminder for everyone but problems happen when multiple instrumentations use problematic eager patterns - if a future MCP, a2a etc instrumentation imported starlette we would likely see a repeat of the httpx issue. Basically we want all instrumentation to be registered as hooks without eagerly importing the instrumented library going forward I think and maybe it's a good topic for a future KubeCon hackathon 😉

Meanwhile I sent #3529 for the version ceiling issue, before realizing I completely missed a similar PR 😭 But on the bright side it means there's some good attention here and hoping it means merges to improve ux for starlette users!