diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index d5ac694fa24..bd0079cabfb 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -161,11 +161,8 @@ class PrometheusLogger(CustomLogger): def get_instance() -> PrometheusLogger | None: """The registered PrometheusLogger, however it was registered. - Searching ``litellm.callbacks`` alone misses the equally supported - ``litellm_settings.success_callback: ["prometheus"]``, which lands the - logger on the success-callback lists instead. Callers of this method - publish metrics from outside the request hooks, so a miss shows up as a - metric that registers and then never leaves zero. + ``litellm.callbacks`` alone misses ``success_callback: ["prometheus"]``, + which shows up as a metric that registers and never leaves zero. """ import litellm diff --git a/litellm/proxy/db/db_pool_metrics.py b/litellm/proxy/db/db_pool_metrics.py index 570b875ba89..0b9fd232337 100644 --- a/litellm/proxy/db/db_pool_metrics.py +++ b/litellm/proxy/db/db_pool_metrics.py @@ -1,15 +1,7 @@ """Bridges the Prisma query engine's connection-pool counters into Prometheus. -The query engine tracks pool occupancy and, separately, how long a query spent -waiting for a pool slot versus executing against the database. That split is the -only way to tell "the database is slow" apart from "we ran out of connections", -so it is sampled here and re-published under ``litellm_db_pool_*`` names. - -Sampling is driven from the DB call path rather than from a scheduled job: an -exporter that runs on a timer stops reporting exactly when the event loop is -saturated, which is the window an operator most needs. Requests are what stall -during pool exhaustion, and they keep arriving, so a throttled sample taken -alongside real DB work stays alive through the incident. +Sampled from the DB call path, not a timer: an exporter on a timer stops +reporting exactly when the event loop is saturated. """ from __future__ import annotations @@ -40,26 +32,17 @@ _QUERY_DURATION_HISTOGRAM_KEY: Final = "prisma_datasource_queries_duration_histo class SupportsPoolSample(Protocol): - """The one method this module needs from a Prisma client wrapper. - - Deliberately not ``runtime_checkable``: ``PrismaWrapper`` resolves most of - its surface through an instance-level ``__getattr__``, which an - ``isinstance`` check against a protocol does not see, so a runtime check - here would reject the very object it exists to accept. - """ + """Not ``runtime_checkable``: ``PrismaWrapper`` resolves through an + instance-level ``__getattr__``, which ``isinstance`` does not see.""" async def get_pool_sample(self) -> DBPoolSample: ... @dataclass(frozen=True, slots=True) class DBPoolSample: - """One reading of the engine's pool counters. - - ``max_connections`` is derived as ``busy + idle`` rather than parsed out of - ``DATABASE_URL``. The engine reports ``idle`` as remaining capacity, so the - sum is the configured ``connection_limit``, and deriving it here keeps the - database credentials out of this path entirely. - """ + """One reading of the engine's pool counters. ``max_connections`` is + ``busy + idle`` because the engine reports idle as remaining capacity, which + also keeps ``DATABASE_URL`` out of this path.""" busy_connections: float idle_connections: float @@ -77,13 +60,8 @@ class DBPoolSample: @dataclass(frozen=True, slots=True) class DBPoolMetricsUpdate: - """A sample plus the cumulative movement since the previous sample. - - The engine reports totals; Prometheus counters take increments. The deltas - are the difference between consecutive samples, and are omitted entirely - when the engine's totals move backwards, which happens when the query engine - restarts and its counters reset. - """ + """A sample plus the movement since the previous one. Deltas are zeroed when + the engine's totals move backwards, which means it restarted.""" sample: DBPoolSample pending_acquirers: float @@ -123,12 +101,8 @@ def parse_pool_sample(metrics: Metrics) -> DBPoolSample: class DBPoolMetricsSampler: - """Throttled reader of the engine's pool counters. - - One instance per process. ``maybe_sample`` is safe to call on every DB - operation: it returns ``None`` without touching the engine until - ``min_interval_seconds`` has elapsed since the last reading. - """ + """Throttled reader of the engine's pool counters. Safe to call on every DB + operation: it touches nothing until ``min_interval_seconds`` has elapsed.""" def __init__( self, @@ -149,19 +123,11 @@ class DBPoolMetricsSampler: return (self._monotonic() - self._last_sampled_at) >= self._min_interval_seconds async def maybe_sample(self, resolve_client: Callable[[], SupportsPoolSample | None]) -> DBPoolMetricsUpdate | None: - """Read the engine's counters, or return ``None`` if not yet due. + """Read the engine's counters, or ``None`` if not yet due. - The interval is consumed as soon as an attempt starts, before the client - is resolved, so a deployment that has no client to sample throttles the - same as one that does rather than retrying on every database call. - - Never raises, and never blocks for long: a wedged query engine costs a - bounded wait. A pool sample is diagnostic, and an engine that cannot - answer one is already the subject of a louder alarm. - - The client is resolved through a callable rather than passed in so that - resolution, which reads proxy module state, happens under the same - throttle and the same exception guard as the read itself. + The interval is consumed before the client is resolved, so a deployment + with nothing to sample throttles the same as one that does. Never raises + and never blocks for long; a pool sample is diagnostic. """ if not self.is_due(): return None @@ -185,16 +151,10 @@ class DBPoolMetricsSampler: def _corrected_pending_acquirers(self, sample: DBPoolSample) -> float: """The engine's waiter gauge, corrected for waiters that timed out. - ``prisma_client_queries_wait`` is decremented when a waiter acquires a - connection but not when it gives up, so every pool timeout latches the - gauge one higher for the life of the process. Observed directly against - a live engine at ``connection_limit=2``: after two bursts that produced - seven P2024s, a fully drained pool still reported seven waiters. - - Free capacity is proof that nobody is queued, so any reading taken while - a connection is idle is exactly the accumulated latch, and subtracting it - recovers the real depth. The baseline re-arms on every idle sample, so - repeated exhaustion stays accurate rather than drifting further each time. + ``prisma_client_queries_wait`` decrements on acquisition but not on + timeout, so each P2024 latches it one higher for the life of the process. + Free capacity proves nobody is queued, so an idle reading is exactly the + accumulated latch; subtracting it recovers the real depth. """ if sample.idle_connections > 0: self._pending_baseline = sample.pending_acquirers diff --git a/litellm/proxy/db/log_db_metrics.py b/litellm/proxy/db/log_db_metrics.py index 1536af4cb0d..3074f435d43 100644 --- a/litellm/proxy/db/log_db_metrics.py +++ b/litellm/proxy/db/log_db_metrics.py @@ -30,33 +30,31 @@ _pool_metrics_sampler: Final = DBPoolMetricsSampler() def _prometheus_logger() -> "PrometheusLogger | None": - """The active PrometheusLogger, or None when prometheus is not configured. - - Imports lazily. Bare ``import litellm`` does not load the prometheus - integration, so hoisting this would make every proxy pay for a module that - only prometheus deployments use. - """ + """The active PrometheusLogger, or None. Imported lazily so a proxy without + prometheus never loads the integration.""" from litellm.integrations.prometheus import PrometheusLogger return PrometheusLogger.get_instance() def _resolve_pool_client() -> "SupportsPoolSample | None": + """The client to sample, or None when there is nothing to sample for. + + Checks for a metrics consumer before returning a client, so a proxy without + prometheus never pays for the engine read. + """ from litellm.proxy.proxy_server import prisma_client - return None if prisma_client is None else prisma_client.db + if prisma_client is None or _prometheus_logger() is None: + return None + return prisma_client.db async def _sample_db_pool_metrics() -> None: - """Publish a throttled reading of the connection pool, if one is due. + """Publish a throttled pool reading, if one is due. - Runs alongside real database work rather than on a timer: a scheduled - exporter goes quiet exactly when the event loop is saturated, which is the - window this metric exists to cover. - - Every step is inside the guard, including resolving the client and locating - the logger. This runs off a database call that has already succeeded, so - nothing here may turn a working query into a failed one. + Every step is inside the guard: this runs off a database call that already + succeeded, so nothing here may turn a working query into a failed one. """ try: update: Final = await _pool_metrics_sampler.maybe_sample(_resolve_pool_client) @@ -70,16 +68,11 @@ async def _sample_db_pool_metrics() -> None: def _record_db_pool_timeout_if_exhausted(e: Exception) -> None: - """Count a pool exhaustion once, without ever displacing the error itself. + """Count a pool exhaustion once, without displacing the error itself. - Decorated database helpers nest: ``get_object_permission`` is called from - inside ``get_key_object``, and both carry this decorator. One P2024 therefore - passes through several ``except`` blocks on its way up, so the exception is - marked the first time it is counted and skipped afterwards. - - The caller re-raises after this returns. Anything escaping here would replace - a P2024 with a metrics error, during the incident this counter exists to - record. + Decorated helpers nest (``get_object_permission`` inside ``get_key_object``), + so one P2024 passes through several ``except`` blocks; the exception is + marked the first time it is counted. """ try: if not PrismaDBExceptionHandler.is_connection_pool_timeout_error(e): diff --git a/litellm/proxy/db/prisma_client.py b/litellm/proxy/db/prisma_client.py index 00f92ddd94d..8d5876c3046 100644 --- a/litellm/proxy/db/prisma_client.py +++ b/litellm/proxy/db/prisma_client.py @@ -800,14 +800,9 @@ class PrismaWrapper: return seconds_left > self.TOKEN_REFRESH_BUFFER_SECONDS async def get_pool_sample(self) -> DBPoolSample: - """This connection pool's current occupancy, as the query engine sees it. - - Declared rather than left to ``__getattr__``, which resolves through an - untyped delegation and so cannot satisfy a typed caller, and which also - makes the underlying client's identity easy to lose track of. Keeping - the engine's raw counter names behind this boundary means the metrics - layer never has to know them. - """ + """This pool's occupancy, as the query engine sees it. Declared rather + than left to ``__getattr__`` so a typed caller can reach it, and so the + engine's counter names stay out of the metrics layer.""" return parse_pool_sample(await self._original_prisma.get_metrics()) def __getattr__(self, name: str): diff --git a/tests/test_litellm/proxy/db/test_log_db_metrics.py b/tests/test_litellm/proxy/db/test_log_db_metrics.py index 84cdcf1ced6..2fcf6815e9c 100644 --- a/tests/test_litellm/proxy/db/test_log_db_metrics.py +++ b/tests/test_litellm/proxy/db/test_log_db_metrics.py @@ -160,3 +160,29 @@ async def test_two_separate_pool_timeouts_are_counted_separately(): await failing(table_name="x") assert logger.record_db_pool_timeout.call_count == 2 + + +@pytest.mark.asyncio +async def test_a_proxy_without_prometheus_never_reads_the_query_engine(): + """The sample is worthless with no collector configured, so the engine read + must be skipped rather than taken and discarded every interval.""" + from litellm.proxy.db.log_db_metrics import _pool_metrics_sampler, _sample_db_pool_metrics + + reads = [] + prisma_client = MagicMock() + + async def _sample(): + reads.append(1) + raise AssertionError("the engine must not be read when prometheus is off") + + prisma_client.db.get_pool_sample = _sample + _pool_metrics_sampler._last_sampled_at = None + + with ( + patch("litellm.proxy.db.log_db_metrics._prometheus_logger", return_value=None), + patch("litellm.proxy.proxy_server.prisma_client", prisma_client), + ): + await _sample_db_pool_metrics() + + assert reads == [] + assert _pool_metrics_sampler.is_due() is False, "the interval must still be consumed"