mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
fix(proxy): let accounting callbacks opt in to no-log requests by attribute
A `no-log: true` request asks that its content not be logged; it is not an opt-out of billing. The filter decided by class name, so the router's budget limiter (`RouterBudgetLimiting`) was silently skipped and deployment, provider and tag budgets could be bypassed -- reported by a reviewer as six out of six no-log calls returning 200 under a 0.05 deployment `max_budget`. Replace the name list with an inherited `CustomLogger.runs_on_no_log_requests` attribute mirroring `enforces_request_content`, set it on `_ProxyDBLogger` and `RouterBudgetLimiting`, and keep the internal `_PROXY_` prefix as a back-compat signal for hooks that predate the attribute. Close the second leak of the same kind: `sanitize_error_information_for_spend_logs` only consulted the proxy-wide `store_prompts_in_spend_logs`, so a no-log request with prompt storage enabled still persisted the prompt echoed back inside provider errors. Thread `litellm_params` through it so it follows the per-request rule, as the vector-store payload path already does. Tests: attribute opt-in and default filtering, the real classes carry the mark, and error_information redaction across the `no-log` x `global_disable_no_log_param` matrix. Signed-off-by: apex-mochen <2756823972@qq.com>
This commit is contained in:
parent
963c607ad4
commit
bbf017d5cf
7 changed files with 116 additions and 19 deletions
|
|
@ -85,6 +85,21 @@ class CustomLogger: # https://docs.litellm.ai/docs/observability/custom_callbac
|
|||
consulted, so a subclass that does not override ``async_pre_call_hook`` inherits nothing.
|
||||
"""
|
||||
|
||||
runs_on_no_log_requests: bool = False
|
||||
"""
|
||||
Whether this callback must still run for a request that asked for ``no-log: true``.
|
||||
|
||||
``no-log`` asks that the request's *content* not be logged; it is not a request to stop
|
||||
accounting for it. A callback that only moves numbers -- the proxy's spend writer, the
|
||||
router's budget limiter -- sets this True, so the request is still billed and still counts
|
||||
against budgets. Exporters and hooks that ship the payload to a third party leave it False.
|
||||
|
||||
Opting in by attribute rather than by class name is deliberate: a class-name check breaks
|
||||
silently when the class is renamed, which is how a ``no-log`` request started being served
|
||||
without being charged. The attribute inherits to subclasses and reads as intent rather than
|
||||
as a naming convention.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
turn_off_message_logging: bool = False,
|
||||
|
|
|
|||
|
|
@ -473,11 +473,6 @@ _INPUT_PRICING_KEY_PREFIXES: Final = (
|
|||
_OUTPUT_PRICING_KEY_PREFIXES: Final = ("output_cost_per_token",)
|
||||
_BATCH_PRICING_KEY_SUFFIX: Final = "_batches"
|
||||
|
||||
# Proxy-internal callbacks that must run even on `no-log` requests: the proxy still has to do
|
||||
# its own spend accounting. The `_PROXY_` handler prefix marks most of them; the spend writer
|
||||
# `_ProxyDBLogger` predates that prefix, so it is listed explicitly (exact names, no substring).
|
||||
_PROXY_INTERNAL_CALLBACK_CLASS_NAMES: Final = frozenset({"_ProxyDBLogger"})
|
||||
|
||||
_NO_CARRIED_RATES: Final[Mapping[str, object]] = MappingProxyType({})
|
||||
|
||||
|
||||
|
|
@ -2927,13 +2922,14 @@ class Logging(LiteLLMLoggingBaseClass):
|
|||
return True
|
||||
|
||||
if litellm_params.get("no-log", False) is True:
|
||||
# Proxy-internal callbacks must still run on `no-log` requests: the proxy's own
|
||||
# spend accounting has to record the request. They use the internal `_PROXY_`
|
||||
# handler prefix or -- for the spend writer, which predates that prefix -- an
|
||||
# explicit entry in `_PROXY_INTERNAL_CALLBACK_CLASS_NAMES`.
|
||||
# A `no-log` request still has to be accounted for: billing, budgets and rate
|
||||
# limits all read the same success path. Callbacks that only move numbers opt in
|
||||
# through `runs_on_no_log_requests`; the internal `_PROXY_` handler prefix stays
|
||||
# as a back-compat signal for hooks that predate the attribute. Exporters stay
|
||||
# filtered, which is the point of `no-log` in the first place.
|
||||
callback_class_name: Final = callback.__class__.__name__
|
||||
runs_on_no_log: Final = isinstance(callback, CustomLogger) and (
|
||||
callback_class_name.startswith("_PROXY_") or callback_class_name in _PROXY_INTERNAL_CALLBACK_CLASS_NAMES
|
||||
callback.runs_on_no_log_requests or callback_class_name.startswith("_PROXY_")
|
||||
)
|
||||
if not runs_on_no_log:
|
||||
verbose_logger.debug("no-log request, skipping logging for %s event", event_hook)
|
||||
|
|
|
|||
|
|
@ -395,6 +395,9 @@ def _attempt_spend_row_kwargs(
|
|||
|
||||
|
||||
class _ProxyDBLogger(CustomLogger):
|
||||
# The proxy still bills a `no-log` request; only its content is withheld.
|
||||
runs_on_no_log_requests = True
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
spend_event_producer: SpendEventProducer | None = None,
|
||||
|
|
|
|||
|
|
@ -910,7 +910,9 @@ def get_logging_payload(
|
|||
if drop_requester_ip_address:
|
||||
clean_metadata["requester_ip_address"] = None
|
||||
clean_metadata["error_information"] = sanitize_error_information_for_spend_logs(
|
||||
clean_metadata["error_information"], original_exception=failure_exception
|
||||
clean_metadata["error_information"],
|
||||
original_exception=failure_exception,
|
||||
request_litellm_params=cast("Mapping[str, object] | None", litellm_params),
|
||||
)
|
||||
|
||||
if litellm.cache is None:
|
||||
|
|
@ -1593,6 +1595,7 @@ def _redact_prompt_fields_in_guardrail_entry(
|
|||
def sanitize_error_information_for_spend_logs(
|
||||
error_information: StandardLoggingPayloadErrorInformation | None,
|
||||
original_exception: BaseException | None = None,
|
||||
request_litellm_params: Mapping[str, object] | None = None,
|
||||
) -> StandardLoggingPayloadErrorInformation | None:
|
||||
"""
|
||||
Sanitize ``error_information`` before it lands in ``LiteLLM_SpendLogs.metadata``.
|
||||
|
|
@ -1603,7 +1606,8 @@ def sanitize_error_information_for_spend_logs(
|
|||
|
||||
- Always: cap ``error_message`` and ``traceback`` with the existing
|
||||
``MAX_STRING_LENGTH_PROMPT_IN_DB`` DB-storage safeguard.
|
||||
- When ``store_prompts_in_spend_logs`` is False: additionally redact
|
||||
- When the request asked for ``no-log``, or ``store_prompts_in_spend_logs`` is False,
|
||||
additionally redact
|
||||
``'input'`` / ``'messages'`` / ``'prompt'`` values *and* Pydantic v2
|
||||
``input_value=...`` assignments inside both ``error_message`` and
|
||||
``traceback`` so prompts cannot leak through either field.
|
||||
|
|
@ -1621,7 +1625,7 @@ def sanitize_error_information_for_spend_logs(
|
|||
)
|
||||
sanitized = cast(dict, {**persisted})
|
||||
|
||||
if not should_store_prompts_and_responses_in_spend_logs():
|
||||
if not should_store_prompts_and_responses_in_spend_logs(request_litellm_params):
|
||||
for field in ("error_message", "traceback"):
|
||||
value = sanitized.get(field)
|
||||
if isinstance(value, str):
|
||||
|
|
|
|||
|
|
@ -120,6 +120,9 @@ def budget_window_seconds(started_at: float, budget_duration: str) -> int:
|
|||
|
||||
|
||||
class RouterBudgetLimiting(CustomLogger):
|
||||
# Deployment, provider and tag budgets are accounting: a `no-log` request still spends.
|
||||
runs_on_no_log_requests = True
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
dual_cache: DualCache,
|
||||
|
|
|
|||
|
|
@ -14740,9 +14740,7 @@ async def test_logger_whose_name_mentions_proxy_runs_without_no_log(
|
|||
assert len(logger.logged) == 1 and logger.logged[0] is result
|
||||
|
||||
|
||||
def test_no_log_keeps_running_proxy_prefixed_handlers(
|
||||
logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
def test_no_log_keeps_running_proxy_prefixed_handlers(logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch):
|
||||
"""The proxy's `_PROXY_*` handler family must keep running on no-log requests."""
|
||||
monkeypatch.setattr(litellm, "global_disable_no_log_param", False)
|
||||
assert (
|
||||
|
|
@ -14753,3 +14751,58 @@ def test_no_log_keeps_running_proxy_prefixed_handlers(
|
|||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
class _AccountingStub(CustomLogger):
|
||||
"""Stands in for an accounting callback (router budgets) that opts in by attribute."""
|
||||
|
||||
runs_on_no_log_requests = True
|
||||
|
||||
|
||||
class _ExportingStub(CustomLogger):
|
||||
"""Default: a callback that ships content stays filtered on `no-log`."""
|
||||
|
||||
|
||||
def test_no_log_runs_callback_that_opts_in_by_attribute(logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch):
|
||||
"""A callback that only moves numbers opts in with `runs_on_no_log_requests`."""
|
||||
monkeypatch.setattr(litellm, "global_disable_no_log_param", False)
|
||||
assert (
|
||||
logging_obj.should_run_callback(
|
||||
callback=_AccountingStub(),
|
||||
litellm_params={"no-log": True},
|
||||
event_hook="success",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def test_no_log_filters_default_callback_without_the_attribute(
|
||||
logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
"""Control: the attribute is an opt-in, so a default CustomLogger stays filtered."""
|
||||
monkeypatch.setattr(litellm, "global_disable_no_log_param", False)
|
||||
assert (
|
||||
logging_obj.should_run_callback(
|
||||
callback=_ExportingStub(),
|
||||
litellm_params={"no-log": True},
|
||||
event_hook="success",
|
||||
)
|
||||
is False
|
||||
)
|
||||
assert (
|
||||
logging_obj.should_run_callback(
|
||||
callback=_ExportingStub(),
|
||||
litellm_params={"no-log": False},
|
||||
event_hook="success",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def test_accounting_callbacks_are_marked_in_the_real_classes():
|
||||
"""#44503 follow-up: the spend writer and the router budget limiter must both opt in."""
|
||||
from litellm.proxy.hooks.proxy_track_cost_callback import _ProxyDBLogger
|
||||
from litellm.router_strategy.budget_limiter import RouterBudgetLimiting
|
||||
|
||||
assert _ProxyDBLogger.runs_on_no_log_requests is True
|
||||
assert RouterBudgetLimiting.runs_on_no_log_requests is True
|
||||
|
|
|
|||
|
|
@ -61,9 +61,7 @@ def _payload(monkeypatch, no_log: bool, store_prompts: bool) -> dict:
|
|||
|
||||
monkeypatch.setattr(proxy_server, "general_settings", {"store_prompts_in_spend_logs": store_prompts})
|
||||
now: Final = datetime.datetime.now(timezone.utc)
|
||||
return get_logging_payload(
|
||||
kwargs=_kwargs(no_log=no_log), response_obj=_response(), start_time=now, end_time=now
|
||||
)
|
||||
return get_logging_payload(kwargs=_kwargs(no_log=no_log), response_obj=_response(), start_time=now, end_time=now)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("store_prompts", [True, False])
|
||||
|
|
@ -166,3 +164,28 @@ def test_vector_store_content_follows_request_no_log(monkeypatch, no_log: bool,
|
|||
payload: Final = get_logging_payload(kwargs=kwargs, response_obj=_response(), start_time=now, end_time=now)
|
||||
|
||||
assert (VECTOR_STORE_SECRET in json.dumps(payload, default=str)) is (not no_log or global_disable_no_log_param)
|
||||
|
||||
|
||||
ERROR_SECRET: Final = "ERROR-SECRET-1d4b9f"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("global_disable_no_log_param", [False, True])
|
||||
@pytest.mark.parametrize("no_log", [True, False])
|
||||
def test_error_information_follows_request_no_log(monkeypatch, no_log: bool, global_disable_no_log_param: bool):
|
||||
"""A provider error can echo the body, so the same per-request rule must redact it."""
|
||||
from litellm.proxy import proxy_server
|
||||
from litellm.proxy.spend_tracking.spend_tracking_utils import (
|
||||
sanitize_error_information_for_spend_logs,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(litellm, "global_disable_no_log_param", global_disable_no_log_param)
|
||||
monkeypatch.setattr(proxy_server, "general_settings", {"store_prompts_in_spend_logs": True})
|
||||
error_information: Final = {
|
||||
"error_message": "validation error [type=string_type, input_value=" + repr(ERROR_SECRET) + ", input_type=str]",
|
||||
"traceback": "payload {'messages': [{'role': 'user', 'content': " + repr(ERROR_SECRET) + "}]}",
|
||||
}
|
||||
|
||||
sanitized = sanitize_error_information_for_spend_logs(error_information, request_litellm_params={"no-log": no_log})
|
||||
|
||||
content_expected: Final = not (no_log and not global_disable_no_log_param)
|
||||
assert (ERROR_SECRET in json.dumps(sanitized, default=str)) is content_expected
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue