Skip to content

Commit 6f11ca5

Browse files
betegoncodexJPeer264
authored
fix(core): resolve MCP capture policy per operation (#23437)
MCP server instrumentation now honors the active Sentry client's GenAI input <br>and output collection settings even when the server was wrapped before that <br>client became available. Explicit wrapper overrides still win independently, <br>and the existing default remains unchanged when no policy is configured. The capture decision belongs to the operation that starts the span. <br>Request/response pairs therefore retain one policy for their full lifetime <br>instead of allowing response timing or a different active scope to change what <br>gets recorded. Notifications resolve against the client active when their <br>operation begins. Explicit options are also snapshotted on the first wrap, <br>preserving the wrapper's idempotent behavior. This is shared transport behavior, so it applies to sessionful MCP SDK v1 and <br>stable MCP SDK v2 without changing the public API. Packaged SDK coverage uses <br>the Node and Cloudflare MCP fixtures. A disposable Cloudflare Worker running <br>the packaged SDK also verified the policy matrix against Sentry for both modern <br>and legacy-compatible requests. ## Root cause `wrapMcpServerWithSentry` read `dataCollection.genAI` once, while the wrapper <br>was being constructed. Common Node import ordering and Cloudflare Durable <br>Object initialization can run that code before Sentry binds a client, causing <br>the `true` fallback to become the permanent capture policy for every operation <br>handled by that server. Fixes #23436 --------- Co-authored-by: OpenAI Codex <codex@openai.com> Co-authored-by: JPeer264 <jan.peer@sentry.io>
1 parent 48924ba commit 6f11ca5

11 files changed

Lines changed: 557 additions & 35 deletions

File tree

dev-packages/e2e-tests/test-applications/cloudflare-mcp-agent/src/index.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
44
import * as z from 'zod';
55

66
class MyMCPAgentBase extends McpAgent<Env, unknown, Record<string, unknown>> {
7-
#mcpServer = new McpServer({
8-
name: 'cloudflare-mcp-agent',
9-
version: '1.0.0',
10-
});
7+
#mcpServer = Sentry.wrapMcpServerWithSentry(
8+
new McpServer({
9+
name: 'cloudflare-mcp-agent',
10+
version: '1.0.0',
11+
}),
12+
);
1113

1214
get server() {
13-
return Sentry.wrapMcpServerWithSentry(this.#mcpServer);
15+
return this.#mcpServer;
1416
}
1517

1618
async init(): Promise<void> {
@@ -31,7 +33,6 @@ class MyMCPAgentBase extends McpAgent<Env, unknown, Record<string, unknown>> {
3133
if (span) {
3234
span.setAttribute('mcp.tool.name', 'my-tool');
3335
span.setAttribute('mcp.tool.extra', 'from-mcpagent');
34-
span.setAttribute('mcp.tool.input', JSON.stringify({ message }));
3536
}
3637

3738
return {
@@ -55,6 +56,12 @@ export const MyMCPAgent = Sentry.instrumentDurableObjectWithSentry(
5556
tunnel: `http://localhost:3031/`,
5657
tracesSampleRate: 1.0,
5758
debug: true,
59+
dataCollection: {
60+
genAI: {
61+
inputs: false,
62+
outputs: false,
63+
},
64+
},
5865
transportOptions: {
5966
bufferSize: 1000,
6067
},
@@ -70,6 +77,14 @@ export default Sentry.withSentry(
7077
tunnel: `http://localhost:3031/`,
7178
tracesSampleRate: 1.0,
7279
debug: true,
80+
// The worker and the Durable Object share one cached client per isolate, so the entrypoint that
81+
// initializes first decides the data collection settings for both.
82+
dataCollection: {
83+
genAI: {
84+
inputs: false,
85+
outputs: false,
86+
},
87+
},
7388
transportOptions: {
7489
bufferSize: 1000,
7590
},

dev-packages/e2e-tests/test-applications/cloudflare-mcp-agent/tests/index.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expect, test } from '@playwright/test';
22
import { waitForRequest } from '@sentry-internal/test-utils';
33

44
test('sends spans for MCP tool calls via MCPAgent (DurableObject)', async ({ baseURL }) => {
5+
const privateMessage = 'cloudflare-agent-private-capture-policy-message';
56
const mcpToolWaiter = waitForRequest('cloudflare-mcp-agent', event => {
67
const transaction = event.envelope[1][0][1];
78
return (
@@ -66,16 +67,18 @@ test('sends spans for MCP tool calls via MCPAgent (DurableObject)', async ({ bas
6667
params: {
6768
name: 'my-tool',
6869
arguments: {
69-
message: 'hello from MCPAgent test',
70+
message: privateMessage,
7071
},
7172
},
7273
}),
7374
});
7475

7576
expect(response.status).toBe(200);
77+
await expect(response.text()).resolves.toContain(`Tool my-tool: ${privateMessage}`);
7678

7779
const mcpData = await mcpToolWaiter;
7880
const mcpEvent = mcpData.envelope[1][0][1];
81+
const traceData = mcpEvent.contexts?.trace?.data;
7982

8083
expect(mcpEvent.contexts?.trace?.trace_id).toBe(mcpData.envelope[0].trace.trace_id);
8184
expect(mcpEvent.contexts?.trace).toEqual({
@@ -91,7 +94,12 @@ test('sends spans for MCP tool calls via MCPAgent (DurableObject)', async ({ bas
9194
'mcp.method.name': 'tools/call',
9295
'mcp.tool.name': 'my-tool',
9396
'mcp.tool.extra': 'from-mcpagent',
94-
'mcp.tool.input': '{"message":"hello from MCPAgent test"}',
97+
'mcp.tool.result.content_count': 1,
98+
'mcp.tool.result.content_type': 'text',
9599
}),
96100
});
101+
expect(traceData?.['mcp.request.argument.message']).toBeUndefined();
102+
expect(traceData?.['mcp.tool.result.content']).toBeUndefined();
103+
expect(traceData?.['mcp.tool.input']).toBeUndefined();
104+
expect(JSON.stringify(traceData)).not.toContain(privateMessage);
97105
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import * as Sentry from '@sentry/node';
2+
// Keep the dedicated MCP server evaluation ahead of initialization without loading Express before its instrumentation.
3+
import './mcpCapturePolicyServer';
24

35
declare global {
46
namespace globalThis {
@@ -14,6 +16,12 @@ Sentry.init({
1416
debug: !!process.env.DEBUG,
1517
tunnel: `http://localhost:3031/`, // proxy server
1618
tracesSampleRate: 1,
19+
dataCollection: {
20+
genAI: {
21+
inputs: false,
22+
outputs: false,
23+
},
24+
},
1725
// Opt into the Sentry OpenTelemetry tracer provider in the "(tracer provider)" e2e variant.
1826
// Leaving it `undefined` otherwise keeps the SDK's default (no provider).
1927
enableOpenTelemetrySetup: process.env.E2E_TEST_OTEL_SETUP === 'true' ? true : undefined,

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js';
55
import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
66
import { z } from 'zod';
77
import { wrapMcpServerWithSentry } from '@sentry/node';
8+
import { capturePolicyServer } from './mcpCapturePolicyServer';
89

910
// Helper to check if request is an initialize request (compatible with all MCP SDK versions)
1011
function isInitializeRequest(body: unknown): boolean {
@@ -60,6 +61,26 @@ server.tool('always-error', {}, async () => {
6061
});
6162

6263
const transports: Record<string, SSEServerTransport> = {};
64+
const capturePolicyTransports: Record<string, SSEServerTransport> = {};
65+
66+
mcpRouter.get('/capture-policy/sse', async (_, res) => {
67+
const transport = new SSEServerTransport('/capture-policy/messages', res);
68+
capturePolicyTransports[transport.sessionId] = transport;
69+
res.on('close', () => {
70+
delete capturePolicyTransports[transport.sessionId];
71+
});
72+
await capturePolicyServer.connect(transport);
73+
});
74+
75+
mcpRouter.post('/capture-policy/messages', async (req, res) => {
76+
const sessionId = req.query.sessionId;
77+
const transport = capturePolicyTransports[sessionId as string];
78+
if (transport) {
79+
await transport.handlePostMessage(req, res, req.body);
80+
} else {
81+
res.status(400).send('No transport found for sessionId');
82+
}
83+
});
6384

6485
mcpRouter.get('/sse', async (_, res) => {
6586
const transport = new SSEServerTransport('/messages', res);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
2+
import { wrapMcpServerWithSentry } from '@sentry/node';
3+
import { z } from 'zod';
4+
5+
export const capturePolicyServer = wrapMcpServerWithSentry(
6+
new McpServer({
7+
name: 'Capture-Policy',
8+
version: '1.0.0',
9+
}),
10+
);
11+
12+
capturePolicyServer.tool('capture-policy', { message: z.string() }, async ({ message }) => {
13+
return {
14+
content: [{ type: 'text', text: `Capture policy result: ${message}` }],
15+
};
16+
});

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,49 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
180180
});
181181
});
182182

183+
test('resolves capture policy when the MCP server is wrapped before Sentry.init', async ({ baseURL }) => {
184+
const transport = new SSEClientTransport(new URL(`${baseURL}/capture-policy/sse`));
185+
const client = new Client({
186+
name: 'capture-policy-client',
187+
version: '1.0.0',
188+
});
189+
await client.connect(transport);
190+
191+
const toolTransactionPromise = waitForTransaction('node-express', transactionEvent => {
192+
return transactionEvent.transaction === 'tools/call capture-policy';
193+
});
194+
const privateMessage = 'node-v1-private-capture-policy-message';
195+
196+
const toolResult = await client.callTool({
197+
name: 'capture-policy',
198+
arguments: {
199+
message: privateMessage,
200+
},
201+
});
202+
203+
expect(toolResult).toMatchObject({
204+
content: [
205+
{
206+
text: `Capture policy result: ${privateMessage}`,
207+
type: 'text',
208+
},
209+
],
210+
});
211+
212+
const toolTransaction = await toolTransactionPromise;
213+
const traceData = toolTransaction.contexts?.trace?.data;
214+
215+
expect(traceData?.['mcp.method.name']).toBe('tools/call');
216+
expect(traceData?.['mcp.tool.name']).toBe('capture-policy');
217+
expect(traceData?.['mcp.tool.result.content_count']).toBe(1);
218+
expect(traceData?.['mcp.tool.result.content_type']).toBe('text');
219+
expect(traceData?.['mcp.request.argument.message']).toBeUndefined();
220+
expect(traceData?.['mcp.tool.result.content']).toBeUndefined();
221+
expect(JSON.stringify(traceData)).not.toContain(privateMessage);
222+
223+
await client.close();
224+
});
225+
183226
/**
184227
* Tests for StreamableHTTPServerTransport (wrapper transport pattern)
185228
*

packages/core/src/integrations/mcp-server/correlation.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,20 @@ function getOrCreateSpanMap(transport: MCPTransport): Map<RequestId, RequestSpan
6969
* @param requestId - Request identifier
7070
* @param span - Active span to correlate
7171
* @param method - MCP method name
72+
* @param capturePolicy - Capture policy resolved when the request began
7273
*/
73-
export function storeSpanForRequest(transport: MCPTransport, requestId: RequestId, span: Span, method: string): void {
74+
export function storeSpanForRequest(
75+
transport: MCPTransport,
76+
requestId: RequestId,
77+
span: Span,
78+
method: string,
79+
capturePolicy: ResolvedMcpOptions,
80+
): void {
7481
const spanMap = getOrCreateSpanMap(transport);
7582
spanMap.set(requestId, {
7683
span,
7784
method,
85+
capturePolicy,
7886
// oxlint-disable-next-line sdk/no-unsafe-random-apis
7987
startTime: Date.now(),
8088
});
@@ -85,14 +93,12 @@ export function storeSpanForRequest(transport: MCPTransport, requestId: RequestI
8593
* @param transport - MCP transport instance
8694
* @param requestId - Request identifier
8795
* @param result - Execution result for attribute extraction
88-
* @param options - Resolved MCP options
8996
* @param hasError - Whether the JSON-RPC response contained an error
9097
*/
9198
export function completeSpanWithResults(
9299
transport: MCPTransport,
93100
requestId: RequestId,
94101
result: unknown,
95-
options: ResolvedMcpOptions,
96102
hasError = false,
97103
): void {
98104
const spanMap = getOrCreateSpanMap(transport);
@@ -119,10 +125,10 @@ export function completeSpanWithResults(
119125
if (hasError) {
120126
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
121127
} else if (method === 'tools/call') {
122-
const toolAttributes = extractToolResultAttributes(result, options.recordOutputs);
128+
const toolAttributes = extractToolResultAttributes(result, spanData.capturePolicy.recordOutputs);
123129
span.setAttributes(toolAttributes);
124130
} else if (method === 'prompts/get') {
125-
const promptAttributes = extractPromptResultAttributes(result, options.recordOutputs);
131+
const promptAttributes = extractPromptResultAttributes(result, spanData.capturePolicy.recordOutputs);
126132
span.setAttributes(promptAttributes);
127133
}
128134

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import { getClient } from '../../currentScopes';
21
import { fill } from '../../utils/object';
32
import { wrapAllMCPHandlers, wrapExistingHandlers } from './handlers';
43
import { wrapTransportError, wrapTransportOnClose, wrapTransportOnMessage, wrapTransportSend } from './transport';
5-
import type { MCPServerInstance, McpServerWrapperOptions, MCPTransport, ResolvedMcpOptions } from './types';
4+
import type { MCPServerInstance, McpServerWrapperOptions, MCPTransport } from './types';
65
import { validateMcpServerInstance } from './validation';
76

87
/**
@@ -60,13 +59,7 @@ export function wrapMcpServerWithSentry<S extends object>(mcpServerInstance: S,
6059
}
6160

6261
const serverInstance = mcpServerInstance as MCPServerInstance;
63-
const client = getClient();
64-
const genAI = client?.getDataCollectionOptions().genAI;
65-
66-
const resolvedOptions: ResolvedMcpOptions = {
67-
recordInputs: options?.recordInputs ?? genAI?.inputs ?? true,
68-
recordOutputs: options?.recordOutputs ?? genAI?.outputs ?? true,
69-
};
62+
const captureOptions: McpServerWrapperOptions = { ...options };
7063

7164
fill(serverInstance, 'connect', originalConnect => {
7265
return async function (this: MCPServerInstance, transport: MCPTransport, ...restArgs: unknown[]) {
@@ -76,8 +69,8 @@ export function wrapMcpServerWithSentry<S extends object>(mcpServerInstance: S,
7669
...restArgs,
7770
);
7871

79-
wrapTransportOnMessage(transport, resolvedOptions);
80-
wrapTransportSend(transport, resolvedOptions);
72+
wrapTransportOnMessage(transport, captureOptions);
73+
wrapTransportSend(transport, captureOptions);
8174
wrapTransportOnClose(transport);
8275
wrapTransportError(transport);
8376

0 commit comments

Comments
 (0)