fix(litellm): store bookkeeping span off-band, not in forwarded metadata#6598
Open
jgreer013 wants to merge 2 commits into
Open
fix(litellm): store bookkeeping span off-band, not in forwarded metadata#6598jgreer013 wants to merge 2 commits into
jgreer013 wants to merge 2 commits into
Conversation
jgreer013
force-pushed
the
jgreer013/issue-6596
branch
from
6fb4fa5 to
5a5f39b
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5a5f39b. Configure here.
jgreer013
force-pushed
the
jgreer013/issue-6596
branch
from
5a5f39b to
986f979
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
jgreer013
force-pushed
the
jgreer013/issue-6596
branch
from
986f979 to
dbb2139
Compare
alexander-alderman-webb
left a comment
alexander-alderman-webb
left a comment
Contributor
There was a problem hiding this comment.
Integration code looks good to me!
Could you remove the new tests @jgreer013?
They rely on private details such as _input_callback and _sentry_span. For a bugfix like this we really would prefer not to add tests that make future code changes more difficult (since the tests rely on private details).
jgreer013
force-pushed
the
jgreer013/issue-6596
branch
2 times, most recently
from
b007228 to
4ea0739
Compare
With LiteLLMIntegration enabled, any call passing caller `metadata` crashed during request serialization. `_input_callback` stored the live Span in the caller's `metadata` dict, and some providers (e.g. Anthropic's /v1/messages passthrough) forward that dict into the outbound request body, so `json.dumps(request_body)` raised `TypeError: Object of type Span is not JSON serializable` before the request was sent. The span (holding the verbatim prompt under send_default_pii) could also leak to the provider. Stash the span on a top-level key of the per-request kwargs dict (litellm's `model_call_details`) that litellm threads through the input/success/failure callbacks, instead of in the forwarded `metadata` sub-dict. This ties the span's lifetime to the request with no module-level tracking, mirroring how the clickhouse/dramatiq integrations stash a span on their per-request object. The Anthropic request body is built only from recognized request params, not from `model_call_details`, so the span is never serialized onto the wire (verified end-to-end against the passthrough). Fixes getsentry#6596 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jgreer013
force-pushed
the
jgreer013/issue-6596
branch
from
4ea0739 to
928b083
Compare
Author
|
Updated to have just one test through the public api with mocks (damn you claude and your incessant need to write tests for private methods) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6596 (Linear PY-2540).
With
LiteLLMIntegrationenabled, any call that passes callermetadatacould crash during request serialization:Root cause
_input_callbackstored the live span via_get_metadata_dict(kwargs)["_sentry_span"] = span. litellm threads the caller'smetadatadict through tolitellm_params["metadata"]— it's the same dict object the caller passed — and some providers (e.g. Anthropic's/v1/messagespassthrough) forward that dict into the outbound request body. So the liveSpanlanded atrequest_body["metadata"]["_sentry_span"]andjson.dumps(request_body)failed before the request was sent.Separately, under
send_default_pii=True+include_prompts=True, that span'sgen_ai.request.messagesholds the verbatim prompt, so any sink that serialized the injected metadata could leak prompt content to the provider.Fix
Store the bookkeeping span off-band in a module-level registry keyed by
litellm_call_id(a per-request UUID that is stable across the input/success/failure callbacks), falling back to the identity of the shared callbackkwargsdict for direct callback invocations that omit it. The span no longer lives in any litellm-visible dict, so it can't be forwarded, serialized, or deep-copied by litellm — fixing both the crash and the prompt-leak vector. The registry entry is removed by the terminal success/failure callback (streaming success peeks and pops only on the final call).Testing
test_caller_metadata_stays_json_serializableasserts the forwardedmetadatastays free of_sentry_spanand JSON-serializable at the serialization moment, while the span is still recorded off-band. It fails onmasterand passes with this change.mypyclean;ruffcheck + format clean.Related (downstream, defense-in-depth)
litellm forwards/serializes this injected metadata into the provider body. Companion issue: BerriAI/litellm#30662. litellm has stripped such span objects in other logging paths (BerriAI/litellm#15728, BerriAI/litellm#12354).