perf(otel): memoize per-request lazy import of otel runtime hooks - #31707
Conversation
|
|
Greptile SummaryThis PR memoizes the OpenTelemetry runtime hook lookup. The main changes are:
Confidence Score: 5/5The change is narrowly scoped to OpenTelemetry runtime hook resolution and preserves no-op behavior when hooks are unavailable. The implementation is covered by focused tests for repeated calls, cache behavior, and SDK-absent handling, with no code issues identified in the changed files.
What T-Rex did
Reviews (1): Last reviewed commit: "perf(otel): memoize per-request lazy imp..." | Re-trigger Greptile |
Greptile SummaryThis PR reduces repeated OTel lazy-import work on the proxy request path. The main changes are:
Confidence Score: 4/5The change is narrowly scoped to memoizing optional OTel hook resolution while preserving no-op behavior when the SDK is unavailable. The added tests cover repeated resolution, cache behavior, and SDK-absent execution, which are the main behavioral risks for this patch. No specific files require follow-up attention.
What T-Rex did
Reviews (2): Last reviewed commit: "perf(otel): memoize per-request lazy imp..." | Re-trigger Greptile |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The proxy auth path calls phase_span() and seed_request_identity() in litellm/integrations/otel/runtime.py on every request, each doing a try/except lazy import of litellm.integrations.otel.logger. When the OpenTelemetry SDK is not installed (the default), that import raises, and CPython never caches a failed import, so every request re-scanned sys.path and contended on the import lock. At 750 concurrent users this cost about 12% throughput versus v1.85.0. Resolve the hooks once and cache the outcome, absence included, with functools.cache, so the import is attempted a single time instead of per request. Throughput returns to the v1.85.0 baseline.
40aaebc to
12b48de
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 12b48de. Configure here.
…rriAI#31707) The proxy auth path calls phase_span() and seed_request_identity() in litellm/integrations/otel/runtime.py on every request, each doing a try/except lazy import of litellm.integrations.otel.logger. When the OpenTelemetry SDK is not installed (the default), that import raises, and CPython never caches a failed import, so every request re-scanned sys.path and contended on the import lock. At 750 concurrent users this cost about 12% throughput versus v1.85.0. Resolve the hooks once and cache the outcome, absence included, with functools.cache, so the import is attempted a single time instead of per request. Throughput returns to the v1.85.0 baseline.
Relevant issues
Linear ticket
LIT-4104
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
The proxy auth path calls
phase_span()andseed_request_identity()(inlitellm/integrations/otel/runtime.py) on every request. Each did atry: from litellm.integrations.otel.logger import ... except Exceptionlazy import.otel.loggerimports the OpenTelemetry SDK at module scope, so when the SDK is not installed (the default) the import raises. CPython never caches a failed import, so every request re-ran a fullfind_specscan ofsys.pathand contended on the global import lockBenchmark is locust against a real litellm proxy, 750 concurrent users, spawn 100/s, 60s, 4 worker processes,
/v1/chat/completionsonly, Postgres and Redis both connected. The upstream is a static mock OpenAI server on purpose so the proxy's own per-request overhead dominates the measurement rather than provider latency; real-provider latency would swamp the signal this PR is about. Same config for every version, proxy run with--num_workers 4Launch and sanity check
Load test
Controlled interleaved A/B (185, 191-unpatched, 191-patched), 2 runs each, constant background load
The fix closes about 79% of the gap. The single-worker cProfile run (cleanest apples-to-apples CPU comparison, no multi-worker scheduling noise) reads 116.2 rps patched vs 117.0 for 1.85.0 vs 105.1 unpatched, so per-request overhead is back to baseline. The residual ~2.6% under 4-worker load sits inside 1.85.0's own run-to-run variance and traces to genuinely new per-request features (per-token cost and cache breakdown, overhead-latency metric) plus a FastAPI/starlette bump, not this code path
Direct evidence of the cause and the fix, captured by instrumenting
PathFinder.find_specFull load-test CSVs, the find_spec probe output, and the cProfile diff: https://gist.github.com/yassin-berriai/c4a5be8099c6467d7e8330e9e82ec0e2
Type
🐛 Bug Fix
Changes
Resolve the SDK-backed OTel hooks once and memoize the outcome, absence included, with
functools.cacheinlitellm/integrations/otel/runtime.py, so the lazy import is attempted a single time instead of on every request. Public behavior is unchanged: the wrappers still no-op when the SDK is absent or V2 is not the active logger, and still nest spans when it isAdded
tests/test_litellm/integrations/otel/test_runtime.py, which counts import attempts across 50 calls and asserts the resolution happens at most once, asserts thefunctools.cacheis memoized, and checks the SDK-absent path still no-ops without raising. The test fails on the pre-fix code (one import attempt per call) and passes with the fixNote
Low Risk
Localized performance fix in the OTel shim with unchanged public no-op/span behavior and targeted regression tests.
Overview
Fixes a proxy auth hot-path regression where
phase_spanandseed_request_identityinlitellm/integrations/otel/runtime.pyattempted a lazyotel.loggerimport on every request. When the OpenTelemetry SDK is not installed, that import fails and CPython does not cache failures, so each call re-scannedsys.pathand hit the import lock.The change adds a
@cache-memoized_otel_runtime()that resolvesphase_spanandseed_request_identityonce (including cachingNonewhen the SDK is missing).phase_spanandseed_request_identitydelegate to that tuple; behavior is unchanged—still no-op without the SDK/V2 logger, still nest spans when OTel is active.Adds
tests/test_litellm/integrations/otel/test_runtime.pyto assert at most one logger import across many calls,functools.cachehit/miss behavior, and no-op wrappers when runtime is absent.Reviewed by Cursor Bugbot for commit 12b48de. Bugbot is set up for automated code reviews on this repo. Configure here.