Skip to content

Commit 87b4e30

Browse files
betegonbetegon
authored
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 ## What's inside - Support for new MCP SDK methods (`mcpServerInstance.tool()`, `mcpServerInstance.resource()`, etc.) - Tracing instrumentation - Error handling ## Tracing 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`. ### Tracing flow 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** ## Error handling 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 37bfcdc commit 87b4e30

27 files changed

+3337
-580
lines changed

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

Lines changed: 3 additions & 3 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-v5', transactionEvent => {
21-
return transactionEvent.transaction === 'mcp-server/tool:echo';
21+
return transactionEvent.transaction === 'tools/call echo';
2222
});
2323

2424
const toolResult = await client.callTool({
@@ -51,7 +51,7 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
5151
return transactionEvent.transaction === 'POST /messages';
5252
});
5353
const resourceTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
54-
return transactionEvent.transaction === 'mcp-server/resource:echo';
54+
return transactionEvent.transaction === 'resources/read echo://foobar';
5555
});
5656

5757
const resourceResult = await client.readResource({
@@ -76,7 +76,7 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
7676
return transactionEvent.transaction === 'POST /messages';
7777
});
7878
const promptTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
79-
return transactionEvent.transaction === 'mcp-server/prompt:echo';
79+
return transactionEvent.transaction === 'prompts/get echo';
8080
});
8181

8282
const promptResult = await client.getPrompt({

‎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
@@ -112,7 +112,7 @@ export { featureFlagsIntegration, type FeatureFlagsIntegration } from './integra
112112
export { profiler } from './profiling';
113113
export { instrumentFetchRequest } from './fetch';
114114
export { trpcMiddleware } from './trpc';
115-
export { wrapMcpServerWithSentry } from './mcp-server';
115+
export { wrapMcpServerWithSentry } from './integrations/mcp-server';
116116
export { captureFeedback } from './feedback';
117117
export type { ReportDialogOptions } from './report-dialog';
118118
export { _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer, _INTERNAL_captureSerializedLog } from './logs/exports';

0 commit comments

Comments
 (0)