Skip to content

Commit 3e4eaa0

Browse files
betegonAbhiPrasad
authored andcommitted
feat(core): MCP server instrumentation without breaking Miniflare (#16817)
Closes #16826, #16654, #16666 #16978 Different approach from #16807 . Using `Proxy` was causing issues in cloudflare #16182. Now using `fill` we shouldn't have those problems as `fill` doesn't create a new wrapper object with a different identity, so now: 1. `fill` just replaces the method on the existing object 2. The transport object keeps its original identity 3. When `transport.start()` runs and accesses private fields, this is still the original transport object 4. The `WeakMap` recognizes it as the same object that owns the private fields - Support for new MCP SDK methods (`mcpServerInstance.tool()`, `mcpServerInstance.resource()`, etc.) - Tracing instrumentation - Error handling It follows [OTEL semantic conventions for MCP](https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md) and adds more attributes we thought are useful. It also handles PII based on user setting of `sendDefaultPii`. 1. **Transport receives tools/call request (`id: 2`)** 2. **Create INACTIVE `mcp.server` span** 3. **Store span in**`requestIdToSpanMap[2] = { span, method: "tools/call", startTime }` 4. **Execute handler with span context** *(handler gets requestId: 2)* 5. **Handler finds span using** `requestId: 2` 6. **Tool execution happens within span context** 7. **Response sent with tool results** 8. `completeSpanWithToolResults(2, result)` **enriches and completes span** 1. **error capture** - `errorCapture.ts` - Non-invasive error reporting that never interferes with MCP service operation - Error context with MCP-specific metadata - PII filtering respects `sendDefaultPii` settings - Resilient to Sentry failures (wrapped in try-catch) 2. **Tool execution error capturing** - `handlers.ts` - Captures exceptions thrown during tool execution - Preserves normal MCP behaviour (errors converted to `isError: true`) - Includes tool name, arguments, and request context - Handles both sync and async tool handlers 3. **Transport error instrumentation** - `transport.ts` - Captures connection errors and network failures - Intercepts JSON-RPC error responses - Includes transport type and session information - Handles error responses being sent back to clients
1 parent 343d18a commit 3e4eaa0

27 files changed

+3443
-577
lines changed
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForTransaction } from '@sentry-internal/test-utils';
3+
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
4+
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
5+
6+
test('Should record transactions for mcp handlers', async ({ baseURL }) => {
7+
const transport = new SSEClientTransport(new URL(`${baseURL}/sse`));
8+
9+
const client = new Client({
10+
name: 'test-client',
11+
version: '1.0.0',
12+
});
13+
14+
await client.connect(transport);
15+
16+
await test.step('tool handler', async () => {
17+
const postTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
18+
return transactionEvent.transaction === 'POST /messages';
19+
});
20+
const toolTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
21+
return transactionEvent.transaction === 'tools/call echo';
22+
});
23+
24+
const toolResult = await client.callTool({
25+
name: 'echo',
26+
arguments: {
27+
message: 'foobar',
28+
},
29+
});
30+
31+
expect(toolResult).toMatchObject({
32+
content: [
33+
{
34+
text: 'Tool echo: foobar',
35+
type: 'text',
36+
},
37+
],
38+
});
39+
40+
const postTransaction = await postTransactionPromise;
41+
expect(postTransaction).toBeDefined();
42+
43+
const toolTransaction = await toolTransactionPromise;
44+
expect(toolTransaction).toBeDefined();
45+
46+
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
47+
});
48+
49+
await test.step('resource handler', async () => {
50+
const postTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
51+
return transactionEvent.transaction === 'POST /messages';
52+
});
53+
const resourceTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
54+
return transactionEvent.transaction === 'resources/read echo://foobar';
55+
});
56+
57+
const resourceResult = await client.readResource({
58+
uri: 'echo://foobar',
59+
});
60+
61+
expect(resourceResult).toMatchObject({
62+
contents: [{ text: 'Resource echo: foobar', uri: 'echo://foobar' }],
63+
});
64+
65+
const postTransaction = await postTransactionPromise;
66+
expect(postTransaction).toBeDefined();
67+
68+
const resourceTransaction = await resourceTransactionPromise;
69+
expect(resourceTransaction).toBeDefined();
70+
71+
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
72+
});
73+
74+
await test.step('prompt handler', async () => {
75+
const postTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
76+
return transactionEvent.transaction === 'POST /messages';
77+
});
78+
const promptTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
79+
return transactionEvent.transaction === 'prompts/get echo';
80+
});
81+
82+
const promptResult = await client.getPrompt({
83+
name: 'echo',
84+
arguments: {
85+
message: 'foobar',
86+
},
87+
});
88+
89+
expect(promptResult).toMatchObject({
90+
messages: [
91+
{
92+
content: {
93+
text: 'Please process this message: foobar',
94+
type: 'text',
95+
},
96+
role: 'user',
97+
},
98+
],
99+
});
100+
101+
const postTransaction = await postTransactionPromise;
102+
expect(postTransaction).toBeDefined();
103+
104+
const promptTransaction = await promptTransactionPromise;
105+
expect(promptTransaction).toBeDefined();
106+
107+
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
108+
});
109+
});

dev-packages/e2e-tests/test-applications/node-express/src/app.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import { mcpRouter } from './mcp';
2525
const app = express();
2626
const port = 3030;
2727

28+
app.use(express.json());
29+
2830
app.use(mcpRouter);
2931

3032
app.get('/crash-in-with-monitor/:id', async (req, res) => {

dev-packages/e2e-tests/test-applications/node-express/src/mcp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ mcpRouter.post('/messages', async (req, res) => {
5555
const sessionId = req.query.sessionId;
5656
const transport = transports[sessionId as string];
5757
if (transport) {
58-
await transport.handlePostMessage(req, res);
58+
await transport.handlePostMessage(req, res, req.body);
5959
} else {
6060
res.status(400).send('No transport found for sessionId');
6161
}

dev-packages/e2e-tests/test-applications/node-express/tests/errors.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ test('Sends correct error event', async ({ baseURL }) => {
1313
expect(errorEvent.exception?.values).toHaveLength(1);
1414
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
1515

16-
expect(errorEvent.request).toEqual({
16+
expect(errorEvent.request).toMatchObject({
1717
method: 'GET',
1818
cookies: {},
1919
headers: expect.any(Object),
@@ -43,11 +43,11 @@ test('Should record caught exceptions with local variable', async ({ baseURL })
4343

4444
test('To not crash app from withMonitor', async ({ baseURL }) => {
4545
const doRequest = async (id: number) => {
46-
const response = await fetch(`${baseURL}/crash-in-with-monitor/${id}`)
46+
const response = await fetch(`${baseURL}/crash-in-with-monitor/${id}`);
4747
return response.json();
48-
}
49-
const [response1, response2] = await Promise.all([doRequest(1), doRequest(2)])
50-
expect(response1.message).toBe('This is an exception withMonitor: 1')
51-
expect(response2.message).toBe('This is an exception withMonitor: 2')
52-
expect(response1.pid).toBe(response2.pid) //Just to double-check, TBS
48+
};
49+
const [response1, response2] = await Promise.all([doRequest(1), doRequest(2)]);
50+
expect(response1.message).toBe('This is an exception withMonitor: 1');
51+
expect(response2.message).toBe('This is an exception withMonitor: 2');
52+
expect(response1.pid).toBe(response2.pid); //Just to double-check, TBS
5353
});

dev-packages/e2e-tests/test-applications/node-express/tests/mcp.test.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
1818
return transactionEvent.transaction === 'POST /messages';
1919
});
2020
const toolTransactionPromise = waitForTransaction('node-express', transactionEvent => {
21-
return transactionEvent.transaction === 'mcp-server/tool:echo';
21+
return transactionEvent.transaction === 'tools/call echo';
2222
});
2323

2424
const toolResult = await client.callTool({
@@ -39,10 +39,12 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
3939

4040
const postTransaction = await postTransactionPromise;
4141
expect(postTransaction).toBeDefined();
42+
expect(postTransaction.contexts?.trace?.op).toEqual('http.server');
4243

4344
const toolTransaction = await toolTransactionPromise;
4445
expect(toolTransaction).toBeDefined();
45-
46+
expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server');
47+
expect(toolTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('tools/call');
4648
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
4749
});
4850

@@ -51,7 +53,7 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
5153
return transactionEvent.transaction === 'POST /messages';
5254
});
5355
const resourceTransactionPromise = waitForTransaction('node-express', transactionEvent => {
54-
return transactionEvent.transaction === 'mcp-server/resource:echo';
56+
return transactionEvent.transaction === 'resources/read echo://foobar';
5557
});
5658

5759
const resourceResult = await client.readResource({
@@ -64,10 +66,12 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
6466

6567
const postTransaction = await postTransactionPromise;
6668
expect(postTransaction).toBeDefined();
69+
expect(postTransaction.contexts?.trace?.op).toEqual('http.server');
6770

6871
const resourceTransaction = await resourceTransactionPromise;
6972
expect(resourceTransaction).toBeDefined();
70-
73+
expect(resourceTransaction.contexts?.trace?.op).toEqual('mcp.server');
74+
expect(resourceTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('resources/read');
7175
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
7276
});
7377

@@ -76,7 +80,7 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
7680
return transactionEvent.transaction === 'POST /messages';
7781
});
7882
const promptTransactionPromise = waitForTransaction('node-express', transactionEvent => {
79-
return transactionEvent.transaction === 'mcp-server/prompt:echo';
83+
return transactionEvent.transaction === 'prompts/get echo';
8084
});
8185

8286
const promptResult = await client.getPrompt({
@@ -100,10 +104,12 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
100104

101105
const postTransaction = await postTransactionPromise;
102106
expect(postTransaction).toBeDefined();
107+
expect(postTransaction.contexts?.trace?.op).toEqual('http.server');
103108

104109
const promptTransaction = await promptTransactionPromise;
105110
expect(promptTransaction).toBeDefined();
106-
111+
expect(promptTransaction.contexts?.trace?.op).toEqual('mcp.server');
112+
expect(promptTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('prompts/get');
107113
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
108114
});
109115
});

packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export { featureFlagsIntegration, type FeatureFlagsIntegration } from './integra
118118
export { profiler } from './profiling';
119119
export { instrumentFetchRequest } from './fetch';
120120
export { trpcMiddleware } from './trpc';
121-
export { wrapMcpServerWithSentry } from './mcp-server';
121+
export { wrapMcpServerWithSentry } from './integrations/mcp-server';
122122
export { captureFeedback } from './feedback';
123123
export type { ReportDialogOptions } from './report-dialog';
124124
export { _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer, _INTERNAL_captureSerializedLog } from './logs/exports';

0 commit comments

Comments
 (0)