fix(gateway): keep the Prometheus /metrics Mount in the gateway route trim - #32317
Conversation
… trim Resolves LIT-4236
Greptile SummaryThis PR fixes a regression in the componentized gateway entrypoint where the Prometheus
Confidence Score: 4/5The fix is isolated to the gateway route-trim predicate and allowlist; no production request path, auth layer, or database access is touched. The core change is a one-line predicate update and a new frozenset constant — both easy to verify and matching the pattern already established for the backend. The new tests exercise the regression directly and the module-level test setup is carefully documented. The only finding is a stale docstring on a helper that is not called from any new code paths. No files require special attention; test_component_allowlists.py has a minor docstring inconsistency in _component_paths but it does not affect test correctness.
|
| Filename | Overview |
|---|---|
| gateway/main.py | _is_gateway_route now consults GATEWAY_MOUNT_PATHS for Starlette Mounts instead of returning False unconditionally; the logic is correct and the fix is minimal |
| gateway/routes/allowlist.py | Adds GATEWAY_MOUNT_PATHS frozenset containing "/metrics"; also fixes a missing trailing comma on "/watsonx". Structure mirrors the existing BACKEND_MOUNT_PATHS pattern. |
| tests/test_litellm/proxy/test_component_allowlists.py | Adds four new Mount-specific tests and complex module-level setup to import _is_gateway_route safely; the _component_paths docstring is now slightly stale relative to the full gateway predicate |
Comments Outside Diff (1)
-
tests/test_litellm/proxy/test_component_allowlists.py, line 89-100 (link)_component_pathsdocstring diverged from the gateway predicateThe docstring says this helper reproduces
gateway.main._is_gateway_route, but after this PR that predicate also handles Mounts viaGATEWAY_MOUNT_PATHS. The helper still skips every Mount unconditionally (if isinstance(r, Mount): continue), so it only reproduces the non-Mount half of_is_gateway_route. The helper is only called intest_gateway_plus_backend_covers_full_app, which intentionally excludes Mounts from its union-coverage check, so behaviour is correct — but the docstring implies full fidelity and will mislead future readers who try to extend this test for Mounts.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "fix(gateway): keep the Prometheus /metri..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a 404 regression on
Confidence Score: 5/5Safe to merge — the change is minimal, well-scoped, and backed by a clear before/after demonstration and dedicated regression tests. The two-line predicate change in No files require special attention; the one style nit is in the test helper docstring.
|
| Filename | Overview |
|---|---|
| gateway/main.py | Core fix: _is_gateway_route now consults GATEWAY_MOUNT_PATHS for Mount instances instead of unconditionally returning False, allowing the Prometheus /metrics Mount to survive the gateway route trim. |
| gateway/routes/allowlist.py | Adds GATEWAY_MOUNT_PATHS = frozenset({"/metrics"}) for Mount-specific allowlisting; also fixes a missing trailing comma after /watsonx in GATEWAY_PATH_PREFIXES. |
| tests/test_litellm/proxy/test_component_allowlists.py | Adds four new tests covering Mount-specific gateway behaviour; imports _is_gateway_route directly with careful env/lifespan teardown. The _component_paths docstring now diverges from the real predicate it claims to reproduce. |
Reviews (2): Last reviewed commit: "fix(gateway): keep the Prometheus /metri..." | Re-trigger Greptile
| def _component_paths(routes, exact_paths, path_prefixes) -> set[str]: | ||
| """Reproduce ``gateway.main._is_gateway_route`` / ``backend.main._is_backend_route``.""" |
There was a problem hiding this comment.
The
_component_paths docstring still claims to reproduce gateway.main._is_gateway_route, but since the fix it no longer does: _is_gateway_route now returns path in GATEWAY_MOUNT_PATHS for Mount instances, while _component_paths skips every Mount unconditionally. A future developer leaning on this docstring to understand the predicate will get a subtly wrong picture.
| def _component_paths(routes, exact_paths, path_prefixes) -> set[str]: | |
| """Reproduce ``gateway.main._is_gateway_route`` / ``backend.main._is_backend_route``.""" | |
| def _component_paths(routes, exact_paths, path_prefixes) -> set[str]: | |
| """Reproduce the non-Mount portion of ``gateway.main._is_gateway_route`` / ``backend.main._is_backend_route``. | |
| Mount handling is intentionally omitted here; it is covered by the dedicated | |
| Mount tests that import ``_is_gateway_route`` directly. | |
| """ |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
|
|
CI status: 122 of 123 checks pass. The one failure is ci/circleci: llm_responses_api_testing, which runs tests/llm_responses_api_testing/** against real provider APIs (Azure, Google AI Studio, Anthropic). That suite is disjoint from this diff, which only touches gateway/main.py, gateway/routes/allowlist.py, and tests/test_litellm/proxy/test_component_allowlists.py; nothing under litellm/ changed, and gateway/ is not imported by any code that suite exercises I ran the suite locally on both this branch and the base litellm_internal_staging with identical results (the same provider-credential-gated tests fail identically on both within seconds, which is local cred noise, not a behavioral delta), and the same job currently passes on other open PRs against the same base. This looks like the known per-run flakiness of the real-API CircleCI jobs. I cannot re-trigger CircleCI from here; a maintainer clicking Rerun failed on workflow https://circleci.com/gh/BerriAI/litellm/2004033 should clear it |
Relevant issues
Fixes #30291
Linear ticket
Resolves LIT-4236
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Both runs use the componentized gateway entrypoint with the same config, no load balancer or ingress in the path (this is what a
kubectl port-forwardof the gateway service sees):Before the fix (base
litellm_internal_staging),/metrics404s while/healthworks, which is the exact symptom from the issue:Control on the same base commit and config: the monolithic entrypoint (
uvicorn litellm.proxy.proxy_server:app) serves/metricswith HTTP 200, confirming the regression is specific to the gateway trimAfter the fix, on the same gateway entrypoint,
/metricsserves the Prometheus exposition and the management/UI surfaces stay trimmed:End to end with a real provider call through the fixed gateway, the per-user request metrics the endpoint exists for show up on the scrape:
Independent e2e run
Reproduced independently on the componentized gateway entrypoint with the config and commands above. BEFORE is the merge-base of this branch with
litellm_internal_staging(git merge-base origin/litellm_internal_staging origin/litellm_gateway_metrics_mount, commit5b93ba0), where_is_gateway_routedrops every Mount; AFTER is this branch (ad0a456), whereGATEWAY_MOUNT_PATHSkeeps/metrics. Both ranuvicorn gateway.main:app, BEFORE on port 4000 and AFTER on port 4001Before and after curl sequence against the running gateway:
BEFORE, on the merge-base,
/metricsis 404 on the gateway even though/health/livelinessis 200:AFTER, on this branch,
/metricsreturns 200 with the Prometheus exposition text (# HELP python_gc_...), and/uiplusPOST /key/generatestay 404:The regression tests in
tests/test_litellm/proxy/test_component_allowlists.pypass on the branch, 9 passed:One environment note for anyone reproducing this locally: FastAPI 0.137+ changed
app.include_routerto attach a lazy_IncludedRouterobject that has no.path, so on those versions the lifespan trim drops the whole included-route surface (health, models, chat, everything), not just Mounts, and the fixed/metricscannot even be observed because nothing else is served. This run pinnedfastapi==0.136.3, the lower bound of thefastapi>=0.136.3,<1.0range inpyproject.toml, whereinclude_routerstill flattens routes so the trim behaves as designed. A reasonable follow-up would be to make_is_gateway_routerecurse into_IncludedRouterso the gateway keeps working on newer FastAPI, since the resolved lockfile currently picks 0.139.0Type
🐛 Bug Fix
Changes
The componentized gateway entrypoint (
gateway/main.py) trims the shared proxy route table at startup. Its_is_gateway_routepredicate rejected every starletteMountbefore consulting the allowlist, but Prometheus registers/metricsas a Mount viaapp.mount("/metrics", make_asgi_app())(litellm/integrations/prometheus.py). The trim runs inside the wrapped lifespan after the proxy's startup hooks have mounted/metrics, so the route was created and then deleted, and the gateway returned 404 even though/metricsis listed ingateway/routes/allowlist.pyand the helm ingress routes/metricsto the gateway serviceThe fix mirrors the pattern the backend component already uses for its
/swaggerMount (BACKEND_MOUNT_PATHS):gateway/routes/allowlist.pygains aGATEWAY_MOUNT_PATHSfrozenset containing/metrics, and the Mount branch of_is_gateway_routenow keeps Mounts whose path is in that set instead of returning False unconditionally. UI static mounts (/ui,/_next,/litellm-asset-prefix/_next) and/swaggerremain trimmedTests in
tests/test_litellm/proxy/test_component_allowlists.pypreviously excluded Mounts from the union-coverage assertion, which is why CI never saw this. The new tests import the real_is_gateway_route(restoring the lifespan wrapper and DATABASE_* env vars its import mutates) and assert that a production-shaped/metricsMount survives the trim, that UI and swagger Mounts are still dropped, and that every Mount on the app is assigned to the gateway, the backend, or the UI container. The core regression test fails on the pre-fix codeLink to Devin session: https://app.devin.ai/sessions/90969e1591404372a4a9f57fcad4e023