mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-07 07:58:09 +00:00
fix(proxy): bound Prometheus route labels for unmatched paths
Unmatched request paths (scanner traffic, 404s) were passed verbatim through normalize_request_route() into Prometheus route labels, causing unbounded metric cardinality. Add resolve_request_route_for_metrics() that prefers the FastAPI route template, falls back to regex normalization for known dynamic APIs, keeps a small set of literal static routes, and maps everything else to /unmatched. Use it when seeding request_route during auth and on auth failures so failed-request metrics cannot be driven by arbitrary paths. Fixes #35666
This commit is contained in:
parent
f6b9518ddb
commit
53acca8a4b
6 changed files with 140 additions and 6 deletions
|
|
@ -15,7 +15,7 @@ from litellm.proxy._types import (
|
|||
ProxyException,
|
||||
UserAPIKeyAuth,
|
||||
)
|
||||
from litellm.proxy.auth.auth_utils import _get_request_ip_address
|
||||
from litellm.proxy.auth.auth_utils import _get_request_ip_address, resolve_request_route_for_metrics
|
||||
from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler
|
||||
from litellm.types.services import ServiceTypes
|
||||
|
||||
|
|
@ -86,7 +86,7 @@ class UserAPIKeyAuthExceptionHandler:
|
|||
token="failed-to-connect-to-db",
|
||||
user_id=DB_UNAVAILABLE_FALLBACK_USER_ID,
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
request_route=route,
|
||||
request_route=resolve_request_route_for_metrics(request, route),
|
||||
)
|
||||
else:
|
||||
# raise the exception to the caller
|
||||
|
|
@ -108,7 +108,7 @@ class UserAPIKeyAuthExceptionHandler:
|
|||
# so the handler is side-effect-free for the caller's identity object.
|
||||
user_api_key_dict = resolved_identity.model_copy() if resolved_identity is not None else UserAPIKeyAuth()
|
||||
user_api_key_dict.parent_otel_span = parent_otel_span
|
||||
user_api_key_dict.request_route = route
|
||||
user_api_key_dict.request_route = resolve_request_route_for_metrics(request, route)
|
||||
user_api_key_dict.api_key = user_api_key_dict.api_key or UserAPIKeyAuth(api_key=api_key).api_key
|
||||
|
||||
# Stamp identity onto the request's server span now, before the request
|
||||
|
|
|
|||
|
|
@ -767,6 +767,57 @@ def normalize_request_route(route: str) -> str:
|
|||
return route
|
||||
|
||||
|
||||
UNMATCHED_REQUEST_ROUTE: Final = "/unmatched"
|
||||
|
||||
# Literal proxy routes safe to emit when FastAPI's route template is unavailable
|
||||
# (e.g. auth failure on an unmatched scope). Dynamic segments must never appear
|
||||
# here — those are covered by get_request_route_template() or normalize_request_route().
|
||||
_LITERAL_STATIC_PROXY_ROUTES: Final = frozenset(
|
||||
{
|
||||
"/",
|
||||
"/chat/completions",
|
||||
"/v1/chat/completions",
|
||||
"/v1/completions",
|
||||
"/v1/embeddings",
|
||||
"/v1/messages",
|
||||
"/v1/models",
|
||||
"/v1/responses",
|
||||
"/health",
|
||||
"/health/liveliness",
|
||||
"/health/readiness",
|
||||
"/metrics",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def resolve_request_route_for_metrics(
|
||||
request: Request | None,
|
||||
route: str,
|
||||
) -> str:
|
||||
"""
|
||||
Resolve a bounded-vocabulary route label for Prometheus metrics.
|
||||
|
||||
Priority:
|
||||
1. FastAPI matched route template (e.g. ``/v1/responses/{response_id}``)
|
||||
2. Regex normalization for known dynamic API paths
|
||||
3. Literal route when it is a known static proxy endpoint
|
||||
4. ``/unmatched`` for scanner traffic and other unknown paths
|
||||
"""
|
||||
if request is not None:
|
||||
template = get_request_route_template(request)
|
||||
if template:
|
||||
return template
|
||||
|
||||
normalized = normalize_request_route(route)
|
||||
if normalized != route:
|
||||
return normalized
|
||||
|
||||
if route in _LITERAL_STATIC_PROXY_ROUTES:
|
||||
return route
|
||||
|
||||
return UNMATCHED_REQUEST_ROUTE
|
||||
|
||||
|
||||
async def check_if_request_size_is_safe(request: Request) -> bool:
|
||||
"""
|
||||
Enterprise Only:
|
||||
|
|
|
|||
|
|
@ -65,6 +65,7 @@ from litellm.proxy.auth.auth_utils import (
|
|||
iter_request_fallback_targets,
|
||||
normalize_request_route,
|
||||
pre_db_read_auth_checks,
|
||||
resolve_request_route_for_metrics,
|
||||
route_in_additonal_public_routes,
|
||||
)
|
||||
from litellm.proxy.auth.handle_jwt import JWTAuthManager, JWTHandler
|
||||
|
|
@ -2670,7 +2671,7 @@ async def user_api_key_auth(
|
|||
user_api_key_auth_obj,
|
||||
model=request_data.get("model") if isinstance(request_data, dict) else None,
|
||||
)
|
||||
user_api_key_auth_obj.request_route = normalize_request_route(route)
|
||||
user_api_key_auth_obj.request_route = resolve_request_route_for_metrics(request, route)
|
||||
|
||||
if body_parse_exception is not None:
|
||||
raise body_parse_exception
|
||||
|
|
|
|||
|
|
@ -425,7 +425,7 @@ def test_route_normalization_for_sub_routes():
|
|||
|
||||
def test_route_normalization_preserves_static_routes():
|
||||
"""Test that static routes are not affected by normalization"""
|
||||
from litellm.proxy.auth.auth_utils import normalize_request_route
|
||||
from litellm.proxy.auth.auth_utils import normalize_request_route, resolve_request_route_for_metrics
|
||||
|
||||
static_routes = [
|
||||
"/chat/completions",
|
||||
|
|
@ -442,10 +442,28 @@ def test_route_normalization_preserves_static_routes():
|
|||
assert (
|
||||
normalized == route
|
||||
), f"Static route should not be modified: {route} -> {normalized}"
|
||||
assert (
|
||||
resolve_request_route_for_metrics(None, route) == route
|
||||
), f"Static route should be preserved for metrics: {route}"
|
||||
|
||||
print(f"✅ {len(static_routes)} static routes preserved")
|
||||
|
||||
|
||||
def test_unmatched_scanner_routes_collapse_for_metrics():
|
||||
"""Unknown paths must not create unbounded Prometheus route labels."""
|
||||
from litellm.proxy.auth.auth_utils import (
|
||||
UNMATCHED_REQUEST_ROUTE,
|
||||
resolve_request_route_for_metrics,
|
||||
)
|
||||
|
||||
scanner_paths = [
|
||||
"/files/pRzAdaV1.php",
|
||||
"/actuator/metrics",
|
||||
"/scanner/path-1",
|
||||
]
|
||||
assert all(resolve_request_route_for_metrics(None, p) == UNMATCHED_REQUEST_ROUTE for p in scanner_paths)
|
||||
|
||||
|
||||
def test_route_normalization_other_dynamic_apis():
|
||||
"""Test normalization for other OpenAI-compatible APIs with dynamic IDs"""
|
||||
from litellm.proxy.auth.auth_utils import normalize_request_route
|
||||
|
|
|
|||
|
|
@ -395,7 +395,7 @@ async def test_route_passed_to_post_call_failure_hook():
|
|||
# Verify post_call_failure_hook was called with the correct route
|
||||
mock_post_call_failure_hook.assert_called_once()
|
||||
call_args = mock_post_call_failure_hook.call_args[1]
|
||||
assert call_args["user_api_key_dict"].request_route == test_route
|
||||
assert call_args["user_api_key_dict"].request_route == "/unmatched"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -2912,6 +2912,70 @@ class TestPricingInjectionBlocked:
|
|||
)
|
||||
|
||||
|
||||
class TestResolveRequestRouteForMetrics:
|
||||
"""resolve_request_route_for_metrics bounds Prometheus route label cardinality."""
|
||||
|
||||
def _request(self, scope):
|
||||
req = MagicMock()
|
||||
req.scope = scope
|
||||
return req
|
||||
|
||||
def test_prefers_fastapi_route_template(self):
|
||||
from litellm.proxy.auth.auth_utils import resolve_request_route_for_metrics
|
||||
|
||||
route = MagicMock()
|
||||
route.path = "/v1/responses/{response_id}"
|
||||
req = self._request({"route": route, "path": "/v1/responses/abc123"})
|
||||
assert (
|
||||
resolve_request_route_for_metrics(req, "/v1/responses/abc123")
|
||||
== "/v1/responses/{response_id}"
|
||||
)
|
||||
|
||||
def test_normalizes_known_dynamic_routes_without_template(self):
|
||||
from litellm.proxy.auth.auth_utils import resolve_request_route_for_metrics
|
||||
|
||||
assert (
|
||||
resolve_request_route_for_metrics(None, "/v1/responses/abc123")
|
||||
== "/v1/responses/{response_id}"
|
||||
)
|
||||
|
||||
def test_preserves_literal_static_routes(self):
|
||||
from litellm.proxy.auth.auth_utils import resolve_request_route_for_metrics
|
||||
|
||||
for route in (
|
||||
"/v1/chat/completions",
|
||||
"/chat/completions",
|
||||
"/health/liveliness",
|
||||
"/metrics",
|
||||
):
|
||||
assert resolve_request_route_for_metrics(None, route) == route
|
||||
|
||||
def test_collapses_unknown_paths_to_unmatched(self):
|
||||
from litellm.proxy.auth.auth_utils import (
|
||||
UNMATCHED_REQUEST_ROUTE,
|
||||
resolve_request_route_for_metrics,
|
||||
)
|
||||
|
||||
scanner_paths = [
|
||||
"/files/pRzAdaV1.php",
|
||||
"/files/J80racNN.php",
|
||||
"/files/ldap.debug.txt",
|
||||
"/api/v1/files/secret_key",
|
||||
"/actuator/metrics",
|
||||
"/solr/admin/metrics",
|
||||
"/scanner/path-1",
|
||||
"/scanner/path-2",
|
||||
]
|
||||
normalized = {resolve_request_route_for_metrics(None, path) for path in scanner_paths}
|
||||
assert normalized == {UNMATCHED_REQUEST_ROUTE}
|
||||
|
||||
def test_repeated_unknown_paths_do_not_increase_cardinality(self):
|
||||
from litellm.proxy.auth.auth_utils import resolve_request_route_for_metrics
|
||||
|
||||
routes = [f"/scanner/random-{i}" for i in range(100)]
|
||||
assert len({resolve_request_route_for_metrics(None, r) for r in routes}) == 1
|
||||
|
||||
|
||||
class TestGetRequestRouteTemplate:
|
||||
"""get_request_route_template returns the low-cardinality FastAPI route
|
||||
template (e.g. /v1/threads/{thread_id}/runs) for http.route, distinct
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue