From 002174638ca88c9cd6d17ead3b2c0492ec34e16a Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Mon, 27 Jul 2026 12:18:53 -0700 Subject: [PATCH] fix(otel/v2): evict tenant tracer providers without synchronous shutdown TenantTracerCache evicted its LRU TracerProvider by calling shutdown() on it, which force-flushes and stops the provider's BatchSpanProcessor synchronously on the request-serving path. In a busy multi-tenant deployment (more than 256 live destination-resource groups) that can block the evicting request on the flush and drop a concurrent request's in-flight spans on the evicted provider. This is the exact hazard TenantFanOutSpanProcessor already avoids by dropping the reference and letting the worker drain, with a comment saying so; the two caches now handle eviction the same way. Extracted _evict_if_full, removed the now-unused _shutdown_provider. The un-shut-down working set stays bounded by the cache size and is reclaimed at process exit. Greptile 4/5 finding on routing.py. --- litellm/integrations/otel/plumbing/routing.py | 30 +++++++------------ .../integrations/otel/test_otel_v2_dynamic.py | 28 +++++++++++++---- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/litellm/integrations/otel/plumbing/routing.py b/litellm/integrations/otel/plumbing/routing.py index 246acb4a19f..0009a67146a 100644 --- a/litellm/integrations/otel/plumbing/routing.py +++ b/litellm/integrations/otel/plumbing/routing.py @@ -50,20 +50,6 @@ _NON_OTLP_KINDS = ("console", "in_memory", "inmemory", "memory") _MAX_CACHED_PROVIDERS = 256 -def _shutdown_provider(provider: TracerProvider) -> None: - """Flush + stop an evicted provider's processors (reclaims their threads). - - ``TracerProvider.shutdown`` force-flushes each ``SpanProcessor`` before - stopping it, so any spans already handed to a ``BatchSpanProcessor`` are - exported rather than dropped. Best-effort: a shutdown failure must not break - the request that triggered the eviction. - """ - try: - provider.shutdown() - except Exception as e: # pragma: no cover - defensive - verbose_logger.debug("OTel V2: error shutting down evicted provider: %s", e) - - class TenantTracerCache: """Destination-scoped ``TracerProvider`` cache keyed by endpoint + headers.""" @@ -78,6 +64,14 @@ class TenantTracerCache: self._tracer_name = tracer_name self._providers: OrderedDict[tuple[object, ...], TracerProvider] = OrderedDict() + def _evict_if_full(self) -> None: + """Drop the least-recently-used provider when over capacity, without a + synchronous ``shutdown``. Mirrors ``TenantFanOutSpanProcessor``: the + evicted provider's worker drains on its own and is reclaimed at process + exit, and the cache stays bounded.""" + if len(self._providers) > _MAX_CACHED_PROVIDERS: + self._providers.popitem(last=False) + def tracers_for(self, default: Tracer, destinations: "tuple[OtelDestination, ...]") -> "tuple[Tracer, ...]": """The tracers for this request's gen-AI span, one per distinct Resource group. @@ -142,9 +136,7 @@ class TenantTracerCache: self._config_with_destinations(tuple(group), include_base_exporters=include_base) ) self._providers[cache_key] = provider - if len(self._providers) > _MAX_CACHED_PROVIDERS: - _, evicted = self._providers.popitem(last=False) - _shutdown_provider(evicted) + self._evict_if_full() return get_tracer(provider, self._tracer_name) def tracer_for(self, default: Tracer, destinations: "tuple[OtelDestination, ...]") -> Tracer: @@ -163,9 +155,7 @@ class TenantTracerCache: else: provider = build_tracer_provider(self._config_with_destinations(destinations)) self._providers[cache_key] = provider - if len(self._providers) > _MAX_CACHED_PROVIDERS: - _, evicted = self._providers.popitem(last=False) - _shutdown_provider(evicted) + self._evict_if_full() return get_tracer(provider, self._tracer_name) def _owned_otlp_kind(self) -> str: diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py b/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py index 2d30779206f..6265ea387b0 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py @@ -105,22 +105,38 @@ def test_destination_set_is_order_independent(): assert len(cache._providers) == 1 -def test_provider_cache_is_bounded_and_evicts_lru(monkeypatch): +def test_provider_cache_evicts_lru_without_shutdown(monkeypatch): + """The provider cache stays bounded, and eviction drops the LRU provider WITHOUT + a synchronous ``shutdown``. Eviction runs on the request-serving path, and + ``TracerProvider.shutdown`` force-flushes/blocks and can drop a concurrent + request's in-flight spans, so it mirrors ``TenantFanOutSpanProcessor`` and lets the + evicted worker drain on its own. Restoring a shutdown-on-eviction fails this.""" + from unittest.mock import MagicMock + from litellm.integrations.otel.plumbing import routing as routing_mod monkeypatch.setattr(routing_mod, "_MAX_CACHED_PROVIDERS", 2) - shut_down = [] - monkeypatch.setattr( - routing_mod, "_shutdown_provider", lambda p: shut_down.append(p) - ) + real_build = routing_mod.build_tracer_provider + created = [] + + def spying_build(config): + provider = real_build(config) + provider.shutdown = MagicMock(wraps=provider.shutdown) + created.append(provider) + return provider + + monkeypatch.setattr(routing_mod, "build_tracer_provider", spying_build) + cache = _cache("langfuse_otel") default = NoOpTracer() cache.tracer_for(default, (_dest("https://1/v1"),)) cache.tracer_for(default, (_dest("https://2/v1"),)) cache.tracer_for(default, (_dest("https://1/v1"),)) # touch "1" -> "2" is LRU cache.tracer_for(default, (_dest("https://3/v1"),)) # overflow -> evict "2" + assert len(cache._providers) == 2 - assert len(shut_down) == 1 + for provider in created: + provider.shutdown.assert_not_called() # --- fan-out: keep the configured exporters, append one per destination ----- #