From 7875f5b7a75f33e92e2892369fbc5e3a4e6de66e Mon Sep 17 00:00:00 2001 From: hclsys Date: Fri, 18 Sep 2026 00:22:00 +0800 Subject: [PATCH] fix(prometheus): answer /metrics without a trailing-slash redirect app.mount("/metrics", ...) makes starlette answer the bare path with 307 Location: /metrics/. Prometheus and grafana alloy do not follow redirects, so the scrape reads an empty body while up stays 1 - metric loss with nothing logged. Register both spellings as routes. ASGIRoute hands the raw ASGI triple to the metrics app so its streaming, gzip and render-coalescing survive, instead of buffering the whole scrape to build one Response. Signed-off-by: hclsys --- litellm/integrations/prometheus.py | 21 +++++++-- .../prometheus_metrics_endpoint.py | 16 +++++++ .../test_prometheus_metrics_endpoint.py | 46 +++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index fb010ab5886..33efbedca48 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -4553,7 +4553,10 @@ class PrometheusLogger(CustomLogger): from prometheus_client import REGISTRY from litellm._logging import verbose_proxy_logger - from litellm.integrations.prometheus_metrics_endpoint import make_metrics_asgi_app + from litellm.integrations.prometheus_metrics_endpoint import ( + ASGIRoute, + make_metrics_asgi_app, + ) from litellm.proxy.proxy_server import app # Create metrics ASGI app @@ -4566,8 +4569,20 @@ class PrometheusLogger(CustomLogger): else: metrics_app = make_metrics_asgi_app(REGISTRY) - # Mount the metrics app to the app - app.mount("/metrics", metrics_app) + # `app.mount("/metrics", ...)` makes starlette answer the bare `/metrics` + # with `307 Location: /metrics/`. Prometheus and grafana alloy do not + # follow redirects, so the scrape reads an empty body while `up` stays 1 + # -- metric loss with no error anywhere (#30079). + # + # Register both spellings as routes instead. `ASGIRoute` is what keeps + # `metrics_app`'s streaming, gzip and render-coalescing intact -- a plain + # function endpoint would have to buffer the whole scrape to hand back a + # single Response. + # `methods` is left at its default: starlette's `Route` resolves None to + # exactly `["GET"]`, so naming it here would only add a mutable literal. + metrics_route: Final = ASGIRoute(metrics_app) + app.add_route("/metrics", metrics_route) + app.add_route("/metrics/", metrics_route) verbose_proxy_logger.debug("Starting Prometheus Metrics on /metrics (no authentication)") diff --git a/litellm/integrations/prometheus_metrics_endpoint.py b/litellm/integrations/prometheus_metrics_endpoint.py index b41cc13a04f..b356c2ad666 100644 --- a/litellm/integrations/prometheus_metrics_endpoint.py +++ b/litellm/integrations/prometheus_metrics_endpoint.py @@ -79,6 +79,22 @@ def _chunks(body: bytes) -> Iterator[bytes]: return (body[start : start + RESPONSE_CHUNK_SIZE_BYTES] for start in range(0, len(body), RESPONSE_CHUNK_SIZE_BYTES)) +class ASGIRoute: + """Adapt an ASGI app so it can be registered with ``Starlette.add_route``. + + Starlette calls a plain function endpoint as ``f(request)`` and expects a + ``Response`` back. A callable object is handed the raw ASGI triple instead, + which lets the mounted app write its own streaming response rather than + having the whole scrape buffered to build one. + """ + + def __init__(self, app: ASGIApp) -> None: + self._app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + await self._app(scope, receive, send) + + def make_metrics_asgi_app(registry: CollectorRegistry) -> ASGIApp: renderer: Final = CoalescedScrapeRenderer(registry) diff --git a/tests/test_litellm/integrations/test_prometheus_metrics_endpoint.py b/tests/test_litellm/integrations/test_prometheus_metrics_endpoint.py index f0e6495ba22..d45c363b454 100644 --- a/tests/test_litellm/integrations/test_prometheus_metrics_endpoint.py +++ b/tests/test_litellm/integrations/test_prometheus_metrics_endpoint.py @@ -274,3 +274,49 @@ async def test_a_finishing_render_does_not_evict_another_that_is_still_in_flight assert collector.collect_calls == 1, "an unrelated render finishing evicted the render still in flight" for response in responses: assert b"gated_metric" in response.content + + +def test_both_metrics_paths_answer_without_a_redirect(monkeypatch: pytest.MonkeyPatch): + """`app.mount("/metrics", ...)` answers the bare path with 307 Location: + /metrics/. Prometheus and grafana alloy do not follow redirects, so the + scrape reads an empty body while `up` stays 1 (#30079). Both spellings must + return the payload directly. + + Drives the real `_mount_metrics_endpoint` against a stand-in app so the + wiring itself is covered, not just `ASGIRoute` in isolation. + """ + import sys + import types + + from fastapi import FastAPI + from fastapi.testclient import TestClient + + # Import before the stub goes in. `_mount_metrics_endpoint` resolves + # `litellm.proxy.proxy_server` lazily, and standing a fake module in that + # slot while this module graph is still being built changes what the real + # imports underneath it resolve to. + from litellm.integrations.prometheus import PrometheusLogger + + app: Final = FastAPI() + proxy_server: Final = types.ModuleType("litellm.proxy.proxy_server") + proxy_server.app = app # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "litellm.proxy.proxy_server", proxy_server) + + PrometheusLogger._mount_metrics_endpoint() + + assert {route.path for route in app.router.routes if "metrics" in getattr(route, "path", "")} == { + "/metrics", + "/metrics/", + } + + client: Final = TestClient(app, follow_redirects=False) + for path in ("/metrics", "/metrics/"): + response = client.get(path) + # 200 rather than the 307 the mount used to emit. What the registry + # holds is deliberately not asserted -- this shares the process-wide + # default REGISTRY, so another test may legitimately have drained it. + assert response.status_code == 200, path + assert response.headers["content-type"].startswith("text/plain"), path + + # The streaming app still owns the response, so its behaviour survives. + assert client.get("/metrics", headers={"accept-encoding": "gzip"}).headers["content-encoding"] == "gzip"