Repository navigation
fix(proxy): keep request metadata out of the cost tracking failure alert - #41950
Conversation
The cost tracking callback f-stringed chosen_metadata, litellm_metadata, and old_metadata into the failed_tracking_spend alert on every failure, at every log level, so one 250-byte request produced a 23 KB alert carrying the client's metadata, headers, and key-auth reprs four times over. The alert now carries the exception, the traceback, the model, and the call type; the metadata keys are logged once at debug level through lazy formatting, so nothing is built at warning level
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
| _metadata_keys(get_litellm_metadata_from_kwargs(kwargs=kwargs)), | ||
| _metadata_keys(failing_litellm_params.get("litellm_metadata")), | ||
| _metadata_keys(failing_litellm_params.get("metadata")), |
There was a problem hiding this comment.
Caller-Controlled Metadata Keys
Debug logging includes every caller-controlled metadata key, so sensitive identifiers embedded in key names can persist in logs. Consider logging fixed categories or counts instead
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!
There was a problem hiding this comment.
At DEBUG the proxy already logs whole request bodies, so key names add no new exposure; values are never stringified, per the ticket's ask
There was a problem hiding this comment.
That context addresses the concern. Since DEBUG already emits the full request body, logging the metadata key names does not add meaningful exposure at that level; importantly, _metadata_keys() never stringifies metadata values, and the failure alert remains free of them. No change is needed for this comment.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9e8a847. Configure here.
TLDR
Problem this solves:
How it solves it:
%sargsDecisions:
()User Flow
Before: a proxy admin whose Redis cache turned into a read-only replica after a failover gets a 23 KB
failed_tracking_spendSlack alert carrying a developer's whole request metadata, headers, and key details, rebuilt on every request whatever the log levelLITELLM_LOG=WARNING, Slack alerting on, andalert_types: ["failed_tracking_spend"]; a failover leaves the Redis cache pointing at a read-only replicaPOST https://litellm-domain/v1/chat/completionswith"model": "healthy-chat", a one-line prompt, and"metadata": {"tags": ["lit7506-repro"], "user_context": "<a 90-character note>"}, about 250 bytes in all200with the completion, as usualfailed_tracking_spendalert of 23 to 24 KB: afterError in tracking cost callback - ... You can't write against a read only replicaand the traceback it carrieschosen_metadata: {...}andold_metadata: {...}, which hold the developer'smetadatablob four times, the request headers four times, two key-auth dumps with the key hash and budgets, and the deployment'sapi_baseERROR ... Error in tracking cost callbackline per request, and the 23 KB string is rebuilt for every request on that model even though the alert itself is sent once a day per modelAfter: the same failover gets the admin a short alert with the error, the traceback, the model, and the call type, and the metadata keys only show up at debug level
LITELLM_LOG=WARNING, Slack alerting on, andalert_types: ["failed_tracking_spend"]; a failover leaves the Redis cache pointing at a read-only replicaPOST https://litellm-domain/v1/chat/completionswith"model": "healthy-chat", a one-line prompt, and"metadata": {"tags": ["lit7506-repro"], "user_context": "<a 90-character note>"}, about 250 bytes in all200with the completion, as usualfailed_tracking_spendalert of 5,295 bytes: theYou can't write against a read only replicaerror, the traceback,model: healthy-chat, andcall_type: acompletion, with nothing from the developer's metadata, headers, or keyERRORline per request and nothing else atWARNING; only withLITELLM_LOG=DEBUGdoes it add one line naming the metadata keys, such aschosen_metadata keys=('headers', 'user_api_key', ...), never their valuesRelevant issues
Affected release
Linear ticket
Resolves LIT-7506
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays 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
Same rig for both legs, only the proxy commit differs. Each run boots a two-worker proxy (
--num_workers 2) on a random port with Postgres behind it andlitellm_settings.cachepointing at a Redis read-only replica on 127.0.0.1:59361, which is what a failover leaves behind, so every request's spend counterINCRBYFLOATfails and the cost tracking callback hits its except block.SLACK_WEBHOOK_URLpoints at a local receiver that records each Slack POST as one JSON line. Config:qa-chat,qa-responses, andqa-messagesall onopenai/gpt-5.4-mini(real OpenAI calls),alerting: ["slack"],alert_types: ["failed_tracking_spend"]. Each leg ran twice,LITELLM_LOG=WARNINGandLITELLM_LOG=DEBUG. Every request carries a per-run sentinel string in anx-lit7506-noteheader and inmetadata.user_context, so grepping the alerts and the proxy log for$SENTINELshows whether the request metadata got out. The WARNING run is shown in full and the DEBUG run shows the alert summary and the log grepsBefore (5fc510a)
LITELLM_LOG=WARNING,SENTINEL=lit7506-sentinel-1789813802-576956200with a real completionmetadatablobERRORper request, no keys line, and the sentinel stays out of the WARNING log (it only ever reached Slack)LITELLM_LOG=DEBUG,SENTINEL=lit7506-sentinel-1789813853-523069, same three requests, all200After (9e8a847)
LITELLM_LOG=WARNING,SENTINEL=lit7506-sentinel-1789813801-284411200with a real completionERRORper request, nothing else at WARNINGLITELLM_LOG=DEBUG,SENTINEL=lit7506-sentinel-1789813881-287290, same three requests, all200One keys line per request (3,201, 4,554, and 4,562 characters; shown cut after the first six keys), naming the metadata keys and never a value
Observations from the runs, none caused by this PR:
model=gpt-5.4-miniis the deployment model, not the alias, both legslitellm_metadataempty, pre-existingType
🐛 Bug Fix
Caveats (if any)
Three CircleCI jobs are red at 9e8a847 and none of them reaches this change (Low).
litellm_utils_testingfailstest_models_by_provideron every main pipeline since #41515 added the Amazon Transcribe cost map entry (main pipelines 89818 and 89834 fail it at tips without this PR, and the merge base 5fc510a already carries that entry); tracked separately.proxy_e2e_anthropic_messages_testsfails the twotest_bedrock_invoke_messages_with_all_beta_headerscases with Bedrock'sinvalid beta flagon the same two main pipelines; tracked in LIT-8149.google_generate_content_endpoint_testinghit a Vertex 429RESOURCE_EXHAUSTEDon gemini-2.5-flash-lite that main's scheduled runs pass; the from-failed rerun (workflow d2ca33c2) got the same 429 on two of the streaming tests, and every PR pipeline started after 10:25Z that day (89859, 89860, 89861) fails the sametest_proxy_genai_sdk_*tests with the same 429 while every pipeline before 10:25Z passes them, so it is the shared Vertex quota, not this changeLive PR risk at 9e8a847
No breaking change and no regression found. The one contract change is the intended one: the
failed_tracking_spendSlack alert text no longer carries thechosen_metadata,litellm_metadataandold_metadatablocks (24,130 bytes to 5,295 bytes for the same failure), so an operator who read those blocks out of the Slack message loses them. Themodel:andcall_type:lines stay, the exception and traceback stay, and LIT-7506 asks for exactly this removal, so it ships as askedRegression risk: none left unreached. Every dependent of the changed block was driven live on base 5fc510a and head 9e8a847 (2 uvicorn workers,
/v1/chat/completions,/v1/responsesand/v1/messagesagainst a real gpt-5.4-mini deployment, the failure forced by a read-only Redis replica,LITELLM_LOGat WARNING and at DEBUG, the alert captured by a forwarding recorder at the webhook boundary)Dependency graph
ProxyLogging.failed_tracking_alerttoSlackAlerting.failed_tracking_alert(litellm/proxy/utils.py,litellm/integrations/SlackAlerting/slack_alerting.py): the only reader oferror_msg; the dedupe key usesfailing_model, which is unchanged. Verified live (same alert count and same per-deployment dedupe on both sides) and tested (tests/test_litellm/proxy/utils/proxy_logging/test_alerting.py, plus the new regression test)spend_log_error(...)ERROR line: outside the diff, arguments unchanged. Verified live (3 lines per run on both sides)verbose_proxy_logger.debug(...)line: readers are the log handlers, JSON formatter included, which format throughgetMessage(). Tested by the new parametrized test with alogging.Handlerat WARNING and at DEBUG; verified live (absent at WARNING, one keys-only line per failing request at DEBUG)_metadata_keys: private, one caller. Tested; Codecov reports every modified line coveredchosen_metadata,Args to _PROXY_track_cost_callback) acrosslitellm/,enterprise/,tests/,ui/and the docs checkout: none;docs/proxy/alerting.mdnames only the alert typeslack_alerting.pybudget wording and none touchingfailed_tracking_alertor this hook. The merged tree (3aa49e8fd0) merges clean, the mapped test file passes on it, and the WARNING scenario re-run on it gives a 5,428-byte alert with no metadata, headers, key auth or sentinel in it, matching the head legNot verified
JSON_LOGS=Trueformatting of the new debug line on a live proxy: covered by the unit test's handler onlyFinal Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
9e8a847 passes /live-pr-risk
Note
Low Risk
Observability-only change to failure alerting and debug logging; spend tracking behavior is unchanged aside from shorter alert text.
Overview
When cost tracking fails,
failed_tracking_spendalerts no longer embed full request metadata (chosen_metadata,litellm_metadata,old_metadata). The Slack/error payload keeps the exception, traceback, model, and call_type only, avoiding huge alerts and accidental leakage of headers or customer metadata on paths like read-only Redis.Metadata is surfaced only at DEBUG via a lazy
verbose_proxy_logger.debugline that logs sorted key names per metadata bucket (never values), using a new_metadata_keyshelper. A parametrized test asserts metadata values are never stringified on the failure path at WARNING or DEBUG.Reviewed by Cursor Bugbot for commit 9e8a847. Bugbot is set up for automated code reviews on this repo. Configure here.