mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(observability): resolve the prometheus logger built by a string callback
`success_callback: ["prometheus"]` is the registration the docs show, and it never reaches a callback list. The logger is constructed lazily on the first request and cached in `_in_memory_loggers`, so `get_instance` searching only the callback lists returned None for the whole life of such a proxy, and every pool metric registered and sat at zero. Verified directly: after the lazy construction the cache holds the logger while `get_instance` still returned None. It now returns that same object. An earlier review raised this and I refuted it on the strength of a live proxy showing 103 metric families after the first request. That measurement was about litellm's own metrics, which the logging path records on the instance directly, not through `get_instance`, so it did not cover this path. The finding was correct. The regression test drives the string registration and the lazy construction rather than placing an object on the list, which is what the previous test did and why it passed throughout.
This commit is contained in:
parent
e97fb89404
commit
bd2a9dbdea
2 changed files with 49 additions and 3 deletions
|
|
@ -159,10 +159,16 @@ class PrometheusLogger(CustomLogger):
|
|||
|
||||
@staticmethod
|
||||
def get_instance() -> PrometheusLogger | None:
|
||||
"""The registered PrometheusLogger, however it was registered.
|
||||
"""The live PrometheusLogger, however it was registered.
|
||||
|
||||
``litellm.callbacks`` alone misses ``success_callback: ["prometheus"]``,
|
||||
which shows up as a metric that registers and never leaves zero.
|
||||
Two registrations have to be covered, and neither one subsumes the other.
|
||||
An object placed in ``litellm.callbacks`` is found through the callback
|
||||
lists. A *string* registration -- ``success_callback: ["prometheus"]``,
|
||||
which is what the docs show -- is different: the logger is constructed
|
||||
lazily on the first request and cached in ``_in_memory_loggers``, and
|
||||
nothing ever adds it to a callback list. Searching only the callback
|
||||
lists therefore returns None for the whole life of such a proxy, and
|
||||
every metric published through here registers and never leaves zero.
|
||||
"""
|
||||
import litellm
|
||||
|
||||
|
|
@ -170,6 +176,15 @@ class PrometheusLogger(CustomLogger):
|
|||
for instance in instances:
|
||||
if isinstance(instance, PrometheusLogger):
|
||||
return instance
|
||||
|
||||
# Imported here, not at module scope: litellm_logging imports this module.
|
||||
from litellm.litellm_core_utils.litellm_logging import (
|
||||
_in_memory_loggers, # pyright: ignore[reportPrivateUsage] # the only registry of lazily built loggers
|
||||
)
|
||||
|
||||
for logger in _in_memory_loggers:
|
||||
if isinstance(logger, PrometheusLogger):
|
||||
return logger
|
||||
return None
|
||||
|
||||
def __init__(
|
||||
|
|
|
|||
|
|
@ -246,6 +246,37 @@ def test_get_instance_finds_a_logger_registered_via_success_callback(logger):
|
|||
litellm.success_callback = saved_success
|
||||
|
||||
|
||||
def test_get_instance_finds_a_logger_registered_by_the_string_prometheus():
|
||||
"""success_callback: ["prometheus"] is the form the docs show. The logger is
|
||||
built lazily on the first request and cached in _in_memory_loggers, and
|
||||
nothing adds it to a callback list, so searching the callback lists alone
|
||||
returns None for the life of the proxy and every pool metric stays at zero."""
|
||||
import litellm
|
||||
import litellm.litellm_core_utils.litellm_logging as litellm_logging
|
||||
from litellm.integrations.prometheus import PrometheusLogger
|
||||
|
||||
saved_callbacks = litellm.callbacks
|
||||
saved_success = litellm.success_callback
|
||||
saved_in_memory = list(litellm_logging._in_memory_loggers)
|
||||
try:
|
||||
litellm.callbacks = []
|
||||
litellm.success_callback = ["prometheus"]
|
||||
litellm_logging._in_memory_loggers.clear()
|
||||
|
||||
assert PrometheusLogger.get_instance() is None, "nothing is constructed before the first request"
|
||||
|
||||
constructed = litellm_logging._init_custom_logger_compatible_class(
|
||||
"prometheus", internal_usage_cache=None, llm_router=None
|
||||
)
|
||||
|
||||
assert PrometheusLogger.get_instance() is constructed
|
||||
finally:
|
||||
litellm_logging._in_memory_loggers.clear()
|
||||
litellm_logging._in_memory_loggers.extend(saved_in_memory)
|
||||
litellm.callbacks = saved_callbacks
|
||||
litellm.success_callback = saved_success
|
||||
|
||||
|
||||
def test_get_instance_returns_none_when_prometheus_is_not_registered(logger):
|
||||
import litellm
|
||||
from litellm.integrations.prometheus import PrometheusLogger
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue