mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-19 00:01:29 +00:00
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.
This commit is contained in:
parent
65dd6b8376
commit
002174638c
2 changed files with 32 additions and 26 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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 ----- #
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue