diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index c7c9d758c76..2cbdf36cfdd 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -756,8 +756,10 @@ class PrometheusLogger(CustomLogger): self.litellm_cronjob_lock_acquisitions_total = self._counter_factory( name="litellm_cronjob_lock_acquisitions_total", documentation=( - "Attempts to take the single-owner lock for a cron job. result is one of acquired, " - "not_acquired, no_redis; no_redis means no Redis is configured, so no pod can be elected" + "Attempts to take the single-owner lock for a cron job. result is one of " + f"{', '.join(r.value for r in LockAttemptResult)}; no_redis means no Redis is " + "configured, so no pod can be elected, and error means the attempt itself failed " + "rather than losing the election" ), labelnames=("cronjob_id", "result"), ) @@ -3211,9 +3213,9 @@ class PrometheusLogger(CustomLogger): if run.items_processed: self.litellm_scheduled_job_items_processed_total.labels(job_name=run.job_name).inc(run.items_processed) - def record_cronjob_lock_attempt(self, cronjob_id: str, result: str) -> None: + def record_cronjob_lock_attempt(self, cronjob_id: str, result: LockAttemptResult) -> None: """Publish the outcome of one single-owner lock attempt.""" - self.litellm_cronjob_lock_acquisitions_total.labels(cronjob_id=cronjob_id, result=result).inc() + self.litellm_cronjob_lock_acquisitions_total.labels(cronjob_id=cronjob_id, result=result.value).inc() def record_db_pool_sample(self, update: DBPoolMetricsUpdate) -> None: """Publish one reading of this worker's Prisma connection pool.""" diff --git a/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py b/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py index 0627fa66c16..33b71627894 100644 --- a/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py +++ b/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py @@ -7,6 +7,7 @@ from litellm._uuid import uuid from litellm.caching.redis_cache import RedisCache from litellm.constants import DEFAULT_CRON_JOB_LOCK_TTL_SECONDS from litellm.proxy.db.db_transaction_queue.base_update_queue import service_logger_obj +from litellm.types.integrations.prometheus import LockAttemptResult from litellm.types.services import ServiceTypes if TYPE_CHECKING: @@ -15,7 +16,7 @@ else: ProxyLogging = Any -def _record_lock_attempt(cronjob_id: str, result: str) -> None: +def _record_lock_attempt(cronjob_id: str, result: LockAttemptResult) -> None: """Publish the outcome of one single-owner lock attempt. Each result is a distinct operational state: no Redis means no pod can ever @@ -70,15 +71,15 @@ end """ if self.redis_cache is None: verbose_proxy_logger.debug("redis_cache is None, skipping acquire_lock") - _record_lock_attempt(cronjob_id, "no_redis") + _record_lock_attempt(cronjob_id, LockAttemptResult.NO_REDIS) return None try: acquired: Final = await self._attempt_acquire_lock(cronjob_id, ttl=ttl, allow_reentrant=allow_reentrant) except Exception as e: verbose_proxy_logger.error("Error acquiring Redis lock for %s: %s", cronjob_id, e) - _record_lock_attempt(cronjob_id, "error") + _record_lock_attempt(cronjob_id, LockAttemptResult.ERROR) return False - _record_lock_attempt(cronjob_id, "acquired" if acquired else "not_acquired") + _record_lock_attempt(cronjob_id, LockAttemptResult.ACQUIRED if acquired else LockAttemptResult.NOT_ACQUIRED) return acquired async def _attempt_acquire_lock( diff --git a/litellm/types/integrations/prometheus.py b/litellm/types/integrations/prometheus.py index fe58af24662..9e745aa7a31 100644 --- a/litellm/types/integrations/prometheus.py +++ b/litellm/types/integrations/prometheus.py @@ -8,6 +8,18 @@ from typing import Any, ClassVar, Final, Literal import litellm +class LockAttemptResult(str, Enum): + """Closed set of outcomes for one cron-job lock attempt, so the `result` + label stays bounded and the metric documentation cannot drift from it.""" + + ACQUIRED = "acquired" + NOT_ACQUIRED = "not_acquired" + # No Redis is configured, so no pod can ever be elected. + NO_REDIS = "no_redis" + # The attempt itself failed, rather than losing the election. + ERROR = "error" + + def _sanitize_prometheus_label_name(label: str) -> str: """ Sanitize a label name to comply with Prometheus label name requirements. diff --git a/tests/test_litellm/integrations/test_prometheus_cronjob_lock_metrics.py b/tests/test_litellm/integrations/test_prometheus_cronjob_lock_metrics.py new file mode 100644 index 00000000000..ffc818b7d06 --- /dev/null +++ b/tests/test_litellm/integrations/test_prometheus_cronjob_lock_metrics.py @@ -0,0 +1,37 @@ +"""The cron-lock metric's advertised result values against the ones actually emitted. + +A consumer builds alerts from the documented set, so a value the code can emit +but the documentation omits is silently dropped from their queries. +""" + +import os +import re +import sys + +sys.path.insert(0, os.path.abspath("../../..")) + +import litellm +from litellm.integrations.prometheus import PrometheusLogger +from litellm.types.integrations.prometheus import LockAttemptResult + + +def test_the_lock_metric_documents_every_result_it_can_emit(monkeypatch): + monkeypatch.setattr(litellm, "callbacks", []) + monkeypatch.setattr(litellm, "success_callback", []) + from prometheus_client import REGISTRY + + for collector in list(REGISTRY._collector_to_names.keys()): + try: + REGISTRY.unregister(collector) + except Exception: + pass + + documentation = PrometheusLogger().litellm_cronjob_lock_acquisitions_total._documentation + + # The advertised set only, not the prose that follows it, so a value merely + # mentioned in passing does not count as documented. + advertised = re.search(r"result is one of ([^;]+);", documentation) + assert advertised is not None, f"no advertised result list in: {documentation}" + documented = {value.strip() for value in advertised.group(1).split(",")} + + assert documented == {result.value for result in LockAttemptResult}