From baf668c9efe9547f874e1f4cf9b99d92fc8d8d67 Mon Sep 17 00:00:00 2001 From: ozz Date: Fri, 24 Jul 2026 09:46:05 +0000 Subject: [PATCH] Propagate W3C trace context in outbound MCP _meta vMCP terminates the MCP protocol and re-issues requests to backends as a client, but it only propagated W3C trace context via HTTP headers. A spec-compliant MCP 2026-07-28 backend reads trace context from params._meta (SEP-414), so backend spans reached through vMCP were not joined to the client->proxy->server trace. The InjectMetaTraceContext helper added for this in #3682 was left unwired (dead code). Add MetaWithTraceContext, a copy-safe wrapper that enriches a _meta map with the active traceparent/tracestate/baggage, and wire it into the vMCP CallTool/ReadResource/GetPrompt calls on both the pooled-client and session-backed backend paths. It returns nil when there is no active trace context so no _meta is emitted for 2025-11-25 peers, keeping the change backward compatible. Note: mcpcompat v0.0.32 does not forward Params.Meta on the non-resume ReadResource/GetPrompt paths yet, so those two are set for forward compatibility and covered by a tripwire test that fails loudly once the SDK starts forwarding them. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MRh394VZgnqbYjJwjrcJWm --- pkg/telemetry/propagation.go | 31 ++- pkg/telemetry/propagation_test.go | 70 ++++++ pkg/vmcp/client/client.go | 11 +- pkg/vmcp/client/meta_integration_test.go | 206 +++++++++++++++++- .../session/internal/backend/mcp_session.go | 15 +- .../backend/mcp_session_capabilities_test.go | 19 +- .../mcp_session_meta_propagation_test.go | 125 +++++++++++ 7 files changed, 463 insertions(+), 14 deletions(-) create mode 100644 pkg/vmcp/session/internal/backend/mcp_session_meta_propagation_test.go diff --git a/pkg/telemetry/propagation.go b/pkg/telemetry/propagation.go index dbc836e1a6..fd93edd2b9 100644 --- a/pkg/telemetry/propagation.go +++ b/pkg/telemetry/propagation.go @@ -5,6 +5,7 @@ package telemetry import ( "context" + "maps" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" @@ -64,7 +65,8 @@ func (c *MetaCarrier) Meta() map[string]interface{} { } // InjectMetaTraceContext injects the current trace context from ctx directly into -// the given meta map using W3C Trace Context format (traceparent, tracestate). +// the given meta map using W3C Trace Context format (traceparent, tracestate) +// plus W3C Baggage, subject to the globally configured OTEL propagator. // // This function operates directly on the meta map contents. Use this when you // already have the _meta map and want to inject trace context fields into it. @@ -75,3 +77,30 @@ func InjectMetaTraceContext(ctx context.Context, meta map[string]interface{}) { carrier := NewMetaCarrier(meta) otel.GetTextMapPropagator().Inject(ctx, carrier) } + +// MetaWithTraceContext returns a COPY of meta enriched with the current W3C +// trace context (traceparent/tracestate/baggage) from ctx, per SEP-414. +// The caller's map is never mutated (copy-before-mutate). Returns nil when the +// result is empty (no caller meta and no active trace context) so callers can +// omit _meta entirely for 2025-11-25 peers that don't expect it. +// +// Overwrite semantics: injecting the CURRENT span's context overwrites any +// inbound traceparent already present in the cloned caller map. This is the +// correct behavior for an intermediary — the outbound request must be a child +// of the span active here, not of whatever the caller inherited upstream. +// +// SECURITY: the injected trace context AND baggage cross the trust boundary into +// backend requests, and backends may be untrusted. Callers must never place +// secrets or PII in baggage, since it is serialized verbatim into the outbound +// _meta. Baggage is intentionally NOT stripped: SEP-414 explicitly reserves it. +func MetaWithTraceContext(ctx context.Context, meta map[string]interface{}) map[string]interface{} { + enriched := maps.Clone(meta) // nil-safe: returns nil for nil input + if enriched == nil { + enriched = make(map[string]interface{}) + } + InjectMetaTraceContext(ctx, enriched) + if len(enriched) == 0 { + return nil + } + return enriched +} diff --git a/pkg/telemetry/propagation_test.go b/pkg/telemetry/propagation_test.go index 3ade00394d..7927d142be 100644 --- a/pkg/telemetry/propagation_test.go +++ b/pkg/telemetry/propagation_test.go @@ -7,7 +7,10 @@ import ( "context" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" sdktrace "go.opentelemetry.io/otel/sdk/trace" ) @@ -152,3 +155,70 @@ func TestInjectMetaTraceContext_NilMeta(t *testing.T) { // Should not panic InjectMetaTraceContext(context.Background(), nil) } + +//nolint:paralleltest // Mutates global OTEL propagator +func TestMetaWithTraceContext(t *testing.T) { + oldPropagator := otel.GetTextMapPropagator() + otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) + defer otel.SetTextMapPropagator(oldPropagator) + + tp := sdktrace.NewTracerProvider() + defer func() { _ = tp.Shutdown(context.Background()) }() + tracer := tp.Tracer("test") + + spanCtx, span := tracer.Start(context.Background(), "test-span") + defer span.End() + + t.Run("active span with nil meta returns traceparent", func(t *testing.T) { + got := MetaWithTraceContext(spanCtx, nil) + require.NotNil(t, got) + tp, ok := got["traceparent"].(string) + require.True(t, ok, "expected traceparent to be present") + assert.NotEmpty(t, tp) + }) + + t.Run("active span with caller meta preserves caller fields without mutating input", func(t *testing.T) { + input := map[string]any{"progressToken": "tok-123"} + got := MetaWithTraceContext(spanCtx, input) + + require.NotNil(t, got) + assert.Equal(t, "tok-123", got["progressToken"]) + tp, ok := got["traceparent"].(string) + require.True(t, ok, "expected traceparent to be present") + assert.NotEmpty(t, tp) + + // Copy-before-mutate: the caller's original map must be untouched. + assert.Equal(t, map[string]any{"progressToken": "tok-123"}, input) + _, present := input["traceparent"] + assert.False(t, present, "input map must not be mutated with traceparent") + }) + + t.Run("no active span with nil meta returns nil", func(t *testing.T) { + got := MetaWithTraceContext(context.Background(), nil) + assert.Nil(t, got) + }) + + t.Run("no active span with caller meta returns unmutated clone", func(t *testing.T) { + input := map[string]any{"progressToken": "tok-456"} + got := MetaWithTraceContext(context.Background(), input) + + require.NotNil(t, got) + assert.Equal(t, input, got) + _, present := got["traceparent"] + assert.False(t, present, "no traceparent expected without an active trace context") + }) + + t.Run("baggage present in ctx is propagated", func(t *testing.T) { + member, err := baggage.NewMember("user.id", "42") + require.NoError(t, err) + bag, err := baggage.New(member) + require.NoError(t, err) + ctx := baggage.ContextWithBaggage(spanCtx, bag) + + got := MetaWithTraceContext(ctx, nil) + require.NotNil(t, got) + bgKey, ok := got["baggage"].(string) + require.True(t, ok, "expected baggage key to be present") + assert.Contains(t, bgKey, "user.id=42") + }) +} diff --git a/pkg/vmcp/client/client.go b/pkg/vmcp/client/client.go index 9c029f1ce3..119de0c829 100644 --- a/pkg/vmcp/client/client.go +++ b/pkg/vmcp/client/client.go @@ -30,6 +30,7 @@ import ( "github.com/stacklok/toolhive-core/mcpcompat/mcp" "github.com/stacklok/toolhive/pkg/auth" "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/telemetry" "github.com/stacklok/toolhive/pkg/versions" "github.com/stacklok/toolhive/pkg/vmcp" vmcpauth "github.com/stacklok/toolhive/pkg/vmcp/auth" @@ -1056,7 +1057,7 @@ func (h *httpBackendClient) CallTool( Params: mcp.CallToolParams{ Name: backendToolName, Arguments: arguments, - Meta: conversion.ToMCPMeta(meta), + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, meta)), }, }) if err != nil { @@ -1160,9 +1161,12 @@ func (h *httpBackendClient) ReadResource( slog.Debug("translating resource URI", "client_uri", uri, "backend_uri", backendURI) } + // Forward-compat: mcpcompat drops Params.Meta on this path today (no-op on + // the wire). See TestOutboundMetaTraceContext for details/tripwire. result, err := c.ReadResource(ctx, mcp.ReadResourceRequest{ Params: mcp.ReadResourceParams{ - URI: backendURI, + URI: backendURI, + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, nil)), }, }) if err != nil { @@ -1215,10 +1219,13 @@ func (h *httpBackendClient) GetPrompt( stringArgs := conversion.ConvertPromptArguments(arguments) + // Forward-compat: mcpcompat drops Params.Meta on this path today (no-op on + // the wire). See TestOutboundMetaTraceContext for details/tripwire. result, err := c.GetPrompt(ctx, mcp.GetPromptRequest{ Params: mcp.GetPromptParams{ Name: backendPromptName, Arguments: stringArgs, + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, nil)), }, }) if err != nil { diff --git a/pkg/vmcp/client/meta_integration_test.go b/pkg/vmcp/client/meta_integration_test.go index e8659709b9..8c8fc563b4 100644 --- a/pkg/vmcp/client/meta_integration_test.go +++ b/pkg/vmcp/client/meta_integration_test.go @@ -10,11 +10,15 @@ import ( "io" "net" "net/http" + "sync" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/propagation" + sdktrace "go.opentelemetry.io/otel/sdk/trace" "github.com/stacklok/toolhive-core/mcpcompat/mcp" "github.com/stacklok/toolhive-core/mcpcompat/server" @@ -29,7 +33,7 @@ func TestMetaPreservation_CallTool(t *testing.T) { t.Parallel() // Create and start a real MCP server that returns _meta - port, cleanup := startTestMCPServer(t) + port, _, cleanup := startTestMCPServer(t) defer cleanup() // Create vMCP backend client with unauthenticated strategy @@ -77,7 +81,7 @@ func TestMetaPreservation_CallTool(t *testing.T) { func TestMetaPreservation_CallTool_NoMeta(t *testing.T) { t.Parallel() - port, cleanup := startTestMCPServer(t) + port, _, cleanup := startTestMCPServer(t) defer cleanup() registry := auth.NewDefaultOutgoingAuthRegistry() @@ -124,7 +128,7 @@ func TestMetaPreservation_CallTool_NoMeta(t *testing.T) { func TestMetaPreservation_CallTool_Error(t *testing.T) { t.Parallel() - port, cleanup := startTestMCPServer(t) + port, _, cleanup := startTestMCPServer(t) defer cleanup() registry := auth.NewDefaultOutgoingAuthRegistry() @@ -171,7 +175,7 @@ func TestMetaPreservation_CallTool_Error(t *testing.T) { func TestMetaPreservation_GetPrompt(t *testing.T) { t.Parallel() - port, cleanup := startTestMCPServer(t) + port, _, cleanup := startTestMCPServer(t) defer cleanup() registry := auth.NewDefaultOutgoingAuthRegistry() @@ -227,7 +231,7 @@ func TestMetaPreservation_GetPrompt(t *testing.T) { func TestMetaPreservation_ReadResource(t *testing.T) { t.Parallel() - port, cleanup := startTestMCPServer(t) + port, _, cleanup := startTestMCPServer(t) defer cleanup() registry := auth.NewDefaultOutgoingAuthRegistry() @@ -263,11 +267,152 @@ func TestMetaPreservation_ReadResource(t *testing.T) { assert.Equal(t, "text/plain", result.Contents[0].MimeType) } +// TestOutboundMetaTraceContext verifies that the vMCP backend client injects the +// current W3C trace context (traceparent) into outbound params._meta (SEP-414) +// for CallTool, and that it is omitted entirely when there is no active span. +// +// ReadResource and GetPrompt are NOT asserted for on-the-wire traceparent +// delivery here: github.com/stacklok/toolhive-core's mcpcompat client (as of +// v0.0.32) builds the underlying go-sdk resources/read and prompts/get +// requests without forwarding request.Params.Meta at all (only CallTool's +// non-resume path converts and forwards Meta - see mcpcompat/client/client.go +// CallTool vs ReadResource/GetPrompt). This means the Meta we set on +// mcp.ReadResourceParams/mcp.GetPromptParams in client.go and mcp_session.go +// is silently dropped before it reaches the wire for these two operations +// today. The code changes are still correct and forward-compatible: they will +// start working the moment mcpcompat forwards Meta the same way it already +// does for CallTool, with no further change needed on our side. This is +// analogous to the pre-existing SDK limitation documented in +// TestMetaPreservation_ReadResource (inbound resource _meta). +// +// This test mutates the global OTEL propagator, so it must NOT run in +// parallel with the other tests in this file (see propagation_test.go for the +// same convention in pkg/telemetry). +func TestOutboundMetaTraceContext(t *testing.T) { //nolint:paralleltest // Mutates global OTEL propagator + oldPropagator := otel.GetTextMapPropagator() + otel.SetTextMapPropagator(propagation.TraceContext{}) + defer otel.SetTextMapPropagator(oldPropagator) + + port, capture, cleanup := startTestMCPServer(t) + defer cleanup() + + registry := auth.NewDefaultOutgoingAuthRegistry() + err := registry.RegisterStrategy("unauthenticated", &strategies.UnauthenticatedStrategy{}) + require.NoError(t, err) + + backendClient, err := vmcpclient.NewHTTPBackendClient(registry) + require.NoError(t, err) + + target := &vmcp.BackendTarget{ + WorkloadID: "test-backend", + WorkloadName: "Test Backend", + BaseURL: "http://127.0.0.1:" + port, + TransportType: "streamable-http", + } + + tp := sdktrace.NewTracerProvider() + defer func() { _ = tp.Shutdown(context.Background()) }() + tracer := tp.Tracer("test") + + //nolint:paralleltest // sequential: shares capture/backendClient state with the sibling subtests below + t.Run("CallTool injects traceparent", func(t *testing.T) { + spanCtx, span := tracer.Start(context.Background(), "test-span") + ctx, cancel := context.WithTimeout(spanCtx, 5*time.Second) + defer cancel() + defer span.End() + + _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil) + require.NoError(t, err) + + got := capture.get("tool") + require.NotNil(t, got, "expected params._meta to be sent") + traceparent, ok := got.AdditionalFields["traceparent"].(string) + require.True(t, ok, "expected traceparent in captured _meta") + // Prove span identity, not just presence: assert the W3C traceparent + // carries the active span's TraceID rather than a stale/global one. + assert.Contains(t, traceparent, span.SpanContext().TraceID().String(), + "traceparent must carry the active span's TraceID") + }) + + //nolint:paralleltest // sequential: shares capture/backendClient state with the sibling subtests + t.Run("ReadResource and GetPrompt: mcpcompat drops outbound Meta (documented SDK limitation)", func(t *testing.T) { + spanCtx, span := tracer.Start(context.Background(), "test-span") + ctx, cancel := context.WithTimeout(spanCtx, 5*time.Second) + defer cancel() + defer span.End() + + // TODO: tripwire for a toolhive-core/mcpcompat limitation (see this + // test's doc comment). These asserts pass ONLY because mcpcompat drops + // outbound Params.Meta on the resources/read and prompts/get paths. When + // they start failing, mcpcompat now forwards Meta: delete the + // forward-compat NOTE comments in client.go/mcp_session.go and replace + // these nil checks with traceparent assertions (as done for CallTool). + _, err := backendClient.ReadResource(ctx, target, "test://capture-resource") + require.NoError(t, err) + assert.Nil(t, capture.get("resource"), + "expected nil ONLY because mcpcompat v0.0.32 drops outbound Meta for resources/read; "+ + "if this is now non-nil, mcpcompat forwards Meta — delete the forward-compat NOTE comments "+ + "in client.go/mcp_session.go and assert traceparent here instead") + + _, err = backendClient.GetPrompt(ctx, target, "test_prompt_capture_meta", nil) + require.NoError(t, err) + assert.Nil(t, capture.get("prompt"), + "expected nil ONLY because mcpcompat v0.0.32 drops outbound Meta for prompts/get; "+ + "if this is now non-nil, mcpcompat forwards Meta — delete the forward-compat NOTE comments "+ + "in client.go/mcp_session.go and assert traceparent here instead") + }) + + //nolint:paralleltest // sequential: reuses the "tool" capture key populated above to prove _meta is now absent + t.Run("CallTool omits _meta without an active span", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil) + require.NoError(t, err) + + got := capture.get("tool") + assert.Nil(t, got, "expected no _meta to be sent without an active trace context") + }) +} + +// metaCapture records the inbound params._meta observed by the test server's +// capture-only tool/resource/prompt handlers, keyed by capability name. It +// lets tests assert what the vMCP client actually sent on the wire (e.g. an +// injected traceparent), as opposed to what the backend echoes back. +type metaCapture struct { + mu sync.Mutex + meta map[string]*mcp.Meta +} + +func newMetaCapture() *metaCapture { + return &metaCapture{meta: make(map[string]*mcp.Meta)} +} + +func (c *metaCapture) record(key string, meta *mcp.Meta) { + c.mu.Lock() + defer c.mu.Unlock() + c.meta[key] = meta +} + +// get returns the captured _meta for key, or nil if the capability was never +// invoked or was invoked without a _meta field. +func (c *metaCapture) get(key string) *mcp.Meta { + c.mu.Lock() + defer c.mu.Unlock() + return c.meta[key] +} + // startTestMCPServer creates and starts a test MCP server with tools that return _meta. -// Returns the port and cleanup function. -func startTestMCPServer(t *testing.T) (string, func()) { +// It also registers capture-only "test_tool_capture_meta", "test_prompt_capture_meta", and +// "test://capture-resource" capabilities that record the inbound params._meta they receive +// into the returned metaCapture, so tests can assert on OUTBOUND _meta (e.g. injected W3C +// trace context) rather than only on what the backend echoes back. +// Returns the port, the meta capture, and a cleanup function. +func startTestMCPServer(t *testing.T) (string, *metaCapture, func()) { t.Helper() + capture := newMetaCapture() + // Create MCP server mcpServer := server.NewMCPServer("test-backend", "1.0.0") @@ -385,6 +530,51 @@ func startTestMCPServer(t *testing.T) (string, func()) { }, ) + // Add capture-only tool/resource/prompt that record inbound params._meta + // for outbound-propagation assertions (see metaCapture). + mcpServer.AddTool( + mcp.NewTool("test_tool_capture_meta", + mcp.WithDescription("Capture-only tool that records inbound _meta"), + ), + func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + capture.record("tool", request.Params.Meta) + return &mcp.CallToolResult{ + Content: []mcp.Content{mcp.NewTextContent("captured")}, + }, nil + }, + ) + mcpServer.AddPrompt( + mcp.NewPrompt("test_prompt_capture_meta", + mcp.WithPromptDescription("Capture-only prompt that records inbound _meta"), + ), + func(_ context.Context, request mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { + capture.record("prompt", request.Params.Meta) + return &mcp.GetPromptResult{ + Messages: []mcp.PromptMessage{ + {Role: "user", Content: mcp.NewTextContent("captured")}, + }, + }, nil + }, + ) + mcpServer.AddResource( + mcp.Resource{ + URI: "test://capture-resource", + Name: "Capture Resource", + Description: "Capture-only resource that records inbound _meta", + MIMEType: "text/plain", + }, + func(_ context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + capture.record("resource", request.Params.Meta) + return []mcp.ResourceContents{ + mcp.TextResourceContents{ + URI: "test://capture-resource", + MIMEType: "text/plain", + Text: "captured", + }, + }, nil + }, + ) + // Create HTTP handler for the MCP server httpHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // MCP over HTTP uses POST requests with JSON-RPC @@ -438,5 +628,5 @@ func startTestMCPServer(t *testing.T) (string, func()) { _ = listener.Close() } - return port, cleanup + return port, capture, cleanup } diff --git a/pkg/vmcp/session/internal/backend/mcp_session.go b/pkg/vmcp/session/internal/backend/mcp_session.go index fdb7412eac..39b7658e45 100644 --- a/pkg/vmcp/session/internal/backend/mcp_session.go +++ b/pkg/vmcp/session/internal/backend/mcp_session.go @@ -17,6 +17,7 @@ import ( "github.com/stacklok/toolhive-core/mcpcompat/mcp" "github.com/stacklok/toolhive/pkg/auth" "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/telemetry" "github.com/stacklok/toolhive/pkg/versions" "github.com/stacklok/toolhive/pkg/vmcp" vmcpauth "github.com/stacklok/toolhive/pkg/vmcp/auth" @@ -135,7 +136,7 @@ func (c *mcpSession) CallTool( Params: mcp.CallToolParams{ Name: backendName, Arguments: arguments, - Meta: conversion.ToMCPMeta(meta), + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, meta)), }, }) if err != nil { @@ -172,8 +173,14 @@ func (c *mcpSession) ReadResource( slog.Debug("Translating resource URI", "clientURI", uri, "backendURI", backendURI) } + // Forward-compat: mcpcompat drops Params.Meta on this path today (no-op on + // the wire). See pkg/vmcp/client's TestOutboundMetaTraceContext for + // details/tripwire. result, err := c.client.ReadResource(ctx, mcp.ReadResourceRequest{ - Params: mcp.ReadResourceParams{URI: backendURI}, + Params: mcp.ReadResourceParams{ + URI: backendURI, + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, nil)), + }, }) if err != nil { return nil, fmt.Errorf("resource %q read failed on backend %s: %w", uri, c.target.WorkloadID, err) @@ -198,10 +205,14 @@ func (c *mcpSession) GetPrompt( stringArgs := conversion.ConvertPromptArguments(arguments) + // Forward-compat: mcpcompat drops Params.Meta on this path today (no-op on + // the wire). See pkg/vmcp/client's TestOutboundMetaTraceContext for + // details/tripwire. result, err := c.client.GetPrompt(ctx, mcp.GetPromptRequest{ Params: mcp.GetPromptParams{ Name: backendName, Arguments: stringArgs, + Meta: conversion.ToMCPMeta(telemetry.MetaWithTraceContext(ctx, nil)), }, }) if err != nil { diff --git a/pkg/vmcp/session/internal/backend/mcp_session_capabilities_test.go b/pkg/vmcp/session/internal/backend/mcp_session_capabilities_test.go index cb94911b0c..ab325a378a 100644 --- a/pkg/vmcp/session/internal/backend/mcp_session_capabilities_test.go +++ b/pkg/vmcp/session/internal/backend/mcp_session_capabilities_test.go @@ -67,6 +67,11 @@ type fakeBackend struct { // method name. Tests asserting transport-chain behavior (e.g. HeaderForward) // use headersFor(method) to inspect the headers a backend actually saw. headersByMethod map[string]http.Header + + // metaByMethod records the raw params._meta keyed by JSON-RPC method name. + // Tests asserting outbound W3C trace context propagation (SEP-414) use + // metaFor(method) to inspect the _meta a backend actually saw on the wire. + metaByMethod map[string]json.RawMessage } type jsonRPCError struct { @@ -82,6 +87,7 @@ func newFakeBackend(t *testing.T, fb *fakeBackend) string { fb.t = t fb.methodCalls = make(map[string]int) fb.headersByMethod = make(map[string]http.Header) + fb.metaByMethod = make(map[string]json.RawMessage) mux := http.NewServeMux() mux.HandleFunc("/mcp", fb.handle) @@ -116,6 +122,15 @@ func (f *fakeBackend) headersFor(method string) http.Header { return h.Clone() } +// metaFor returns the raw params._meta recorded for the most recent JSON-RPC +// request with the given method, or nil if no such request was seen or it +// carried no _meta field. +func (f *fakeBackend) metaFor(method string) json.RawMessage { + f.mu.Lock() + defer f.mu.Unlock() + return f.metaByMethod[method] +} + // handle implements the JSON-RPC subset needed for backend init. The // streamable-HTTP transport sends POST requests with Accept: // application/json, text/event-stream — we always reply with @@ -142,7 +157,8 @@ func (f *fakeBackend) handle(w http.ResponseWriter, r *http.Request) { ID json.RawMessage `json:"id"` Method string `json:"method"` Params struct { - Cursor string `json:"cursor"` + Cursor string `json:"cursor"` + Meta json.RawMessage `json:"_meta"` } `json:"params"` } if err := json.Unmarshal(body, &msg); err != nil { @@ -154,6 +170,7 @@ func (f *fakeBackend) handle(w http.ResponseWriter, r *http.Request) { f.mu.Lock() f.methodCalls[msg.Method]++ f.headersByMethod[msg.Method] = r.Header.Clone() + f.metaByMethod[msg.Method] = msg.Params.Meta f.mu.Unlock() // Notifications (no id, e.g. notifications/initialized) get an empty 202. diff --git a/pkg/vmcp/session/internal/backend/mcp_session_meta_propagation_test.go b/pkg/vmcp/session/internal/backend/mcp_session_meta_propagation_test.go new file mode 100644 index 0000000000..ff3de16f5f --- /dev/null +++ b/pkg/vmcp/session/internal/backend/mcp_session_meta_propagation_test.go @@ -0,0 +1,125 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/propagation" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + + "github.com/stacklok/toolhive-core/mcpcompat/mcp" + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// TestHTTPSession_CallTool_InjectsTraceparent asserts that the session-backed +// connector (pkg/vmcp/session/internal/backend) injects the current W3C trace +// context (traceparent) into outbound params._meta (SEP-414) for CallTool, and +// that it is omitted entirely when there is no active span. This mirrors the +// coverage in pkg/vmcp/client's TestOutboundMetaTraceContext for the +// session-side connector. +// +// ReadResource and GetPrompt are not covered here for the same reason +// documented on pkg/vmcp/client's TestOutboundMetaTraceContext: mcpcompat's +// non-resume client does not forward Params.Meta to the wire for those two +// operations (only CallTool does), so there is nothing to observe on fakeBackend +// today. See mcp_session.go's ReadResource/GetPrompt for the code that will +// start working once mcpcompat closes that gap. +// +// This test mutates the global OTEL propagator, so it must NOT run in +// parallel with the rest of this package's tests. +func TestHTTPSession_CallTool_InjectsTraceparent(t *testing.T) { //nolint:paralleltest // Mutates global OTEL propagator + oldPropagator := otel.GetTextMapPropagator() + otel.SetTextMapPropagator(propagation.TraceContext{}) + defer otel.SetTextMapPropagator(oldPropagator) + + fb := &fakeBackend{advertiseTools: true, tools: []mcp.Tool{{Name: "echo"}}} + url := newFakeBackend(t, fb) + + target := &vmcp.BackendTarget{ + WorkloadID: "trace-context-backend", + WorkloadName: "trace-context-backend", + BaseURL: url, + TransportType: "streamable-http", + } + + registry := newTestRegistry(t) + connector := NewHTTPConnector(registry) + + initCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + sess, _, err := connector(initCtx, target, nil, "") + require.NoError(t, err) + t.Cleanup(func() { _ = sess.Close() }) + + tp := sdktrace.NewTracerProvider() + defer func() { _ = tp.Shutdown(context.Background()) }() + tracer := tp.Tracer("test") + + //nolint:paralleltest // sequential: shares the fakeBackend/session with the sibling subtests below + t.Run("active span: traceparent on the wire carries this span's TraceID", func(t *testing.T) { + spanCtx, span := tracer.Start(context.Background(), "test-span") + defer span.End() + + _, err := sess.CallTool(spanCtx, "echo", map[string]any{}, nil) + require.NoError(t, err) + + raw := fb.metaFor(string(mcp.MethodToolsCall)) + require.NotEmpty(t, raw, "expected params._meta to be sent") + + var meta map[string]any + require.NoError(t, json.Unmarshal(raw, &meta)) + traceparent, ok := meta["traceparent"].(string) + require.True(t, ok, "expected traceparent in captured _meta") + + // Prove span identity, not just presence: a stale or global context would + // still yield a non-empty traceparent. The W3C format is + // "---"; the trace-id field must match + // the span active at the call site. + wantTraceID := span.SpanContext().TraceID().String() + assert.Contains(t, traceparent, wantTraceID, + "traceparent on the wire must carry the active span's TraceID") + }) + + //nolint:paralleltest // sequential: shares the fakeBackend/session with the sibling subtests + t.Run("progressToken and traceparent coexist on the wire", func(t *testing.T) { + spanCtx, span := tracer.Start(context.Background(), "test-span") + defer span.End() + + // progressToken serializes via Meta.ProgressToken while the injected + // traceparent rides Meta.AdditionalFields — assert both survive the round + // trip into the real serialized _meta bytes. progressToken is caller + // _meta (the 4th CallTool arg), not a tool argument. + _, err := sess.CallTool(spanCtx, "echo", map[string]any{}, map[string]any{"progressToken": "tok"}) + require.NoError(t, err) + + raw := fb.metaFor(string(mcp.MethodToolsCall)) + require.NotEmpty(t, raw, "expected params._meta to be sent") + + var meta map[string]any + require.NoError(t, json.Unmarshal(raw, &meta)) + + assert.Equal(t, "tok", meta["progressToken"], "progressToken must survive on the wire") + traceparent, ok := meta["traceparent"].(string) + require.True(t, ok, "expected traceparent alongside progressToken in captured _meta") + assert.Contains(t, traceparent, span.SpanContext().TraceID().String(), + "traceparent must carry the active span's TraceID when coexisting with progressToken") + }) + + //nolint:paralleltest // sequential: reuses the "tools/call" capture key populated above + t.Run("no active span: _meta is omitted", func(t *testing.T) { + _, err := sess.CallTool(context.Background(), "echo", map[string]any{}, nil) + require.NoError(t, err) + + raw := fb.metaFor(string(mcp.MethodToolsCall)) + assert.Empty(t, raw, "expected no _meta to be sent without an active trace context") + }) +}