From 68eefb371e022bf0c0754551e2199abb0914f30d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 16:18:25 +0000 Subject: [PATCH 1/8] feat(core): Enrich MCP SDK spans instead of duplicating them @modelcontextprotocol/server starts its own OpenTelemetry span for every MCP request and notification once a tracer provider is registered. With the Sentry tracer provider (e.g. @sentry/nextjs, @sentry/sveltekit, @sentry/deno, or enableOpenTelemetrySetup: true) every request ended up with two server spans. The MCP server instrumentation now adds its attributes to the span the MCP SDK starts and only starts its own span when the SDK records nothing. Refs #26078 Co-Authored-By: Francesco Gringl-Novy --- .../node-express-mcp-v2/instrument.mjs | 2 + .../node-express-mcp-v2/package.json | 13 +- .../tests/mcp-sdk-otel.test.ts | 95 ++++++ .../node-express-mcp-v2/tests/mcp.test.ts | 3 + .../integrations/mcp-server/correlation.ts | 92 +++--- .../core/src/integrations/mcp-server/index.ts | 15 +- .../core/src/integrations/mcp-server/spans.ts | 68 ++-- .../src/integrations/mcp-server/transport.ts | 160 ++++++++-- .../core/src/integrations/mcp-server/types.ts | 13 - .../mcp-server/upstreamInstrumentation.ts | 202 ++++++++++++ .../upstreamInstrumentation.test.ts | 293 ++++++++++++++++++ 11 files changed, 844 insertions(+), 112 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts create mode 100644 packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts create mode 100644 packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs index f3dd95215d03..a3d6c930a0a8 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs @@ -6,4 +6,6 @@ Sentry.init({ debug: !!process.env.DEBUG, tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1, + // Registers the Sentry tracer provider, so the MCP SDK's built-in OpenTelemetry instrumentation records spans. + enableOpenTelemetrySetup: process.env.MCP_SDK_OTEL === '1', }); diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json index 6482c7f3a8e0..6a5886e1732a 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json @@ -8,7 +8,9 @@ "test": "playwright test", "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install && pnpm build", - "test:assert": "pnpm test" + "test:build-mcp-sdk-otel": "pnpm install && pnpm add --config.block-exotic-subdeps=false https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@919bb8e https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@919bb8e @opentelemetry/api@^1.9.0 && pnpm add --config.block-exotic-subdeps=false -D https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@919bb8e && pnpm build", + "test:assert": "pnpm test", + "test:assert-mcp-sdk-otel": "MCP_SDK_OTEL=1 pnpm test" }, "dependencies": { "@cfworker/json-schema": "^4.0.0", @@ -28,6 +30,13 @@ }, "type": "module", "sentryTest": { - "optional": true + "optional": true, + "optionalVariants": [ + { + "build-command": "pnpm test:build-mcp-sdk-otel", + "assert-command": "pnpm test:assert-mcp-sdk-otel", + "label": "node-express-mcp-v2 (MCP SDK OpenTelemetry)" + } + ] } } diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts new file mode 100644 index 000000000000..551d92b38ae1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts @@ -0,0 +1,95 @@ +import { expect, test } from '@playwright/test'; +import { collectStreamedSpans, getSpanOp, type SerializedStreamedSpan } from '@sentry-internal/test-utils'; +import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; + +// Runs in the `MCP SDK OpenTelemetry` variant, where `@modelcontextprotocol/server` emits its own +// OpenTelemetry spans and Sentry registers its tracer provider (`enableOpenTelemetrySetup: true`). +test.skip(process.env.MCP_SDK_OTEL !== '1', 'Only runs in the MCP SDK OpenTelemetry variant'); + +const PROXY_SERVER_NAME = 'node-express-mcp-v2'; + +function mcpSpansFor(spans: SerializedStreamedSpan[], method: string): SerializedStreamedSpan[] { + return spans.filter(span => span.attributes?.['mcp.method.name']?.value === method); +} + +/** The trace of one MCP request: its `POST /mcp` segment and every span below it. */ +function collectMcpRequestTrace(spanName: string): Promise { + return collectStreamedSpans( + PROXY_SERVER_NAME, + spans => spans.some(span => span.is_segment) && spans.some(span => span.name === spanName), + ); +} + +test('enriches the MCP SDK spans instead of duplicating them', async ({ baseURL }) => { + const client = new Client({ name: 'test-client-v2', version: '1.0.0' }); + const transport = new StreamableHTTPClientTransport(new URL(`${baseURL}/mcp`)); + + const initializeTracePromise = collectMcpRequestTrace('initialize'); + await client.connect(transport); + + await test.step('initialize', async () => { + const spans = await initializeTracePromise; + const initializeSpans = mcpSpansFor(spans, 'initialize'); + expect(initializeSpans).toHaveLength(1); + + const [initializeSpan] = initializeSpans; + expect(getSpanOp(initializeSpan!)).toBe('mcp.server'); + expect(initializeSpan!.attributes?.['sentry.kind']?.value).toBe('server'); + expect(initializeSpan!.attributes?.['mcp.client.name']?.value).toBe('test-client-v2'); + expect(initializeSpan!.attributes?.['mcp.server.name']?.value).toBe('Echo-V2'); + expect(initializeSpan!.attributes?.['mcp.transport']?.value).toMatch(/StreamableHTTPServerTransport/); + + // The MCP SDK starts its span inside the request, so it is no longer a segment of its own. + expect(initializeSpan!.is_segment).toBe(false); + expect(getSpanOp(spans.find(span => span.is_segment)!)).toBe('http.server'); + }); + + await test.step('tools/call', async () => { + const toolTracePromise = collectMcpRequestTrace('tools/call echo'); + + const toolResult = await client.callTool({ name: 'echo', arguments: { message: 'foobar' } }); + expect(toolResult).toMatchObject({ content: [{ text: 'Tool echo: foobar', type: 'text' }] }); + + const spans = await toolTracePromise; + const toolSpans = mcpSpansFor(spans, 'tools/call'); + expect(toolSpans).toHaveLength(1); + + const [toolSpan] = toolSpans; + expect(toolSpan!.name).toBe('tools/call echo'); + expect(getSpanOp(toolSpan!)).toBe('mcp.server'); + expect(toolSpan!.attributes?.['sentry.kind']?.value).toBe('server'); + expect(toolSpan!.attributes?.['mcp.tool.name']?.value).toBe('echo'); + expect(toolSpan!.attributes?.['mcp.request.argument.message']?.value).toBe('"foobar"'); + expect(toolSpan!.attributes?.['mcp.tool.result.content_count']?.value).toBe(1); + }); + + await test.step('prompts/get', async () => { + const promptTracePromise = collectMcpRequestTrace('prompts/get echo'); + + await client.getPrompt({ name: 'echo', arguments: { message: 'foobar' } }); + + const spans = await promptTracePromise; + const promptSpans = mcpSpansFor(spans, 'prompts/get'); + expect(promptSpans).toHaveLength(1); + expect(getSpanOp(promptSpans[0]!)).toBe('mcp.server'); + expect(promptSpans[0]!.attributes?.['mcp.prompt.name']?.value).toBe('echo'); + }); + + await test.step('tool error', async () => { + const toolTracePromise = collectMcpRequestTrace('tools/call always-error'); + + try { + await client.callTool({ name: 'always-error', arguments: {} }); + } catch { + // Expected: MCP SDK throws when the tool returns a JSON-RPC error + } + + const spans = await toolTracePromise; + const toolSpans = mcpSpansFor(spans, 'tools/call'); + expect(toolSpans).toHaveLength(1); + expect(getSpanOp(toolSpans[0]!)).toBe('mcp.server'); + expect(toolSpans[0]!.status).toBe('error'); + }); + + await client.close(); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts index c7e7e2dae2e2..1e0d7daff1b2 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts @@ -3,6 +3,9 @@ import { waitForStreamedSpan, getSpanOp } from '@sentry-internal/test-utils'; import { Client } from '@modelcontextprotocol/client'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; +// The `MCP SDK OpenTelemetry` variant is covered by `mcp-sdk-otel.test.ts`. +test.skip(process.env.MCP_SDK_OTEL === '1', 'MCP SDK spans reach Sentry in this variant'); + // The server in this app is never wrapped with `wrapMcpServerWithSentry` — spans are produced // solely by the auto-instrumenting `mcpServer` integration. test('auto-instruments a stable MCP SDK v2 server (no manual wrap)', async ({ baseURL }) => { diff --git a/packages/core/src/integrations/mcp-server/correlation.ts b/packages/core/src/integrations/mcp-server/correlation.ts index 4ca42839787d..34348e031a3c 100644 --- a/packages/core/src/integrations/mcp-server/correlation.ts +++ b/packages/core/src/integrations/mcp-server/correlation.ts @@ -94,6 +94,58 @@ export function storeSpanForRequest( }); } +/** + * Records a request's response on its span: session metadata from `initialize`, the JSON-RPC + * error, and tool or prompt results. Does not end the span. + * @param transport - MCP transport instance + * @param span - The request span + * @param method - MCP method name + * @param result - Execution result for attribute extraction + * @param error - The JSON-RPC error response, if any + * @param capturePolicy - Capture policy resolved when the request began + */ +export function applyResponseToSpan( + transport: MCPTransport, + span: Span, + method: string, + result: unknown, + error: JsonRpcError | undefined, + capturePolicy: ResolvedMcpOptions, +): void { + const responseSessionData = + method === 'initialize' ? extractSessionDataFromInitializeResponse(result) : extractSessionDataFromResponse(result); + if (method === 'initialize' && (responseSessionData.protocolVersion || responseSessionData.serverInfo)) { + updateSessionDataForTransport(transport, responseSessionData); + } + const responseAttributes: Record = { + ...buildServerAttributesFromInfo(responseSessionData.serverInfo), + }; + if (responseSessionData.protocolVersion) { + responseAttributes[MCP_PROTOCOL_VERSION] = responseSessionData.protocolVersion; + } + if (Object.keys(responseAttributes).length > 0) { + span.setAttributes(responseAttributes); + } + + if (error) { + span.setAttributes({ [RPC_RESPONSE_STATUS_CODE]: String(error.code) }); + if (isJsonRpcServerError(error.code)) { + span.setAttributes({ [ERROR_TYPE]: String(error.code) }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: error.message }); + } + } else if (method === 'tools/call') { + const toolAttributes = extractToolResultAttributes(result, capturePolicy.recordOutputs); + span.setAttributes(toolAttributes); + // oxlint-disable-next-line typescript/no-deprecated -- Preserve classification based on the existing tool result attribute. + if (toolAttributes[MCP_TOOL_RESULT_IS_ERROR] === true) { + span.setAttributes({ [ERROR_TYPE]: 'tool_error' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + } + } else if (method === 'prompts/get') { + span.setAttributes(extractPromptResultAttributes(result, capturePolicy.recordOutputs)); + } +} + /** * Completes span with results and cleans up correlation * @param transport - MCP transport instance @@ -110,44 +162,8 @@ export function completeSpanWithResults( const spanMap = getOrCreateSpanMap(transport); const spanData = spanMap.get(requestId); if (spanData) { - const { span, method } = spanData; - const responseSessionData = - method === 'initialize' - ? extractSessionDataFromInitializeResponse(result) - : extractSessionDataFromResponse(result); - if (method === 'initialize' && (responseSessionData.protocolVersion || responseSessionData.serverInfo)) { - updateSessionDataForTransport(transport, responseSessionData); - } - const responseAttributes: Record = { - ...buildServerAttributesFromInfo(responseSessionData.serverInfo), - }; - if (responseSessionData.protocolVersion) { - responseAttributes[MCP_PROTOCOL_VERSION] = responseSessionData.protocolVersion; - } - if (Object.keys(responseAttributes).length > 0) { - span.setAttributes(responseAttributes); - } - - if (error) { - span.setAttributes({ [RPC_RESPONSE_STATUS_CODE]: String(error.code) }); - if (isJsonRpcServerError(error.code)) { - span.setAttributes({ [ERROR_TYPE]: String(error.code) }); - span.setStatus({ code: SPAN_STATUS_ERROR, message: error.message }); - } - } else if (method === 'tools/call') { - const toolAttributes = extractToolResultAttributes(result, spanData.capturePolicy.recordOutputs); - span.setAttributes(toolAttributes); - // oxlint-disable-next-line typescript/no-deprecated -- Preserve classification based on the existing tool result attribute. - if (toolAttributes[MCP_TOOL_RESULT_IS_ERROR] === true) { - span.setAttributes({ [ERROR_TYPE]: 'tool_error' }); - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - } - } else if (method === 'prompts/get') { - const promptAttributes = extractPromptResultAttributes(result, spanData.capturePolicy.recordOutputs); - span.setAttributes(promptAttributes); - } - - span.end(); + applyResponseToSpan(transport, spanData.span, spanData.method, result, error, spanData.capturePolicy); + spanData.span.end(); spanMap.delete(requestId); } } diff --git a/packages/core/src/integrations/mcp-server/index.ts b/packages/core/src/integrations/mcp-server/index.ts index ebea8edd74f6..a667fa5ed542 100644 --- a/packages/core/src/integrations/mcp-server/index.ts +++ b/packages/core/src/integrations/mcp-server/index.ts @@ -2,6 +2,7 @@ import { fill } from '../../utils/object'; import { wrapAllMCPHandlers, wrapExistingHandlers } from './handlers'; import { wrapTransportError, wrapTransportOnClose, wrapTransportOnMessage, wrapTransportSend } from './transport'; import type { MCPServerInstance, McpServerWrapperOptions, MCPTransport } from './types'; +import { decorateUpstreamInstrumentation } from './upstreamInstrumentation'; import { validateMcpServerInstance } from './validation'; /** @@ -34,9 +35,13 @@ function applyMissingMcpOptions(target: McpServerWrapperOptions, source: McpServ } } -function instrumentTransport(transport: MCPTransport, options: McpServerWrapperOptions): void { - wrapTransportOnMessage(transport, options); - wrapTransportSend(transport, options); +function instrumentTransport( + transport: MCPTransport, + options: McpServerWrapperOptions, + serverInstance: MCPServerInstance, +): void { + wrapTransportOnMessage(transport, options, serverInstance); + wrapTransportSend(transport, options, serverInstance); wrapTransportOnClose(transport); wrapTransportError(transport); } @@ -187,7 +192,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S, } isTransportInstrumented = true; - instrumentTransport(transport, wrapperOptions); + instrumentTransport(transport, wrapperOptions, serverInstance); }; const restoreStart = interceptTransportStart(transport, instrumentTransportOnce); @@ -207,6 +212,8 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S, }; }); + decorateUpstreamInstrumentation(serverInstance); + wrapAllMCPHandlers(serverInstance); wrapExistingHandlers(serverInstance); diff --git a/packages/core/src/integrations/mcp-server/spans.ts b/packages/core/src/integrations/mcp-server/spans.ts index 9f370a5d1b98..cb7892fd0974 100644 --- a/packages/core/src/integrations/mcp-server/spans.ts +++ b/packages/core/src/integrations/mcp-server/spans.ts @@ -23,7 +23,7 @@ import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, - McpSpanConfig, + McpSpanType, MCPTransport, ResolvedMcpOptions, } from './types'; @@ -45,7 +45,7 @@ function createSpanName(method: string, target?: string): string { * @param type - Span type configuration * @returns Sentry-specific attributes */ -function buildSentryAttributes(type: McpSpanConfig['type']): Record { +function buildSentryAttributes(type: McpSpanType): Record { let op: string; let origin: string; @@ -72,43 +72,51 @@ function buildSentryAttributes(type: McpSpanConfig['type']): Record } { const { method } = message; const params = message.params; const client = getClient(); const spanStreamingEnabled = !!client && hasSpanStreamingEnabled(client); - // Determine span name based on type and OTEL conventions - let spanName: string; - if (type === 'request') { - const targetInfo = extractTargetInfo(method, params || {}); - const target = spanStreamingEnabled && !targetInfo.targetIsLowCardinality ? undefined : targetInfo.target; - spanName = method ? createSpanName(method, target) : MCP_SERVER_SPAN_NAME_FALLBACK; - } else { - // For notifications, use method name directly per OpenTelemetry conventions. - // With span streaming, span names have to be low cardinality, so a message without a method name gets a static name. - spanName = method || (spanStreamingEnabled ? MCP_NOTIFICATION_SPAN_NAME_FALLBACK : method); - } + // For notifications, use method name directly per OpenTelemetry conventions. + // With span streaming, span names have to be low cardinality, so a message without a method name gets a static name. + const name = method || (spanStreamingEnabled ? MCP_NOTIFICATION_SPAN_NAME_FALLBACK : method); const rawAttributes: Record = { ...buildTransportAttributes(transport, extra, message), [MCP_METHOD_NAME]: method, - ...buildTypeSpecificAttributes(type, message, params, options?.recordInputs), + ...buildTypeSpecificAttributes(type, message, params, options.recordInputs), ...buildSentryAttributes(type), }; const userInfo = Boolean(client?.getDataCollectionOptions().userInfo); const attributes = filterMcpPiiFromSpanData(rawAttributes, userInfo) as Record; + return { name, attributes }; +} + +function createMcpNotificationSpanWithCallback( + type: 'notification-incoming' | 'notification-outgoing', + message: JsonRpcNotification, + transport: MCPTransport, + options: ResolvedMcpOptions, + callback: () => unknown, + extra?: ExtraHandlerData, +): unknown { + const { name, attributes } = buildMcpNotificationSpanConfig(type, message, transport, options, extra); + return startSpan( { - name: spanName, + name, // oxlint-disable-next-line typescript/no-deprecated forceTransaction: true, attributes, @@ -133,14 +141,14 @@ export function createMcpNotificationSpan( options: ResolvedMcpOptions, callback: () => unknown, ): unknown { - return createMcpSpan({ - type: 'notification-incoming', - message: jsonRpcMessage, + return createMcpNotificationSpanWithCallback( + 'notification-incoming', + jsonRpcMessage, transport, - extra, - callback, options, - }); + callback, + extra, + ); } /** @@ -157,13 +165,7 @@ export function createMcpOutgoingNotificationSpan( options: ResolvedMcpOptions, callback: () => unknown, ): unknown { - return createMcpSpan({ - type: 'notification-outgoing', - message: jsonRpcMessage, - transport, - options, - callback, - }); + return createMcpNotificationSpanWithCallback('notification-outgoing', jsonRpcMessage, transport, options, callback); } /** diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index df0703128aef..00dbe83e66b9 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -10,15 +10,40 @@ import { getClient, getIsolationScope, withIsolationScope } from '../../currentS import { DEBUG_BUILD } from '../../debug-build'; import { withActiveSpan } from '../../tracing'; import { startInactiveSpan } from '../../tracing/trace'; +import type { Span } from '../../types/span'; +import { getActiveSpan } from '../../utils/spanUtils'; import { isObjectLike } from '../../utils/is'; import { fill } from '../../utils/object'; import { safeCallback } from '../../utils/safeCallback'; -import { cleanupPendingSpansForTransport, completeSpanWithResults, storeSpanForRequest } from './correlation'; +import { + applyResponseToSpan, + cleanupPendingSpansForTransport, + completeSpanWithResults, + storeSpanForRequest, +} from './correlation'; import { captureError, isJsonRpcServerError } from './errorCapture'; import { buildClientAttributesFromInfo, extractSessionDataFromInitializeRequest } from './sessionExtraction'; import { cleanupSessionDataForTransport, updateSessionDataForTransport } from './sessionManagement'; -import { buildMcpServerSpanConfig, createMcpNotificationSpan, createMcpOutgoingNotificationSpan } from './spans'; -import type { ExtraHandlerData, McpServerWrapperOptions, MCPTransport, ResolvedMcpOptions, SessionData } from './types'; +import { + buildMcpNotificationSpanConfig, + buildMcpServerSpanConfig, + createMcpNotificationSpan, + createMcpOutgoingNotificationSpan, +} from './spans'; +import type { + ExtraHandlerData, + JsonRpcRequest, + McpServerWrapperOptions, + MCPTransport, + ResolvedMcpOptions, + SessionData, +} from './types'; +import type { UpstreamOutcome } from './upstreamInstrumentation'; +import { + getUpstreamSpanForOutboundNotification, + shouldHandOffToUpstream, + withInboundHandoff, +} from './upstreamInstrumentation'; import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse } from './validation'; function resolveMcpOptions(options: McpServerWrapperOptions): ResolvedMcpOptions { @@ -66,8 +91,13 @@ function resolveOAuthClientName(options: McpServerWrapperOptions, extra?: ExtraH * Modern request metadata is read directly when building each message's span. * @param transport - MCP transport instance to wrap * @param options - MCP instrumentation options + * @param serverInstance - The MCP server the transport is connected to */ -export function wrapTransportOnMessage(transport: MCPTransport, options: McpServerWrapperOptions): void { +export function wrapTransportOnMessage( + transport: MCPTransport, + options: McpServerWrapperOptions, + serverInstance?: object, +): void { if (transport.onmessage) { fill(transport, 'onmessage', originalOnMessage => { return function (this: MCPTransport, message: unknown, extra?: unknown) { @@ -96,30 +126,87 @@ export function wrapTransportOnMessage(transport: MCPTransport, options: McpServ if (oauthClientName) { spanConfig.attributes[MCP_AUTH_CLIENT_NAME] = oauthClientName; } - const span = startInactiveSpan(spanConfig); - - if (request.method === 'initialize' && messageSessionData) { - span.setAttributes({ - ...buildClientAttributesFromInfo(messageSessionData.clientInfo), - ...(messageSessionData.protocolVersion && { - [MCP_PROTOCOL_VERSION]: messageSessionData.protocolVersion, - }), - }); - } + const sessionAttributes = + request.method === 'initialize' && messageSessionData + ? { + ...buildClientAttributesFromInfo(messageSessionData.clientInfo), + ...(messageSessionData.protocolVersion && { + [MCP_PROTOCOL_VERSION]: messageSessionData.protocolVersion, + }), + } + : undefined; + + const startRequestSpan = (): Span => { + const span = startInactiveSpan(spanConfig); + if (sessionAttributes) { + span.setAttributes(sessionAttributes); + } + storeSpanForRequest(transport, request.id, span, request.method, resolvedOptions); + return span; + }; - storeSpanForRequest(transport, request.id, span, request.method, resolvedOptions); + const callOriginal = (): unknown => + (originalOnMessage as (...args: unknown[]) => unknown).call(this, request, extra); + + if (shouldHandOffToUpstream(serverInstance)) { + return withInboundHandoff( + { + method: request.method, + id: request.id, + adopt: span => { + span.setAttributes({ ...spanConfig.attributes, ...sessionAttributes }); + return outcome => applyUpstreamOutcome(transport, span, request, outcome, resolvedOptions); + }, + runInOwnSpan: fn => withActiveSpan(startRequestSpan(), fn), + }, + callOriginal, + ); + } - return withActiveSpan(span, () => { - return (originalOnMessage as (...args: unknown[]) => unknown).call(this, request, extra); - }); + return withActiveSpan(startRequestSpan(), callOriginal); }); } if (notification) { const resolvedOptions = resolveMcpOptions(options); - return createMcpNotificationSpan(notification, transport, extra as ExtraHandlerData, resolvedOptions, () => { - return (originalOnMessage as (...args: unknown[]) => unknown).call(this, notification, extra); - }); + const callOriginal = (): unknown => + (originalOnMessage as (...args: unknown[]) => unknown).call(this, notification, extra); + + if (shouldHandOffToUpstream(serverInstance)) { + return withInboundHandoff( + { + method: notification.method, + adopt: span => { + span.setAttributes( + buildMcpNotificationSpanConfig( + 'notification-incoming', + notification, + transport, + resolvedOptions, + extra as ExtraHandlerData, + ).attributes, + ); + }, + runInOwnSpan: fn => + createMcpNotificationSpan( + notification, + transport, + extra as ExtraHandlerData, + resolvedOptions, + fn, + ) as ReturnType, + }, + callOriginal, + ); + } + + return createMcpNotificationSpan( + notification, + transport, + extra as ExtraHandlerData, + resolvedOptions, + callOriginal, + ); } return (originalOnMessage as (...args: unknown[]) => unknown).call(this, message, extra); @@ -128,14 +215,34 @@ export function wrapTransportOnMessage(transport: MCPTransport, options: McpServ } } +/** Records the response on the MCP SDK's span before the SDK ends it. */ +function applyUpstreamOutcome( + transport: MCPTransport, + span: Span, + request: JsonRpcRequest, + outcome: UpstreamOutcome, + capturePolicy: ResolvedMcpOptions, +): void { + if (outcome.type === 'result' && 'result' in outcome) { + applyResponseToSpan(transport, span, request.method, outcome.result, undefined, capturePolicy); + } else if (outcome.type === 'error' && 'error' in outcome) { + applyResponseToSpan(transport, span, request.method, undefined, outcome.error, capturePolicy); + } +} + /** * Wraps transport.send to handle outgoing messages and response correlation. * Caches legacy initialize metadata and applies modern result metadata only to * the corresponding request span. * @param transport - MCP transport instance to wrap * @param options - MCP capture overrides + * @param serverInstance - The MCP server the transport is connected to */ -export function wrapTransportSend(transport: MCPTransport, options: McpServerWrapperOptions): void { +export function wrapTransportSend( + transport: MCPTransport, + options: McpServerWrapperOptions, + serverInstance?: object, +): void { if (transport.send) { fill(transport, 'send', originalSend => { return async function (this: MCPTransport, ...args: unknown[]) { @@ -143,6 +250,15 @@ export function wrapTransportSend(transport: MCPTransport, options: McpServerWra if (isJsonRpcNotification(message)) { const resolvedOptions = resolveMcpOptions(options); + const upstreamSpan = + shouldHandOffToUpstream(serverInstance) && + getUpstreamSpanForOutboundNotification(getActiveSpan(), message.method); + if (upstreamSpan) { + upstreamSpan.setAttributes( + buildMcpNotificationSpanConfig('notification-outgoing', message, transport, resolvedOptions).attributes, + ); + return (originalSend as (...args: unknown[]) => unknown).call(this, ...args); + } return createMcpOutgoingNotificationSpan(message, transport, resolvedOptions, () => { return (originalSend as (...args: unknown[]) => unknown).call(this, ...args); }); diff --git a/packages/core/src/integrations/mcp-server/types.ts b/packages/core/src/integrations/mcp-server/types.ts index 5a7ed20c92c3..7585dae9476f 100644 --- a/packages/core/src/integrations/mcp-server/types.ts +++ b/packages/core/src/integrations/mcp-server/types.ts @@ -159,19 +159,6 @@ export interface ExtraHandlerData { /** Types of MCP spans */ export type McpSpanType = 'request' | 'notification-incoming' | 'notification-outgoing'; -/** - * Configuration for creating MCP spans - * @internal - */ -export interface McpSpanConfig { - type: McpSpanType; - message: JsonRpcRequest | JsonRpcNotification; - transport: MCPTransport; - extra?: ExtraHandlerData; - callback: () => unknown; - options?: ResolvedMcpOptions; -} - export type SessionId = string; export type RequestId = string | number; diff --git a/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts new file mode 100644 index 000000000000..3be21b58179c --- /dev/null +++ b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts @@ -0,0 +1,202 @@ +/** + * Hand-off to the OpenTelemetry instrumentation built into `@modelcontextprotocol/server`. + * + * Newer `@modelcontextprotocol/server` versions start their own span for every request and + * notification through the global OpenTelemetry API. When a tracer provider is registered, those + * spans would duplicate ours, so the server span the SDK starts is enriched with our attributes + * instead of starting a second one. Without a tracer provider the SDK records nothing and the + * transport instrumentation keeps starting its own spans. + */ + +import { MCP_METHOD_NAME } from '@sentry/conventions/attributes'; +import { getClient } from '../../currentScopes'; +import { DEBUG_BUILD } from '../../debug-build'; +import type { Span } from '../../types/span'; +import { debug } from '../../utils/debug-logger'; +import { isObjectLike } from '../../utils/is'; +import { spanToJSON } from '../../utils/spanUtils'; +import { GLOBAL_OBJ } from '../../utils/worldwide'; +import type { JsonRpcError, RequestId } from './types'; + +const OTEL_API_GLOBAL_KEY = Symbol.for('opentelemetry.js.api.1'); + +/** The outcome the MCP SDK reports when it ends an operation. */ +export type UpstreamOutcome = + | { type: 'result'; result: unknown } + | { type: 'error'; error: JsonRpcError; cause?: unknown } + | { type: string }; + +interface UpstreamOperation { + method: string; + id?: RequestId; +} + +interface UpstreamScope { + run(fn: () => T): T; + end(outcome: UpstreamOutcome): void; +} + +interface UpstreamInstrumentation { + send?: (operation: UpstreamOperation) => UpstreamScope | undefined; + receive?: (operation: UpstreamOperation) => UpstreamScope | undefined; +} + +/** What the transport instrumentation hands over for one inbound message. */ +export interface InboundHandoff { + method: string; + id?: RequestId; + /** Enriches the span the MCP SDK started; the returned callback runs right before the SDK ends it. */ + adopt(span: Span): ((outcome: UpstreamOutcome) => void) | void; + /** Runs the message's work in a Sentry span when the SDK's span did not reach Sentry. */ + runInOwnSpan(fn: () => T): T; +} + +const decoratedProtocols = new WeakSet(); + +let pendingHandoff: InboundHandoff | undefined; + +/** + * Whether an OpenTelemetry tracer provider is registered globally, i.e. whether the MCP SDK's + * built-in instrumentation records spans. + */ +function hasGlobalTracerProvider(): boolean { + const registry = (GLOBAL_OBJ as unknown as Record | undefined>)[OTEL_API_GLOBAL_KEY]; + return !!registry?.trace; +} + +/** The `Protocol` instance behind an `McpServer` (`server.server`) or a low-level `Server`. */ +function getProtocol(serverInstance: object): Record | undefined { + const inner = (serverInstance as { server?: unknown }).server; + const protocol = isObjectLike(inner) ? inner : serverInstance; + const instrumentation = (protocol as Record)._instrumentation; + return isObjectLike(instrumentation) && typeof (instrumentation as UpstreamInstrumentation).receive === 'function' + ? (protocol as Record) + : undefined; +} + +/** + * Installs the hand-off on the MCP SDK's built-in instrumentation of this server, if it has one. + * `_instrumentation` is package-internal to the MCP SDK; without it, nothing changes. + */ +export function decorateUpstreamInstrumentation(serverInstance: object): void { + const protocol = getProtocol(serverInstance); + if (!protocol || decoratedProtocols.has(protocol)) { + return; + } + + const original = protocol._instrumentation as UpstreamInstrumentation; + const decorated: UpstreamInstrumentation = { + ...original, + receive: operation => receiveWithHandoff(original, operation), + }; + + try { + protocol._instrumentation = decorated; + decoratedProtocols.add(protocol); + } catch { + DEBUG_BUILD && debug.warn('Could not hand off MCP spans to the MCP SDK instrumentation.'); + } +} + +/** + * Whether inbound messages of this server should be handed off to the MCP SDK's instrumentation + * instead of being wrapped in a span by the transport instrumentation. + */ +export function shouldHandOffToUpstream(serverInstance: object | undefined): boolean { + if (!serverInstance) { + return false; + } + const protocol = getProtocol(serverInstance); + return !!protocol && decoratedProtocols.has(protocol) && hasGlobalTracerProvider(); +} + +/** + * Runs `callback` (the original `transport.onmessage`) with `handoff` pending. The MCP SDK starts + * its instrumentation scope synchronously while the message is dispatched, which takes it. + */ +export function withInboundHandoff(handoff: InboundHandoff, callback: () => T): T { + const previous = pendingHandoff; + pendingHandoff = handoff; + try { + return callback(); + } finally { + pendingHandoff = previous; + } +} + +/** + * The span the MCP SDK started for an outbound notification. The SDK sends the notification inside + * its scope, so the span is the active one at `transport.send`. + */ +export function getUpstreamSpanForOutboundNotification(activeSpan: Span | undefined, method: string): Span | undefined { + if (!activeSpan) { + return undefined; + } + return spanToJSON(activeSpan).attributes[MCP_METHOD_NAME] === method ? activeSpan : undefined; +} + +function takeHandoff(operation: UpstreamOperation): InboundHandoff | undefined { + const handoff = pendingHandoff; + if (!handoff || handoff.method !== operation.method || handoff.id !== operation.id) { + return undefined; + } + pendingHandoff = undefined; + return handoff; +} + +function receiveWithHandoff( + original: UpstreamInstrumentation, + operation: UpstreamOperation, +): UpstreamScope | undefined { + const handoff = takeHandoff(operation); + if (!handoff) { + return original.receive?.(operation); + } + + const { scope, span } = captureStartedSpan(() => original.receive?.(operation)); + + if (span && scope) { + let beforeEnd: ((outcome: UpstreamOutcome) => void) | void = undefined; + try { + beforeEnd = handoff.adopt(span); + } catch { + // noop + } + return { + run: fn => scope.run(fn), + end: outcome => { + try { + beforeEnd?.(outcome); + } catch { + // noop + } + scope.end(outcome); + }, + }; + } + + // The SDK's span did not reach Sentry, e.g. because the registered tracer provider isn't Sentry's. + return { + run: fn => handoff.runInOwnSpan(() => (scope ? scope.run(fn) : fn())), + end: outcome => scope?.end(outcome), + }; +} + +function captureStartedSpan(start: () => UpstreamScope | undefined): { + scope: UpstreamScope | undefined; + span: Span | undefined; +} { + let span: Span | undefined; + const unsubscribe = getClient()?.on('spanStart', startedSpan => { + if (!span && spanToJSON(startedSpan).attributes[MCP_METHOD_NAME] !== undefined) { + span = startedSpan; + } + }); + + try { + const scope = start(); + return { scope, span }; + } finally { + unsubscribe?.(); + } +} diff --git a/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts new file mode 100644 index 000000000000..20126d4c336c --- /dev/null +++ b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts @@ -0,0 +1,293 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { setCurrentClient } from '../../../../src'; +import { wrapMcpServerWithSentry } from '../../../../src/integrations/mcp-server'; +import { withActiveSpan } from '../../../../src/tracing'; +import { startInactiveSpan, startSpan } from '../../../../src/tracing/trace'; +import type { Span, StreamedSpanJSON } from '../../../../src/types/span'; +import { getActiveSpan, spanToJSON } from '../../../../src/utils/spanUtils'; +import { GLOBAL_OBJ } from '../../../../src/utils/worldwide'; +import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client'; +import { resetGlobals } from '../../../testutils'; + +const OTEL_API_GLOBAL_KEY = Symbol.for('opentelemetry.js.api.1'); + +type Outcome = { type: 'result'; result: unknown } | { type: 'error'; error: { code: number; message: string } }; + +interface Scope { + run(fn: () => T): T; + end(outcome: Outcome): void; +} + +interface Operation { + method: string; + id?: string | number; +} + +interface Instrumentation { + send?: (operation: Operation) => Scope | undefined; + receive?: (operation: Operation) => Scope | undefined; +} + +/** + * The MCP SDK's built-in instrumentation as the `SentryTracerProvider` turns it into Sentry spans: + * a span per operation, active while the handler runs, ended before the response is sent. + */ +function sentryTracedInstrumentation(): Instrumentation { + const start = (operation: Operation, kind: 'client' | 'server'): Scope => { + const span = startInactiveSpan({ + name: operation.method, + attributes: { 'mcp.method.name': operation.method, 'sentry.kind': kind }, + }); + return { run: fn => withActiveSpan(span, fn), end: () => span.end() }; + }; + return { send: operation => start(operation, 'client'), receive: operation => start(operation, 'server') }; +} + +/** The instrumentation when the registered tracer provider is not Sentry's: no Sentry span is started. */ +function foreignTracedInstrumentation(): Instrumentation { + const scope: Scope = { run: fn => fn(), end: () => undefined }; + return { send: () => scope, receive: () => scope }; +} + +class InMemoryTransport { + public onmessage?: (message: unknown, extra?: unknown) => void; + public onclose?: () => void; + public onerror?: (error: Error) => void; + public sent: unknown[] = []; + + public async start(): Promise {} + + public async send(message: unknown): Promise { + this.sent.push(message); + } +} + +type ToolHandler = (args: Record) => Promise; + +/** + * Minimal stand-in for `McpServer` with the dispatch order of `@modelcontextprotocol/server`'s + * `Protocol`: the instrumentation scope is started synchronously in `transport.onmessage`, the handler + * runs inside it, and the scope ends before the response goes out. + */ +function createServer(instrumentation: Instrumentation | undefined) { + const tools: Record = {}; + const protocol: { _instrumentation?: Instrumentation; transport?: InMemoryTransport } = { + _instrumentation: instrumentation, + }; + + const mcpServer = { + server: protocol, + registerTool(name: string, _config: unknown, handler: ToolHandler) { + tools[name] = handler; + }, + registerResource() {}, + registerPrompt() {}, + async connect(transport: InMemoryTransport) { + protocol.transport = transport; + transport.onmessage = (message: unknown) => { + const { id, method, params } = message as { + id?: number; + method: string; + params?: { name?: string; arguments?: Record }; + }; + const scope = protocol._instrumentation?.receive?.({ method, ...(id !== undefined && { id }) }); + const run = (fn: () => T): T => (scope ? scope.run(fn) : fn()); + + if (id === undefined) { + scope?.end({ type: 'result', result: undefined }); + return; + } + + const handler = (): Promise => + method === 'tools/call' + ? tools[params!.name!]!(params!.arguments ?? {}) + : Promise.resolve({ serverInfo: { name: 'test-server', version: '1.0.0' }, protocolVersion: '2025-06-18' }); + + // Unlike the real `Protocol`, the handler starts synchronously: these tests run without an + // async context strategy, so the active span does not carry across a microtask. + void run(handler).then(async result => { + scope?.end({ type: 'result', result }); + await transport.send({ jsonrpc: '2.0', id, result }); + }); + }; + await transport.start(); + }, + async notify(method: string) { + const scope = protocol._instrumentation?.send?.({ method }); + const send = () => protocol.transport!.send({ jsonrpc: '2.0', method, params: {} }); + await (scope ? scope.run(send) : send()); + scope?.end({ type: 'result', result: undefined }); + }, + }; + + return mcpServer; +} + +async function callTool(transport: InMemoryTransport, name: string, args: Record): Promise { + const responses = transport.sent.length; + transport.onmessage?.({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args } }, {}); + await vi.waitFor(() => expect(transport.sent.length).toBeGreaterThan(responses)); +} + +describe('MCP server hand-off to the MCP SDK instrumentation', () => { + let client: TestClient; + let endedSpans: StreamedSpanJSON[]; + let originalOtelRegistry: unknown; + + const mcpSpans = (): StreamedSpanJSON[] => + endedSpans.filter(span => span.attributes['mcp.method.name'] !== undefined); + const registerTracerProvider = (): void => { + (GLOBAL_OBJ as unknown as Record)[OTEL_API_GLOBAL_KEY] = { version: '1.9.0', trace: {} }; + }; + + beforeEach(() => { + resetGlobals(); + originalOtelRegistry = (GLOBAL_OBJ as unknown as Record)[OTEL_API_GLOBAL_KEY]; + client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); + setCurrentClient(client); + client.init(); + endedSpans = []; + client.on('spanEnd', span => { + endedSpans.push(spanToJSON(span)); + }); + }); + + afterEach(() => { + (GLOBAL_OBJ as unknown as Record)[OTEL_API_GLOBAL_KEY] = originalOtelRegistry; + }); + + it('enriches the span the MCP SDK starts instead of starting a second one', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(sentryTracedInstrumentation())); + let handlerParent: Span | undefined; + server.registerTool('echo', {}, async ({ message }) => { + handlerParent = getActiveSpan(); + startSpan({ name: 'inside-tool' }, () => undefined); + return { content: [{ type: 'text', text: `echo: ${message as string}` }] }; + }); + + const transport = new InMemoryTransport(); + await server.connect(transport); + await callTool(transport, 'echo', { message: 'hi' }); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + const [toolSpan] = spans; + expect(toolSpan!.attributes).toMatchObject({ + 'sentry.kind': 'server', + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.function.mcp_server', + 'mcp.tool.name': 'echo', + 'mcp.request.argument.message': '"hi"', + 'mcp.tool.result.content_count': 1, + }); + expect(spanToJSON(handlerParent!).span_id).toBe(toolSpan!.span_id); + expect(endedSpans.find(span => span.name === 'inside-tool')!.parent_span_id).toBe(toolSpan!.span_id); + }); + + it('records initialize session data on the MCP SDK span', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(sentryTracedInstrumentation())); + const transport = new InMemoryTransport(); + await server.connect(transport); + + transport.onmessage?.( + { + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-06-18', clientInfo: { name: 'test-client', version: '1.0.0' } }, + }, + {}, + ); + await vi.waitFor(() => expect(transport.sent).toHaveLength(1)); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ + 'sentry.op': 'mcp.server', + 'mcp.client.name': 'test-client', + 'mcp.server.name': 'test-server', + 'mcp.protocol.version': '2025-06-18', + }); + }); + + it('enriches the MCP SDK span of an incoming notification', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(sentryTracedInstrumentation())); + const transport = new InMemoryTransport(); + await server.connect(transport); + + transport.onmessage?.({ jsonrpc: '2.0', method: 'notifications/initialized' }, {}); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ + 'sentry.kind': 'server', + 'sentry.op': 'mcp.notification.client_to_server', + }); + }); + + it('enriches the MCP SDK span of an outgoing notification', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(sentryTracedInstrumentation())); + const transport = new InMemoryTransport(); + await server.connect(transport); + + await server.notify('notifications/tools/list_changed'); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ + 'sentry.kind': 'client', + 'sentry.op': 'mcp.notification.server_to_client', + }); + }); + + it('starts its own span when the MCP SDK span does not reach Sentry', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(foreignTracedInstrumentation())); + let handlerSpan: Span | undefined; + server.registerTool('echo', {}, async () => { + handlerSpan = getActiveSpan(); + return { content: [{ type: 'text', text: 'echo' }] }; + }); + + const transport = new InMemoryTransport(); + await server.connect(transport); + await callTool(transport, 'echo', {}); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); + expect(spans[0]!.attributes['sentry.kind']).toBeUndefined(); + expect(spanToJSON(handlerSpan!).span_id).toBe(spans[0]!.span_id); + }); + + it('keeps starting its own spans when no tracer provider is registered', async () => { + const server = wrapMcpServerWithSentry(createServer(foreignTracedInstrumentation())); + server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'echo' }] })); + + const transport = new InMemoryTransport(); + await server.connect(transport); + await callTool(transport, 'echo', {}); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); + }); + + it('keeps starting its own spans when the MCP SDK has no instrumentation', async () => { + registerTracerProvider(); + const server = wrapMcpServerWithSentry(createServer(undefined)); + server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'echo' }] })); + + const transport = new InMemoryTransport(); + await server.connect(transport); + await callTool(transport, 'echo', {}); + + const spans = mcpSpans(); + expect(spans).toHaveLength(1); + expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); + }); +}); From aa020ccc9a147f7885c38df17b62ee3cfe524e9c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:19:33 +0000 Subject: [PATCH 2/8] ref(core): Trim MCP SDK hand-off to stay within the bundle size budget --- .../src/integrations/mcp-server/transport.ts | 17 +++----- .../mcp-server/upstreamInstrumentation.ts | 39 +++---------------- 2 files changed, 12 insertions(+), 44 deletions(-) diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 00dbe83e66b9..3783c6baf25d 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -5,13 +5,13 @@ * @see https://modelcontextprotocol.io/specification/2025-06-18/basic/transports */ -import { MCP_AUTH_CLIENT_NAME, MCP_PROTOCOL_VERSION } from '@sentry/conventions/attributes'; +import { MCP_AUTH_CLIENT_NAME, MCP_METHOD_NAME, MCP_PROTOCOL_VERSION } from '@sentry/conventions/attributes'; import { getClient, getIsolationScope, withIsolationScope } from '../../currentScopes'; import { DEBUG_BUILD } from '../../debug-build'; import { withActiveSpan } from '../../tracing'; import { startInactiveSpan } from '../../tracing/trace'; import type { Span } from '../../types/span'; -import { getActiveSpan } from '../../utils/spanUtils'; +import { getActiveSpan, spanToJSON } from '../../utils/spanUtils'; import { isObjectLike } from '../../utils/is'; import { fill } from '../../utils/object'; import { safeCallback } from '../../utils/safeCallback'; @@ -39,11 +39,7 @@ import type { SessionData, } from './types'; import type { UpstreamOutcome } from './upstreamInstrumentation'; -import { - getUpstreamSpanForOutboundNotification, - shouldHandOffToUpstream, - withInboundHandoff, -} from './upstreamInstrumentation'; +import { shouldHandOffToUpstream, withInboundHandoff } from './upstreamInstrumentation'; import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse } from './validation'; function resolveMcpOptions(options: McpServerWrapperOptions): ResolvedMcpOptions { @@ -250,10 +246,9 @@ export function wrapTransportSend( if (isJsonRpcNotification(message)) { const resolvedOptions = resolveMcpOptions(options); - const upstreamSpan = - shouldHandOffToUpstream(serverInstance) && - getUpstreamSpanForOutboundNotification(getActiveSpan(), message.method); - if (upstreamSpan) { + // The MCP SDK sends a notification inside its own scope, so its span is the active one. + const upstreamSpan = shouldHandOffToUpstream(serverInstance) ? getActiveSpan() : undefined; + if (upstreamSpan && spanToJSON(upstreamSpan).attributes[MCP_METHOD_NAME] === message.method) { upstreamSpan.setAttributes( buildMcpNotificationSpanConfig('notification-outgoing', message, transport, resolvedOptions).attributes, ); diff --git a/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts index 3be21b58179c..d573a06d6ac5 100644 --- a/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts +++ b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts @@ -10,9 +10,7 @@ import { MCP_METHOD_NAME } from '@sentry/conventions/attributes'; import { getClient } from '../../currentScopes'; -import { DEBUG_BUILD } from '../../debug-build'; import type { Span } from '../../types/span'; -import { debug } from '../../utils/debug-logger'; import { isObjectLike } from '../../utils/is'; import { spanToJSON } from '../../utils/spanUtils'; import { GLOBAL_OBJ } from '../../utils/worldwide'; @@ -85,17 +83,11 @@ export function decorateUpstreamInstrumentation(serverInstance: object): void { } const original = protocol._instrumentation as UpstreamInstrumentation; - const decorated: UpstreamInstrumentation = { + protocol._instrumentation = { ...original, - receive: operation => receiveWithHandoff(original, operation), + receive: (operation: UpstreamOperation) => receiveWithHandoff(original, operation), }; - - try { - protocol._instrumentation = decorated; - decoratedProtocols.add(protocol); - } catch { - DEBUG_BUILD && debug.warn('Could not hand off MCP spans to the MCP SDK instrumentation.'); - } + decoratedProtocols.add(protocol); } /** @@ -124,34 +116,15 @@ export function withInboundHandoff(handoff: InboundHandoff, callback: () => T } } -/** - * The span the MCP SDK started for an outbound notification. The SDK sends the notification inside - * its scope, so the span is the active one at `transport.send`. - */ -export function getUpstreamSpanForOutboundNotification(activeSpan: Span | undefined, method: string): Span | undefined { - if (!activeSpan) { - return undefined; - } - return spanToJSON(activeSpan).attributes[MCP_METHOD_NAME] === method ? activeSpan : undefined; -} - -function takeHandoff(operation: UpstreamOperation): InboundHandoff | undefined { - const handoff = pendingHandoff; - if (!handoff || handoff.method !== operation.method || handoff.id !== operation.id) { - return undefined; - } - pendingHandoff = undefined; - return handoff; -} - function receiveWithHandoff( original: UpstreamInstrumentation, operation: UpstreamOperation, ): UpstreamScope | undefined { - const handoff = takeHandoff(operation); - if (!handoff) { + const handoff = pendingHandoff; + if (!handoff || handoff.method !== operation.method || handoff.id !== operation.id) { return original.receive?.(operation); } + pendingHandoff = undefined; const { scope, span } = captureStartedSpan(() => original.receive?.(operation)); From 1c4f6a48bd2e75dc3430581a4b9a530d0370a60d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:07:31 +0000 Subject: [PATCH 3/8] test(e2e): Simplify MCP SDK OpenTelemetry variant install --- .../test-applications/node-express-mcp-v2/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json index 6a5886e1732a..0aaa50d7b84b 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json @@ -8,7 +8,7 @@ "test": "playwright test", "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install && pnpm build", - "test:build-mcp-sdk-otel": "pnpm install && pnpm add --config.block-exotic-subdeps=false https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@919bb8e https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@919bb8e @opentelemetry/api@^1.9.0 && pnpm add --config.block-exotic-subdeps=false -D https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@919bb8e && pnpm build", + "test:build-mcp-sdk-otel": "pnpm install && pnpm add --config.block-exotic-subdeps=false https://pkg.pr.new/@modelcontextprotocol/server@919bb8e https://pkg.pr.new/@modelcontextprotocol/node@919bb8e https://pkg.pr.new/@modelcontextprotocol/client@919bb8e && pnpm build", "test:assert": "pnpm test", "test:assert-mcp-sdk-otel": "MCP_SDK_OTEL=1 pnpm test" }, From 9b31a9d37b501520be5993a9f5a393083dda274a Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:10:47 +0000 Subject: [PATCH 4/8] ref(core): Simplify MCP response attribute handling --- .../integrations/mcp-server/correlation.ts | 15 ++++-------- .../mcp-server/capturePolicy.test.ts | 3 +-- .../transportInstrumentation.test.ts | 23 +++++++++++-------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/packages/core/src/integrations/mcp-server/correlation.ts b/packages/core/src/integrations/mcp-server/correlation.ts index 34348e031a3c..954a91a49d62 100644 --- a/packages/core/src/integrations/mcp-server/correlation.ts +++ b/packages/core/src/integrations/mcp-server/correlation.ts @@ -117,20 +117,15 @@ export function applyResponseToSpan( if (method === 'initialize' && (responseSessionData.protocolVersion || responseSessionData.serverInfo)) { updateSessionDataForTransport(transport, responseSessionData); } - const responseAttributes: Record = { - ...buildServerAttributesFromInfo(responseSessionData.serverInfo), - }; + span.setAttributes(buildServerAttributesFromInfo(responseSessionData.serverInfo)); if (responseSessionData.protocolVersion) { - responseAttributes[MCP_PROTOCOL_VERSION] = responseSessionData.protocolVersion; - } - if (Object.keys(responseAttributes).length > 0) { - span.setAttributes(responseAttributes); + span.setAttribute(MCP_PROTOCOL_VERSION, responseSessionData.protocolVersion); } if (error) { - span.setAttributes({ [RPC_RESPONSE_STATUS_CODE]: String(error.code) }); + span.setAttribute(RPC_RESPONSE_STATUS_CODE, String(error.code)); if (isJsonRpcServerError(error.code)) { - span.setAttributes({ [ERROR_TYPE]: String(error.code) }); + span.setAttribute(ERROR_TYPE, String(error.code)); span.setStatus({ code: SPAN_STATUS_ERROR, message: error.message }); } } else if (method === 'tools/call') { @@ -138,7 +133,7 @@ export function applyResponseToSpan( span.setAttributes(toolAttributes); // oxlint-disable-next-line typescript/no-deprecated -- Preserve classification based on the existing tool result attribute. if (toolAttributes[MCP_TOOL_RESULT_IS_ERROR] === true) { - span.setAttributes({ [ERROR_TYPE]: 'tool_error' }); + span.setAttribute(ERROR_TYPE, 'tool_error'); span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); } } else if (method === 'prompts/get') { diff --git a/packages/core/test/lib/integrations/mcp-server/capturePolicy.test.ts b/packages/core/test/lib/integrations/mcp-server/capturePolicy.test.ts index 73cff92f2481..b6f8d5225acb 100644 --- a/packages/core/test/lib/integrations/mcp-server/capturePolicy.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/capturePolicy.test.ts @@ -121,8 +121,7 @@ describe('MCP Server Capture Policy', () => { } function expectToolResult(span: MockSpan, content?: string): void { - expect(span.setAttributes).toHaveBeenCalledOnce(); - expect(span.setAttributes).toHaveBeenCalledWith({ + expect(span.setAttributes).toHaveBeenLastCalledWith({ 'mcp.tool.result.content_count': 1, 'mcp.tool.result.content_type': 'text', ...(content !== undefined && { 'mcp.tool.result.content': content }), diff --git a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts index a4e9f24407aa..93d5f0e06c72 100644 --- a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts @@ -343,6 +343,7 @@ describe('MCP Server Transport Instrumentation', () => { it('should set span status to error when JSON-RPC error response is sent', async () => { const mockSpan = { + setAttribute: vi.fn(), setAttributes: vi.fn(), setStatus: vi.fn(), end: vi.fn(), @@ -375,6 +376,7 @@ describe('MCP Server Transport Instrumentation', () => { it('should not set error span status for successful JSON-RPC responses', async () => { const mockSpan = { + setAttribute: vi.fn(), setAttributes: vi.fn(), setStatus: vi.fn(), end: vi.fn(), @@ -874,7 +876,7 @@ describe('MCP Server Transport Instrumentation', () => { await wrappedMcpServer.connect(transport); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan); transport.onmessage?.( @@ -903,7 +905,7 @@ describe('MCP Server Transport Instrumentation', () => { await wrappedMcpServer.connect(transport); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); transport.onmessage?.( @@ -975,7 +977,7 @@ describe('MCP Server Transport Instrumentation', () => { const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); const transport = createMockTransport(); transport.sessionId = ''; - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); await wrappedMcpServer.connect(transport); @@ -1002,7 +1004,10 @@ describe('MCP Server Transport Instrumentation', () => { }); expect(getSessionDataForTransport(transport)).toBeUndefined(); - expect(mockSpan.setAttributes).not.toHaveBeenCalled(); + expect(mockSpan.setAttributes).not.toHaveBeenCalledWith( + expect.objectContaining({ 'mcp.server.name': 'application-server' }), + ); + expect(mockSpan.setAttribute).not.toHaveBeenCalled(); expect(mockSpan.end).toHaveBeenCalledOnce(); }); @@ -1054,7 +1059,7 @@ describe('MCP Server Transport Instrumentation', () => { const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); const transport = createMockTransport(); transport.sessionId = ''; - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); await wrappedMcpServer.connect(transport); @@ -1202,7 +1207,7 @@ describe('MCP Server Transport Instrumentation', () => { // Connect using the wrapper transport (what users do) await wrappedMcpServer.connect(wrapper); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); // Simulate incoming request - due to getter/setter, onmessage runs on inner transport @@ -1244,7 +1249,7 @@ describe('MCP Server Transport Instrumentation', () => { await wrappedMcpServer.connect(wrapper); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); inner.onmessage?.call( @@ -1331,7 +1336,7 @@ describe('MCP Server Transport Instrumentation', () => { await wrappedMcpServer.connect(wrapper); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); // Initialize request @@ -1382,7 +1387,7 @@ describe('MCP Server Transport Instrumentation', () => { await wrappedMcpServer.connect(wrapper); - const mockSpan = { setAttributes: vi.fn(), end: vi.fn(), setStatus: vi.fn() }; + const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn(), setStatus: vi.fn() }; startInactiveSpanSpy.mockReturnValue(mockSpan as any); // Start a request but don't complete it From 3103b3e6f34dab216de42cedc46d980d5a7c61c6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:17:21 +0000 Subject: [PATCH 5/8] ref(core): Simplify MCP SDK span hand-off Co-Authored-By: Francesco Gringl-Novy --- .../node-express-mcp-v2/instrument.mjs | 2 +- .../tests/mcp-sdk-otel.test.ts | 112 ++++------- .../node-express-mcp-v2/tests/mcp.test.ts | 3 +- .../core/src/integrations/mcp-server/index.ts | 11 +- .../core/src/integrations/mcp-server/spans.ts | 23 +-- .../src/integrations/mcp-server/transport.ts | 100 +++------- .../mcp-server/upstreamInstrumentation.ts | 179 ++++++------------ .../upstreamInstrumentation.test.ts | 38 ++-- 8 files changed, 152 insertions(+), 316 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs index a3d6c930a0a8..0c50a79bbaca 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/instrument.mjs @@ -6,6 +6,6 @@ Sentry.init({ debug: !!process.env.DEBUG, tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1, - // Registers the Sentry tracer provider, so the MCP SDK's built-in OpenTelemetry instrumentation records spans. + // Registers a tracer provider, which turns on the MCP SDK's built-in OpenTelemetry spans. enableOpenTelemetrySetup: process.env.MCP_SDK_OTEL === '1', }); diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts index 551d92b38ae1..c95867116435 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts @@ -2,94 +2,48 @@ import { expect, test } from '@playwright/test'; import { collectStreamedSpans, getSpanOp, type SerializedStreamedSpan } from '@sentry-internal/test-utils'; import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; -// Runs in the `MCP SDK OpenTelemetry` variant, where `@modelcontextprotocol/server` emits its own -// OpenTelemetry spans and Sentry registers its tracer provider (`enableOpenTelemetrySetup: true`). test.skip(process.env.MCP_SDK_OTEL !== '1', 'Only runs in the MCP SDK OpenTelemetry variant'); -const PROXY_SERVER_NAME = 'node-express-mcp-v2'; - -function mcpSpansFor(spans: SerializedStreamedSpan[], method: string): SerializedStreamedSpan[] { - return spans.filter(span => span.attributes?.['mcp.method.name']?.value === method); -} - -/** The trace of one MCP request: its `POST /mcp` segment and every span below it. */ -function collectMcpRequestTrace(spanName: string): Promise { - return collectStreamedSpans( - PROXY_SERVER_NAME, +/** Resolves with the single MCP span of the request whose span is named `spanName`, once its trace is complete. */ +async function getMcpSpan(spanName: string): Promise { + const spans = await collectStreamedSpans( + 'node-express-mcp-v2', spans => spans.some(span => span.is_segment) && spans.some(span => span.name === spanName), ); + const mcpSpans = spans.filter(span => span.attributes?.['mcp.method.name']); + expect(mcpSpans).toHaveLength(1); + expect(getSpanOp(spans.find(span => span.is_segment)!)).toBe('http.server'); + + const [mcpSpan] = mcpSpans; + expect(mcpSpan!.name).toBe(spanName); + expect(mcpSpan!.is_segment).toBe(false); + expect(mcpSpan!.attributes?.['sentry.kind']?.value).toBe('server'); + expect(getSpanOp(mcpSpan!)).toBe('mcp.server'); + return mcpSpan!; } test('enriches the MCP SDK spans instead of duplicating them', async ({ baseURL }) => { const client = new Client({ name: 'test-client-v2', version: '1.0.0' }); - const transport = new StreamableHTTPClientTransport(new URL(`${baseURL}/mcp`)); - - const initializeTracePromise = collectMcpRequestTrace('initialize'); - await client.connect(transport); - - await test.step('initialize', async () => { - const spans = await initializeTracePromise; - const initializeSpans = mcpSpansFor(spans, 'initialize'); - expect(initializeSpans).toHaveLength(1); - - const [initializeSpan] = initializeSpans; - expect(getSpanOp(initializeSpan!)).toBe('mcp.server'); - expect(initializeSpan!.attributes?.['sentry.kind']?.value).toBe('server'); - expect(initializeSpan!.attributes?.['mcp.client.name']?.value).toBe('test-client-v2'); - expect(initializeSpan!.attributes?.['mcp.server.name']?.value).toBe('Echo-V2'); - expect(initializeSpan!.attributes?.['mcp.transport']?.value).toMatch(/StreamableHTTPServerTransport/); - - // The MCP SDK starts its span inside the request, so it is no longer a segment of its own. - expect(initializeSpan!.is_segment).toBe(false); - expect(getSpanOp(spans.find(span => span.is_segment)!)).toBe('http.server'); - }); - - await test.step('tools/call', async () => { - const toolTracePromise = collectMcpRequestTrace('tools/call echo'); - - const toolResult = await client.callTool({ name: 'echo', arguments: { message: 'foobar' } }); - expect(toolResult).toMatchObject({ content: [{ text: 'Tool echo: foobar', type: 'text' }] }); - - const spans = await toolTracePromise; - const toolSpans = mcpSpansFor(spans, 'tools/call'); - expect(toolSpans).toHaveLength(1); - - const [toolSpan] = toolSpans; - expect(toolSpan!.name).toBe('tools/call echo'); - expect(getSpanOp(toolSpan!)).toBe('mcp.server'); - expect(toolSpan!.attributes?.['sentry.kind']?.value).toBe('server'); - expect(toolSpan!.attributes?.['mcp.tool.name']?.value).toBe('echo'); - expect(toolSpan!.attributes?.['mcp.request.argument.message']?.value).toBe('"foobar"'); - expect(toolSpan!.attributes?.['mcp.tool.result.content_count']?.value).toBe(1); - }); - - await test.step('prompts/get', async () => { - const promptTracePromise = collectMcpRequestTrace('prompts/get echo'); - - await client.getPrompt({ name: 'echo', arguments: { message: 'foobar' } }); - - const spans = await promptTracePromise; - const promptSpans = mcpSpansFor(spans, 'prompts/get'); - expect(promptSpans).toHaveLength(1); - expect(getSpanOp(promptSpans[0]!)).toBe('mcp.server'); - expect(promptSpans[0]!.attributes?.['mcp.prompt.name']?.value).toBe('echo'); - }); - - await test.step('tool error', async () => { - const toolTracePromise = collectMcpRequestTrace('tools/call always-error'); - - try { - await client.callTool({ name: 'always-error', arguments: {} }); - } catch { - // Expected: MCP SDK throws when the tool returns a JSON-RPC error - } - const spans = await toolTracePromise; - const toolSpans = mcpSpansFor(spans, 'tools/call'); - expect(toolSpans).toHaveLength(1); - expect(getSpanOp(toolSpans[0]!)).toBe('mcp.server'); - expect(toolSpans[0]!.status).toBe('error'); - }); + const initializeSpanPromise = getMcpSpan('initialize'); + await client.connect(new StreamableHTTPClientTransport(new URL(`${baseURL}/mcp`))); + const initializeSpan = await initializeSpanPromise; + expect(initializeSpan.attributes?.['mcp.client.name']?.value).toBe('test-client-v2'); + expect(initializeSpan.attributes?.['mcp.server.name']?.value).toBe('Echo-V2'); + + const toolSpanPromise = getMcpSpan('tools/call echo'); + await client.callTool({ name: 'echo', arguments: { message: 'foobar' } }); + const toolSpan = await toolSpanPromise; + expect(toolSpan.attributes?.['mcp.request.argument.message']?.value).toBe('"foobar"'); + expect(toolSpan.attributes?.['mcp.tool.result.content_count']?.value).toBe(1); + + const promptSpanPromise = getMcpSpan('prompts/get echo'); + await client.getPrompt({ name: 'echo', arguments: { message: 'foobar' } }); + expect((await promptSpanPromise).attributes?.['mcp.prompt.name']?.value).toBe('echo'); + + const errorSpanPromise = getMcpSpan('tools/call always-error'); + await client.callTool({ name: 'always-error', arguments: {} }).catch(() => undefined); + expect((await errorSpanPromise).status).toBe('error'); await client.close(); }); diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts index 1e0d7daff1b2..826821dca3af 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp.test.ts @@ -3,8 +3,7 @@ import { waitForStreamedSpan, getSpanOp } from '@sentry-internal/test-utils'; import { Client } from '@modelcontextprotocol/client'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; -// The `MCP SDK OpenTelemetry` variant is covered by `mcp-sdk-otel.test.ts`. -test.skip(process.env.MCP_SDK_OTEL === '1', 'MCP SDK spans reach Sentry in this variant'); +test.skip(process.env.MCP_SDK_OTEL === '1', 'Covered by mcp-sdk-otel.test.ts'); // The server in this app is never wrapped with `wrapMcpServerWithSentry` — spans are produced // solely by the auto-instrumenting `mcpServer` integration. diff --git a/packages/core/src/integrations/mcp-server/index.ts b/packages/core/src/integrations/mcp-server/index.ts index a667fa5ed542..1701bc6bb3f9 100644 --- a/packages/core/src/integrations/mcp-server/index.ts +++ b/packages/core/src/integrations/mcp-server/index.ts @@ -38,10 +38,10 @@ function applyMissingMcpOptions(target: McpServerWrapperOptions, source: McpServ function instrumentTransport( transport: MCPTransport, options: McpServerWrapperOptions, - serverInstance: MCPServerInstance, + upstreamInstrumented: boolean, ): void { - wrapTransportOnMessage(transport, options, serverInstance); - wrapTransportSend(transport, options, serverInstance); + wrapTransportOnMessage(transport, options, upstreamInstrumented); + wrapTransportSend(transport, options, upstreamInstrumented); wrapTransportOnClose(transport); wrapTransportError(transport); } @@ -182,6 +182,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S, const serverInstance = mcpServerInstance as MCPServerInstance; const wrapperOptions: McpServerWrapperOptions = { ...options }; wrappedMcpServerOptions.set(mcpServerInstance, wrapperOptions); + const upstreamInstrumented = decorateUpstreamInstrumentation(serverInstance); fill(serverInstance, 'connect', originalConnect => { return async function (this: MCPServerInstance, transport: MCPTransport, ...restArgs: unknown[]) { @@ -192,7 +193,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S, } isTransportInstrumented = true; - instrumentTransport(transport, wrapperOptions, serverInstance); + instrumentTransport(transport, wrapperOptions, upstreamInstrumented); }; const restoreStart = interceptTransportStart(transport, instrumentTransportOnce); @@ -212,8 +213,6 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S, }; }); - decorateUpstreamInstrumentation(serverInstance); - wrapAllMCPHandlers(serverInstance); wrapExistingHandlers(serverInstance); diff --git a/packages/core/src/integrations/mcp-server/spans.ts b/packages/core/src/integrations/mcp-server/spans.ts index cb7892fd0974..337b1a7f4d5a 100644 --- a/packages/core/src/integrations/mcp-server/spans.ts +++ b/packages/core/src/integrations/mcp-server/spans.ts @@ -104,16 +104,10 @@ export function buildMcpNotificationSpanConfig( return { name, attributes }; } -function createMcpNotificationSpanWithCallback( - type: 'notification-incoming' | 'notification-outgoing', - message: JsonRpcNotification, - transport: MCPTransport, - options: ResolvedMcpOptions, +function startMcpNotificationSpan( + { name, attributes }: { name: string; attributes: Record }, callback: () => unknown, - extra?: ExtraHandlerData, ): unknown { - const { name, attributes } = buildMcpNotificationSpanConfig(type, message, transport, options, extra); - return startSpan( { name, @@ -141,13 +135,9 @@ export function createMcpNotificationSpan( options: ResolvedMcpOptions, callback: () => unknown, ): unknown { - return createMcpNotificationSpanWithCallback( - 'notification-incoming', - jsonRpcMessage, - transport, - options, + return startMcpNotificationSpan( + buildMcpNotificationSpanConfig('notification-incoming', jsonRpcMessage, transport, options, extra), callback, - extra, ); } @@ -165,7 +155,10 @@ export function createMcpOutgoingNotificationSpan( options: ResolvedMcpOptions, callback: () => unknown, ): unknown { - return createMcpNotificationSpanWithCallback('notification-outgoing', jsonRpcMessage, transport, options, callback); + return startMcpNotificationSpan( + buildMcpNotificationSpanConfig('notification-outgoing', jsonRpcMessage, transport, options), + callback, + ); } /** diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 3783c6baf25d..0fa967586d9f 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -10,7 +10,6 @@ import { getClient, getIsolationScope, withIsolationScope } from '../../currentS import { DEBUG_BUILD } from '../../debug-build'; import { withActiveSpan } from '../../tracing'; import { startInactiveSpan } from '../../tracing/trace'; -import type { Span } from '../../types/span'; import { getActiveSpan, spanToJSON } from '../../utils/spanUtils'; import { isObjectLike } from '../../utils/is'; import { fill } from '../../utils/object'; @@ -30,16 +29,8 @@ import { createMcpNotificationSpan, createMcpOutgoingNotificationSpan, } from './spans'; -import type { - ExtraHandlerData, - JsonRpcRequest, - McpServerWrapperOptions, - MCPTransport, - ResolvedMcpOptions, - SessionData, -} from './types'; -import type { UpstreamOutcome } from './upstreamInstrumentation'; -import { shouldHandOffToUpstream, withInboundHandoff } from './upstreamInstrumentation'; +import type { ExtraHandlerData, McpServerWrapperOptions, MCPTransport, ResolvedMcpOptions, SessionData } from './types'; +import { hasGlobalTracerProvider, withInboundHandoff } from './upstreamInstrumentation'; import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse } from './validation'; function resolveMcpOptions(options: McpServerWrapperOptions): ResolvedMcpOptions { @@ -87,12 +78,12 @@ function resolveOAuthClientName(options: McpServerWrapperOptions, extra?: ExtraH * Modern request metadata is read directly when building each message's span. * @param transport - MCP transport instance to wrap * @param options - MCP instrumentation options - * @param serverInstance - The MCP server the transport is connected to + * @param upstreamInstrumented - Whether the server has the MCP SDK's built-in instrumentation */ export function wrapTransportOnMessage( transport: MCPTransport, options: McpServerWrapperOptions, - serverInstance?: object, + upstreamInstrumented = false, ): void { if (transport.onmessage) { fill(transport, 'onmessage', originalOnMessage => { @@ -131,35 +122,32 @@ export function wrapTransportOnMessage( }), } : undefined; - - const startRequestSpan = (): Span => { + const callOriginal = (): unknown => + (originalOnMessage as (...args: unknown[]) => unknown).call(this, request, extra); + const startOwnSpan = (fn: () => unknown): unknown => { const span = startInactiveSpan(spanConfig); if (sessionAttributes) { span.setAttributes(sessionAttributes); } storeSpanForRequest(transport, request.id, span, request.method, resolvedOptions); - return span; + return withActiveSpan(span, fn); }; - const callOriginal = (): unknown => - (originalOnMessage as (...args: unknown[]) => unknown).call(this, request, extra); - - if (shouldHandOffToUpstream(serverInstance)) { + if (upstreamInstrumented && hasGlobalTracerProvider()) { return withInboundHandoff( { method: request.method, id: request.id, - adopt: span => { - span.setAttributes({ ...spanConfig.attributes, ...sessionAttributes }); - return outcome => applyUpstreamOutcome(transport, span, request, outcome, resolvedOptions); - }, - runInOwnSpan: fn => withActiveSpan(startRequestSpan(), fn), + attributes: { ...spanConfig.attributes, ...sessionAttributes }, + onResponse: (span, result, error) => + applyResponseToSpan(transport, span, request.method, result, error, resolvedOptions), + startOwnSpan, }, callOriginal, ); } - return withActiveSpan(startRequestSpan(), callOriginal); + return startOwnSpan(callOriginal); }); } @@ -167,42 +155,27 @@ export function wrapTransportOnMessage( const resolvedOptions = resolveMcpOptions(options); const callOriginal = (): unknown => (originalOnMessage as (...args: unknown[]) => unknown).call(this, notification, extra); + const startOwnSpan = (fn: () => unknown): unknown => + createMcpNotificationSpan(notification, transport, extra as ExtraHandlerData, resolvedOptions, fn); - if (shouldHandOffToUpstream(serverInstance)) { + if (upstreamInstrumented && hasGlobalTracerProvider()) { return withInboundHandoff( { method: notification.method, - adopt: span => { - span.setAttributes( - buildMcpNotificationSpanConfig( - 'notification-incoming', - notification, - transport, - resolvedOptions, - extra as ExtraHandlerData, - ).attributes, - ); - }, - runInOwnSpan: fn => - createMcpNotificationSpan( - notification, - transport, - extra as ExtraHandlerData, - resolvedOptions, - fn, - ) as ReturnType, + attributes: buildMcpNotificationSpanConfig( + 'notification-incoming', + notification, + transport, + resolvedOptions, + extra as ExtraHandlerData, + ).attributes, + startOwnSpan, }, callOriginal, ); } - return createMcpNotificationSpan( - notification, - transport, - extra as ExtraHandlerData, - resolvedOptions, - callOriginal, - ); + return startOwnSpan(callOriginal); } return (originalOnMessage as (...args: unknown[]) => unknown).call(this, message, extra); @@ -211,33 +184,18 @@ export function wrapTransportOnMessage( } } -/** Records the response on the MCP SDK's span before the SDK ends it. */ -function applyUpstreamOutcome( - transport: MCPTransport, - span: Span, - request: JsonRpcRequest, - outcome: UpstreamOutcome, - capturePolicy: ResolvedMcpOptions, -): void { - if (outcome.type === 'result' && 'result' in outcome) { - applyResponseToSpan(transport, span, request.method, outcome.result, undefined, capturePolicy); - } else if (outcome.type === 'error' && 'error' in outcome) { - applyResponseToSpan(transport, span, request.method, undefined, outcome.error, capturePolicy); - } -} - /** * Wraps transport.send to handle outgoing messages and response correlation. * Caches legacy initialize metadata and applies modern result metadata only to * the corresponding request span. * @param transport - MCP transport instance to wrap * @param options - MCP capture overrides - * @param serverInstance - The MCP server the transport is connected to + * @param upstreamInstrumented - Whether the server has the MCP SDK's built-in instrumentation */ export function wrapTransportSend( transport: MCPTransport, options: McpServerWrapperOptions, - serverInstance?: object, + upstreamInstrumented = false, ): void { if (transport.send) { fill(transport, 'send', originalSend => { @@ -247,7 +205,7 @@ export function wrapTransportSend( if (isJsonRpcNotification(message)) { const resolvedOptions = resolveMcpOptions(options); // The MCP SDK sends a notification inside its own scope, so its span is the active one. - const upstreamSpan = shouldHandOffToUpstream(serverInstance) ? getActiveSpan() : undefined; + const upstreamSpan = upstreamInstrumented && hasGlobalTracerProvider() ? getActiveSpan() : undefined; if (upstreamSpan && spanToJSON(upstreamSpan).attributes[MCP_METHOD_NAME] === message.method) { upstreamSpan.setAttributes( buildMcpNotificationSpanConfig('notification-outgoing', message, transport, resolvedOptions).attributes, diff --git a/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts index d573a06d6ac5..91a3de27ea5e 100644 --- a/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts +++ b/packages/core/src/integrations/mcp-server/upstreamInstrumentation.ts @@ -1,11 +1,7 @@ /** - * Hand-off to the OpenTelemetry instrumentation built into `@modelcontextprotocol/server`. - * - * Newer `@modelcontextprotocol/server` versions start their own span for every request and - * notification through the global OpenTelemetry API. When a tracer provider is registered, those - * spans would duplicate ours, so the server span the SDK starts is enriched with our attributes - * instead of starting a second one. Without a tracer provider the SDK records nothing and the - * transport instrumentation keeps starting its own spans. + * Newer `@modelcontextprotocol/server` versions start their own OpenTelemetry span for every inbound + * message once a tracer provider is registered. In that case we add our attributes to that span + * instead of starting a second one. */ import { MCP_METHOD_NAME } from '@sentry/conventions/attributes'; @@ -16,17 +12,10 @@ import { spanToJSON } from '../../utils/spanUtils'; import { GLOBAL_OBJ } from '../../utils/worldwide'; import type { JsonRpcError, RequestId } from './types'; -const OTEL_API_GLOBAL_KEY = Symbol.for('opentelemetry.js.api.1'); - -/** The outcome the MCP SDK reports when it ends an operation. */ -export type UpstreamOutcome = - | { type: 'result'; result: unknown } - | { type: 'error'; error: JsonRpcError; cause?: unknown } - | { type: string }; - -interface UpstreamOperation { - method: string; - id?: RequestId; +interface UpstreamOutcome { + type: string; + result?: unknown; + error?: JsonRpcError; } interface UpstreamScope { @@ -35,141 +24,99 @@ interface UpstreamScope { } interface UpstreamInstrumentation { - send?: (operation: UpstreamOperation) => UpstreamScope | undefined; - receive?: (operation: UpstreamOperation) => UpstreamScope | undefined; + receive?: (operation: { method: string; id?: RequestId }) => UpstreamScope | undefined; } -/** What the transport instrumentation hands over for one inbound message. */ export interface InboundHandoff { method: string; id?: RequestId; - /** Enriches the span the MCP SDK started; the returned callback runs right before the SDK ends it. */ - adopt(span: Span): ((outcome: UpstreamOutcome) => void) | void; - /** Runs the message's work in a Sentry span when the SDK's span did not reach Sentry. */ - runInOwnSpan(fn: () => T): T; + attributes: Record; + onResponse?: (span: Span, result: unknown, error: JsonRpcError | undefined) => void; + /** Used when the MCP SDK's span does not reach Sentry, e.g. because a non-Sentry tracer provider is registered. */ + startOwnSpan: (fn: () => unknown) => unknown; } -const decoratedProtocols = new WeakSet(); - let pendingHandoff: InboundHandoff | undefined; -/** - * Whether an OpenTelemetry tracer provider is registered globally, i.e. whether the MCP SDK's - * built-in instrumentation records spans. - */ -function hasGlobalTracerProvider(): boolean { - const registry = (GLOBAL_OBJ as unknown as Record | undefined>)[OTEL_API_GLOBAL_KEY]; - return !!registry?.trace; -} - -/** The `Protocol` instance behind an `McpServer` (`server.server`) or a low-level `Server`. */ -function getProtocol(serverInstance: object): Record | undefined { - const inner = (serverInstance as { server?: unknown }).server; - const protocol = isObjectLike(inner) ? inner : serverInstance; - const instrumentation = (protocol as Record)._instrumentation; - return isObjectLike(instrumentation) && typeof (instrumentation as UpstreamInstrumentation).receive === 'function' - ? (protocol as Record) - : undefined; +/** Whether the MCP SDK's built-in instrumentation records spans. */ +export function hasGlobalTracerProvider(): boolean { + const otelApi = (GLOBAL_OBJ as unknown as Record)[ + Symbol.for('opentelemetry.js.api.1') + ]; + return !!otelApi?.trace; } /** - * Installs the hand-off on the MCP SDK's built-in instrumentation of this server, if it has one. - * `_instrumentation` is package-internal to the MCP SDK; without it, nothing changes. + * Hooks into the MCP SDK's built-in instrumentation (the package-internal `_instrumentation`). + * Returns `false` when the server has none. */ -export function decorateUpstreamInstrumentation(serverInstance: object): void { - const protocol = getProtocol(serverInstance); - if (!protocol || decoratedProtocols.has(protocol)) { - return; +export function decorateUpstreamInstrumentation(serverInstance: object): boolean { + const inner = (serverInstance as { server?: unknown }).server; + const protocol = (isObjectLike(inner) ? inner : serverInstance) as { _instrumentation?: UpstreamInstrumentation }; + const original = protocol._instrumentation; + if (typeof original?.receive !== 'function') { + return false; } - const original = protocol._instrumentation as UpstreamInstrumentation; protocol._instrumentation = { ...original, - receive: (operation: UpstreamOperation) => receiveWithHandoff(original, operation), + receive: operation => { + const handoff = pendingHandoff; + if (handoff?.method !== operation.method || handoff.id !== operation.id) { + return original.receive?.(operation); + } + pendingHandoff = undefined; + return receiveWithHandoff(handoff, () => original.receive?.(operation)); + }, }; - decoratedProtocols.add(protocol); -} - -/** - * Whether inbound messages of this server should be handed off to the MCP SDK's instrumentation - * instead of being wrapped in a span by the transport instrumentation. - */ -export function shouldHandOffToUpstream(serverInstance: object | undefined): boolean { - if (!serverInstance) { - return false; - } - const protocol = getProtocol(serverInstance); - return !!protocol && decoratedProtocols.has(protocol) && hasGlobalTracerProvider(); + return true; } -/** - * Runs `callback` (the original `transport.onmessage`) with `handoff` pending. The MCP SDK starts - * its instrumentation scope synchronously while the message is dispatched, which takes it. - */ +/** Runs the original `transport.onmessage`, during which the MCP SDK starts its scope synchronously. */ export function withInboundHandoff(handoff: InboundHandoff, callback: () => T): T { - const previous = pendingHandoff; pendingHandoff = handoff; try { return callback(); } finally { - pendingHandoff = previous; + pendingHandoff = undefined; } } function receiveWithHandoff( - original: UpstreamInstrumentation, - operation: UpstreamOperation, + handoff: InboundHandoff, + receive: () => UpstreamScope | undefined, ): UpstreamScope | undefined { - const handoff = pendingHandoff; - if (!handoff || handoff.method !== operation.method || handoff.id !== operation.id) { - return original.receive?.(operation); - } - pendingHandoff = undefined; - - const { scope, span } = captureStartedSpan(() => original.receive?.(operation)); - - if (span && scope) { - let beforeEnd: ((outcome: UpstreamOutcome) => void) | void = undefined; - try { - beforeEnd = handoff.adopt(span); - } catch { - // noop - } - return { - run: fn => scope.run(fn), - end: outcome => { - try { - beforeEnd?.(outcome); - } catch { - // noop - } - scope.end(outcome); - }, - }; - } - - // The SDK's span did not reach Sentry, e.g. because the registered tracer provider isn't Sentry's. - return { - run: fn => handoff.runInOwnSpan(() => (scope ? scope.run(fn) : fn())), - end: outcome => scope?.end(outcome), - }; -} - -function captureStartedSpan(start: () => UpstreamScope | undefined): { - scope: UpstreamScope | undefined; - span: Span | undefined; -} { let span: Span | undefined; const unsubscribe = getClient()?.on('spanStart', startedSpan => { - if (!span && spanToJSON(startedSpan).attributes[MCP_METHOD_NAME] !== undefined) { + if (!span && spanToJSON(startedSpan).attributes[MCP_METHOD_NAME]) { span = startedSpan; } }); - + let scope: UpstreamScope | undefined; try { - const scope = start(); - return { scope, span }; + scope = receive(); } finally { unsubscribe?.(); } + + if (!scope || !span) { + return { + run: fn => handoff.startOwnSpan(() => (scope ? scope.run(fn) : fn())) as ReturnType, + end: outcome => scope?.end(outcome), + }; + } + + const upstreamSpan = span; + const upstreamScope = scope; + upstreamSpan.setAttributes(handoff.attributes); + return { + run: fn => upstreamScope.run(fn), + // The MCP SDK ends its span before the response is sent, so the response is recorded here. + end: outcome => { + if (outcome.type === 'result' || outcome.type === 'error') { + handoff.onResponse?.(upstreamSpan, outcome.result, outcome.error); + } + upstreamScope.end(outcome); + }, + }; } diff --git a/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts index 20126d4c336c..56fb981f1f98 100644 --- a/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts @@ -28,10 +28,8 @@ interface Instrumentation { receive?: (operation: Operation) => Scope | undefined; } -/** - * The MCP SDK's built-in instrumentation as the `SentryTracerProvider` turns it into Sentry spans: - * a span per operation, active while the handler runs, ended before the response is sent. - */ +// Mirrors the MCP SDK instrumentation under the Sentry tracer provider: one span per operation, +// ended before the response is sent. function sentryTracedInstrumentation(): Instrumentation { const start = (operation: Operation, kind: 'client' | 'server'): Scope => { const span = startInactiveSpan({ @@ -43,7 +41,6 @@ function sentryTracedInstrumentation(): Instrumentation { return { send: operation => start(operation, 'client'), receive: operation => start(operation, 'server') }; } -/** The instrumentation when the registered tracer provider is not Sentry's: no Sentry span is started. */ function foreignTracedInstrumentation(): Instrumentation { const scope: Scope = { run: fn => fn(), end: () => undefined }; return { send: () => scope, receive: () => scope }; @@ -64,11 +61,8 @@ class InMemoryTransport { type ToolHandler = (args: Record) => Promise; -/** - * Minimal stand-in for `McpServer` with the dispatch order of `@modelcontextprotocol/server`'s - * `Protocol`: the instrumentation scope is started synchronously in `transport.onmessage`, the handler - * runs inside it, and the scope ends before the response goes out. - */ +// Follows the dispatch order of the MCP SDK's `Protocol`: the scope starts synchronously in +// `transport.onmessage`, the handler runs inside it, and the scope ends before the response is sent. function createServer(instrumentation: Instrumentation | undefined) { const tools: Record = {}; const protocol: { _instrumentation?: Instrumentation; transport?: InMemoryTransport } = { @@ -264,22 +258,14 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { expect(spanToJSON(handlerSpan!).span_id).toBe(spans[0]!.span_id); }); - it('keeps starting its own spans when no tracer provider is registered', async () => { - const server = wrapMcpServerWithSentry(createServer(foreignTracedInstrumentation())); - server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'echo' }] })); - - const transport = new InMemoryTransport(); - await server.connect(transport); - await callTool(transport, 'echo', {}); - - const spans = mcpSpans(); - expect(spans).toHaveLength(1); - expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); - }); - - it('keeps starting its own spans when the MCP SDK has no instrumentation', async () => { - registerTracerProvider(); - const server = wrapMcpServerWithSentry(createServer(undefined)); + it.each([ + ['no tracer provider is registered', false, foreignTracedInstrumentation()], + ['the MCP SDK has no instrumentation', true, undefined], + ])('keeps starting its own spans when %s', async (_, withTracerProvider, instrumentation) => { + if (withTracerProvider) { + registerTracerProvider(); + } + const server = wrapMcpServerWithSentry(createServer(instrumentation)); server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'echo' }] })); const transport = new InMemoryTransport(); From c95d2afbf0457a2d40b484f3ae2e7f5e589bfd1e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:19:09 +0000 Subject: [PATCH 6/8] ref(core): Fold initialize client attributes into the MCP request span config --- .../src/integrations/mcp-server/transport.ts | 20 +++++++------------ .../transportInstrumentation.test.ts | 18 ++++++++++------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 0fa967586d9f..965fff242bfc 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -113,22 +113,16 @@ export function wrapTransportOnMessage( if (oauthClientName) { spanConfig.attributes[MCP_AUTH_CLIENT_NAME] = oauthClientName; } - const sessionAttributes = - request.method === 'initialize' && messageSessionData - ? { - ...buildClientAttributesFromInfo(messageSessionData.clientInfo), - ...(messageSessionData.protocolVersion && { - [MCP_PROTOCOL_VERSION]: messageSessionData.protocolVersion, - }), - } - : undefined; + if (request.method === 'initialize' && messageSessionData) { + Object.assign(spanConfig.attributes, buildClientAttributesFromInfo(messageSessionData.clientInfo)); + if (messageSessionData.protocolVersion) { + spanConfig.attributes[MCP_PROTOCOL_VERSION] = messageSessionData.protocolVersion; + } + } const callOriginal = (): unknown => (originalOnMessage as (...args: unknown[]) => unknown).call(this, request, extra); const startOwnSpan = (fn: () => unknown): unknown => { const span = startInactiveSpan(spanConfig); - if (sessionAttributes) { - span.setAttributes(sessionAttributes); - } storeSpanForRequest(transport, request.id, span, request.method, resolvedOptions); return withActiveSpan(span, fn); }; @@ -138,7 +132,7 @@ export function wrapTransportOnMessage( { method: request.method, id: request.id, - attributes: { ...spanConfig.attributes, ...sessionAttributes }, + attributes: spanConfig.attributes, onResponse: (span, result, error) => applyResponseToSpan(transport, span, request.method, result, error, resolvedOptions), startOwnSpan, diff --git a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts index 93d5f0e06c72..81b89cf96448 100644 --- a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts @@ -889,11 +889,13 @@ describe('MCP Server Transport Instrumentation', () => { {}, ); - expect(mockSpan.setAttributes).toHaveBeenCalledWith( + expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - 'mcp.client.name': 'test-client', - 'mcp.client.version': '1.0.0', - 'mcp.protocol.version': '2025-06-18', + attributes: expect.objectContaining({ + 'mcp.client.name': 'test-client', + 'mcp.client.version': '1.0.0', + 'mcp.protocol.version': '2025-06-18', + }), }), ); }); @@ -1365,10 +1367,12 @@ describe('MCP Server Transport Instrumentation', () => { }); // Span should have client and server info attributes - expect(mockSpan.setAttributes).toHaveBeenCalledWith( + expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - 'mcp.client.name': 'test-client', - 'mcp.client.version': '1.0.0', + attributes: expect.objectContaining({ + 'mcp.client.name': 'test-client', + 'mcp.client.version': '1.0.0', + }), }), ); expect(mockSpan.setAttributes).toHaveBeenCalledWith( From ffad06f63ef61a86dc07031b3050b404f4937872 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:47:33 +0000 Subject: [PATCH 7/8] test(core): Use semantic attribute constants in MCP SDK hand-off tests Moves the pnpm exotic-subdeps setting into the e2e app's pnpm-workspace.yaml. --- .../node-express-mcp-v2/package.json | 5 +- .../node-express-mcp-v2/pnpm-workspace.yaml | 1 + .../tests/mcp-sdk-otel.test.ts | 28 ++++++--- .../upstreamInstrumentation.test.ts | 57 ++++++++++++------- 4 files changed, 60 insertions(+), 31 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json index 0aaa50d7b84b..91145f279d48 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/package.json @@ -8,7 +8,7 @@ "test": "playwright test", "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install && pnpm build", - "test:build-mcp-sdk-otel": "pnpm install && pnpm add --config.block-exotic-subdeps=false https://pkg.pr.new/@modelcontextprotocol/server@919bb8e https://pkg.pr.new/@modelcontextprotocol/node@919bb8e https://pkg.pr.new/@modelcontextprotocol/client@919bb8e && pnpm build", + "test:build-mcp-sdk-otel": "pnpm install && pnpm add https://pkg.pr.new/@modelcontextprotocol/server@919bb8e https://pkg.pr.new/@modelcontextprotocol/node@919bb8e https://pkg.pr.new/@modelcontextprotocol/client@919bb8e && pnpm build", "test:assert": "pnpm test", "test:assert-mcp-sdk-otel": "MCP_SDK_OTEL=1 pnpm test" }, @@ -26,7 +26,8 @@ "devDependencies": { "@modelcontextprotocol/client": "^2.0.0", "@playwright/test": "1.64.1", - "@sentry-internal/test-utils": "file:../../packed/sentry-internal-test-utils-packed.tgz" + "@sentry-internal/test-utils": "file:../../packed/sentry-internal-test-utils-packed.tgz", + "@sentry/conventions": "0.32.0" }, "type": "module", "sentryTest": { diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml index a1c031fc3310..2f7a2ef5a32a 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml @@ -1 +1,2 @@ minimumReleaseAge: 0 +blockExoticSubdeps: false diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts index c95867116435..ae81a7445ed8 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/tests/mcp-sdk-otel.test.ts @@ -1,4 +1,14 @@ import { expect, test } from '@playwright/test'; +import { + MCP_CLIENT_NAME, + MCP_METHOD_NAME, + MCP_PROMPT_NAME, + MCP_REQUEST_ARGUMENT_KEY_BASE, + MCP_SERVER_NAME, + MCP_TOOL_RESULT_CONTENT_COUNT, + SENTRY_KIND, +} from '@sentry/conventions/attributes'; +import { HTTP_SERVER, MCP_SERVER } from '@sentry/conventions/op'; import { collectStreamedSpans, getSpanOp, type SerializedStreamedSpan } from '@sentry-internal/test-utils'; import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; @@ -10,15 +20,15 @@ async function getMcpSpan(spanName: string): Promise { 'node-express-mcp-v2', spans => spans.some(span => span.is_segment) && spans.some(span => span.name === spanName), ); - const mcpSpans = spans.filter(span => span.attributes?.['mcp.method.name']); + const mcpSpans = spans.filter(span => span.attributes?.[MCP_METHOD_NAME]); expect(mcpSpans).toHaveLength(1); - expect(getSpanOp(spans.find(span => span.is_segment)!)).toBe('http.server'); + expect(getSpanOp(spans.find(span => span.is_segment)!)).toBe(HTTP_SERVER); const [mcpSpan] = mcpSpans; expect(mcpSpan!.name).toBe(spanName); expect(mcpSpan!.is_segment).toBe(false); - expect(mcpSpan!.attributes?.['sentry.kind']?.value).toBe('server'); - expect(getSpanOp(mcpSpan!)).toBe('mcp.server'); + expect(mcpSpan!.attributes?.[SENTRY_KIND]?.value).toBe('server'); + expect(getSpanOp(mcpSpan!)).toBe(MCP_SERVER); return mcpSpan!; } @@ -28,18 +38,18 @@ test('enriches the MCP SDK spans instead of duplicating them', async ({ baseURL const initializeSpanPromise = getMcpSpan('initialize'); await client.connect(new StreamableHTTPClientTransport(new URL(`${baseURL}/mcp`))); const initializeSpan = await initializeSpanPromise; - expect(initializeSpan.attributes?.['mcp.client.name']?.value).toBe('test-client-v2'); - expect(initializeSpan.attributes?.['mcp.server.name']?.value).toBe('Echo-V2'); + expect(initializeSpan.attributes?.[MCP_CLIENT_NAME]?.value).toBe('test-client-v2'); + expect(initializeSpan.attributes?.[MCP_SERVER_NAME]?.value).toBe('Echo-V2'); const toolSpanPromise = getMcpSpan('tools/call echo'); await client.callTool({ name: 'echo', arguments: { message: 'foobar' } }); const toolSpan = await toolSpanPromise; - expect(toolSpan.attributes?.['mcp.request.argument.message']?.value).toBe('"foobar"'); - expect(toolSpan.attributes?.['mcp.tool.result.content_count']?.value).toBe(1); + expect(toolSpan.attributes?.[`${MCP_REQUEST_ARGUMENT_KEY_BASE}.message`]?.value).toBe('"foobar"'); + expect(toolSpan.attributes?.[MCP_TOOL_RESULT_CONTENT_COUNT]?.value).toBe(1); const promptSpanPromise = getMcpSpan('prompts/get echo'); await client.getPrompt({ name: 'echo', arguments: { message: 'foobar' } }); - expect((await promptSpanPromise).attributes?.['mcp.prompt.name']?.value).toBe('echo'); + expect((await promptSpanPromise).attributes?.[MCP_PROMPT_NAME]?.value).toBe('echo'); const errorSpanPromise = getMcpSpan('tools/call always-error'); await client.callTool({ name: 'always-error', arguments: {} }).catch(() => undefined); diff --git a/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts index 56fb981f1f98..ca4eed280ab0 100644 --- a/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/upstreamInstrumentation.test.ts @@ -1,6 +1,24 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + MCP_CLIENT_NAME, + MCP_METHOD_NAME, + MCP_PROTOCOL_VERSION, + MCP_REQUEST_ARGUMENT_KEY_BASE, + MCP_SERVER_NAME, + MCP_TOOL_NAME, + MCP_TOOL_RESULT_CONTENT_COUNT, + SENTRY_KIND, + SENTRY_OP, + SENTRY_ORIGIN, +} from '@sentry/conventions/attributes'; +import { + MCP_NOTIFICATION_CLIENT_TO_SERVER, + MCP_NOTIFICATION_SERVER_TO_CLIENT, + MCP_SERVER, +} from '@sentry/conventions/op'; import { setCurrentClient } from '../../../../src'; import { wrapMcpServerWithSentry } from '../../../../src/integrations/mcp-server'; +import { MCP_FUNCTION_ORIGIN_VALUE } from '../../../../src/integrations/mcp-server/attributes'; import { withActiveSpan } from '../../../../src/tracing'; import { startInactiveSpan, startSpan } from '../../../../src/tracing/trace'; import type { Span, StreamedSpanJSON } from '../../../../src/types/span'; @@ -34,7 +52,7 @@ function sentryTracedInstrumentation(): Instrumentation { const start = (operation: Operation, kind: 'client' | 'server'): Scope => { const span = startInactiveSpan({ name: operation.method, - attributes: { 'mcp.method.name': operation.method, 'sentry.kind': kind }, + attributes: { [MCP_METHOD_NAME]: operation.method, [SENTRY_KIND]: kind }, }); return { run: fn => withActiveSpan(span, fn), end: () => span.end() }; }; @@ -128,8 +146,7 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { let endedSpans: StreamedSpanJSON[]; let originalOtelRegistry: unknown; - const mcpSpans = (): StreamedSpanJSON[] => - endedSpans.filter(span => span.attributes['mcp.method.name'] !== undefined); + const mcpSpans = (): StreamedSpanJSON[] => endedSpans.filter(span => span.attributes[MCP_METHOD_NAME] !== undefined); const registerTracerProvider = (): void => { (GLOBAL_OBJ as unknown as Record)[OTEL_API_GLOBAL_KEY] = { version: '1.9.0', trace: {} }; }; @@ -168,12 +185,12 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { expect(spans).toHaveLength(1); const [toolSpan] = spans; expect(toolSpan!.attributes).toMatchObject({ - 'sentry.kind': 'server', - 'sentry.op': 'mcp.server', - 'sentry.origin': 'auto.function.mcp_server', - 'mcp.tool.name': 'echo', - 'mcp.request.argument.message': '"hi"', - 'mcp.tool.result.content_count': 1, + [SENTRY_KIND]: 'server', + [SENTRY_OP]: MCP_SERVER, + [SENTRY_ORIGIN]: MCP_FUNCTION_ORIGIN_VALUE, + [MCP_TOOL_NAME]: 'echo', + [`${MCP_REQUEST_ARGUMENT_KEY_BASE}.message`]: '"hi"', + [MCP_TOOL_RESULT_CONTENT_COUNT]: 1, }); expect(spanToJSON(handlerParent!).span_id).toBe(toolSpan!.span_id); expect(endedSpans.find(span => span.name === 'inside-tool')!.parent_span_id).toBe(toolSpan!.span_id); @@ -199,10 +216,10 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { const spans = mcpSpans(); expect(spans).toHaveLength(1); expect(spans[0]!.attributes).toMatchObject({ - 'sentry.op': 'mcp.server', - 'mcp.client.name': 'test-client', - 'mcp.server.name': 'test-server', - 'mcp.protocol.version': '2025-06-18', + [SENTRY_OP]: MCP_SERVER, + [MCP_CLIENT_NAME]: 'test-client', + [MCP_SERVER_NAME]: 'test-server', + [MCP_PROTOCOL_VERSION]: '2025-06-18', }); }); @@ -217,8 +234,8 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { const spans = mcpSpans(); expect(spans).toHaveLength(1); expect(spans[0]!.attributes).toMatchObject({ - 'sentry.kind': 'server', - 'sentry.op': 'mcp.notification.client_to_server', + [SENTRY_KIND]: 'server', + [SENTRY_OP]: MCP_NOTIFICATION_CLIENT_TO_SERVER, }); }); @@ -233,8 +250,8 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { const spans = mcpSpans(); expect(spans).toHaveLength(1); expect(spans[0]!.attributes).toMatchObject({ - 'sentry.kind': 'client', - 'sentry.op': 'mcp.notification.server_to_client', + [SENTRY_KIND]: 'client', + [SENTRY_OP]: MCP_NOTIFICATION_SERVER_TO_CLIENT, }); }); @@ -253,8 +270,8 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { const spans = mcpSpans(); expect(spans).toHaveLength(1); - expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); - expect(spans[0]!.attributes['sentry.kind']).toBeUndefined(); + expect(spans[0]!.attributes).toMatchObject({ [SENTRY_OP]: MCP_SERVER, [MCP_TOOL_RESULT_CONTENT_COUNT]: 1 }); + expect(spans[0]!.attributes[SENTRY_KIND]).toBeUndefined(); expect(spanToJSON(handlerSpan!).span_id).toBe(spans[0]!.span_id); }); @@ -274,6 +291,6 @@ describe('MCP server hand-off to the MCP SDK instrumentation', () => { const spans = mcpSpans(); expect(spans).toHaveLength(1); - expect(spans[0]!.attributes).toMatchObject({ 'sentry.op': 'mcp.server', 'mcp.tool.result.content_count': 1 }); + expect(spans[0]!.attributes).toMatchObject({ [SENTRY_OP]: MCP_SERVER, [MCP_TOOL_RESULT_CONTENT_COUNT]: 1 }); }); }); From c63a8ef346d8e26c9c9a11670e8b980c1b080805 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:58:44 +0000 Subject: [PATCH 8/8] test(node): Explain why the MCP e2e app allows exotic subdeps --- .../test-applications/node-express-mcp-v2/pnpm-workspace.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml index 2f7a2ef5a32a..0706e381d258 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml +++ b/dev-packages/e2e-tests/test-applications/node-express-mcp-v2/pnpm-workspace.yaml @@ -1,2 +1,5 @@ minimumReleaseAge: 0 +# The MCP SDK OpenTelemetry variant installs unreleased pkg.pr.new previews, which depend on +# @modelcontextprotocol/core through a pkg.pr.new tarball URL. Remove this once an npm release of +# @modelcontextprotocol/server ships the OpenTelemetry instrumentation and the variant pins that version. blockExoticSubdeps: false