fix(otel): stamp an MCP tool failure on the request that carried it - #34551
Conversation
|
|
Greptile SummaryThe PR moves MCP transport-span propagation from shared session authentication state to each message's ASGI request scope, uses the live span for failure attribution and request-root anchoring, and adds regression coverage for stale and completed spans Confidence Score: 5/5The PR appears safe to merge No blocking failures remain
|
| Filename | Overview |
|---|---|
| litellm/integrations/otel/logger.py | Prefers the current MCP message's writable transport span when recording proxy-level failures |
| litellm/integrations/otel/plumbing/context.py | Carries the live per-message transport span through context and re-anchors request-level telemetry while it is recording |
| litellm/proxy/_experimental/mcp_server/server.py | Publishes transport spans on request-local ASGI scope and scopes their use to MCP message handling |
| litellm/proxy/_experimental/mcp_server/auth/litellm_auth_handler.py | Removes transport telemetry state from the session-retained authentication object |
| tests/test_litellm/integrations/otel/test_otel_v2_logger.py | Adds focused regression tests for failure attribution, completed-span fallback, and guardrail parenting |
Reviews (2): Last reviewed commit: "fix(otel): stamp an MCP tool failure on ..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
c7eaeeb to
4f61825
Compare
|
@greptileai please review the current head 4f61825. Both findings are addressed: the transport span moved off the shared session auth object onto the per-request ASGI scope, read back through |
A failed MCP tool call aimed its error.* attributes at request_root_span(), a ContextVar written on the ASGI request task. A stateful streamable-HTTP session runs every message on the single task the session's initialize POST spawned, so inside the message handler that ContextVar still holds the initialize request's SERVER span. That span ended long ago, so the SDK dropped every write (five 'Setting attribute on ended span' warnings plus set_status and _add_event per failed call) and the POST that actually failed carried no error at all. The identity attributes seeded onto the server span went the same way. Publish the live transport span on the ASGI scope of the request being handled and read it back in the message handler through req_ctx.request, the Request the streamable-HTTP transport attaches to each message. That replaces the session-scoped field with a per-message one: a JSON-RPC response POST deliberately skips the per-session lock, since it can arrive while the tool call awaiting it is still in flight, so a field on the shared auth object could be overwritten mid-call and send the tool call's telemetry to the response's request. A scope also dies with its request rather than holding a finished span on idle session state. Publishing re-anchors the request root for the message so guardrail spans and identity seeding follow, and only a transport still open for writes is anchored or stamped: a notification POST can answer before the session task is done, and moving dropped writes from one finished span to another is no fix. Live capture goes from seven ended-span warnings and an unmarked transaction to zero warnings and ERROR on the POST that carried the call.
4f61825 to
0515579
Compare
502d360
into
litellm_internal_staging
…erriAI#34551) A failed MCP tool call aimed its error.* attributes at request_root_span(), a ContextVar written on the ASGI request task. A stateful streamable-HTTP session runs every message on the single task the session's initialize POST spawned, so inside the message handler that ContextVar still holds the initialize request's SERVER span. That span ended long ago, so the SDK dropped every write (five 'Setting attribute on ended span' warnings plus set_status and _add_event per failed call) and the POST that actually failed carried no error at all. The identity attributes seeded onto the server span went the same way. Publish the live transport span on the ASGI scope of the request being handled and read it back in the message handler through req_ctx.request, the Request the streamable-HTTP transport attaches to each message. That replaces the session-scoped field with a per-message one: a JSON-RPC response POST deliberately skips the per-session lock, since it can arrive while the tool call awaiting it is still in flight, so a field on the shared auth object could be overwritten mid-call and send the tool call's telemetry to the response's request. A scope also dies with its request rather than holding a finished span on idle session state. Publishing re-anchors the request root for the message so guardrail spans and identity seeding follow, and only a transport still open for writes is anchored or stamped: a notification POST can answer before the session task is done, and moving dropped writes from one finished span to another is no fix. Live capture goes from seven ended-span warnings and an unmarked transaction to zero warnings and ERROR on the POST that carried the call.
TLDR
Problem this solves:
error.*on the span of whichever request opened the session, which has already ended, so the SDK drops the write and the POST that actually failed is left unmarkedHow it solves it:
Relevant issues
Follows #34537, which fixed the trace placement of the same span and has merged. This branch is rebased onto
litellm_internal_stagingand carries only the change below.Linear ticket
Resolves LIT-4784
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
Same rig as #34537: live proxy with otel_v2 exporting OTLP/HTTP to a local collector that prints one line per span, one upstream MCP server, and a session driven with curl the way MCP Inspector drives it, ending in a
tools/callfor a tool that fails upstream. The wire is unchanged throughout, HTTP 200 withisError: true.Before, on
litellm_internal_stagingwith #34537 in it. Every failed tool call logs:and the transport carries nothing, because all of it was aimed at the
initializerequest's finished span:After, on this branch, same sequence:
No writes to ended spans, and the
POSTthat carried the call is the one marked ERROR. The two POSTs that did not fail,initializeandnotifications/initialized, stay UNSET rather than absorbing another request's failure.Type
🐛 Bug Fix
Changes
async_post_call_failure_hooknow prefers the transport span published for the message being handled, falling back to the request-root anchor and then touser_api_key_dict.parent_otel_spanexactly as before, so the pre-call failure paths (auth rejection, malformed body) are untouched.The transport is published on the ASGI scope of the request being handled and read back through
req_ctx.request, theRequestthe streamable-HTTP transport attaches to every message. That makes it per-message rather than per-session, which matters because a JSON-RPC response POST deliberately skips the per-session lock: it can arrive while the tool call awaiting it is still in flight, so a field on the shared auth object would be overwritten mid-call and the tool call's telemetry would land on the response's request. A scope belongs to one request and dies with it, so nothing keeps a finished span alive on idle session state either. This replaces the auth-context field #34537 introduced, which is removed.It is the live
Span, not theSpanContext#34537 carried. Parenting and linking only need the context, but stamping needs a span still open for writes, so one value now serves both and the context accessor derives from it.mcp_message_transport_spangates onis_recording(), notis_recordable_span: a finished span keeps a valid context forever, so the latter would hand back exactly the spans the SDK then refuses to write. The POST carrying atools/callstays open until the result is written, so it is live for the duration of the call, while a notification POST can answer first, which is the case the gate exists for.Publishing also re-anchors the request root for that message, so the identity attributes seeded onto the server span and any guardrail span emitted during the call land on the same request rather than on the session opener. The MCP SDK dispatches each message on its own task, so the anchor is scoped to the message. An already-finished transport is never anchored, since that would only move the dropped writes from one dead span to another.
Tests in
test_otel_v2_logger.pyadd three cases, each emitting from a task spawned before the current request so the stale anchor is real rather than simulated: the failure lands on this message's transport and not on the session opener, a transport that already answered is skipped in favor of the anchor, and a guardrail emitted during the message parents to this message's transport.QA runbook
mcp_servers, and start the proxy withLITELLM_OTEL_V2=1andcallbacks: ["otel"]pointed at any span exporterinitializeand thentools/callfor a tool that fails, so the result is HTTP 200 withisError: trueSetting attribute on ended spanorTried calling set_status on an ended spanwarningsPOST /<mcp_server_name>/mcpthat carried the call has ERROR status and theerror.*attributes, and that theinitializePOST does nottools/callon the same session and confirm the error follows the second POST, not the firstFinal Attestation