Skip to content

Commit ceb8517

Browse files
betegoncodex
andauthored
fix(core): instrument MCP transports before start (#23978)
Fixes missing MCP spans when a transport delivers queued requests during `server.connect()`. The MCP SDK installs its callbacks before calling `transport.start()`, so instrumentation must run at that boundary. The temporary interceptor restores `start()` before invoking it, preserving its receiver, Promise and synchronous errors. Transports without a patchable `start()` retain the post-connect fallback. Regression tests cover MCP v1 and v2 in ESM/CJS, including optional Sentry-managed OpenTelemetry setup. A deployed Worker A/B reproduced the missing spans without the fix; the modern protocol entry path was unaffected. This does not change OpenTelemetry providers, exporters or propagation. Fixes #23977 --------- Co-authored-by: OpenAI Codex <codex@openai.com>
1 parent 1b2f6bd commit ceb8517

7 files changed

Lines changed: 372 additions & 12 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/mcp-server-streamed/instrument.mjs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,14 @@ Sentry.init({
66
release: '1.0',
77
tracesSampleRate: 1.0,
88
transport: loggingTransport,
9+
enableOpenTelemetrySetup: process.env.ENABLE_OTEL === 'true',
10+
});
11+
12+
let initializeSpansStarted = 0;
13+
Sentry.getClient()?.on('spanStart', span => {
14+
const attributes = Sentry.spanToJSON(span).attributes;
15+
if (attributes['sentry.op'] === 'mcp.server' && attributes['mcp.method.name'] === 'initialize') {
16+
initializeSpansStarted += 1;
17+
span.setAttribute('test.mcp.initialize_spans_started', initializeSpansStarted);
18+
}
919
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { Client } from '@modelcontextprotocol/client';
2+
import { InMemoryTransport, McpServer } from '@modelcontextprotocol/server';
3+
import { wrapMcpServerWithSentry } from '@sentry/node';
4+
5+
const server = wrapMcpServerWithSentry(new McpServer({ name: 'Echo', version: '1.0.0' }));
6+
7+
async function run() {
8+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
9+
const client = new Client({ name: 'test-client', version: '1.0.0' }, { versionNegotiation: { mode: 'legacy' } });
10+
const originalSend = clientTransport.send.bind(clientTransport);
11+
const requestQueued = new Promise(resolve => {
12+
clientTransport.send = async (...args) => {
13+
const result = await originalSend(...args);
14+
if (args[0]?.method === 'initialize') {
15+
resolve();
16+
}
17+
return result;
18+
};
19+
});
20+
21+
const clientConnection = client.connect(clientTransport);
22+
await requestQueued;
23+
await server.connect(serverTransport);
24+
await clientConnection;
25+
26+
await client.close();
27+
await server.close();
28+
}
29+
30+
run();
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
2+
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
3+
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
4+
import { wrapMcpServerWithSentry } from '@sentry/node';
5+
6+
const server = wrapMcpServerWithSentry(new McpServer({ name: 'Echo', version: '1.0.0' }));
7+
8+
async function run() {
9+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
10+
const client = new Client({ name: 'test-client', version: '1.0.0' });
11+
const originalSend = clientTransport.send.bind(clientTransport);
12+
const requestQueued = new Promise(resolve => {
13+
clientTransport.send = async (...args) => {
14+
const result = await originalSend(...args);
15+
if (args[0]?.method === 'initialize') {
16+
resolve();
17+
}
18+
return result;
19+
};
20+
});
21+
22+
const clientConnection = client.connect(clientTransport);
23+
await requestQueued;
24+
await server.connect(serverTransport);
25+
await clientConnection;
26+
27+
await client.close();
28+
await server.close();
29+
}
30+
31+
run();

‎dev-packages/node-integration-tests/suites/tracing/mcp-server-streamed/test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@ function mcpSpans(container: SerializedStreamedSpanContainer): SerializedStreame
66
return container.items.filter(item => item.attributes['sentry.op']?.value === 'mcp.server');
77
}
88

9+
function assertInitializeSpan(container: SerializedStreamedSpanContainer): void {
10+
const initializeSpans = mcpSpans(container).filter(
11+
span => span.attributes['mcp.method.name']?.value === 'initialize',
12+
);
13+
14+
expect(initializeSpans).toHaveLength(1);
15+
const initializeSpan = initializeSpans[0]!;
16+
expect(initializeSpan.name).toBe('initialize');
17+
expect(initializeSpan.status).toBe('ok');
18+
expect(initializeSpan.attributes['sentry.op']).toEqual({ type: 'string', value: 'mcp.server' });
19+
expect(initializeSpan.attributes['sentry.origin']).toEqual({ type: 'string', value: 'auto.function.mcp_server' });
20+
expect(initializeSpan.attributes['test.mcp.initialize_spans_started']).toEqual({ type: 'integer', value: 1 });
21+
}
22+
923
describe('MCP server spans (streamed)', () => {
1024
afterAll(() => {
1125
cleanupChildProcesses();
@@ -43,4 +57,31 @@ describe('MCP server spans (streamed)', () => {
4357
.completed();
4458
});
4559
});
60+
61+
createEsmAndCjsTests(__dirname, 'scenario-start-v2.mjs', 'instrument.mjs', (createTestRunner, test) => {
62+
test('captures an MCP v2 initialize request queued before transport start once', async () => {
63+
await createTestRunner().unordered().expect({ span: assertInitializeSpan }).start().completed();
64+
});
65+
66+
test('captures the queued request with Sentry OpenTelemetry setup enabled', async () => {
67+
await createTestRunner()
68+
.withEnv({ ENABLE_OTEL: 'true' })
69+
.unordered()
70+
.expect({ span: assertInitializeSpan })
71+
.start()
72+
.completed();
73+
});
74+
});
75+
76+
createEsmAndCjsTests(
77+
__dirname,
78+
'scenario-v1.mjs',
79+
'instrument.mjs',
80+
(createTestRunner, test) => {
81+
test('captures an MCP v1 initialize request queued before transport start once', async () => {
82+
await createTestRunner().unordered().expect({ span: assertInitializeSpan }).start().completed();
83+
});
84+
},
85+
{ additionalDependencies: { '@modelcontextprotocol/sdk': '1.30.0' } },
86+
);
4687
});

‎packages/core/src/integrations/mcp-server/index.ts‎

Lines changed: 97 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,79 @@ import { validateMcpServerInstance } from './validation';
1010
*/
1111
const wrappedMcpServerInstances = new WeakSet();
1212

13+
function instrumentTransport(transport: MCPTransport, options: McpServerWrapperOptions): void {
14+
wrapTransportOnMessage(transport, options);
15+
wrapTransportSend(transport, options);
16+
wrapTransportOnClose(transport);
17+
wrapTransportError(transport);
18+
}
19+
20+
function interceptTransportStart(transport: MCPTransport, beforeStart: () => void): () => void {
21+
let transportStart: MCPTransport['start'];
22+
let originalDescriptor: PropertyDescriptor | undefined;
23+
24+
try {
25+
transportStart = transport.start;
26+
originalDescriptor = Object.getOwnPropertyDescriptor(transport, 'start');
27+
} catch {
28+
return () => undefined;
29+
}
30+
31+
if (typeof transportStart !== 'function') {
32+
return () => undefined;
33+
}
34+
35+
const originalStart = transportStart;
36+
let isInstalled = false;
37+
38+
const restoreStart = (): void => {
39+
if (!isInstalled) {
40+
return;
41+
}
42+
43+
try {
44+
const currentDescriptor = Object.getOwnPropertyDescriptor(transport, 'start');
45+
if (currentDescriptor?.value !== interceptedStart) {
46+
isInstalled = false;
47+
return;
48+
}
49+
50+
if (originalDescriptor) {
51+
Object.defineProperty(transport, 'start', originalDescriptor);
52+
isInstalled = false;
53+
} else if (Reflect.deleteProperty(transport, 'start')) {
54+
isInstalled = false;
55+
}
56+
} catch {}
57+
};
58+
59+
function interceptedStart(this: MCPTransport): Promise<void> {
60+
// Restoring first keeps recursive calls and user-observed method identity identical to the original transport.
61+
restoreStart();
62+
beforeStart();
63+
return originalStart.call(this);
64+
}
65+
66+
const replacementDescriptor: PropertyDescriptor =
67+
originalDescriptor && 'value' in originalDescriptor
68+
? { ...originalDescriptor, value: interceptedStart }
69+
: {
70+
configurable: originalDescriptor?.configurable ?? true,
71+
enumerable: originalDescriptor?.enumerable ?? false,
72+
writable: true,
73+
value: interceptedStart,
74+
};
75+
76+
try {
77+
Object.defineProperty(transport, 'start', replacementDescriptor);
78+
isInstalled = true;
79+
} catch {
80+
// The post-connect fallback preserves the previous behavior for transports which cannot be patched.
81+
}
82+
83+
return restoreStart;
84+
}
85+
1386
/**
1487
* Wraps an MCP Server instance with Sentry instrumentation.
1588
*
@@ -63,18 +136,30 @@ export function wrapMcpServerWithSentry<S extends object>(mcpServerInstance: S,
63136

64137
fill(serverInstance, 'connect', originalConnect => {
65138
return async function (this: MCPServerInstance, transport: MCPTransport, ...restArgs: unknown[]) {
66-
const result = await (originalConnect as (...args: unknown[]) => Promise<unknown>).call(
67-
this,
68-
transport,
69-
...restArgs,
70-
);
71-
72-
wrapTransportOnMessage(transport, captureOptions);
73-
wrapTransportSend(transport, captureOptions);
74-
wrapTransportOnClose(transport);
75-
wrapTransportError(transport);
76-
77-
return result;
139+
let isTransportInstrumented = false;
140+
const instrumentTransportOnce = (): void => {
141+
if (isTransportInstrumented) {
142+
return;
143+
}
144+
145+
isTransportInstrumented = true;
146+
instrumentTransport(transport, captureOptions);
147+
};
148+
const restoreStart = interceptTransportStart(transport, instrumentTransportOnce);
149+
150+
try {
151+
const result = await (originalConnect as (...args: unknown[]) => Promise<unknown>).call(
152+
this,
153+
transport,
154+
...restArgs,
155+
);
156+
157+
instrumentTransportOnce();
158+
159+
return result;
160+
} finally {
161+
restoreStart();
162+
}
78163
};
79164
});
80165

‎packages/core/src/integrations/mcp-server/types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ export interface JsonRpcNotification {
6565
* @description Abstraction for MCP communication transport layer
6666
*/
6767
export interface MCPTransport {
68+
/** Starts the transport lifecycle. */
69+
start?: () => Promise<void>;
70+
6871
/**
6972
* Message handler for incoming JSON-RPC messages
7073
* The first argument is a JSON RPC message

0 commit comments

Comments
 (0)