fix(litellm): Store span off-band, not in forwarded metadata - #6598
Conversation
Sorry, something went wrong.
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.
Sorry, something went wrong.
alexander-alderman-webb
left a comment
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).
Sorry, something went wrong.
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>
|
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) |
Sorry, something went wrong.
|
The httpx import in your test additions fail because it's not installed in the environment. Happy to merge without the tests as before 😄. https://github.com/getsentry/sentry-python/blob/master/scripts/populate_tox/README.md |
Sorry, something went wrong.
It imported httpx at module top, which isn't installed in the common/gevent collection envs, breaking collection there. Maintainer preferred merging the fix without the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Gotcha, dropped it! CC @alexander-alderman-webb since the update needs workflow approvals to be re-run |
Sorry, something went wrong.
…ry#6598) Store LiteLLM spans in a shared dictionary reference under `_sentry_span`. The same reference is passed to all LiteLLM callbacks. This prevents a `TypeError` caused by trying to serialize a `Span` when preparing an LLM request, as the `metadata` in which the span was previously stored can be part of a request. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Alex Alderman Webb <alexander.webb@sentry.io>
Summary
Fixes #6596 (Linear PY-2540).
With LiteLLMIntegration enabled, any call that passes caller metadata could crash during request serialization:
Root cause
_input_callback stored the live span via _get_metadata_dict(kwargs)["_sentry_span"] = span. litellm threads the caller's metadata dict through to litellm_params["metadata"] — it's the same dict object the caller passed — and some providers (e.g. Anthropic's /v1/messages passthrough) forward that dict into the outbound request body. So the live Span landed at request_body["metadata"]["_sentry_span"] and json.dumps(request_body) failed before the request was sent.
Separately, under send_default_pii=True + include_prompts=True, that span's gen_ai.request.messages holds 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 callback kwargs dict 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
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).