mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-16 23:41:43 +00:00
Merge pull request #41170 from BerriAI/litellm_prometheus_401_failed_requests_metric
fix(prometheus): count 401 auth failures in litellm_proxy_failed_requests_metric
This commit is contained in:
commit
367393405c
4 changed files with 94 additions and 35 deletions
|
|
@ -2610,12 +2610,6 @@ class PrometheusLogger(CustomLogger):
|
|||
StandardLoggingPayloadSetup,
|
||||
)
|
||||
|
||||
if self._should_skip_metrics_for_invalid_key(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
exception=original_exception,
|
||||
):
|
||||
return
|
||||
|
||||
status_code: Final = self._extract_status_code(exception=original_exception)
|
||||
|
||||
try:
|
||||
|
|
@ -2633,7 +2627,7 @@ class PrometheusLogger(CustomLogger):
|
|||
end_user=user_api_key_dict.end_user_id,
|
||||
user=user_api_key_dict.user_id,
|
||||
user_email=user_api_key_dict.user_email,
|
||||
hashed_api_key=user_api_key_dict.api_key,
|
||||
hashed_api_key=None if status_code == 401 else user_api_key_dict.api_key,
|
||||
api_key_alias=user_api_key_dict.key_alias,
|
||||
team=user_api_key_dict.team_id,
|
||||
team_alias=user_api_key_dict.team_alias,
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from litellm.proxy.auth.auth_utils import (
|
|||
_get_request_ip_address,
|
||||
is_invalid_virtual_key_error,
|
||||
mark_invalid_virtual_key_error,
|
||||
normalize_request_route,
|
||||
)
|
||||
from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler
|
||||
from litellm.types.services import ServiceTypes
|
||||
|
|
@ -172,7 +173,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 = normalize_request_route(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
|
||||
|
|
|
|||
|
|
@ -1,18 +1,18 @@
|
|||
"""
|
||||
Unit tests for Prometheus invalid API key request filtering.
|
||||
|
||||
Tests functionality that prevents invalid API key requests (401 status codes)
|
||||
from being recorded in Prometheus metrics.
|
||||
Tests the 401 detection helpers, that LLM-level metrics skip invalid API key
|
||||
requests, and that the proxy-level failed request counter still records them.
|
||||
"""
|
||||
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
from prometheus_client import REGISTRY
|
||||
|
||||
|
||||
from litellm.integrations.prometheus import PrometheusLogger
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
from litellm.proxy._types import ProxyErrorTypes, ProxyException, UserAPIKeyAuth
|
||||
|
||||
|
||||
@pytest.fixture(scope="function")
|
||||
|
|
@ -129,28 +129,29 @@ class TestSkipMetricsValidation:
|
|||
|
||||
|
||||
class TestAsyncHooks:
|
||||
"""Test async hook methods skip metrics for invalid API keys."""
|
||||
|
||||
@pytest.fixture
|
||||
def mock_user_api_key(self):
|
||||
"""Create a mock UserAPIKeyAuth object."""
|
||||
user_key = Mock(spec=UserAPIKeyAuth)
|
||||
user_key.api_key = "test-key"
|
||||
user_key.end_user_id = None
|
||||
user_key.user_id = None
|
||||
user_key.user_email = None
|
||||
user_key.key_alias = None
|
||||
user_key.team_id = None
|
||||
user_key.team_alias = None
|
||||
user_key.request_route = "/test"
|
||||
return user_key
|
||||
"""Test how async hook methods treat invalid API key requests."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_call_failure_hook_skips_401(
|
||||
self, prometheus_logger, mock_user_api_key
|
||||
@pytest.mark.parametrize(
|
||||
"exception",
|
||||
[
|
||||
HTTPException(
|
||||
status_code=401,
|
||||
detail="LiteLLM Virtual Key expected. Received=nota****tall, expected to start with 'sk-'.",
|
||||
),
|
||||
ProxyException(
|
||||
message="Authentication Error, Invalid proxy server token passed.",
|
||||
type=ProxyErrorTypes.token_not_found_in_db,
|
||||
param="key",
|
||||
code=401,
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_post_call_failure_hook_counts_401_without_key_hash(
|
||||
self, prometheus_logger, exception
|
||||
):
|
||||
exception = ExceptionWithCode("401")
|
||||
exception.__class__.__name__ = "ProxyException"
|
||||
unauthenticated = UserAPIKeyAuth(request_route="/v1/chat/completions")
|
||||
unauthenticated.api_key = "notakeyatall"
|
||||
|
||||
with (
|
||||
patch.object(
|
||||
|
|
@ -160,15 +161,50 @@ class TestAsyncHooks:
|
|||
prometheus_logger, "litellm_proxy_total_requests_metric"
|
||||
) as mock_total,
|
||||
):
|
||||
|
||||
await prometheus_logger.async_post_call_failure_hook(
|
||||
request_data={"model": "test-model"},
|
||||
original_exception=exception,
|
||||
user_api_key_dict=mock_user_api_key,
|
||||
user_api_key_dict=unauthenticated,
|
||||
)
|
||||
|
||||
mock_failed.labels.assert_not_called()
|
||||
mock_total.labels.assert_not_called()
|
||||
failed_labels = mock_failed.labels.call_args.kwargs
|
||||
assert failed_labels["exception_status"] == "401"
|
||||
assert failed_labels["hashed_api_key"] is None
|
||||
assert failed_labels["route"] == "/v1/chat/completions"
|
||||
mock_failed.labels.return_value.inc.assert_called_once()
|
||||
assert mock_total.labels.call_args.kwargs["status_code"] == "401"
|
||||
mock_total.labels.return_value.inc.assert_called_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_call_failure_hook_keeps_resolved_identity_labels_for_401(
|
||||
self, prometheus_logger
|
||||
):
|
||||
expired_key = UserAPIKeyAuth(
|
||||
api_key="sk-expired",
|
||||
key_alias="expired-alias",
|
||||
team_id="team-1",
|
||||
)
|
||||
exception = ProxyException(
|
||||
message="Authentication Error - Expired Key.",
|
||||
type=ProxyErrorTypes.expired_key,
|
||||
param="key",
|
||||
code=401,
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
prometheus_logger, "litellm_proxy_failed_requests_metric"
|
||||
) as mock_failed:
|
||||
await prometheus_logger.async_post_call_failure_hook(
|
||||
request_data={"model": "test-model"},
|
||||
original_exception=exception,
|
||||
user_api_key_dict=expired_key,
|
||||
)
|
||||
|
||||
failed_labels = mock_failed.labels.call_args.kwargs
|
||||
assert failed_labels["exception_status"] == "401"
|
||||
assert failed_labels["hashed_api_key"] is None
|
||||
assert failed_labels["api_key_alias"] == "expired-alias"
|
||||
assert failed_labels["team"] == "team-1"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_log_failure_event_skips_401(self, prometheus_logger):
|
||||
|
|
|
|||
|
|
@ -487,6 +487,34 @@ async def test_route_passed_to_post_call_failure_hook():
|
|||
assert call_args["user_api_key_dict"].request_route == test_route
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dynamic_route_normalized_on_auth_failure():
|
||||
handler = UserAPIKeyAuthExceptionHandler()
|
||||
|
||||
with (
|
||||
patch( # test-quality-ok: handler reads proxy_server globals at call time
|
||||
"litellm.proxy.proxy_server.proxy_logging_obj.post_call_failure_hook",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_post_call_failure_hook,
|
||||
patch( # test-quality-ok: handler reads proxy_server globals at call time
|
||||
"litellm.proxy.proxy_server.general_settings", {}
|
||||
),
|
||||
pytest.raises(ProxyException),
|
||||
):
|
||||
await handler._handle_authentication_error(
|
||||
HTTPException(status_code=401, detail="Authentication Error, Invalid proxy server token passed"),
|
||||
MagicMock(),
|
||||
{},
|
||||
"/v1/responses/resp_attacker_controlled_id",
|
||||
None,
|
||||
"sk-doesnotexist",
|
||||
)
|
||||
|
||||
hook_kwargs = mock_post_call_failure_hook.call_args.kwargs
|
||||
assert hook_kwargs["route"] == "/v1/responses/resp_attacker_controlled_id"
|
||||
assert hook_kwargs["user_api_key_dict"].request_route == "/v1/responses/{response_id}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolved_identity_exported_on_auth_failure():
|
||||
"""Regression: when auth fails AFTER the key/team/user identity is resolved
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue