fix(logging): bind litellm_metadata by reference in function_setup so guardrail info reaches spend logs - #35292
Conversation
Greptile SummaryThis PR keeps
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/utils.py | Binds the logging object's litellm_metadata to the request dictionary while preserving the separate fallback metadata copy. |
| tests/test_litellm/litellm_core_utils/test_litellm_logging.py | Adds focused regression coverage proving that post-setup guardrail mutations remain visible when constructing standard logging metadata. |
Reviews (2): Last reviewed commit: "fix(logging): bind litellm_metadata by r..." | Re-trigger Greptile
|
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 6241148. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… guardrail info reaches spend logs
6241148 to
899ed67
Compare
|
@greptileai please review the current head 899ed67 |
|
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 899ed67. Configure here.
TLDR
Problem this solves:
How it solves it:
Relevant issues
Linear ticket
Resolves LIT-4512
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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
All evidence is from a live proxy (Postgres backed) with a
litellm_content_filterpre-call guardrail attached to a freshly generated virtual key, hitting real provider APIs (no mocks), then readingGET /spend/logs. Before runs are at base commit8e287652c6, after runs are at the fix commit (same tree as head899ed67860, code-identical rebase of62411488cf)One nuance the evidence has to work around: some provider handlers rebind
litellm_metadatainto the logging object later in the call (Logging.update_from_kwargsin the shared HTTP handler), which masks this bug on those paths even without the fix. The paths with no later rebind are the OpenAI-SDK ones; in particular, /v1/messages with an OpenAI model routes through the Responses API bridge by default (_should_route_to_responses_api), so the /v1/responses run below exercises the same broken path the reported /v1/messages traffic takes. The OpenAI account ran out of credits mid-capture (the after run below consumed the last of them; a direct probe now returns "You have no credits remaining"), so a same-provider /v1/messages differential could not be replayed; happy to re-capture one on a funded OpenAI key1) Differential on the broken path, real OpenAI gpt-4o-mini via /v1/responses
Before, base
8e287652c6; the request succeeds and costs real money, the spend log has nothing for the guardrail:After, at the fix:
2) /v1/messages and /v1/chat/completions with the fix, real Gemini through an OpenAI-SDK path
Model
openai/gemini-2.5-flashagainst Google's OpenAI-compatible endpoint withuse_chat_completions_url_for_anthropic_messages: true, so /v1/messages goes through the completion adapter on the OpenAI SDK client:(The same two legs also pass on base code because the chat adapter is one of the rebind-masked paths; the regression test in this PR plus run 1 above pin the unmasked behavior)
3) Real Bedrock (claude-haiku-4-5), all three endpoints at the fix
A streaming /v1/messages request on the same rig also persists both fields
4) Admin UI at the fix: /v1/messages request now shows the Guardrails panel
The /v1/messages (anthropic_messages) success row on the bundled dashboard Logs page, with the Guardrails and Policy Compliance panel, request lifecycle showing the passed pre-call guardrail, and evaluation details:
Same key on /v1/chat/completions for comparison:
Type
🐛 Bug Fix
Changes
function_setup(litellm/utils.py) storedkwargs["litellm_metadata"]into the logging object'slitellm_paramsvia.copy()while storingkwargs["metadata"]by reference. The proxy creates the logging object before pre-call guardrails run (common_request_processing.py, deliberately, so rejected requests still get logged), so on routes that carry proxy state inlitellm_metadata(/v1/messages, /v1/responses, batches, files) every guardrail write that happens after setup (standard_logging_guardrail_information,applied_guardrails) landed in the request dict but never in the snapshot the spend-log payload is built from. /v1/chat/completions only worked because itsmetadatadict is aliasedThe fix binds
litellm_metadataby reference, the same shapeLogging.update_from_kwargs(litellm_core_utils/litellm_logging.py line 600) has always used on this exact dict, and the same aliasingmetadatahas always had on the chat path. The.copy()being removed was introduced by a review-bot suggested edit (17804edc78) with no test or issue behind it. Line 1057's fallback copy intolitellm_params["metadata"]is intentionally kept: it preservesmetadata is not litellm_metadata, which an existing test asserts, andmerge_litellm_metadatafills the guardrail keys from the livelitellm_metadatasince they never exist in the stale fallbackThe regression test extends the existing
function_setuplitellm_metadata suite intests/test_litellm/litellm_core_utils/test_litellm_logging.py(the mapped suite for this behavior;litellm/utils.pyhas no dedicated mirror file and the sibling tests live here). It reproduces the production sequence (setup, then a bucket write viaget_or_create_metadata_bucket) and fails if the.copy()is reintroducedBehavior changes
Spend logs and logging callbacks on litellm_metadata routes now receive
guardrail_informationandapplied_guardrailsthat were previously dropped; that is the fix itself. The logging object'slitellm_params["litellm_metadata"]is now the same dict as the request data's bucket, identical to the long-standing chat-path semantics; a two-pass regression sweep found no consumer that relied on the snapshot (the one snapshot-dependent site, the router's mid-stream fallback, already deep-copies both metadata dicts). Pre-existing gaps deliberately not covered here are tracked in LIT-5022 (failure-path spend logs drop guardrail_information for blocked requests on every endpoint, including chat) and LIT-5023 (litellm_metadata bucket hardening follow-ups)Why the aliasing is a pre-existing pattern, not a new one
Four pieces of merged code establish that binding these metadata dicts by reference is the codebase's long-standing convention, and that the
.copy()this PR removes was the outliermetadataby reference since January 2024 (a299ac2328), on the line directly above the one changed here, untouched by this PR. Every /v1/chat/completions request that ever logged guardrail info depended on that aliasingLogging.update_from_kwargsalready uses the identical shape on the identical dict, reference forlitellm_metadatawith a.copy()only for themetadatafallback, and it runs on the live /v1/messages success path (llm_http_handler.py:2031). Production traffic has been flowing through this exact aliasing all along, which is why many routes behave identically before and after this PR; the fix extends the aliasing to the window between function_setup and the provider handler, where guardrails runMerged code documents the alias as a contract it relies on (feat(spend): track prompt compression saved tokens in daily spend aggregates #33810, merged 2026-07-18):
litellm/integrations/compression_interception/handler.pyupdates the existing dict in place "because the proxy and the logging object hold references to the same object; replacing it would orphan writes made through those references"The exact line changed here was introduced as a reference in
8c3d6db482and flipped to.copy()hours later in17804edc78, an applied review-bot suggestion with no test or issue behind it. This PR restores the line's original form, which is also the form of its three siblings aboveFinal Attestation