This commit is contained in:
hcl 2026-09-25 00:32:17 +00:00 • committed by GitHub
commit 2928c6177a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 80 additions and 3 deletions

View file

@ -4571,7 +4571,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
@ -4584,8 +4587,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)")

View file

@ -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)

View file

@ -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"