diff --git a/litellm/integrations/prometheus_services.py b/litellm/integrations/prometheus_services.py index 52b32c12764..55ce758ece6 100644 --- a/litellm/integrations/prometheus_services.py +++ b/litellm/integrations/prometheus_services.py @@ -105,13 +105,11 @@ class PrometheusServicesLogger: return metrics def is_metric_registered(self, metric_name) -> bool: - # Use registry's collector dict to avoid REGISTRY.collect(), which is O(all metrics) - # and causes severe perf regression when a new Router (and thus this logger) is - # created per request (e.g. hierarchical router_settings from DB). + # Use _names_to_collectors (O(1)) instead of REGISTRY.collect() (O(n)) to avoid + # perf regression when a new Router is created per request (e.g. router_settings in DB). names_to_collectors = getattr(self.REGISTRY, "_names_to_collectors", None) if names_to_collectors is not None: return metric_name in names_to_collectors - # Fallback for custom registries that don't expose _names_to_collectors for metric in self.REGISTRY.collect(): if metric_name == metric.name: return True diff --git a/tests/test_litellm/integrations/test_prometheus_services.py b/tests/test_litellm/integrations/test_prometheus_services.py index 3b2b806084b..ff80d7d9f8b 100644 --- a/tests/test_litellm/integrations/test_prometheus_services.py +++ b/tests/test_litellm/integrations/test_prometheus_services.py @@ -19,15 +19,9 @@ sys.path.insert( def test_is_metric_registered_does_not_use_registry_collect(): - """ - Validates the perf regression fix: is_metric_registered() must NOT call REGISTRY.collect() - when _names_to_collectors is available. collect() is O(all metrics) and causes severe - slowdown when a new Router (and thus PrometheusServicesLogger) is created per request - (e.g. hierarchical router_settings from DB). See GitHub issue #19921. - """ + """is_metric_registered() must use _names_to_collectors, not REGISTRY.collect() (perf; #19921).""" from prometheus_client import CollectorRegistry, Counter, Histogram - # Simulate a proxy with many metrics (e.g. router, redis, postgres, llm providers) registry = CollectorRegistry() for i in range(80): Counter( @@ -46,7 +40,6 @@ def test_is_metric_registered_does_not_use_registry_collect(): pl = PrometheusServicesLogger() pl.REGISTRY = registry - # Mock collect() - the bug is that is_metric_registered() calls it (expensive) original_collect = registry.collect collect_called = [] @@ -56,8 +49,7 @@ def test_is_metric_registered_does_not_use_registry_collect(): registry.collect = track_collect - # Simulate Router init: many is_metric_registered() calls; time to show fix vs no-fix difference - n_calls = 30 * 2 # 30 iters × 2 names + n_calls = 30 * 2 start = time.perf_counter() for _ in range(30): pl.is_metric_registered("litellm_service_0_latency") @@ -67,27 +59,19 @@ def test_is_metric_registered_does_not_use_registry_collect(): per_call_us = (elapsed_s / n_calls) * 1_000_000 if n_calls else 0 n_collect = len(collect_called) - # Clear latency output (visible with pytest -s) path = "slow (REGISTRY.collect)" if n_collect else "fast (_names_to_collectors)" print( - f"\n is_metric_registered latency: {elapsed_ms:.2f} ms total | " - f"{per_call_us:.1f} µs/call | {n_calls} calls | {n_collect} collect() invocations | {path}\n" + f"\n is_metric_registered: {elapsed_ms:.2f} ms total | " + f"{per_call_us:.1f} µs/call | {n_calls} calls | {n_collect} collect() | {path}\n" ) - # With fix: _names_to_collectors is used, collect() must not be called, elapsed ~ms - # With fix commented out: collect() every time → slow (e.g. 50–500ms+), test fails assert n_collect == 0, ( f"is_metric_registered() must not use REGISTRY.collect() when _names_to_collectors " - f"is available (perf regression).\n" - f" Latency: {elapsed_ms:.2f} ms total, {per_call_us:.1f} µs per call, {n_calls} calls.\n" - f" collect() was called {n_collect} times (slow path).\n" - f" With fix: typically <1 ms total, 0 collect() calls. " - "Uncomment the _names_to_collectors branch in prometheus_services.is_metric_registered()" + f"is available. Latency: {elapsed_ms:.2f} ms, {per_call_us:.1f} µs/call, {n_calls} calls, " + f"collect() called {n_collect} times." ) - # With fix, assert we're in the fast path (optional; keeps regression visible) assert elapsed_s < 0.05, ( - f"is_metric_registered() took {elapsed_ms:.2f} ms for {n_calls} calls " - f"({per_call_us:.1f} µs/call); expected <50 ms with _names_to_collectors." + f"is_metric_registered() took {elapsed_ms:.2f} ms for {n_calls} calls; expected <50 ms." )