Repository navigation
fix(mcp): propagate active trace context - #63412
Yagi-Michael wants to merge 1 commit into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks right, and I verified it with your tests. All 16 cases in mcp-tracing.test.ts fail on the base and pass with this PR. The MCP SDK's streamable HTTP transport merges the per-request headers with configured, Authorization and Mcp-Param-* headers, and keeps them on retries. The other MCP tests and the core/util typecheck:tsc runs pass. The one MCP test that fails also fails on the base. I did not run a full OpenCode session against a real MCP server.
Public API: no OpenCode public API changes (no config, SDK, plugin or CLI surface). What changes on the wire is that every MCP request now includes traceparent, and tracestate/baggage when they exist, in params._meta and, over HTTP, as request headers.
Things worth addressing:
- Trace context is sent even when telemetry is off. Effect's built-in tracer still creates spans, so every MCP server and gateway gets a sampled
traceparentpointing at spans that are never exported (see inline). The issue suggested leaving it out when tracing isn't enabled. A maintainer should decide which behaviour they want. - The baggage code can't be reached from OpenCode today (see inline).
- The tests are large for what they check (see inline).
| import { Context, Effect, Option, References, Tracer } from "effect" | ||
|
|
||
| /** W3C context for the executing caller, without installing global instrumentation or propagators. */ | ||
| export const current: Effect.Effect<Record<string, string> | undefined> = Effect.gen(function* () { |
There was a problem hiding this comment.
With no OTLP endpoint configured, Effect's built-in tracer still creates spans, and this returns a sampled context for them. I checked: TraceContext.current.pipe(Effect.withSpan("MCP.callTool")) with no tracing layer gives { traceparent: "00-<random>-<random>-01" }. So every user's MCP servers and gateways, e.g. LiteLLM, will parent their spans onto OpenCode spans that are never exported, and their backends will show traces with a missing parent. #46856 suggested omitting traceparent when tracing isn't enabled. Should this only propagate when an OTel tracer is actually installed (i.e. the parent is an OTel span or an external span)? Or is this meant to match Effect's HttpClient, which also injects for its requests?
| } | ||
| const state = span.traceState?.serialize() | ||
| if (state) carrier.tracestate = state | ||
| const baggage = propagation |
There was a problem hiding this comment.
Nothing in OpenCode ever puts OTel baggage into the active context: no setBaggage/createBaggage anywhere in packages/*/src. So this hand-written baggage encoding, with its size limits, only ever runs in its own test. Unless a plugin or the AI SDK is expected to set baggage, I'd drop it, along with the OTLP-server baggage test in observability-trace-context.test.ts. That keeps the change to traceparent/tracestate. If it stays, W3CBaggagePropagator from @opentelemetry/core already handles encoding and limits.
| } | ||
| }), | ||
| ) | ||
|
|
There was a problem hiding this comment.
This file is 354 lines with a test fixture that runs a real HTTP server and an in-memory server for three transports. It checks concurrency, retry and cancellation across all of them. That's a lot for a change that adds a context field. Could it be cut down to the core cases? Something like: one stdio test showing _meta arrives, one HTTP test (legacy or modern) checking the traceparent header next to configured headers, and one test that a call without a parent span sends nothing. The concurrency/cancellation matrix, which uses fixed arrival counts and promise latches, mostly tests the SDK and adds upkeep.
Issue for this PR
Closes #46856
Type of change
What does this PR do?
Forward the executing caller's W3C trace context across existing MCP tool, prompt, resource and catalog requests. These requests currently succeed without propagation, leaving receiving services in disconnected traces.
Send
traceparent,tracestateandbaggagein_metaand supported request-scoped HTTP headers. Reading context per execution avoids connection-time attribution and mixed callers on a shared connection. Session/progress metadata, configured headers, OAuth retries and cancellation remain intact; auth discovery/token requests do not receive the trace overlay.Flag: none, treating this as missing standard behavior for existing traced operations. No tracing provider, global propagator, dependency or configuration surface is added. This is the proposed fix classification, not prior maintainer design approval. New experimental Events tracing and #62305 span enrichment are out of scope.
How did you verify your code works?
bun run check: 35/35 tasks. Canonical lint has 525 existing warnings; separate Effect-pattern lint retains 35 baseline-identical diagnostics.17b5826a. Local SDK/OTLP fixtures, not a live LiteLLM gateway, full app, Windows or CI run.Independent final source review found no verified correctness defects and reran core tracing/Events 23/23, util tracing 3/3, real-client OTLP 187/187, external-parent 21/21 and the supplemental auth fixture 7/7. The auth fixture uses local paths on one loopback server, not a distinct OAuth host; cross-origin separation was source-reviewed. Final commit
e2f434dc6921639e09ac0f090a82d73e56da52d2contains the exact five reviewed file hashes.Screenshots / recordings
No UI change. Protocol payload and exported-span assertions provide the before/after evidence.
Checklist
Added by the opencode bot (AI-generated).
Related: #28295, #62305