From dff20805eb18197d7b845cbc37d619a8cf90cc5a Mon Sep 17 00:00:00 2001 From: betegon Date: Fri, 27 Jun 2025 20:46:18 +0200 Subject: [PATCH 01/22] feat(mcp-server): Enhance transport handling and request instrumentation --- packages/core/src/mcp-server.ts | 361 ++++++++++++++++++-------------- 1 file changed, 200 insertions(+), 161 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 1a6f626a83f2..80f831cb7742 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -25,6 +25,9 @@ interface MCPServerInstance { // The first arg is always a name, the last arg should always be a callback function (ie a handler). prompt: (name: string, ...args: unknown[]) => void; connect(transport: MCPTransport): Promise; + server?: { + setRequestHandler: (schema: unknown, handler: (...args: unknown[]) => unknown) => void; + }; } const wrappedMcpServerInstances = new WeakSet(); @@ -45,164 +48,213 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): return mcpServerInstance; } - // eslint-disable-next-line @typescript-eslint/unbound-method + // Wrap connect() to intercept AFTER Protocol sets up transport handlers mcpServerInstance.connect = new Proxy(mcpServerInstance.connect, { - apply(target, thisArg, argArray) { + async apply(target, thisArg, argArray) { const [transport, ...restArgs] = argArray as [MCPTransport, ...unknown[]]; - if (!transport.onclose) { - transport.onclose = () => { - if (transport.sessionId) { - handleTransportOnClose(transport.sessionId); + // Call the original connect first to let Protocol set up its handlers + const result = await Reflect.apply(target, thisArg, [transport, ...restArgs]); + + + // NOW intercept the transport's onmessage after Protocol has set it up + if (transport.onmessage) { + const protocolOnMessage = transport.onmessage; + + transport.onmessage = new Proxy(protocolOnMessage, { + apply(onMessageTarget, onMessageThisArg, onMessageArgs) { + const [jsonRpcMessage, extra] = onMessageArgs; + + + // TODO(bete): Instrument responses/notifications (not sure if they are RPC) + if (isJsonRpcRequest(jsonRpcMessage)) { + return createMcpServerSpan(jsonRpcMessage, transport, extra, () => { + return onMessageTarget.apply(onMessageThisArg, onMessageArgs); + }); + } + + return onMessageTarget.apply(onMessageThisArg, onMessageArgs); } - }; - } - - if (!transport.onmessage) { - transport.onmessage = jsonRpcMessage => { - if (transport.sessionId && isJsonRPCMessageWithRequestId(jsonRpcMessage)) { - handleTransportOnMessage(transport.sessionId, jsonRpcMessage.id); + }); + } + + // Handle transport lifecycle events + if (transport.onclose) { + const originalOnClose = transport.onclose; + transport.onclose = new Proxy(originalOnClose, { + apply(onCloseTarget, onCloseThisArg, onCloseArgs) { + if (transport.sessionId) { + handleTransportOnClose(transport.sessionId); + } + return onCloseTarget.apply(onCloseThisArg, onCloseArgs); } - }; + }); } - - const patchedTransport = new Proxy(transport, { - set(target, key, value) { - if (key === 'onmessage') { - target[key] = new Proxy(value, { - apply(onMessageTarget, onMessageThisArg, onMessageArgArray) { - const [jsonRpcMessage] = onMessageArgArray; - if (transport.sessionId && isJsonRPCMessageWithRequestId(jsonRpcMessage)) { - handleTransportOnMessage(transport.sessionId, jsonRpcMessage.id); - } - return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgArray); - }, - }); - } else if (key === 'onclose') { - target[key] = new Proxy(value, { - apply(onCloseTarget, onCloseThisArg, onCloseArgArray) { - if (transport.sessionId) { - handleTransportOnClose(transport.sessionId); - } - return Reflect.apply(onCloseTarget, onCloseThisArg, onCloseArgArray); - }, - }); - } else { - target[key as keyof MCPTransport] = value; - } - return true; - }, - }); - - return Reflect.apply(target, thisArg, [patchedTransport, ...restArgs]); + return result; }, }); - mcpServerInstance.resource = new Proxy(mcpServerInstance.resource, { - apply(target, thisArg, argArray) { - const resourceName: unknown = argArray[0]; - const resourceHandler: unknown = argArray[argArray.length - 1]; - - if (typeof resourceName !== 'string' || typeof resourceHandler !== 'function') { - return target.apply(thisArg, argArray); - } + wrappedMcpServerInstances.add(mcpServerInstance); + return mcpServerInstance as S; +} - const wrappedResourceHandler = new Proxy(resourceHandler, { - apply(resourceHandlerTarget, resourceHandlerThisArg, resourceHandlerArgArray) { - const extraHandlerDataWithRequestId = resourceHandlerArgArray.find(isExtraHandlerDataWithRequestId); - return associateContextWithRequestSpan(extraHandlerDataWithRequestId, () => { - return startSpan( - { - name: `mcp-server/resource:${resourceName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.resource': resourceName, - }, - }, - () => resourceHandlerTarget.apply(resourceHandlerThisArg, resourceHandlerArgArray), - ); - }); - }, - }); - - return Reflect.apply(target, thisArg, [...argArray.slice(0, -1), wrappedResourceHandler]); - }, +function createMcpServerSpan( + jsonRpcMessage: JsonRpcRequest, + transport: MCPTransport, + extra: any, + callback: () => any +) { + const { method, id: requestId, params } = jsonRpcMessage; + + // Extract target from method and params for proper description + const target = extractTarget(method, params); + const description = target ? `${method} ${target}` : method; + + // Session ID should come from the transport itself, not the RPC message + const sessionId = transport.sessionId; + + // Extract client information from extra/request data + const clientAddress = extra?.requestInfo?.remoteAddress || + extra?.clientAddress || + extra?.request?.ip || + extra?.request?.connection?.remoteAddress; + const clientPort = extra?.requestInfo?.remotePort || + extra?.clientPort || + extra?.request?.connection?.remotePort; + + // Determine transport types + const { mcpTransport, networkTransport } = getTransportTypes(transport); + + const attributes: Record = { + 'mcp.method.name': method, + + ...(requestId !== undefined && { 'mcp.request.id': String(requestId) }), + ...(target && getTargetAttributes(method, target)), + ...(sessionId && { 'mcp.session.id': sessionId }), + ...(clientAddress && { 'client.address': clientAddress }), + ...(clientPort && { 'client.port': clientPort }), + 'mcp.transport': mcpTransport, // Application level: "http", "sse", "stdio", "websocket" + 'network.transport': networkTransport, // Network level: "tcp", "pipe", "udp", "quic" + 'network.protocol.version': '2.0', // JSON-RPC version + + // Opt-in: Tool arguments (if enabled) + ...getRequestArguments(method, params), + + // Sentry-specific attributes + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'mcp.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp_server', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' + }; + + return startSpan({ + name: description, + forceTransaction: true, + attributes + }, () => { + return callback(); }); +} - mcpServerInstance.tool = new Proxy(mcpServerInstance.tool, { - apply(target, thisArg, argArray) { - const toolName: unknown = argArray[0]; - const toolHandler: unknown = argArray[argArray.length - 1]; - - if (typeof toolName !== 'string' || typeof toolHandler !== 'function') { - return target.apply(thisArg, argArray); - } - - const wrappedToolHandler = new Proxy(toolHandler, { - apply(toolHandlerTarget, toolHandlerThisArg, toolHandlerArgArray) { - const extraHandlerDataWithRequestId = toolHandlerArgArray.find(isExtraHandlerDataWithRequestId); - return associateContextWithRequestSpan(extraHandlerDataWithRequestId, () => { - return startSpan( - { - name: `mcp-server/tool:${toolName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.tool': toolName, - }, - }, - () => toolHandlerTarget.apply(toolHandlerThisArg, toolHandlerArgArray), - ); - }); - }, - }); - - return Reflect.apply(target, thisArg, [...argArray.slice(0, -1), wrappedToolHandler]); - }, - }); +function extractTarget(method: string, params: any): string | undefined { + switch (method) { + case 'tools/call': + return params?.name; // Tool name + case 'resources/read': + case 'resources/subscribe': + case 'resources/unsubscribe': + return params?.uri; // Resource URI + case 'prompts/get': + return params?.name; // Prompt name + default: + return undefined; + } +} - mcpServerInstance.prompt = new Proxy(mcpServerInstance.prompt, { - apply(target, thisArg, argArray) { - const promptName: unknown = argArray[0]; - const promptHandler: unknown = argArray[argArray.length - 1]; +function getTargetAttributes(method: string, target: string): Record { + switch (method) { + case 'tools/call': + return { 'mcp.tool.name': target }; + case 'resources/read': + case 'resources/subscribe': + case 'resources/unsubscribe': + return { 'mcp.resource.uri': target }; + case 'prompts/get': + return { 'mcp.prompt.name': target }; + default: + return {}; + } +} - if (typeof promptName !== 'string' || typeof promptHandler !== 'function') { - return target.apply(thisArg, argArray); +function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { + // Try to determine transport type from transport properties/constructor + const transportName = transport.constructor?.name?.toLowerCase() || ''; + + if (transportName.includes('sse')) { + return { mcpTransport: 'sse', networkTransport: 'tcp' }; + } + if (transportName.includes('http')) { + return { mcpTransport: 'http', networkTransport: 'tcp' }; + } + if (transportName.includes('websocket') || transportName.includes('ws')) { + return { mcpTransport: 'websocket', networkTransport: 'tcp' }; + } + if (transportName.includes('stdio')) { + return { mcpTransport: 'stdio', networkTransport: 'pipe' }; + } + + // Default assumption based on your setup (HTTP server) + return { mcpTransport: 'http', networkTransport: 'tcp' }; +} +function getRequestArguments(method: string, params: any): Record { + const args: Record = {}; + + // Only include arguments for certain methods (security consideration) + switch (method) { + case 'tools/call': + if (params?.arguments) { + // Convert arguments to JSON strings as per MCP conventions + for (const [key, value] of Object.entries(params.arguments)) { + args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); + } } + break; + case 'resources/read': + if (params?.uri) { + args['mcp.request.argument.uri'] = JSON.stringify(params.uri); + } + break; + case 'prompts/get': + if (params?.name) { + args['mcp.request.argument.name'] = JSON.stringify(params.name); + } + if (params?.arguments) { + for (const [key, value] of Object.entries(params.arguments)) { + args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); + } + } + break; + } + + return args; +} - const wrappedPromptHandler = new Proxy(promptHandler, { - apply(promptHandlerTarget, promptHandlerThisArg, promptHandlerArgArray) { - const extraHandlerDataWithRequestId = promptHandlerArgArray.find(isExtraHandlerDataWithRequestId); - return associateContextWithRequestSpan(extraHandlerDataWithRequestId, () => { - return startSpan( - { - name: `mcp-server/prompt:${promptName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.prompt': promptName, - }, - }, - () => promptHandlerTarget.apply(promptHandlerThisArg, promptHandlerArgArray), - ); - }); - }, - }); - - return Reflect.apply(target, thisArg, [...argArray.slice(0, -1), wrappedPromptHandler]); - }, - }); - - wrappedMcpServerInstances.add(mcpServerInstance); +function isJsonRpcRequest(message: any): message is JsonRpcRequest { + const isRequest = ( + typeof message === 'object' && + message !== null && + message.jsonrpc === '2.0' && + 'method' in message && + 'id' in message + ); + + return isRequest; +} - return mcpServerInstance as S; +interface JsonRpcRequest { + jsonrpc: '2.0'; + method: string; + id: string | number; + params?: any; } function isMcpServerInstance(mcpServerInstance: unknown): mcpServerInstance is MCPServerInstance { @@ -216,35 +268,21 @@ function isMcpServerInstance(mcpServerInstance: unknown): mcpServerInstance is M 'prompt' in mcpServerInstance && typeof mcpServerInstance.prompt === 'function' && 'connect' in mcpServerInstance && - typeof mcpServerInstance.connect === 'function' + typeof mcpServerInstance.connect === 'function' && + 'server' in mcpServerInstance && + typeof mcpServerInstance.server === 'object' && + mcpServerInstance.server !== null && + 'setRequestHandler' in mcpServerInstance.server && + typeof mcpServerInstance.server.setRequestHandler === 'function' ); } -function isJsonRPCMessageWithRequestId(target: unknown): target is { id: RequestId } { - return ( - typeof target === 'object' && - target !== null && - 'id' in target && - (typeof target.id === 'number' || typeof target.id === 'string') - ); -} interface ExtraHandlerDataWithRequestId { sessionId: SessionId; requestId: RequestId; } -// Note that not all versions of the MCP library have `requestId` as a field on the extra data. -function isExtraHandlerDataWithRequestId(target: unknown): target is ExtraHandlerDataWithRequestId { - return ( - typeof target === 'object' && - target !== null && - 'sessionId' in target && - typeof target.sessionId === 'string' && - 'requestId' in target && - (typeof target.requestId === 'number' || typeof target.requestId === 'string') - ); -} type SessionId = string; type RequestId = string | number; @@ -255,6 +293,7 @@ function handleTransportOnClose(sessionId: SessionId): void { sessionAndRequestToRequestParentSpanMap.delete(sessionId); } +// TODO(bete): refactor this and associateContextWithRequestSpan to use the new span API. function handleTransportOnMessage(sessionId: SessionId, requestId: RequestId): void { const activeSpan = getActiveSpan(); if (activeSpan) { From cb28cccccbae9636587bda052d4fe3762e7d4340 Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 1 Jul 2025 21:26:54 +0200 Subject: [PATCH 02/22] test transport layer --- packages/core/test/lib/mcp-server.test.ts | 330 ++++++++++------------ 1 file changed, 149 insertions(+), 181 deletions(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 12e85f9f370e..cae801832a53 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -1,10 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapMcpServerWithSentry } from '../../src/mcp-server'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '../../src/semanticAttributes'; import * as tracingModule from '../../src/tracing'; vi.mock('../../src/tracing'); @@ -16,37 +11,18 @@ describe('wrapMcpServerWithSentry', () => { vi.mocked(tracingModule.startSpan).mockImplementation((_, cb) => cb()); }); - it('should wrap valid MCP server instance methods with Sentry spans', () => { - // Create a mock MCP server instance - const mockResource = vi.fn(); - const mockTool = vi.fn(); - const mockPrompt = vi.fn(); - - const mockMcpServer = { - resource: mockResource, - tool: mockTool, - prompt: mockPrompt, - connect: vi.fn(), - }; - - // Wrap the MCP server + it('should return the same instance (modified) if it is a valid MCP server instance', () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - // Verify it returns the same instance (modified) expect(wrappedMcpServer).toBe(mockMcpServer); - - // Original methods should be wrapped - expect(wrappedMcpServer.resource).not.toBe(mockResource); - expect(wrappedMcpServer.tool).not.toBe(mockTool); - expect(wrappedMcpServer.prompt).not.toBe(mockPrompt); }); it('should return the input unchanged if it is not a valid MCP server instance', () => { const invalidMcpServer = { - // Missing required methods resource: () => {}, tool: () => {}, - // No prompt method + // Missing required methods }; const result = wrapMcpServerWithSentry(invalidMcpServer); @@ -61,210 +37,202 @@ describe('wrapMcpServerWithSentry', () => { }); it('should not wrap the same instance twice', () => { - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - }; - - // First wrap + const mockMcpServer = createMockMcpServer(); + const wrappedOnce = wrapMcpServerWithSentry(mockMcpServer); - - // Store references to wrapped methods - const wrappedResource = wrappedOnce.resource; - const wrappedTool = wrappedOnce.tool; - const wrappedPrompt = wrappedOnce.prompt; - - // Second wrap const wrappedTwice = wrapMcpServerWithSentry(wrappedOnce); - // Should be the same instance with the same wrapped methods expect(wrappedTwice).toBe(wrappedOnce); - expect(wrappedTwice.resource).toBe(wrappedResource); - expect(wrappedTwice.tool).toBe(wrappedTool); - expect(wrappedTwice.prompt).toBe(wrappedPrompt); }); - describe('resource method wrapping', () => { - it('should create a span with proper attributes when resource is called', () => { - const mockResourceHandler = vi.fn(); - const resourceName = 'test-resource'; - - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; - + describe('Transport-level instrumentation', () => { + it('should proxy the connect method', () => { + const mockMcpServer = createMockMcpServer(); + const originalConnect = mockMcpServer.connect; + const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - wrappedMcpServer.resource(resourceName, {}, mockResourceHandler); + + expect(wrappedMcpServer.connect).not.toBe(originalConnect); + }); - // The original registration should use a wrapped handler - expect(mockMcpServer.resource).toHaveBeenCalledWith(resourceName, {}, expect.any(Function)); + it('should intercept transport onmessage handler', async () => { + const mockMcpServer = createMockMcpServer(); + const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - // Invoke the wrapped handler to trigger Sentry span - const wrappedResourceHandler = (mockMcpServer.resource as any).mock.calls[0][2]; - wrappedResourceHandler('test-uri', { foo: 'bar' }); + const mockTransport = createMockTransport(); + const originalOnMessage = mockTransport.onmessage; - expect(tracingModule.startSpan).toHaveBeenCalledTimes(1); - expect(tracingModule.startSpan).toHaveBeenCalledWith( - { - name: `mcp-server/resource:${resourceName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.resource': resourceName, - }, - }, - expect.any(Function), - ); + await wrappedMcpServer.connect(mockTransport); - // Verify the original handler was called within the span - expect(mockResourceHandler).toHaveBeenCalledWith('test-uri', { foo: 'bar' }); + // onmessage should be wrapped + expect(mockTransport.onmessage).not.toBe(originalOnMessage); }); - it('should call the original resource method directly if name or handler is not valid', () => { - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; + it('should intercept transport send handler', async () => { + const mockMcpServer = createMockMcpServer(); + const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + + const mockTransport = createMockTransport(); + const originalSend = mockTransport.send; + + await wrappedMcpServer.connect(mockTransport); + // send should be wrapped + expect(mockTransport.send).not.toBe(originalSend); + }); + + it('should intercept transport onclose handler', async () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - // Call without string name - wrappedMcpServer.resource({} as any, 'handler'); + const mockTransport = createMockTransport(); + const originalOnClose = mockTransport.onclose; - // Call without function handler - wrappedMcpServer.resource('name', 'not-a-function'); + await wrappedMcpServer.connect(mockTransport); - // Original method should be called directly without creating spans - expect(mockMcpServer.resource).toHaveBeenCalledTimes(2); - expect(tracingModule.startSpan).not.toHaveBeenCalled(); + // onclose should be wrapped + expect(mockTransport.onclose).not.toBe(originalOnClose); }); - }); - describe('tool method wrapping', () => { - it('should create a span with proper attributes when tool is called', () => { - const mockToolHandler = vi.fn(); - const toolName = 'test-tool'; + it('should call original connect and preserve functionality', async () => { + const mockMcpServer = createMockMcpServer(); + const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; + const mockTransport = createMockTransport(); + + await wrappedMcpServer.connect(mockTransport); + // Original connect should have been called + expect(mockMcpServer.connect).toHaveBeenCalledWith(mockTransport); + }); + + it('should create spans for incoming JSON-RPC requests', async () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - wrappedMcpServer.tool(toolName, {}, mockToolHandler); - // The original registration should use a wrapped handler - expect(mockMcpServer.tool).toHaveBeenCalledWith(toolName, {}, expect.any(Function)); + const mockTransport = createMockTransport(); + await wrappedMcpServer.connect(mockTransport); + + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/call', + id: 'req-1', + params: { name: 'get-weather' } + }; - // Invoke the wrapped handler to trigger Sentry span - const wrappedToolHandler = (mockMcpServer.tool as any).mock.calls[0][2]; - wrappedToolHandler({ arg: 'value' }, { foo: 'baz' }); + // Simulate incoming message + expect(mockTransport.onmessage).toBeDefined(); + mockTransport.onmessage?.(jsonRpcRequest, {}); - expect(tracingModule.startSpan).toHaveBeenCalledTimes(1); expect(tracingModule.startSpan).toHaveBeenCalledWith( - { - name: `mcp-server/tool:${toolName}`, + expect.objectContaining({ + name: 'tools/call get-weather', forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.tool': toolName, - }, - }, - expect.any(Function), + }), + expect.any(Function) ); - - // Verify the original handler was called within the span - expect(mockToolHandler).toHaveBeenCalledWith({ arg: 'value' }, { foo: 'baz' }); }); - it('should call the original tool method directly if name or handler is not valid', () => { - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; - + it('should create spans for incoming JSON-RPC notifications', async () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - // Call without string name - wrappedMcpServer.tool({} as any, 'handler'); + const mockTransport = createMockTransport(); + await wrappedMcpServer.connect(mockTransport); - // Original method should be called directly without creating spans - expect(mockMcpServer.tool).toHaveBeenCalledTimes(1); - expect(tracingModule.startSpan).not.toHaveBeenCalled(); - }); - }); + const jsonRpcNotification = { + jsonrpc: '2.0', + method: 'notifications/initialized', + // No 'id' field - this makes it a notification + }; - describe('prompt method wrapping', () => { - it('should create a span with proper attributes when prompt is called', () => { - const mockPromptHandler = vi.fn(); - const promptName = 'test-prompt'; + // Simulate incoming notification + expect(mockTransport.onmessage).toBeDefined(); + mockTransport.onmessage?.(jsonRpcNotification, {}); - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/initialized', + forceTransaction: true, + }), + expect.any(Function) + ); + }); + it('should create spans for outgoing notifications', async () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - wrappedMcpServer.prompt(promptName, {}, mockPromptHandler); - // The original registration should use a wrapped handler - expect(mockMcpServer.prompt).toHaveBeenCalledWith(promptName, {}, expect.any(Function)); + const mockTransport = createMockTransport(); + await wrappedMcpServer.connect(mockTransport); + + const outgoingNotification = { + jsonrpc: '2.0', + method: 'notifications/tools/list_changed', + // No 'id' field + }; - // Invoke the wrapped handler to trigger Sentry span - const wrappedPromptHandler = (mockMcpServer.prompt as any).mock.calls[0][2]; - wrappedPromptHandler({ msg: 'hello' }, { data: 123 }); + // Simulate outgoing notification + expect(mockTransport.send).toBeDefined(); + await mockTransport.send?.(outgoingNotification); - expect(tracingModule.startSpan).toHaveBeenCalledTimes(1); expect(tracingModule.startSpan).toHaveBeenCalledWith( - { - name: `mcp-server/prompt:${promptName}`, + expect.objectContaining({ + name: 'notifications/tools/list_changed', forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp-server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'mcp_server.prompt': promptName, - }, - }, - expect.any(Function), + }), + expect.any(Function) ); - - // Verify the original handler was called within the span - expect(mockPromptHandler).toHaveBeenCalledWith({ msg: 'hello' }, { data: 123 }); }); - it('should call the original prompt method directly if name or handler is not valid', () => { - const mockMcpServer = { - resource: vi.fn(), - tool: vi.fn(), - prompt: vi.fn(), - connect: vi.fn(), - }; - + it('should not create spans for non-JSON-RPC messages', async () => { + const mockMcpServer = createMockMcpServer(); const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - // Call without function handler - wrappedMcpServer.prompt('name', 'not-a-function'); + const mockTransport = createMockTransport(); + await wrappedMcpServer.connect(mockTransport); + + // Simulate non-JSON-RPC message + expect(mockTransport.onmessage).toBeDefined(); + mockTransport.onmessage?.({ some: 'data' }, {}); - // Original method should be called directly without creating spans - expect(mockMcpServer.prompt).toHaveBeenCalledTimes(1); expect(tracingModule.startSpan).not.toHaveBeenCalled(); }); + + it('should handle transport onclose events', async () => { + const mockMcpServer = createMockMcpServer(); + const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + + const mockTransport = createMockTransport(); + mockTransport.sessionId = 'test-session-123'; + + await wrappedMcpServer.connect(mockTransport); + + // Trigger onclose - should not throw + expect(mockTransport.onclose).toBeDefined(); + expect(() => mockTransport.onclose?.()).not.toThrow(); + }); }); }); + +// Test helpers +function createMockMcpServer() { + return { + resource: vi.fn(), + tool: vi.fn(), + prompt: vi.fn(), + connect: vi.fn().mockResolvedValue(undefined), + server: { + setRequestHandler: vi.fn(), + }, + }; +} + +function createMockTransport() { + return { + onmessage: vi.fn(), + onclose: vi.fn(), + send: vi.fn().mockResolvedValue(undefined), + sessionId: 'test-session-123', + }; +} From 1480b783c89954ce892cbd98483ac7c806d097ce Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 1 Jul 2025 21:33:08 +0200 Subject: [PATCH 03/22] refactor(mcp-server.test): Simplify test setup by using beforeEach for mock instances --- packages/core/test/lib/mcp-server.test.ts | 104 +++++++--------------- 1 file changed, 30 insertions(+), 74 deletions(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index cae801832a53..d2d7e194f16e 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -46,73 +46,53 @@ describe('wrapMcpServerWithSentry', () => { }); describe('Transport-level instrumentation', () => { + let mockMcpServer: ReturnType; + let wrappedMcpServer: ReturnType; + let mockTransport: ReturnType; + + beforeEach(async () => { + mockMcpServer = createMockMcpServer(); + wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + mockTransport = createMockTransport(); + + // Connect the server to transport - this is common to most tests + await wrappedMcpServer.connect(mockTransport); + }); + it('should proxy the connect method', () => { - const mockMcpServer = createMockMcpServer(); - const originalConnect = mockMcpServer.connect; + // We need to test this before connection, so create fresh instances + const freshMockMcpServer = createMockMcpServer(); + const originalConnect = freshMockMcpServer.connect; - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + const freshWrappedMcpServer = wrapMcpServerWithSentry(freshMockMcpServer); - expect(wrappedMcpServer.connect).not.toBe(originalConnect); + expect(freshWrappedMcpServer.connect).not.toBe(originalConnect); }); - it('should intercept transport onmessage handler', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); + it('should intercept transport onmessage handler', () => { const originalOnMessage = mockTransport.onmessage; - - await wrappedMcpServer.connect(mockTransport); - - // onmessage should be wrapped + // onmessage should be wrapped after connection expect(mockTransport.onmessage).not.toBe(originalOnMessage); }); - it('should intercept transport send handler', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); + it('should intercept transport send handler', () => { const originalSend = mockTransport.send; - - await wrappedMcpServer.connect(mockTransport); - - // send should be wrapped + // send should be wrapped after connection expect(mockTransport.send).not.toBe(originalSend); }); - it('should intercept transport onclose handler', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); + it('should intercept transport onclose handler', () => { const originalOnClose = mockTransport.onclose; - - await wrappedMcpServer.connect(mockTransport); - - // onclose should be wrapped + // onclose should be wrapped after connection expect(mockTransport.onclose).not.toBe(originalOnClose); }); - it('should call original connect and preserve functionality', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); - - await wrappedMcpServer.connect(mockTransport); - - // Original connect should have been called + it('should call original connect and preserve functionality', () => { + // Original connect should have been called during beforeEach expect(mockMcpServer.connect).toHaveBeenCalledWith(mockTransport); }); - it('should create spans for incoming JSON-RPC requests', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); - await wrappedMcpServer.connect(mockTransport); - + it('should create spans for incoming JSON-RPC requests', () => { const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/call', @@ -133,13 +113,7 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should create spans for incoming JSON-RPC notifications', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); - await wrappedMcpServer.connect(mockTransport); - + it('should create spans for incoming JSON-RPC notifications', () => { const jsonRpcNotification = { jsonrpc: '2.0', method: 'notifications/initialized', @@ -160,12 +134,6 @@ describe('wrapMcpServerWithSentry', () => { }); it('should create spans for outgoing notifications', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); - await wrappedMcpServer.connect(mockTransport); - const outgoingNotification = { jsonrpc: '2.0', method: 'notifications/tools/list_changed', @@ -185,13 +153,7 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should not create spans for non-JSON-RPC messages', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); - await wrappedMcpServer.connect(mockTransport); - + it('should not create spans for non-JSON-RPC messages', () => { // Simulate non-JSON-RPC message expect(mockTransport.onmessage).toBeDefined(); mockTransport.onmessage?.({ some: 'data' }, {}); @@ -199,15 +161,9 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).not.toHaveBeenCalled(); }); - it('should handle transport onclose events', async () => { - const mockMcpServer = createMockMcpServer(); - const wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); - - const mockTransport = createMockTransport(); + it('should handle transport onclose events', () => { mockTransport.sessionId = 'test-session-123'; - await wrappedMcpServer.connect(mockTransport); - // Trigger onclose - should not throw expect(mockTransport.onclose).toBeDefined(); expect(() => mockTransport.onclose?.()).not.toThrow(); From 5a97d6942864b8fafb6612642b7d2c9a12d522db Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 1 Jul 2025 22:42:16 +0200 Subject: [PATCH 04/22] test(mcp-server): Add tests for span creation and semantic conventions in MCP server --- packages/core/test/lib/mcp-server.test.ts | 184 ++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index d2d7e194f16e..21b12bb02221 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -169,6 +169,190 @@ describe('wrapMcpServerWithSentry', () => { expect(() => mockTransport.onclose?.()).not.toThrow(); }); }); + + describe('Span Creation & Semantic Conventions', () => { + let mockMcpServer: ReturnType; + let wrappedMcpServer: ReturnType; + let mockTransport: ReturnType; + + beforeEach(async () => { + mockMcpServer = createMockMcpServer(); + wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + mockTransport = createMockTransport(); + mockTransport.sessionId = 'test-session-123'; + + await wrappedMcpServer.connect(mockTransport); + }); + + it('should create spans with correct MCP server semantic attributes for tool operations', () => { + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/call', + id: 'req-1', + params: { + name: 'get-weather', + arguments: { + location: 'Seattle, WA', + units: 'metric' + } + } + }; + + const extraWithClientInfo = { + requestInfo: { + remoteAddress: '192.168.1.100', + remotePort: 54321 + } + }; + + mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo); + + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'tools/call get-weather', + op: 'mcp.server', + forceTransaction: true, + attributes: expect.objectContaining({ + // Required + 'mcp.method.name': 'tools/call', + // Conditionally Required (tool operation) + 'mcp.tool.name': 'get-weather', + 'mcp.request.id': 'req-1', + // Recommended + 'mcp.session.id': 'test-session-123', + 'client.address': '192.168.1.100', + 'client.port': 54321, + // Transport attributes + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', + // Tool arguments (JSON-stringified) + 'mcp.request.argument.location': '"Seattle, WA"', + 'mcp.request.argument.units': '"metric"', + // Sentry-specific + 'sentry.origin': 'auto.function.mcp_server', + }), + }), + expect.any(Function) + ); + }); + + it('should create spans with correct attributes for resource operations', () => { + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'resources/read', + id: 'req-2', + params: { uri: 'file:///docs/api.md' } + }; + + mockTransport.onmessage?.(jsonRpcRequest, {}); + + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'resources/read file:///docs/api.md', + op: 'mcp.server', + attributes: expect.objectContaining({ + // Required + 'mcp.method.name': 'resources/read', + // Conditionally Required (resource operation) + 'mcp.resource.uri': 'file:///docs/api.md', + 'mcp.request.id': 'req-2', + // Recommended + 'mcp.session.id': 'test-session-123', + }), + }), + expect.any(Function) + ); + }); + + it('should create spans with correct attributes for prompt operations', () => { + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'prompts/get', + id: 'req-3', + params: { name: 'analyze-code' } + }; + + mockTransport.onmessage?.(jsonRpcRequest, {}); + + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'prompts/get analyze-code', + op: 'mcp.server', + attributes: expect.objectContaining({ + // Required + 'mcp.method.name': 'prompts/get', + // Conditionally Required (prompt operation) + 'mcp.prompt.name': 'analyze-code', + 'mcp.request.id': 'req-3', + // Recommended + 'mcp.session.id': 'test-session-123', + }), + }), + expect.any(Function) + ); + }); + + it('should create spans with correct attributes for notifications (no request id)', () => { + const jsonRpcNotification = { + jsonrpc: '2.0', + method: 'notifications/tools/list_changed', + params: {} + }; + + mockTransport.onmessage?.(jsonRpcNotification, {}); + + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/tools/list_changed', + op: 'mcp.server', + attributes: expect.objectContaining({ + // Required + 'mcp.method.name': 'notifications/tools/list_changed', + // Recommended + 'mcp.session.id': 'test-session-123', + // Notification-specific + 'mcp.notification.direction': 'client_to_server', + // Sentry-specific + 'sentry.origin': 'auto.mcp.notification', + }), + }), + expect.any(Function) + ); + + // Should not include mcp.request.id for notifications + const callArgs = vi.mocked(tracingModule.startSpan).mock.calls[0]; + expect(callArgs).toBeDefined(); + const attributes = callArgs?.[0]?.attributes; + expect(attributes).not.toHaveProperty('mcp.request.id'); + }); + + it('should create spans for list operations without target in name', () => { + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/list', + id: 'req-4', + params: {} + }; + + mockTransport.onmessage?.(jsonRpcRequest, {}); + + expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'tools/list', + op: 'mcp.server', + attributes: expect.objectContaining({ + 'mcp.method.name': 'tools/list', + 'mcp.request.id': 'req-4', + 'mcp.session.id': 'test-session-123', + }), + }), + expect.any(Function) + ); + }); + + + }); }); // Test helpers From 03077f8fd0ac334562e0e00de794e6da1c1aa8ca Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 09:46:38 +0200 Subject: [PATCH 05/22] test(mcp-server): Update tests to control transport connection in individual cases --- packages/core/test/lib/mcp-server.test.ts | 74 +++++++++++++++-------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 21b12bb02221..03a77367e221 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -50,13 +50,11 @@ describe('wrapMcpServerWithSentry', () => { let wrappedMcpServer: ReturnType; let mockTransport: ReturnType; - beforeEach(async () => { + beforeEach(() => { mockMcpServer = createMockMcpServer(); wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); mockTransport = createMockTransport(); - - // Connect the server to transport - this is common to most tests - await wrappedMcpServer.connect(mockTransport); + // Don't connect transport here. let individual tests control when connection happens }); it('should proxy the connect method', () => { @@ -69,30 +67,43 @@ describe('wrapMcpServerWithSentry', () => { expect(freshWrappedMcpServer.connect).not.toBe(originalConnect); }); - it('should intercept transport onmessage handler', () => { + it('should intercept transport onmessage handler', async () => { const originalOnMessage = mockTransport.onmessage; + + await wrappedMcpServer.connect(mockTransport); + // onmessage should be wrapped after connection expect(mockTransport.onmessage).not.toBe(originalOnMessage); }); - it('should intercept transport send handler', () => { + it('should intercept transport send handler', async () => { const originalSend = mockTransport.send; + + await wrappedMcpServer.connect(mockTransport); + // send should be wrapped after connection expect(mockTransport.send).not.toBe(originalSend); }); - it('should intercept transport onclose handler', () => { + it('should intercept transport onclose handler', async () => { const originalOnClose = mockTransport.onclose; + + await wrappedMcpServer.connect(mockTransport); + // onclose should be wrapped after connection expect(mockTransport.onclose).not.toBe(originalOnClose); }); - it('should call original connect and preserve functionality', () => { - // Original connect should have been called during beforeEach + it('should call original connect and preserve functionality', async () => { + await wrappedMcpServer.connect(mockTransport); + + // Original connect should have been called expect(mockMcpServer.connect).toHaveBeenCalledWith(mockTransport); }); - it('should create spans for incoming JSON-RPC requests', () => { + it('should create spans for incoming JSON-RPC requests', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/call', @@ -101,7 +112,6 @@ describe('wrapMcpServerWithSentry', () => { }; // Simulate incoming message - expect(mockTransport.onmessage).toBeDefined(); mockTransport.onmessage?.(jsonRpcRequest, {}); expect(tracingModule.startSpan).toHaveBeenCalledWith( @@ -113,7 +123,9 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should create spans for incoming JSON-RPC notifications', () => { + it('should create spans for incoming JSON-RPC notifications', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcNotification = { jsonrpc: '2.0', method: 'notifications/initialized', @@ -121,7 +133,6 @@ describe('wrapMcpServerWithSentry', () => { }; // Simulate incoming notification - expect(mockTransport.onmessage).toBeDefined(); mockTransport.onmessage?.(jsonRpcNotification, {}); expect(tracingModule.startSpan).toHaveBeenCalledWith( @@ -134,6 +145,8 @@ describe('wrapMcpServerWithSentry', () => { }); it('should create spans for outgoing notifications', async () => { + await wrappedMcpServer.connect(mockTransport); + const outgoingNotification = { jsonrpc: '2.0', method: 'notifications/tools/list_changed', @@ -141,7 +154,6 @@ describe('wrapMcpServerWithSentry', () => { }; // Simulate outgoing notification - expect(mockTransport.send).toBeDefined(); await mockTransport.send?.(outgoingNotification); expect(tracingModule.startSpan).toHaveBeenCalledWith( @@ -153,19 +165,20 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should not create spans for non-JSON-RPC messages', () => { + it('should not create spans for non-JSON-RPC messages', async () => { + await wrappedMcpServer.connect(mockTransport); + // Simulate non-JSON-RPC message - expect(mockTransport.onmessage).toBeDefined(); mockTransport.onmessage?.({ some: 'data' }, {}); expect(tracingModule.startSpan).not.toHaveBeenCalled(); }); - it('should handle transport onclose events', () => { + it('should handle transport onclose events', async () => { + await wrappedMcpServer.connect(mockTransport); mockTransport.sessionId = 'test-session-123'; // Trigger onclose - should not throw - expect(mockTransport.onclose).toBeDefined(); expect(() => mockTransport.onclose?.()).not.toThrow(); }); }); @@ -175,16 +188,17 @@ describe('wrapMcpServerWithSentry', () => { let wrappedMcpServer: ReturnType; let mockTransport: ReturnType; - beforeEach(async () => { + beforeEach(() => { mockMcpServer = createMockMcpServer(); wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); mockTransport = createMockTransport(); mockTransport.sessionId = 'test-session-123'; - - await wrappedMcpServer.connect(mockTransport); + // Don't connect here - let individual tests control when connection happens }); - it('should create spans with correct MCP server semantic attributes for tool operations', () => { + it('should create spans with correct MCP server semantic attributes for tool operations', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/call', @@ -237,7 +251,9 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should create spans with correct attributes for resource operations', () => { + it('should create spans with correct attributes for resource operations', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcRequest = { jsonrpc: '2.0', method: 'resources/read', @@ -265,7 +281,9 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should create spans with correct attributes for prompt operations', () => { + it('should create spans with correct attributes for prompt operations', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcRequest = { jsonrpc: '2.0', method: 'prompts/get', @@ -293,7 +311,9 @@ describe('wrapMcpServerWithSentry', () => { ); }); - it('should create spans with correct attributes for notifications (no request id)', () => { + it('should create spans with correct attributes for notifications (no request id)', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcNotification = { jsonrpc: '2.0', method: 'notifications/tools/list_changed', @@ -327,7 +347,9 @@ describe('wrapMcpServerWithSentry', () => { expect(attributes).not.toHaveProperty('mcp.request.id'); }); - it('should create spans for list operations without target in name', () => { + it('should create spans for list operations without target in name', async () => { + await wrappedMcpServer.connect(mockTransport); + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/list', From cac9bd05fa6b9e33f04f83d27c8260d294585e66 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 10:40:00 +0200 Subject: [PATCH 06/22] test(mcp-server): Refine span attributes and transport details --- packages/core/test/lib/mcp-server.test.ts | 45 ++++++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 03a77367e221..1c0e7316a2ec 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -224,7 +224,6 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/call get-weather', - op: 'mcp.server', forceTransaction: true, attributes: expect.objectContaining({ // Required @@ -244,7 +243,9 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.request.argument.location': '"Seattle, WA"', 'mcp.request.argument.units': '"metric"', // Sentry-specific + 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.function.mcp_server', + 'sentry.source': 'route', }), }), expect.any(Function) @@ -266,7 +267,7 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).toHaveBeenCalledWith( expect.objectContaining({ name: 'resources/read file:///docs/api.md', - op: 'mcp.server', + forceTransaction: true, attributes: expect.objectContaining({ // Required 'mcp.method.name': 'resources/read', @@ -275,6 +276,16 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.request.id': 'req-2', // Recommended 'mcp.session.id': 'test-session-123', + // Transport attributes + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', + // Request arguments (JSON-stringified) + 'mcp.request.argument.uri': '"file:///docs/api.md"', + // Sentry-specific + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.function.mcp_server', + 'sentry.source': 'route', }), }), expect.any(Function) @@ -296,7 +307,7 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).toHaveBeenCalledWith( expect.objectContaining({ name: 'prompts/get analyze-code', - op: 'mcp.server', + forceTransaction: true, attributes: expect.objectContaining({ // Required 'mcp.method.name': 'prompts/get', @@ -305,6 +316,16 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.request.id': 'req-3', // Recommended 'mcp.session.id': 'test-session-123', + // Transport attributes + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', + // Request arguments (JSON-stringified) + 'mcp.request.argument.name': '"analyze-code"', + // Sentry-specific + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.function.mcp_server', + 'sentry.source': 'route', }), }), expect.any(Function) @@ -325,7 +346,7 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).toHaveBeenCalledWith( expect.objectContaining({ name: 'notifications/tools/list_changed', - op: 'mcp.server', + forceTransaction: true, attributes: expect.objectContaining({ // Required 'mcp.method.name': 'notifications/tools/list_changed', @@ -333,8 +354,14 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.session.id': 'test-session-123', // Notification-specific 'mcp.notification.direction': 'client_to_server', + // Transport attributes + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', // Sentry-specific + 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', }), }), expect.any(Function) @@ -362,11 +389,19 @@ describe('wrapMcpServerWithSentry', () => { expect(tracingModule.startSpan).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/list', - op: 'mcp.server', + forceTransaction: true, attributes: expect.objectContaining({ 'mcp.method.name': 'tools/list', 'mcp.request.id': 'req-4', 'mcp.session.id': 'test-session-123', + // Transport attributes + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', + // Sentry-specific + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.function.mcp_server', + 'sentry.source': 'route', }), }), expect.any(Function) From 37ef9a920282c18071e0e1493a1b4ddd7268a738 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 11:17:52 +0200 Subject: [PATCH 07/22] test(mcp-server): Replace direct tracing module calls with spies for improved test isolation --- packages/core/test/lib/mcp-server.test.ts | 88 ++++++++--------------- 1 file changed, 28 insertions(+), 60 deletions(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 1c0e7316a2ec..49c5eb41c692 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -2,13 +2,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapMcpServerWithSentry } from '../../src/mcp-server'; import * as tracingModule from '../../src/tracing'; -vi.mock('../../src/tracing'); - describe('wrapMcpServerWithSentry', () => { + const startSpanSpy = vi.spyOn(tracingModule, 'startSpan'); + beforeEach(() => { vi.clearAllMocks(); - // @ts-expect-error mocking span is annoying - vi.mocked(tracingModule.startSpan).mockImplementation((_, cb) => cb()); }); it('should return the same instance (modified) if it is a valid MCP server instance', () => { @@ -33,7 +31,7 @@ describe('wrapMcpServerWithSentry', () => { expect(result.tool).toBe(invalidMcpServer.tool); // No calls to startSpan - expect(tracingModule.startSpan).not.toHaveBeenCalled(); + expect(startSpanSpy).not.toHaveBeenCalled(); }); it('should not wrap the same instance twice', () => { @@ -114,7 +112,7 @@ describe('wrapMcpServerWithSentry', () => { // Simulate incoming message mockTransport.onmessage?.(jsonRpcRequest, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/call get-weather', forceTransaction: true, @@ -135,7 +133,7 @@ describe('wrapMcpServerWithSentry', () => { // Simulate incoming notification mockTransport.onmessage?.(jsonRpcNotification, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'notifications/initialized', forceTransaction: true, @@ -156,7 +154,7 @@ describe('wrapMcpServerWithSentry', () => { // Simulate outgoing notification await mockTransport.send?.(outgoingNotification); - expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'notifications/tools/list_changed', forceTransaction: true, @@ -171,7 +169,7 @@ describe('wrapMcpServerWithSentry', () => { // Simulate non-JSON-RPC message mockTransport.onmessage?.({ some: 'data' }, {}); - expect(tracingModule.startSpan).not.toHaveBeenCalled(); + expect(startSpanSpy).not.toHaveBeenCalled(); }); it('should handle transport onclose events', async () => { @@ -203,13 +201,7 @@ describe('wrapMcpServerWithSentry', () => { jsonrpc: '2.0', method: 'tools/call', id: 'req-1', - params: { - name: 'get-weather', - arguments: { - location: 'Seattle, WA', - units: 'metric' - } - } + params: { name: 'get-weather', arguments: { location: 'Seattle, WA' }} }; const extraWithClientInfo = { @@ -221,33 +213,26 @@ describe('wrapMcpServerWithSentry', () => { mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo); - expect(tracingModule.startSpan).toHaveBeenCalledWith( - expect.objectContaining({ + expect(startSpanSpy).toHaveBeenCalledWith( + { name: 'tools/call get-weather', forceTransaction: true, - attributes: expect.objectContaining({ - // Required + attributes: { 'mcp.method.name': 'tools/call', - // Conditionally Required (tool operation) 'mcp.tool.name': 'get-weather', 'mcp.request.id': 'req-1', - // Recommended 'mcp.session.id': 'test-session-123', 'client.address': '192.168.1.100', 'client.port': 54321, - // Transport attributes 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', - // Tool arguments (JSON-stringified) 'mcp.request.argument.location': '"Seattle, WA"', - 'mcp.request.argument.units': '"metric"', - // Sentry-specific 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.function.mcp_server', 'sentry.source': 'route', - }), - }), + }, + }, expect.any(Function) ); }); @@ -264,30 +249,24 @@ describe('wrapMcpServerWithSentry', () => { mockTransport.onmessage?.(jsonRpcRequest, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( - expect.objectContaining({ + expect(startSpanSpy).toHaveBeenCalledWith( + { name: 'resources/read file:///docs/api.md', forceTransaction: true, - attributes: expect.objectContaining({ - // Required + attributes: { 'mcp.method.name': 'resources/read', - // Conditionally Required (resource operation) 'mcp.resource.uri': 'file:///docs/api.md', 'mcp.request.id': 'req-2', - // Recommended 'mcp.session.id': 'test-session-123', - // Transport attributes 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', - // Request arguments (JSON-stringified) 'mcp.request.argument.uri': '"file:///docs/api.md"', - // Sentry-specific 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.function.mcp_server', 'sentry.source': 'route', - }), - }), + }, + }, expect.any(Function) ); }); @@ -304,30 +283,24 @@ describe('wrapMcpServerWithSentry', () => { mockTransport.onmessage?.(jsonRpcRequest, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( - expect.objectContaining({ + expect(startSpanSpy).toHaveBeenCalledWith( + { name: 'prompts/get analyze-code', forceTransaction: true, - attributes: expect.objectContaining({ - // Required + attributes: { 'mcp.method.name': 'prompts/get', - // Conditionally Required (prompt operation) 'mcp.prompt.name': 'analyze-code', 'mcp.request.id': 'req-3', - // Recommended 'mcp.session.id': 'test-session-123', - // Transport attributes 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', - // Request arguments (JSON-stringified) 'mcp.request.argument.name': '"analyze-code"', - // Sentry-specific 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.function.mcp_server', 'sentry.source': 'route', - }), - }), + }, + }, expect.any(Function) ); }); @@ -343,27 +316,22 @@ describe('wrapMcpServerWithSentry', () => { mockTransport.onmessage?.(jsonRpcNotification, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( - expect.objectContaining({ + expect(startSpanSpy).toHaveBeenCalledWith( + { name: 'notifications/tools/list_changed', forceTransaction: true, - attributes: expect.objectContaining({ - // Required + attributes: { 'mcp.method.name': 'notifications/tools/list_changed', - // Recommended 'mcp.session.id': 'test-session-123', - // Notification-specific 'mcp.notification.direction': 'client_to_server', - // Transport attributes 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', - // Sentry-specific 'sentry.op': 'mcp.server', 'sentry.origin': 'auto.mcp.notification', 'sentry.source': 'route', - }), - }), + }, + }, expect.any(Function) ); From ac015ce98258f0d68c05c731e922578e30bd60ad Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 13:10:23 +0200 Subject: [PATCH 08/22] feat(mcp-server): Add TypeScript type definitions for MCP server instrumentation --- packages/core/src/utils/mcp-server/types.ts | 475 ++++++++++++++++++++ 1 file changed, 475 insertions(+) create mode 100644 packages/core/src/utils/mcp-server/types.ts diff --git a/packages/core/src/utils/mcp-server/types.ts b/packages/core/src/utils/mcp-server/types.ts new file mode 100644 index 000000000000..1ec71be1cced --- /dev/null +++ b/packages/core/src/utils/mcp-server/types.ts @@ -0,0 +1,475 @@ +/** + * TypeScript type definitions for MCP server instrumentation + */ + +// ============================================================================= +// JSON-RPC TYPES +// ============================================================================= + +/** + * JSON-RPC 2.0 request object + */ +export interface JsonRpcRequest { + jsonrpc: '2.0'; + method: string; + id: string | number; + params?: Record; +} + +/** + * JSON-RPC 2.0 notification object + * Note: Notifications do NOT have an 'id' field - this is what distinguishes them from requests + */ +export interface JsonRpcNotification { + jsonrpc: '2.0'; + method: string; + params?: Record; +} + +/** + * JSON-RPC 2.0 response object + */ +export interface JsonRpcResponse { + jsonrpc: '2.0'; + id: string | number | null; + result?: unknown; + error?: JsonRpcError; +} + +/** + * JSON-RPC 2.0 error object + */ +export interface JsonRpcError { + code: number; + message: string; + data?: unknown; +} + +/** + * Union type for all JSON-RPC message types + */ +export type JsonRpcMessage = JsonRpcRequest | JsonRpcNotification | JsonRpcResponse; + +// ============================================================================= +// MCP TRANSPORT TYPES +// ============================================================================= + +/** + * MCP transport interface + */ +export interface MCPTransport { + /** + * Message handler for incoming JSON-RPC messages + * The first argument is a JSON RPC message + */ + onmessage?: (...args: unknown[]) => void; + + /** + * Close handler for transport lifecycle + */ + onclose?: (...args: unknown[]) => void; + + /** + * Send method for outgoing messages + */ + send?: (message: JsonRpcMessage, options?: Record) => Promise; + + /** + * Optional session identifier + */ + sessionId?: string; +} + +/** + * MCP server instance interface + */ +export interface MCPServerInstance { + /** + * Register a resource handler + * The first arg is always a name, the last arg should always be a callback function (ie a handler). + */ + resource: (name: string, ...args: unknown[]) => void; + + /** + * Register a tool handler + * The first arg is always a name, the last arg should always be a callback function (ie a handler). + */ + tool: (name: string, ...args: unknown[]) => void; + + /** + * Register a prompt handler + * The first arg is always a name, the last arg should always be a callback function (ie a handler). + */ + prompt: (name: string, ...args: unknown[]) => void; + + /** + * Connect the server to a transport + */ + connect(transport: MCPTransport): Promise; + + /** + * Optional server configuration + */ + server?: { + setRequestHandler: (schema: unknown, handler: (...args: unknown[]) => unknown) => void; + }; +} + +// ============================================================================= +// SPAN AND ATTRIBUTE TYPES +// ============================================================================= + +/** + * Span attributes for MCP instrumentation + */ +export interface McpSpanAttributes { + // Core MCP attributes + 'mcp.method.name': string; + 'mcp.request.id'?: string; + 'mcp.session.id'?: string; + 'mcp.transport': string; + + // Method-specific attributes + 'mcp.tool.name'?: string; + 'mcp.resource.uri'?: string; + 'mcp.resource.name'?: string; + 'mcp.prompt.name'?: string; + 'mcp.resource.protocol'?: string; + + // Notification attributes + 'mcp.notification.direction'?: 'client_to_server' | 'server_to_client'; + 'mcp.cancelled.request_id'?: string; + 'mcp.cancelled.reason'?: string; + 'mcp.progress.token'?: string; + 'mcp.progress.current'?: number; + 'mcp.progress.total'?: number; + 'mcp.progress.percentage'?: number; + 'mcp.progress.message'?: string; + 'mcp.logging.level'?: string; + 'mcp.logging.logger'?: string; + 'mcp.logging.data_type'?: string; + 'mcp.logging.message'?: string; + + // Network attributes + 'network.transport': string; + 'network.protocol.version'?: string; + 'client.address'?: string; + 'client.port'?: number; + + // Error attributes + 'error.type'?: string; + 'rpc.jsonrpc.error_code'?: number; + + // Request arguments (dynamic keys) + [key: `mcp.request.argument.${string}`]: string; +} + +/** + * Transport types for MCP + */ +export type McpTransportType = 'stdio' | 'sse' | 'http' | 'websocket'; + +/** + * Network transport types + */ +export type NetworkTransportType = 'pipe' | 'tcp' | 'udp' | 'quic' | 'unix'; + +/** + * Transport type mapping result + */ +export interface TransportTypesResult { + mcpTransport: McpTransportType; + networkTransport: NetworkTransportType; +} + +// ============================================================================= +// SESSION AND REQUEST CORRELATION TYPES +// ============================================================================= + +/** + * Session identifier type + */ +export type SessionId = string; + +/** + * Request identifier type + */ +export type RequestId = string | number; + +/** + * Extra handler data with request correlation information + */ +export interface ExtraHandlerDataWithRequestId { + sessionId: SessionId; + requestId: RequestId; +} + +/** + * Extra data passed to message handlers + */ +export interface ExtraHandlerData { + requestInfo?: { + remoteAddress?: string; + remotePort?: number; + }; + clientAddress?: string; + clientPort?: number; + request?: { + ip?: string; + connection?: { + remoteAddress?: string; + remotePort?: number; + }; + }; + sessionId?: SessionId; + requestId?: RequestId; +} + +// ============================================================================= +// MCP METHOD PARAMETER TYPES +// ============================================================================= + +/** + * Parameters for tools/call method + */ +export interface ToolCallParams { + name: string; + arguments?: Record; +} + +/** + * Parameters for resources/read method + */ +export interface ResourceReadParams { + uri: string; +} + +/** + * Parameters for resources/subscribe method + */ +export interface ResourceSubscribeParams { + uri: string; +} + +/** + * Parameters for resources/unsubscribe method + */ +export interface ResourceUnsubscribeParams { + uri: string; +} + +/** + * Parameters for prompts/get method + */ +export interface PromptGetParams { + name: string; + arguments?: Record; +} + +/** + * Parameters for notifications/cancelled + */ +export interface NotificationCancelledParams { + requestId: RequestId; + reason?: string; +} + +/** + * Parameters for notifications/progress + */ +export interface NotificationProgressParams { + progressToken: string; + progress?: number; + total?: number; + message?: string; +} + +/** + * Parameters for notifications/message + */ +export interface NotificationMessageParams { + level: string; + logger?: string; + data: unknown; +} + +/** + * Parameters for notifications/resources/updated + */ +export interface NotificationResourceUpdatedParams { + uri: string; +} + +// ============================================================================= +// UTILITY TYPES +// ============================================================================= + +/** + * Generic type for method parameters + */ +export type MethodParams = + | ToolCallParams + | ResourceReadParams + | ResourceSubscribeParams + | ResourceUnsubscribeParams + | PromptGetParams + | NotificationCancelledParams + | NotificationProgressParams + | NotificationMessageParams + | NotificationResourceUpdatedParams + | Record; + +/** + * Type guard function type + */ +export type TypeGuard = (value: unknown) => value is T; + +/** + * Callback function type for instrumentation + */ +export type InstrumentationCallback = () => T; + +/** + * Span creation configuration + */ +export interface SpanConfig { + name: string; + forceTransaction?: boolean; + attributes: Record; +} + +// ============================================================================= +// TRACE PROPAGATION TYPES +// ============================================================================= + +/** + * Trace data for propagation + */ +export interface TraceData { + 'sentry-trace'?: string; + baggage?: string; + traceparent?: string; // W3C format +} + +/** + * MCP trace metadata in params._meta + */ +export interface McpTraceMetadata { + 'sentry-trace'?: string; + baggage?: string; + traceparent?: string; // W3C format support +} + +/** + * Request with trace metadata + */ +export interface JsonRpcRequestWithTrace extends JsonRpcRequest { + params?: Record & { + _meta?: McpTraceMetadata; + }; +} + +/** + * Notification with trace metadata + */ +export interface JsonRpcNotificationWithTrace extends JsonRpcNotification { + params?: Record & { + _meta?: McpTraceMetadata; + }; +} + +// ============================================================================= +// CONFIGURATION TYPES +// ============================================================================= + +/** + * Options for MCP server instrumentation + */ +export interface McpServerInstrumentationOptions { + /** + * Whether to capture request arguments (security consideration) + * Default: false + */ + captureRequestArguments?: boolean; + + /** + * Which request arguments to capture (if captureRequestArguments is true) + * Default: [] (none) + */ + allowedRequestArguments?: string[]; + + /** + * Maximum length for logging message content + * Default: 1000 + */ + maxLoggingMessageLength?: number; + + /** + * Whether to capture client information (address, port) + * Default: true + */ + captureClientInfo?: boolean; + + /** + * Custom attribute extraction function + */ + customAttributeExtractor?: (method: string, params: MethodParams) => Record; +} + +// ============================================================================= +// ERROR TYPES +// ============================================================================= + +/** + * MCP-specific error interface + */ +export interface McpError extends Error { + code?: number; + data?: unknown; +} + +/** + * Tool execution result with error flag + */ +export interface CallToolResult { + content?: unknown; + isError?: boolean; + error?: string; +} + +// ============================================================================= +// EXPORT UTILITY TYPES +// ============================================================================= + +/** + * All MCP method names as a union type + */ +export type McpMethodName = + | 'initialize' + | 'ping' + | 'tools/list' + | 'tools/call' + | 'resources/list' + | 'resources/read' + | 'resources/subscribe' + | 'resources/unsubscribe' + | 'resources/templates/list' + | 'prompts/list' + | 'prompts/get' + | 'roots/list' + | 'completion/complete' + | 'sampling/createMessage' + | 'logging/setLevel' + | 'notifications/initialized' + | 'notifications/cancelled' + | 'notifications/message' + | 'notifications/prompts/list_changed' + | 'notifications/resources/list_changed' + | 'notifications/resources/updated' + | 'notifications/roots/list_changed' + | 'notifications/tools/list_changed'; + +/** + * JSON-RPC error codes as a union type + */ +export type JsonRpcErrorCode = -32700 | -32600 | -32601 | -32602 | -32603 | number; \ No newline at end of file From c2f3e821b9a8ae843330b3451c4729d1cd58bf05 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 13:10:35 +0200 Subject: [PATCH 09/22] feat(mcp-server): Introduce MCP attributes and methods for enhanced tracing and monitoring --- .../core/src/utils/mcp-server/attributes.ts | 399 ++++++++++++++++++ 1 file changed, 399 insertions(+) create mode 100644 packages/core/src/utils/mcp-server/attributes.ts diff --git a/packages/core/src/utils/mcp-server/attributes.ts b/packages/core/src/utils/mcp-server/attributes.ts new file mode 100644 index 000000000000..90132a92aac5 --- /dev/null +++ b/packages/core/src/utils/mcp-server/attributes.ts @@ -0,0 +1,399 @@ +/** + * Model Context Protocol (MCP) Semantic Conventions for Sentry + * + * Based on OpenTelemetry MCP semantic conventions: + * https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/mcp.md + * + * These attributes follow the MCP specification for distributed tracing and monitoring. + */ + +// ============================================================================= +// CORE MCP ATTRIBUTES +// ============================================================================= + +/** + * The name of the request or notification method + * @see https://github.com/open-telemetry/semantic-conventions/blob/main/docs/registry/attributes/mcp.md + * + * Well-known values: + * - completion/complete + * - initialize + * - logging/setLevel + * - notifications/cancelled + * - notifications/initialized + * - notifications/message + * - notifications/prompts/list_changed + * - notifications/resources/list_changed + * - notifications/resources/updated + * - notifications/roots/list_changed + * - notifications/tools/list_changed + * - ping + * - prompts/get + * - prompts/list + * - resources/list + * - resources/read + * - resources/subscribe + * - resources/templates/list + * - resources/unsubscribe + * - roots/list + * - sampling/createMessage + * - tools/call + * - tools/list + */ +export const MCP_METHOD_NAME_ATTRIBUTE = 'mcp.method.name'; + +/** + * Unique identifier for the request + * Examples: "42", "req_123456" + */ +export const MCP_REQUEST_ID_ATTRIBUTE = 'mcp.request.id'; + +/** + * Identifies the MCP session + * Examples: "191c4850af6c49e08843a3f6c80e5046" + */ +export const MCP_SESSION_ID_ATTRIBUTE = 'mcp.session.id'; + +/** + * Transport method used for MCP communication + * Values: "stdio", "sse", "http", "websocket" + */ +export const MCP_TRANSPORT_ATTRIBUTE = 'mcp.transport'; + +// ============================================================================= +// METHOD-SPECIFIC ATTRIBUTES +// ============================================================================= + +/** + * Name of the tool being called + * Examples: "get-weather", "execute_command", "search_docs" + */ +export const MCP_TOOL_NAME_ATTRIBUTE = 'mcp.tool.name'; + +/** + * The resource URI being accessed + * Examples: "file:///home/user/documents/report.pdf", "postgres://db/customers" + */ +export const MCP_RESOURCE_URI_ATTRIBUTE = 'mcp.resource.uri'; + +/** + * Human-readable resource name + * Examples: "sentry-docs-platform", "project-config" + */ +export const MCP_RESOURCE_NAME_ATTRIBUTE = 'mcp.resource.name'; + +/** + * Name of the prompt template + * Examples: "analyze-code", "generate-summary" + */ +export const MCP_PROMPT_NAME_ATTRIBUTE = 'mcp.prompt.name'; + +/** + * Resource protocol extracted from URI + * Examples: "file:", "postgres:", "http:" + */ +export const MCP_RESOURCE_PROTOCOL_ATTRIBUTE = 'mcp.resource.protocol'; + +// ============================================================================= +// REQUEST ARGUMENT ATTRIBUTES +// ============================================================================= + +/** + * Base prefix for request arguments + * Security Note: Instrumentations SHOULD require explicit configuration + * of which arguments are captured to avoid leaking sensitive information. + */ +export const MCP_REQUEST_ARGUMENT_PREFIX = 'mcp.request.argument.'; + +/** + * Helper function to create request argument attribute names + * @param key The argument key (will be lowercased) + * @returns Full attribute name + */ +export function getMcpRequestArgumentAttribute(key: string): string { + return `${MCP_REQUEST_ARGUMENT_PREFIX}${key.toLowerCase()}`; +} + +// ============================================================================= +// NOTIFICATION ATTRIBUTES +// ============================================================================= + +/** + * Direction of the notification + * Values: "client_to_server", "server_to_client" + */ +export const MCP_NOTIFICATION_DIRECTION_ATTRIBUTE = 'mcp.notification.direction'; + +/** + * Request ID for cancelled notifications + */ +export const MCP_CANCELLED_REQUEST_ID_ATTRIBUTE = 'mcp.cancelled.request_id'; + +/** + * Reason for cancellation + */ +export const MCP_CANCELLED_REASON_ATTRIBUTE = 'mcp.cancelled.reason'; + +/** + * Progress token identifier + */ +export const MCP_PROGRESS_TOKEN_ATTRIBUTE = 'mcp.progress.token'; + +/** + * Current progress value + */ +export const MCP_PROGRESS_CURRENT_ATTRIBUTE = 'mcp.progress.current'; + +/** + * Total progress value + */ +export const MCP_PROGRESS_TOTAL_ATTRIBUTE = 'mcp.progress.total'; + +/** + * Progress percentage (calculated) + */ +export const MCP_PROGRESS_PERCENTAGE_ATTRIBUTE = 'mcp.progress.percentage'; + +/** + * Progress message + */ +export const MCP_PROGRESS_MESSAGE_ATTRIBUTE = 'mcp.progress.message'; + +/** + * Logging level + */ +export const MCP_LOGGING_LEVEL_ATTRIBUTE = 'mcp.logging.level'; + +/** + * Logger name + */ +export const MCP_LOGGING_LOGGER_ATTRIBUTE = 'mcp.logging.logger'; + +/** + * Type of logging data + */ +export const MCP_LOGGING_DATA_TYPE_ATTRIBUTE = 'mcp.logging.data_type'; + +/** + * Actual logging message content + */ +export const MCP_LOGGING_MESSAGE_ATTRIBUTE = 'mcp.logging.message'; + +// ============================================================================= +// NETWORK ATTRIBUTES (OpenTelemetry Standard) +// ============================================================================= + +/** + * OSI transport layer protocol + * Values: "pipe" (for stdio), "tcp" (for HTTP/SSE), "udp", "quic", "unix" + */ +export const NETWORK_TRANSPORT_ATTRIBUTE = 'network.transport'; + +/** + * Version of JSON RPC protocol used + * Examples: "1.1", "2.0" + */ +export const NETWORK_PROTOCOL_VERSION_ATTRIBUTE = 'network.protocol.version'; + +/** + * Client address - domain name if available without reverse DNS lookup; otherwise, IP address or Unix domain socket name + * Examples: "client.example.com", "10.1.2.80", "/tmp/my.sock" + */ +export const CLIENT_ADDRESS_ATTRIBUTE = 'client.address'; + +/** + * Client port number + * Example: 65123 + */ +export const CLIENT_PORT_ATTRIBUTE = 'client.port'; + +// ============================================================================= +// ERROR ATTRIBUTES (OpenTelemetry Standard) +// ============================================================================= + +/** + * Error type for failed operations + * - Should be set to the string representation of the JSON RPC error code + * - When JSON RPC call is successful but an error is returned within the result payload, + * should be set to low-cardinality string representation of the error + * - When CallToolResult is returned with isError set to true, should be set to "tool_error" + */ +export const ERROR_TYPE_ATTRIBUTE = 'error.type'; + +/** + * JSON-RPC error code (numeric) + * Examples: -32700, 100 + */ +export const RPC_JSONRPC_ERROR_CODE_ATTRIBUTE = 'rpc.jsonrpc.error_code'; + +// ============================================================================= +// COMMON JSON-RPC ERROR CODES +// ============================================================================= + +export const JSON_RPC_ERROR_CODES = { + PARSE_ERROR: -32700, + INVALID_REQUEST: -32600, + METHOD_NOT_FOUND: -32601, + INVALID_PARAMS: -32602, + INTERNAL_ERROR: -32603, + // Server error range: -32000 to -32099 +} as const; + +export const JSON_RPC_ERROR_MESSAGES = { + [JSON_RPC_ERROR_CODES.PARSE_ERROR]: 'Parse error', + [JSON_RPC_ERROR_CODES.INVALID_REQUEST]: 'Invalid Request', + [JSON_RPC_ERROR_CODES.METHOD_NOT_FOUND]: 'Method not found', + [JSON_RPC_ERROR_CODES.INVALID_PARAMS]: 'Invalid params', + [JSON_RPC_ERROR_CODES.INTERNAL_ERROR]: 'Internal error', +} as const; + +/** + * Special error type for tool execution failures + */ +export const TOOL_ERROR_TYPE = 'tool_error'; + +// ============================================================================= +// TRANSPORT TYPE MAPPINGS +// ============================================================================= + +/** + * MCP transport types (application level) + */ +export const MCP_TRANSPORT_TYPES = { + STDIO: 'stdio', + SSE: 'sse', + HTTP: 'http', + WEBSOCKET: 'websocket', +} as const; + +/** + * Network transport types (network level) + */ +export const NETWORK_TRANSPORT_TYPES = { + PIPE: 'pipe', // For stdio + TCP: 'tcp', // For HTTP/SSE/WebSocket + UDP: 'udp', + QUIC: 'quic', + UNIX: 'unix', +} as const; + +/** + * Mapping from MCP transport to network transport + */ +export const MCP_TO_NETWORK_TRANSPORT_MAP = { + [MCP_TRANSPORT_TYPES.STDIO]: NETWORK_TRANSPORT_TYPES.PIPE, + [MCP_TRANSPORT_TYPES.SSE]: NETWORK_TRANSPORT_TYPES.TCP, + [MCP_TRANSPORT_TYPES.HTTP]: NETWORK_TRANSPORT_TYPES.TCP, + [MCP_TRANSPORT_TYPES.WEBSOCKET]: NETWORK_TRANSPORT_TYPES.TCP, +} as const; + +// ============================================================================= +// WELL-KNOWN MCP METHOD NAMES +// ============================================================================= + +export const MCP_METHODS = { + // Core methods + INITIALIZE: 'initialize', + PING: 'ping', + + // Tool operations + TOOLS_LIST: 'tools/list', + TOOLS_CALL: 'tools/call', + + // Resource operations + RESOURCES_LIST: 'resources/list', + RESOURCES_READ: 'resources/read', + RESOURCES_SUBSCRIBE: 'resources/subscribe', + RESOURCES_UNSUBSCRIBE: 'resources/unsubscribe', + RESOURCES_TEMPLATES_LIST: 'resources/templates/list', + + // Prompt operations + PROMPTS_LIST: 'prompts/list', + PROMPTS_GET: 'prompts/get', + + // Root operations + ROOTS_LIST: 'roots/list', + + // Completion operations + COMPLETION_COMPLETE: 'completion/complete', + + // Sampling operations + SAMPLING_CREATE_MESSAGE: 'sampling/createMessage', + + // Logging operations + LOGGING_SET_LEVEL: 'logging/setLevel', + + // Notifications + NOTIFICATIONS_INITIALIZED: 'notifications/initialized', + NOTIFICATIONS_CANCELLED: 'notifications/cancelled', + NOTIFICATIONS_MESSAGE: 'notifications/message', + NOTIFICATIONS_PROMPTS_LIST_CHANGED: 'notifications/prompts/list_changed', + NOTIFICATIONS_RESOURCES_LIST_CHANGED: 'notifications/resources/list_changed', + NOTIFICATIONS_RESOURCES_UPDATED: 'notifications/resources/updated', + NOTIFICATIONS_ROOTS_LIST_CHANGED: 'notifications/roots/list_changed', + NOTIFICATIONS_TOOLS_LIST_CHANGED: 'notifications/tools/list_changed', +} as const; + +// ============================================================================= +// ATTRIBUTE GROUPS FOR DIFFERENT OPERATIONS +// ============================================================================= + +/** + * Required attributes for all MCP server spans + */ +export const MCP_SERVER_REQUIRED_ATTRIBUTES = [ + MCP_METHOD_NAME_ATTRIBUTE, +] as const; + +/** + * Conditionally required attributes (when applicable) + */ +export const MCP_SERVER_CONDITIONALLY_REQUIRED_ATTRIBUTES = [ + ERROR_TYPE_ATTRIBUTE, // If operation fails + RPC_JSONRPC_ERROR_CODE_ATTRIBUTE, // If response contains error code + MCP_REQUEST_ID_ATTRIBUTE, // When client executes a request + MCP_TOOL_NAME_ATTRIBUTE, // When operation is related to a specific tool + MCP_PROMPT_NAME_ATTRIBUTE, // When operation is related to a specific prompt + MCP_RESOURCE_URI_ATTRIBUTE, // When client executes a request type that includes a resource URI parameter +] as const; + +/** + * Recommended attributes for MCP server spans + */ +export const MCP_SERVER_RECOMMENDED_ATTRIBUTES = [ + MCP_SESSION_ID_ATTRIBUTE, + CLIENT_ADDRESS_ATTRIBUTE, + CLIENT_PORT_ATTRIBUTE, + NETWORK_TRANSPORT_ATTRIBUTE, + NETWORK_PROTOCOL_VERSION_ATTRIBUTE, +] as const; + +/** + * Tool-specific attributes + */ +export const MCP_TOOL_ATTRIBUTES = [ + MCP_TOOL_NAME_ATTRIBUTE, +] as const; + +/** + * Resource-specific attributes + */ +export const MCP_RESOURCE_ATTRIBUTES = [ + MCP_RESOURCE_URI_ATTRIBUTE, + MCP_RESOURCE_NAME_ATTRIBUTE, + MCP_RESOURCE_PROTOCOL_ATTRIBUTE, +] as const; + +/** + * Prompt-specific attributes + */ +export const MCP_PROMPT_ATTRIBUTES = [ + MCP_PROMPT_NAME_ATTRIBUTE, +] as const; + +/** + * Notification-specific attributes + */ +export const MCP_NOTIFICATION_ATTRIBUTES = [ + MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, +] as const; \ No newline at end of file From 094574f292a218d52ae549645bdbaf1fd7ebdc3d Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 17:24:08 +0200 Subject: [PATCH 10/22] test(mcp-server): Add tests for span creation with various notification types --- packages/core/test/lib/mcp-server.test.ts | 126 ++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 49c5eb41c692..e52c3390711e 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -376,6 +376,132 @@ describe('wrapMcpServerWithSentry', () => { ); }); + it('should create spans with logging attributes for notifications/message', async () => { + await wrappedMcpServer.connect(mockTransport); + + const loggingNotification = { + jsonrpc: '2.0', + method: 'notifications/message', + params: { + level: 'info', + logger: 'math-service', + data: 'Addition completed: 2 + 5 = 7' + } + }; + + mockTransport.onmessage?.(loggingNotification, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + { + name: 'notifications/message logger:math-service', + forceTransaction: true, + attributes: { + 'mcp.method.name': 'notifications/message', + 'mcp.session.id': 'test-session-123', + 'mcp.notification.direction': 'client_to_server', + 'mcp.transport': 'http', + 'network.transport': 'tcp', + 'network.protocol.version': '2.0', + 'mcp.logging.level': 'info', + 'mcp.logging.logger': 'math-service', + 'mcp.logging.data_type': 'string', + 'mcp.logging.message': 'Addition completed: 2 + 5 = 7', + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', + }, + }, + expect.any(Function) + ); + }); + + it('should create spans with attributes for other notification types', async () => { + await wrappedMcpServer.connect(mockTransport); + + // Test notifications/cancelled + const cancelledNotification = { + jsonrpc: '2.0', + method: 'notifications/cancelled', + params: { + requestId: 'req-123', + reason: 'user_requested' + } + }; + + mockTransport.onmessage?.(cancelledNotification, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/cancelled request:req-123', + attributes: expect.objectContaining({ + 'mcp.method.name': 'notifications/cancelled', + 'mcp.cancelled.request_id': 'req-123', + 'mcp.cancelled.reason': 'user_requested', + 'mcp.notification.direction': 'client_to_server', + }), + }), + expect.any(Function) + ); + + vi.clearAllMocks(); + + // Test notifications/progress + const progressNotification = { + jsonrpc: '2.0', + method: 'notifications/progress', + params: { + progressToken: 'token-456', + progress: 75, + total: 100, + message: 'Processing files...' + } + }; + + mockTransport.onmessage?.(progressNotification, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/progress token:token-456', + attributes: expect.objectContaining({ + 'mcp.method.name': 'notifications/progress', + 'mcp.progress.token': 'token-456', + 'mcp.progress.current': 75, + 'mcp.progress.total': 100, + 'mcp.progress.percentage': 75, + 'mcp.progress.message': 'Processing files...', + 'mcp.notification.direction': 'client_to_server', + }), + }), + expect.any(Function) + ); + + vi.clearAllMocks(); + + // Test notifications/resources/updated + const resourceUpdatedNotification = { + jsonrpc: '2.0', + method: 'notifications/resources/updated', + params: { + uri: 'file:///tmp/data.json' + } + }; + + mockTransport.onmessage?.(resourceUpdatedNotification, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/resources/updated file:///tmp/data.json', + attributes: expect.objectContaining({ + 'mcp.method.name': 'notifications/resources/updated', + 'mcp.resource.uri': 'file:///tmp/data.json', + 'mcp.resource.protocol': 'file:', + 'mcp.notification.direction': 'client_to_server', + }), + }), + expect.any(Function) + ); + }); + }); }); From 9972b09e49ad8e06e9f1bb8bc7d0190b73eaddb9 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 17:35:07 +0200 Subject: [PATCH 11/22] test(mcp-server): Update test to use spy for startSpan --- packages/core/test/lib/mcp-server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index e52c3390711e..1cfd0eecf2b6 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -354,7 +354,7 @@ describe('wrapMcpServerWithSentry', () => { mockTransport.onmessage?.(jsonRpcRequest, {}); - expect(tracingModule.startSpan).toHaveBeenCalledWith( + expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'tools/list', forceTransaction: true, From ef52da53ab956b4cd7e203d41b2f923978a6819a Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 18:13:19 +0200 Subject: [PATCH 12/22] refactor(mcp-server): improve span handling and attribute extraction --- packages/core/src/mcp-server.ts | 348 ++++++++++++++++---------------- 1 file changed, 178 insertions(+), 170 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 80f831cb7742..98f25b7a0bd9 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -1,4 +1,3 @@ -import { DEBUG_BUILD } from './debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -6,29 +5,45 @@ import { } from './semanticAttributes'; import { startSpan, withActiveSpan } from './tracing'; import type { Span } from './types-hoist/span'; -import { logger } from './utils/logger'; import { getActiveSpan } from './utils/spanUtils'; - -interface MCPTransport { - // The first argument is a JSON RPC message - onmessage?: (...args: unknown[]) => void; - onclose?: (...args: unknown[]) => void; - sessionId?: string; -} - -interface MCPServerInstance { - // The first arg is always a name, the last arg should always be a callback function (ie a handler). - // TODO: We could also make use of the resource uri argument somehow. - resource: (name: string, ...args: unknown[]) => void; - // The first arg is always a name, the last arg should always be a callback function (ie a handler). - tool: (name: string, ...args: unknown[]) => void; - // The first arg is always a name, the last arg should always be a callback function (ie a handler). - prompt: (name: string, ...args: unknown[]) => void; - connect(transport: MCPTransport): Promise; - server?: { - setRequestHandler: (schema: unknown, handler: (...args: unknown[]) => unknown) => void; - }; -} +import { + MCP_METHOD_NAME_ATTRIBUTE, + MCP_REQUEST_ID_ATTRIBUTE, + MCP_SESSION_ID_ATTRIBUTE, + MCP_TRANSPORT_ATTRIBUTE, + NETWORK_TRANSPORT_ATTRIBUTE, + NETWORK_PROTOCOL_VERSION_ATTRIBUTE, + CLIENT_ADDRESS_ATTRIBUTE, + CLIENT_PORT_ATTRIBUTE, + MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, + MCP_SERVER_OP_VALUE, + MCP_FUNCTION_ORIGIN_VALUE, + MCP_NOTIFICATION_ORIGIN_VALUE, + MCP_ROUTE_SOURCE_VALUE, +} from './utils/mcp-server/attributes'; +import type { + JsonRpcRequest, + JsonRpcNotification, + MCPTransport, + MCPServerInstance, + SessionId, + RequestId, + ExtraHandlerData, +} from './utils/mcp-server/types'; +import { + isJsonRpcRequest, + isJsonRpcNotification, + extractTarget, + getTargetAttributes, + getRequestArguments, + getTransportTypes, + getNotificationDescription, + getNotificationAttributes, + extractClientAddress, + extractClientPort, + validateMcpServerInstance, + createSpanName, +} from './utils/mcp-server/utils'; const wrappedMcpServerInstances = new WeakSet(); @@ -43,21 +58,21 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): return mcpServerInstance; } - if (!isMcpServerInstance(mcpServerInstance)) { - DEBUG_BUILD && logger.warn('Did not patch MCP server. Interface is incompatible.'); + if (!validateMcpServerInstance(mcpServerInstance)) { return mcpServerInstance; } + const serverInstance = mcpServerInstance as MCPServerInstance; + // Wrap connect() to intercept AFTER Protocol sets up transport handlers - mcpServerInstance.connect = new Proxy(mcpServerInstance.connect, { + serverInstance.connect = new Proxy(serverInstance.connect, { async apply(target, thisArg, argArray) { const [transport, ...restArgs] = argArray as [MCPTransport, ...unknown[]]; // Call the original connect first to let Protocol set up its handlers const result = await Reflect.apply(target, thisArg, [transport, ...restArgs]); - - // NOW intercept the transport's onmessage after Protocol has set it up + // Intercept incoming messages via onmessage if (transport.onmessage) { const protocolOnMessage = transport.onmessage; @@ -65,10 +80,14 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): apply(onMessageTarget, onMessageThisArg, onMessageArgs) { const [jsonRpcMessage, extra] = onMessageArgs; - - // TODO(bete): Instrument responses/notifications (not sure if they are RPC) + // Instrument both requests and notifications if (isJsonRpcRequest(jsonRpcMessage)) { - return createMcpServerSpan(jsonRpcMessage, transport, extra, () => { + return createMcpServerSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { + return onMessageTarget.apply(onMessageThisArg, onMessageArgs); + }); + } + if (isJsonRpcNotification(jsonRpcMessage)) { + return createMcpNotificationSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { return onMessageTarget.apply(onMessageThisArg, onMessageArgs); }); } @@ -76,7 +95,27 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): return onMessageTarget.apply(onMessageThisArg, onMessageArgs); } }); - } + } + + // Intercept outgoing messages via send + if (transport.send) { + const originalSend = transport.send; + + transport.send = new Proxy(originalSend, { + async apply(sendTarget, sendThisArg, sendArgs) { + const [message, options] = sendArgs; + + // Instrument outgoing notifications (but not requests/responses) + if (isJsonRpcNotification(message)) { + return createMcpOutgoingNotificationSpan(message, transport, options as Record, () => { + return sendTarget.apply(sendThisArg, sendArgs); + }); + } + + return sendTarget.apply(sendThisArg, sendArgs); + } + }); + } // Handle transport lifecycle events if (transport.onclose) { @@ -101,49 +140,44 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): function createMcpServerSpan( jsonRpcMessage: JsonRpcRequest, transport: MCPTransport, - extra: any, - callback: () => any + extra: ExtraHandlerData, + callback: () => unknown ) { const { method, id: requestId, params } = jsonRpcMessage; // Extract target from method and params for proper description - const target = extractTarget(method, params); - const description = target ? `${method} ${target}` : method; + const target = extractTarget(method, params as Record); + const description = createSpanName(method, target); // Session ID should come from the transport itself, not the RPC message const sessionId = transport.sessionId; // Extract client information from extra/request data - const clientAddress = extra?.requestInfo?.remoteAddress || - extra?.clientAddress || - extra?.request?.ip || - extra?.request?.connection?.remoteAddress; - const clientPort = extra?.requestInfo?.remotePort || - extra?.clientPort || - extra?.request?.connection?.remotePort; + const clientAddress = extractClientAddress(extra); + const clientPort = extractClientPort(extra); // Determine transport types const { mcpTransport, networkTransport } = getTransportTypes(transport); const attributes: Record = { - 'mcp.method.name': method, + [MCP_METHOD_NAME_ATTRIBUTE]: method, - ...(requestId !== undefined && { 'mcp.request.id': String(requestId) }), + ...(requestId !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(requestId) }), ...(target && getTargetAttributes(method, target)), - ...(sessionId && { 'mcp.session.id': sessionId }), - ...(clientAddress && { 'client.address': clientAddress }), - ...(clientPort && { 'client.port': clientPort }), - 'mcp.transport': mcpTransport, // Application level: "http", "sse", "stdio", "websocket" - 'network.transport': networkTransport, // Network level: "tcp", "pipe", "udp", "quic" - 'network.protocol.version': '2.0', // JSON-RPC version + ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), + ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), + ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, // Application level: "http", "sse", "stdio", "websocket" + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, // Network level: "tcp", "pipe", "udp", "quic" + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', // JSON-RPC version // Opt-in: Tool arguments (if enabled) - ...getRequestArguments(method, params), + ...getRequestArguments(method, params as Record), // Sentry-specific attributes - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'mcp.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.mcp_server', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_FUNCTION_ORIGIN_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE }; return startSpan({ @@ -151,142 +185,116 @@ function createMcpServerSpan( forceTransaction: true, attributes }, () => { + // TODO(bete): add proper error handling. Handle JSON RPC errors in the result return callback(); }); } -function extractTarget(method: string, params: any): string | undefined { - switch (method) { - case 'tools/call': - return params?.name; // Tool name - case 'resources/read': - case 'resources/subscribe': - case 'resources/unsubscribe': - return params?.uri; // Resource URI - case 'prompts/get': - return params?.name; // Prompt name - default: - return undefined; - } -} +function createMcpNotificationSpan( + jsonRpcMessage: JsonRpcNotification, + transport: MCPTransport, + extra: ExtraHandlerData, + callback: () => unknown +) { + const { method, params } = jsonRpcMessage; + + const description = getNotificationDescription(method, params as Record); + + const sessionId = transport.sessionId; + + // Extract client information + const clientAddress = extractClientAddress(extra); + const clientPort = extractClientPort(extra); -function getTargetAttributes(method: string, target: string): Record { - switch (method) { - case 'tools/call': - return { 'mcp.tool.name': target }; - case 'resources/read': - case 'resources/subscribe': - case 'resources/unsubscribe': - return { 'mcp.resource.uri': target }; - case 'prompts/get': - return { 'mcp.prompt.name': target }; - default: - return {}; - } + // Determine transport types + const { mcpTransport, networkTransport } = getTransportTypes(transport); + + const notificationAttribs = getNotificationAttributes(method, params as Record); + + const attributes: Record = { + [MCP_METHOD_NAME_ATTRIBUTE]: method, + [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'client_to_server', // Incoming notification + + ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), + ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), + ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', + + // Notification-specific attributes + ...notificationAttribs, + + // Sentry-specific attributes + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_NOTIFICATION_ORIGIN_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE + }; + + return startSpan({ + name: description, + forceTransaction: true, + attributes + }, () => { + const result = callback(); + return result; + }); } -function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { - // Try to determine transport type from transport properties/constructor - const transportName = transport.constructor?.name?.toLowerCase() || ''; - - if (transportName.includes('sse')) { - return { mcpTransport: 'sse', networkTransport: 'tcp' }; - } - if (transportName.includes('http')) { - return { mcpTransport: 'http', networkTransport: 'tcp' }; - } - if (transportName.includes('websocket') || transportName.includes('ws')) { - return { mcpTransport: 'websocket', networkTransport: 'tcp' }; - } - if (transportName.includes('stdio')) { - return { mcpTransport: 'stdio', networkTransport: 'pipe' }; - } +function createMcpOutgoingNotificationSpan( + jsonRpcMessage: JsonRpcNotification, + transport: MCPTransport, + options: Record, + callback: () => unknown +) { + const { method, params } = jsonRpcMessage; - // Default assumption based on your setup (HTTP server) - return { mcpTransport: 'http', networkTransport: 'tcp' }; -} -function getRequestArguments(method: string, params: any): Record { - const args: Record = {}; + const description = getNotificationDescription(method, params as Record); - // Only include arguments for certain methods (security consideration) - switch (method) { - case 'tools/call': - if (params?.arguments) { - // Convert arguments to JSON strings as per MCP conventions - for (const [key, value] of Object.entries(params.arguments)) { - args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); - } - } - break; - case 'resources/read': - if (params?.uri) { - args['mcp.request.argument.uri'] = JSON.stringify(params.uri); - } - break; - case 'prompts/get': - if (params?.name) { - args['mcp.request.argument.name'] = JSON.stringify(params.name); - } - if (params?.arguments) { - for (const [key, value] of Object.entries(params.arguments)) { - args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); - } - } - break; - } + const sessionId = transport.sessionId; - return args; -} + // Determine transport types + const { mcpTransport, networkTransport } = getTransportTypes(transport); -function isJsonRpcRequest(message: any): message is JsonRpcRequest { - const isRequest = ( - typeof message === 'object' && - message !== null && - message.jsonrpc === '2.0' && - 'method' in message && - 'id' in message - ); - - return isRequest; -} + const notificationAttribs = getNotificationAttributes(method, params as Record); -interface JsonRpcRequest { - jsonrpc: '2.0'; - method: string; - id: string | number; - params?: any; -} + const attributes: Record = { + [MCP_METHOD_NAME_ATTRIBUTE]: method, + [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'server_to_client', // Outgoing notification + + ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', + + // Notification-specific attributes + ...notificationAttribs, + + // Sentry-specific attributes + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_NOTIFICATION_ORIGIN_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE + }; -function isMcpServerInstance(mcpServerInstance: unknown): mcpServerInstance is MCPServerInstance { - return ( - typeof mcpServerInstance === 'object' && - mcpServerInstance !== null && - 'resource' in mcpServerInstance && - typeof mcpServerInstance.resource === 'function' && - 'tool' in mcpServerInstance && - typeof mcpServerInstance.tool === 'function' && - 'prompt' in mcpServerInstance && - typeof mcpServerInstance.prompt === 'function' && - 'connect' in mcpServerInstance && - typeof mcpServerInstance.connect === 'function' && - 'server' in mcpServerInstance && - typeof mcpServerInstance.server === 'object' && - mcpServerInstance.server !== null && - 'setRequestHandler' in mcpServerInstance.server && - typeof mcpServerInstance.server.setRequestHandler === 'function' - ); + return startSpan({ + name: description, + forceTransaction: true, + attributes + }, () => { + const result = callback(); + return result; + }); } +// ============================================================================= +// SESSION AND REQUEST CORRELATION (Legacy support) +// ============================================================================= interface ExtraHandlerDataWithRequestId { sessionId: SessionId; requestId: RequestId; } - -type SessionId = string; -type RequestId = string | number; - const sessionAndRequestToRequestParentSpanMap = new Map>(); function handleTransportOnClose(sessionId: SessionId): void { From aee709b0aaca978f27e04cc969b023cd36971e03 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 18:20:40 +0200 Subject: [PATCH 13/22] simplify attributes --- .../core/src/utils/mcp-server/attributes.ts | 333 ++---------------- 1 file changed, 26 insertions(+), 307 deletions(-) diff --git a/packages/core/src/utils/mcp-server/attributes.ts b/packages/core/src/utils/mcp-server/attributes.ts index 90132a92aac5..bd848ede0b9f 100644 --- a/packages/core/src/utils/mcp-server/attributes.ts +++ b/packages/core/src/utils/mcp-server/attributes.ts @@ -1,10 +1,8 @@ /** - * Model Context Protocol (MCP) Semantic Conventions for Sentry + * Essential MCP attribute constants for Sentry instrumentation * - * Based on OpenTelemetry MCP semantic conventions: - * https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/mcp.md - * - * These attributes follow the MCP specification for distributed tracing and monitoring. + * Based on OpenTelemetry MCP semantic conventions + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md */ // ============================================================================= @@ -13,50 +11,25 @@ /** * The name of the request or notification method - * @see https://github.com/open-telemetry/semantic-conventions/blob/main/docs/registry/attributes/mcp.md - * - * Well-known values: - * - completion/complete - * - initialize - * - logging/setLevel - * - notifications/cancelled - * - notifications/initialized - * - notifications/message - * - notifications/prompts/list_changed - * - notifications/resources/list_changed - * - notifications/resources/updated - * - notifications/roots/list_changed - * - notifications/tools/list_changed - * - ping - * - prompts/get - * - prompts/list - * - resources/list - * - resources/read - * - resources/subscribe - * - resources/templates/list - * - resources/unsubscribe - * - roots/list - * - sampling/createMessage - * - tools/call - * - tools/list + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#required-attributes */ export const MCP_METHOD_NAME_ATTRIBUTE = 'mcp.method.name'; /** * Unique identifier for the request - * Examples: "42", "req_123456" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#recommended-attributes */ export const MCP_REQUEST_ID_ATTRIBUTE = 'mcp.request.id'; /** * Identifies the MCP session - * Examples: "191c4850af6c49e08843a3f6c80e5046" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#recommended-attributes */ export const MCP_SESSION_ID_ATTRIBUTE = 'mcp.session.id'; /** * Transport method used for MCP communication - * Values: "stdio", "sse", "http", "websocket" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#recommended-attributes */ export const MCP_TRANSPORT_ATTRIBUTE = 'mcp.transport'; @@ -66,334 +39,80 @@ export const MCP_TRANSPORT_ATTRIBUTE = 'mcp.transport'; /** * Name of the tool being called - * Examples: "get-weather", "execute_command", "search_docs" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#method-specific-attributes */ export const MCP_TOOL_NAME_ATTRIBUTE = 'mcp.tool.name'; /** * The resource URI being accessed - * Examples: "file:///home/user/documents/report.pdf", "postgres://db/customers" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#method-specific-attributes */ export const MCP_RESOURCE_URI_ATTRIBUTE = 'mcp.resource.uri'; -/** - * Human-readable resource name - * Examples: "sentry-docs-platform", "project-config" - */ -export const MCP_RESOURCE_NAME_ATTRIBUTE = 'mcp.resource.name'; - /** * Name of the prompt template - * Examples: "analyze-code", "generate-summary" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#method-specific-attributes */ export const MCP_PROMPT_NAME_ATTRIBUTE = 'mcp.prompt.name'; -/** - * Resource protocol extracted from URI - * Examples: "file:", "postgres:", "http:" - */ -export const MCP_RESOURCE_PROTOCOL_ATTRIBUTE = 'mcp.resource.protocol'; - -// ============================================================================= -// REQUEST ARGUMENT ATTRIBUTES -// ============================================================================= - -/** - * Base prefix for request arguments - * Security Note: Instrumentations SHOULD require explicit configuration - * of which arguments are captured to avoid leaking sensitive information. - */ -export const MCP_REQUEST_ARGUMENT_PREFIX = 'mcp.request.argument.'; - -/** - * Helper function to create request argument attribute names - * @param key The argument key (will be lowercased) - * @returns Full attribute name - */ -export function getMcpRequestArgumentAttribute(key: string): string { - return `${MCP_REQUEST_ARGUMENT_PREFIX}${key.toLowerCase()}`; -} - // ============================================================================= // NOTIFICATION ATTRIBUTES // ============================================================================= /** - * Direction of the notification - * Values: "client_to_server", "server_to_client" + * Direction of the notification (client_to_server or server_to_client) + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#notification-attributes */ export const MCP_NOTIFICATION_DIRECTION_ATTRIBUTE = 'mcp.notification.direction'; -/** - * Request ID for cancelled notifications - */ -export const MCP_CANCELLED_REQUEST_ID_ATTRIBUTE = 'mcp.cancelled.request_id'; - -/** - * Reason for cancellation - */ -export const MCP_CANCELLED_REASON_ATTRIBUTE = 'mcp.cancelled.reason'; - -/** - * Progress token identifier - */ -export const MCP_PROGRESS_TOKEN_ATTRIBUTE = 'mcp.progress.token'; - -/** - * Current progress value - */ -export const MCP_PROGRESS_CURRENT_ATTRIBUTE = 'mcp.progress.current'; - -/** - * Total progress value - */ -export const MCP_PROGRESS_TOTAL_ATTRIBUTE = 'mcp.progress.total'; - -/** - * Progress percentage (calculated) - */ -export const MCP_PROGRESS_PERCENTAGE_ATTRIBUTE = 'mcp.progress.percentage'; - -/** - * Progress message - */ -export const MCP_PROGRESS_MESSAGE_ATTRIBUTE = 'mcp.progress.message'; - -/** - * Logging level - */ -export const MCP_LOGGING_LEVEL_ATTRIBUTE = 'mcp.logging.level'; - -/** - * Logger name - */ -export const MCP_LOGGING_LOGGER_ATTRIBUTE = 'mcp.logging.logger'; - -/** - * Type of logging data - */ -export const MCP_LOGGING_DATA_TYPE_ATTRIBUTE = 'mcp.logging.data_type'; - -/** - * Actual logging message content - */ -export const MCP_LOGGING_MESSAGE_ATTRIBUTE = 'mcp.logging.message'; - // ============================================================================= // NETWORK ATTRIBUTES (OpenTelemetry Standard) // ============================================================================= /** * OSI transport layer protocol - * Values: "pipe" (for stdio), "tcp" (for HTTP/SSE), "udp", "quic", "unix" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#network-attributes */ export const NETWORK_TRANSPORT_ATTRIBUTE = 'network.transport'; /** - * Version of JSON RPC protocol used - * Examples: "1.1", "2.0" + * The version of JSON RPC protocol used + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#network-attributes */ export const NETWORK_PROTOCOL_VERSION_ATTRIBUTE = 'network.protocol.version'; /** * Client address - domain name if available without reverse DNS lookup; otherwise, IP address or Unix domain socket name - * Examples: "client.example.com", "10.1.2.80", "/tmp/my.sock" + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#network-attributes */ export const CLIENT_ADDRESS_ATTRIBUTE = 'client.address'; /** * Client port number - * Example: 65123 + * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#network-attributes */ export const CLIENT_PORT_ATTRIBUTE = 'client.port'; // ============================================================================= -// ERROR ATTRIBUTES (OpenTelemetry Standard) -// ============================================================================= - -/** - * Error type for failed operations - * - Should be set to the string representation of the JSON RPC error code - * - When JSON RPC call is successful but an error is returned within the result payload, - * should be set to low-cardinality string representation of the error - * - When CallToolResult is returned with isError set to true, should be set to "tool_error" - */ -export const ERROR_TYPE_ATTRIBUTE = 'error.type'; - -/** - * JSON-RPC error code (numeric) - * Examples: -32700, 100 - */ -export const RPC_JSONRPC_ERROR_CODE_ATTRIBUTE = 'rpc.jsonrpc.error_code'; - -// ============================================================================= -// COMMON JSON-RPC ERROR CODES -// ============================================================================= - -export const JSON_RPC_ERROR_CODES = { - PARSE_ERROR: -32700, - INVALID_REQUEST: -32600, - METHOD_NOT_FOUND: -32601, - INVALID_PARAMS: -32602, - INTERNAL_ERROR: -32603, - // Server error range: -32000 to -32099 -} as const; - -export const JSON_RPC_ERROR_MESSAGES = { - [JSON_RPC_ERROR_CODES.PARSE_ERROR]: 'Parse error', - [JSON_RPC_ERROR_CODES.INVALID_REQUEST]: 'Invalid Request', - [JSON_RPC_ERROR_CODES.METHOD_NOT_FOUND]: 'Method not found', - [JSON_RPC_ERROR_CODES.INVALID_PARAMS]: 'Invalid params', - [JSON_RPC_ERROR_CODES.INTERNAL_ERROR]: 'Internal error', -} as const; - -/** - * Special error type for tool execution failures - */ -export const TOOL_ERROR_TYPE = 'tool_error'; - -// ============================================================================= -// TRANSPORT TYPE MAPPINGS -// ============================================================================= - -/** - * MCP transport types (application level) - */ -export const MCP_TRANSPORT_TYPES = { - STDIO: 'stdio', - SSE: 'sse', - HTTP: 'http', - WEBSOCKET: 'websocket', -} as const; - -/** - * Network transport types (network level) - */ -export const NETWORK_TRANSPORT_TYPES = { - PIPE: 'pipe', // For stdio - TCP: 'tcp', // For HTTP/SSE/WebSocket - UDP: 'udp', - QUIC: 'quic', - UNIX: 'unix', -} as const; - -/** - * Mapping from MCP transport to network transport - */ -export const MCP_TO_NETWORK_TRANSPORT_MAP = { - [MCP_TRANSPORT_TYPES.STDIO]: NETWORK_TRANSPORT_TYPES.PIPE, - [MCP_TRANSPORT_TYPES.SSE]: NETWORK_TRANSPORT_TYPES.TCP, - [MCP_TRANSPORT_TYPES.HTTP]: NETWORK_TRANSPORT_TYPES.TCP, - [MCP_TRANSPORT_TYPES.WEBSOCKET]: NETWORK_TRANSPORT_TYPES.TCP, -} as const; - -// ============================================================================= -// WELL-KNOWN MCP METHOD NAMES +// SENTRY-SPECIFIC MCP ATTRIBUTE VALUES // ============================================================================= -export const MCP_METHODS = { - // Core methods - INITIALIZE: 'initialize', - PING: 'ping', - - // Tool operations - TOOLS_LIST: 'tools/list', - TOOLS_CALL: 'tools/call', - - // Resource operations - RESOURCES_LIST: 'resources/list', - RESOURCES_READ: 'resources/read', - RESOURCES_SUBSCRIBE: 'resources/subscribe', - RESOURCES_UNSUBSCRIBE: 'resources/unsubscribe', - RESOURCES_TEMPLATES_LIST: 'resources/templates/list', - - // Prompt operations - PROMPTS_LIST: 'prompts/list', - PROMPTS_GET: 'prompts/get', - - // Root operations - ROOTS_LIST: 'roots/list', - - // Completion operations - COMPLETION_COMPLETE: 'completion/complete', - - // Sampling operations - SAMPLING_CREATE_MESSAGE: 'sampling/createMessage', - - // Logging operations - LOGGING_SET_LEVEL: 'logging/setLevel', - - // Notifications - NOTIFICATIONS_INITIALIZED: 'notifications/initialized', - NOTIFICATIONS_CANCELLED: 'notifications/cancelled', - NOTIFICATIONS_MESSAGE: 'notifications/message', - NOTIFICATIONS_PROMPTS_LIST_CHANGED: 'notifications/prompts/list_changed', - NOTIFICATIONS_RESOURCES_LIST_CHANGED: 'notifications/resources/list_changed', - NOTIFICATIONS_RESOURCES_UPDATED: 'notifications/resources/updated', - NOTIFICATIONS_ROOTS_LIST_CHANGED: 'notifications/roots/list_changed', - NOTIFICATIONS_TOOLS_LIST_CHANGED: 'notifications/tools/list_changed', -} as const; - -// ============================================================================= -// ATTRIBUTE GROUPS FOR DIFFERENT OPERATIONS -// ============================================================================= - -/** - * Required attributes for all MCP server spans - */ -export const MCP_SERVER_REQUIRED_ATTRIBUTES = [ - MCP_METHOD_NAME_ATTRIBUTE, -] as const; - -/** - * Conditionally required attributes (when applicable) - */ -export const MCP_SERVER_CONDITIONALLY_REQUIRED_ATTRIBUTES = [ - ERROR_TYPE_ATTRIBUTE, // If operation fails - RPC_JSONRPC_ERROR_CODE_ATTRIBUTE, // If response contains error code - MCP_REQUEST_ID_ATTRIBUTE, // When client executes a request - MCP_TOOL_NAME_ATTRIBUTE, // When operation is related to a specific tool - MCP_PROMPT_NAME_ATTRIBUTE, // When operation is related to a specific prompt - MCP_RESOURCE_URI_ATTRIBUTE, // When client executes a request type that includes a resource URI parameter -] as const; - -/** - * Recommended attributes for MCP server spans - */ -export const MCP_SERVER_RECOMMENDED_ATTRIBUTES = [ - MCP_SESSION_ID_ATTRIBUTE, - CLIENT_ADDRESS_ATTRIBUTE, - CLIENT_PORT_ATTRIBUTE, - NETWORK_TRANSPORT_ATTRIBUTE, - NETWORK_PROTOCOL_VERSION_ATTRIBUTE, -] as const; - /** - * Tool-specific attributes + * Sentry operation value for MCP server spans */ -export const MCP_TOOL_ATTRIBUTES = [ - MCP_TOOL_NAME_ATTRIBUTE, -] as const; +export const MCP_SERVER_OP_VALUE = 'mcp.server'; /** - * Resource-specific attributes + * Sentry origin value for MCP function spans */ -export const MCP_RESOURCE_ATTRIBUTES = [ - MCP_RESOURCE_URI_ATTRIBUTE, - MCP_RESOURCE_NAME_ATTRIBUTE, - MCP_RESOURCE_PROTOCOL_ATTRIBUTE, -] as const; +export const MCP_FUNCTION_ORIGIN_VALUE = 'auto.function.mcp_server'; /** - * Prompt-specific attributes + * Sentry origin value for MCP notification spans */ -export const MCP_PROMPT_ATTRIBUTES = [ - MCP_PROMPT_NAME_ATTRIBUTE, -] as const; +export const MCP_NOTIFICATION_ORIGIN_VALUE = 'auto.mcp.notification'; /** - * Notification-specific attributes + * Sentry source value for MCP route spans */ -export const MCP_NOTIFICATION_ATTRIBUTES = [ - MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, -] as const; \ No newline at end of file +export const MCP_ROUTE_SOURCE_VALUE = 'route'; \ No newline at end of file From edc4e3c2ed2485ceed01e44088c3d0133820787e Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 18:27:23 +0200 Subject: [PATCH 14/22] refactor(mcp-server): improve types --- packages/core/src/utils/mcp-server/types.ts | 396 +------------------- 1 file changed, 18 insertions(+), 378 deletions(-) diff --git a/packages/core/src/utils/mcp-server/types.ts b/packages/core/src/utils/mcp-server/types.ts index 1ec71be1cced..b008d088bb27 100644 --- a/packages/core/src/utils/mcp-server/types.ts +++ b/packages/core/src/utils/mcp-server/types.ts @@ -1,11 +1,7 @@ /** - * TypeScript type definitions for MCP server instrumentation + * types for MCP server instrumentation */ -// ============================================================================= -// JSON-RPC TYPES -// ============================================================================= - /** * JSON-RPC 2.0 request object */ @@ -16,16 +12,6 @@ export interface JsonRpcRequest { params?: Record; } -/** - * JSON-RPC 2.0 notification object - * Note: Notifications do NOT have an 'id' field - this is what distinguishes them from requests - */ -export interface JsonRpcNotification { - jsonrpc: '2.0'; - method: string; - params?: Record; -} - /** * JSON-RPC 2.0 response object */ @@ -46,13 +32,14 @@ export interface JsonRpcError { } /** - * Union type for all JSON-RPC message types + * JSON-RPC 2.0 notification object + * Note: Notifications do NOT have an 'id' field - this is what distinguishes them from requests */ -export type JsonRpcMessage = JsonRpcRequest | JsonRpcNotification | JsonRpcResponse; - -// ============================================================================= -// MCP TRANSPORT TYPES -// ============================================================================= +export interface JsonRpcNotification { + jsonrpc: '2.0'; + method: string; + params?: Record; +} /** * MCP transport interface @@ -80,6 +67,12 @@ export interface MCPTransport { sessionId?: string; } +/** + * Union type for all JSON-RPC message types + */ +export type JsonRpcMessage = JsonRpcRequest | JsonRpcNotification | JsonRpcResponse; + + /** * MCP server instance interface */ @@ -106,370 +99,17 @@ export interface MCPServerInstance { * Connect the server to a transport */ connect(transport: MCPTransport): Promise; - - /** - * Optional server configuration - */ - server?: { - setRequestHandler: (schema: unknown, handler: (...args: unknown[]) => unknown) => void; - }; -} - -// ============================================================================= -// SPAN AND ATTRIBUTE TYPES -// ============================================================================= - -/** - * Span attributes for MCP instrumentation - */ -export interface McpSpanAttributes { - // Core MCP attributes - 'mcp.method.name': string; - 'mcp.request.id'?: string; - 'mcp.session.id'?: string; - 'mcp.transport': string; - - // Method-specific attributes - 'mcp.tool.name'?: string; - 'mcp.resource.uri'?: string; - 'mcp.resource.name'?: string; - 'mcp.prompt.name'?: string; - 'mcp.resource.protocol'?: string; - - // Notification attributes - 'mcp.notification.direction'?: 'client_to_server' | 'server_to_client'; - 'mcp.cancelled.request_id'?: string; - 'mcp.cancelled.reason'?: string; - 'mcp.progress.token'?: string; - 'mcp.progress.current'?: number; - 'mcp.progress.total'?: number; - 'mcp.progress.percentage'?: number; - 'mcp.progress.message'?: string; - 'mcp.logging.level'?: string; - 'mcp.logging.logger'?: string; - 'mcp.logging.data_type'?: string; - 'mcp.logging.message'?: string; - - // Network attributes - 'network.transport': string; - 'network.protocol.version'?: string; - 'client.address'?: string; - 'client.port'?: number; - - // Error attributes - 'error.type'?: string; - 'rpc.jsonrpc.error_code'?: number; - - // Request arguments (dynamic keys) - [key: `mcp.request.argument.${string}`]: string; -} - -/** - * Transport types for MCP - */ -export type McpTransportType = 'stdio' | 'sse' | 'http' | 'websocket'; - -/** - * Network transport types - */ -export type NetworkTransportType = 'pipe' | 'tcp' | 'udp' | 'quic' | 'unix'; - -/** - * Transport type mapping result - */ -export interface TransportTypesResult { - mcpTransport: McpTransportType; - networkTransport: NetworkTransportType; -} - -// ============================================================================= -// SESSION AND REQUEST CORRELATION TYPES -// ============================================================================= - -/** - * Session identifier type - */ -export type SessionId = string; - -/** - * Request identifier type - */ -export type RequestId = string | number; - -/** - * Extra handler data with request correlation information - */ -export interface ExtraHandlerDataWithRequestId { - sessionId: SessionId; - requestId: RequestId; } -/** - * Extra data passed to message handlers - */ export interface ExtraHandlerData { - requestInfo?: { - remoteAddress?: string; - remotePort?: number; - }; + requestInfo?: { remoteAddress?: string; remotePort?: number }; clientAddress?: string; clientPort?: number; request?: { ip?: string; - connection?: { - remoteAddress?: string; - remotePort?: number; - }; - }; - sessionId?: SessionId; - requestId?: RequestId; -} - -// ============================================================================= -// MCP METHOD PARAMETER TYPES -// ============================================================================= - -/** - * Parameters for tools/call method - */ -export interface ToolCallParams { - name: string; - arguments?: Record; -} - -/** - * Parameters for resources/read method - */ -export interface ResourceReadParams { - uri: string; -} - -/** - * Parameters for resources/subscribe method - */ -export interface ResourceSubscribeParams { - uri: string; -} - -/** - * Parameters for resources/unsubscribe method - */ -export interface ResourceUnsubscribeParams { - uri: string; -} - -/** - * Parameters for prompts/get method - */ -export interface PromptGetParams { - name: string; - arguments?: Record; -} - -/** - * Parameters for notifications/cancelled - */ -export interface NotificationCancelledParams { - requestId: RequestId; - reason?: string; -} - -/** - * Parameters for notifications/progress - */ -export interface NotificationProgressParams { - progressToken: string; - progress?: number; - total?: number; - message?: string; -} - -/** - * Parameters for notifications/message - */ -export interface NotificationMessageParams { - level: string; - logger?: string; - data: unknown; -} - -/** - * Parameters for notifications/resources/updated - */ -export interface NotificationResourceUpdatedParams { - uri: string; -} - -// ============================================================================= -// UTILITY TYPES -// ============================================================================= - -/** - * Generic type for method parameters - */ -export type MethodParams = - | ToolCallParams - | ResourceReadParams - | ResourceSubscribeParams - | ResourceUnsubscribeParams - | PromptGetParams - | NotificationCancelledParams - | NotificationProgressParams - | NotificationMessageParams - | NotificationResourceUpdatedParams - | Record; - -/** - * Type guard function type - */ -export type TypeGuard = (value: unknown) => value is T; - -/** - * Callback function type for instrumentation - */ -export type InstrumentationCallback = () => T; - -/** - * Span creation configuration - */ -export interface SpanConfig { - name: string; - forceTransaction?: boolean; - attributes: Record; -} - -// ============================================================================= -// TRACE PROPAGATION TYPES -// ============================================================================= - -/** - * Trace data for propagation - */ -export interface TraceData { - 'sentry-trace'?: string; - baggage?: string; - traceparent?: string; // W3C format -} - -/** - * MCP trace metadata in params._meta - */ -export interface McpTraceMetadata { - 'sentry-trace'?: string; - baggage?: string; - traceparent?: string; // W3C format support -} - -/** - * Request with trace metadata - */ -export interface JsonRpcRequestWithTrace extends JsonRpcRequest { - params?: Record & { - _meta?: McpTraceMetadata; + connection?: { remoteAddress?: string; remotePort?: number }; }; } -/** - * Notification with trace metadata - */ -export interface JsonRpcNotificationWithTrace extends JsonRpcNotification { - params?: Record & { - _meta?: McpTraceMetadata; - }; -} - -// ============================================================================= -// CONFIGURATION TYPES -// ============================================================================= - -/** - * Options for MCP server instrumentation - */ -export interface McpServerInstrumentationOptions { - /** - * Whether to capture request arguments (security consideration) - * Default: false - */ - captureRequestArguments?: boolean; - - /** - * Which request arguments to capture (if captureRequestArguments is true) - * Default: [] (none) - */ - allowedRequestArguments?: string[]; - - /** - * Maximum length for logging message content - * Default: 1000 - */ - maxLoggingMessageLength?: number; - - /** - * Whether to capture client information (address, port) - * Default: true - */ - captureClientInfo?: boolean; - - /** - * Custom attribute extraction function - */ - customAttributeExtractor?: (method: string, params: MethodParams) => Record; -} - -// ============================================================================= -// ERROR TYPES -// ============================================================================= - -/** - * MCP-specific error interface - */ -export interface McpError extends Error { - code?: number; - data?: unknown; -} - -/** - * Tool execution result with error flag - */ -export interface CallToolResult { - content?: unknown; - isError?: boolean; - error?: string; -} - -// ============================================================================= -// EXPORT UTILITY TYPES -// ============================================================================= - -/** - * All MCP method names as a union type - */ -export type McpMethodName = - | 'initialize' - | 'ping' - | 'tools/list' - | 'tools/call' - | 'resources/list' - | 'resources/read' - | 'resources/subscribe' - | 'resources/unsubscribe' - | 'resources/templates/list' - | 'prompts/list' - | 'prompts/get' - | 'roots/list' - | 'completion/complete' - | 'sampling/createMessage' - | 'logging/setLevel' - | 'notifications/initialized' - | 'notifications/cancelled' - | 'notifications/message' - | 'notifications/prompts/list_changed' - | 'notifications/resources/list_changed' - | 'notifications/resources/updated' - | 'notifications/roots/list_changed' - | 'notifications/tools/list_changed'; - -/** - * JSON-RPC error codes as a union type - */ -export type JsonRpcErrorCode = -32700 | -32600 | -32601 | -32602 | -32603 | number; \ No newline at end of file +export type SessionId = string; +export type RequestId = string | number; From 62ca0f31d40320afa2d115a7ece29a8d4c8e41e8 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 2 Jul 2025 20:37:14 +0200 Subject: [PATCH 15/22] refactor(mcp-server): refactor span handling and utility functions for MCP server instrumentation --- packages/core/src/mcp-server.ts | 304 +++-------- .../core/src/utils/mcp-server/attributes.ts | 4 +- packages/core/src/utils/mcp-server/types.ts | 15 +- packages/core/src/utils/mcp-server/utils.ts | 504 ++++++++++++++++++ packages/core/test/lib/mcp-server.test.ts | 106 ++-- 5 files changed, 625 insertions(+), 308 deletions(-) create mode 100644 packages/core/src/utils/mcp-server/utils.ts diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 98f25b7a0bd9..d5bf2921081a 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -1,48 +1,17 @@ -import { - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from './semanticAttributes'; -import { startSpan, withActiveSpan } from './tracing'; import type { Span } from './types-hoist/span'; -import { getActiveSpan } from './utils/spanUtils'; -import { - MCP_METHOD_NAME_ATTRIBUTE, - MCP_REQUEST_ID_ATTRIBUTE, - MCP_SESSION_ID_ATTRIBUTE, - MCP_TRANSPORT_ATTRIBUTE, - NETWORK_TRANSPORT_ATTRIBUTE, - NETWORK_PROTOCOL_VERSION_ATTRIBUTE, - CLIENT_ADDRESS_ATTRIBUTE, - CLIENT_PORT_ATTRIBUTE, - MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, - MCP_SERVER_OP_VALUE, - MCP_FUNCTION_ORIGIN_VALUE, - MCP_NOTIFICATION_ORIGIN_VALUE, - MCP_ROUTE_SOURCE_VALUE, -} from './utils/mcp-server/attributes'; import type { - JsonRpcRequest, - JsonRpcNotification, - MCPTransport, + ExtraHandlerData, MCPServerInstance, + MCPTransport, SessionId, - RequestId, - ExtraHandlerData, } from './utils/mcp-server/types'; import { - isJsonRpcRequest, + createMcpNotificationSpan, + createMcpOutgoingNotificationSpan, + createMcpServerSpan, isJsonRpcNotification, - extractTarget, - getTargetAttributes, - getRequestArguments, - getTransportTypes, - getNotificationDescription, - getNotificationAttributes, - extractClientAddress, - extractClientPort, + isJsonRpcRequest, validateMcpServerInstance, - createSpanName, } from './utils/mcp-server/utils'; const wrappedMcpServerInstances = new WeakSet(); @@ -65,55 +34,56 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): const serverInstance = mcpServerInstance as MCPServerInstance; // Wrap connect() to intercept AFTER Protocol sets up transport handlers - serverInstance.connect = new Proxy(serverInstance.connect, { + const originalConnect = serverInstance.connect; + serverInstance.connect = new Proxy(originalConnect, { async apply(target, thisArg, argArray) { const [transport, ...restArgs] = argArray as [MCPTransport, ...unknown[]]; // Call the original connect first to let Protocol set up its handlers const result = await Reflect.apply(target, thisArg, [transport, ...restArgs]); - + // Intercept incoming messages via onmessage if (transport.onmessage) { const protocolOnMessage = transport.onmessage; - + transport.onmessage = new Proxy(protocolOnMessage, { apply(onMessageTarget, onMessageThisArg, onMessageArgs) { const [jsonRpcMessage, extra] = onMessageArgs; - + // Instrument both requests and notifications if (isJsonRpcRequest(jsonRpcMessage)) { return createMcpServerSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { - return onMessageTarget.apply(onMessageThisArg, onMessageArgs); + return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); }); } if (isJsonRpcNotification(jsonRpcMessage)) { return createMcpNotificationSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { - return onMessageTarget.apply(onMessageThisArg, onMessageArgs); + return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); }); } - - return onMessageTarget.apply(onMessageThisArg, onMessageArgs); - } + + return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); + }, }); } // Intercept outgoing messages via send if (transport.send) { const originalSend = transport.send; - + transport.send = new Proxy(originalSend, { async apply(sendTarget, sendThisArg, sendArgs) { const [message, options] = sendArgs; - + // Instrument outgoing notifications (but not requests/responses) if (isJsonRpcNotification(message)) { return createMcpOutgoingNotificationSpan(message, transport, options as Record, () => { - return sendTarget.apply(sendThisArg, sendArgs); + return Reflect.apply(sendTarget, sendThisArg, sendArgs); }); } - - return sendTarget.apply(sendThisArg, sendArgs); - } + + return Reflect.apply(sendTarget, sendThisArg, sendArgs); + }, }); } @@ -125,8 +95,8 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): if (transport.sessionId) { handleTransportOnClose(transport.sessionId); } - return onCloseTarget.apply(onCloseThisArg, onCloseArgs); - } + return Reflect.apply(onCloseTarget, onCloseThisArg, onCloseArgs); + }, }); } return result; @@ -137,203 +107,49 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): return mcpServerInstance as S; } -function createMcpServerSpan( - jsonRpcMessage: JsonRpcRequest, - transport: MCPTransport, - extra: ExtraHandlerData, - callback: () => unknown -) { - const { method, id: requestId, params } = jsonRpcMessage; - - // Extract target from method and params for proper description - const target = extractTarget(method, params as Record); - const description = createSpanName(method, target); - - // Session ID should come from the transport itself, not the RPC message - const sessionId = transport.sessionId; - - // Extract client information from extra/request data - const clientAddress = extractClientAddress(extra); - const clientPort = extractClientPort(extra); - - // Determine transport types - const { mcpTransport, networkTransport } = getTransportTypes(transport); - - const attributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - - ...(requestId !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(requestId) }), - ...(target && getTargetAttributes(method, target)), - ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), - ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), - ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), - [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, // Application level: "http", "sse", "stdio", "websocket" - [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, // Network level: "tcp", "pipe", "udp", "quic" - [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', // JSON-RPC version - - // Opt-in: Tool arguments (if enabled) - ...getRequestArguments(method, params as Record), - - // Sentry-specific attributes - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_FUNCTION_ORIGIN_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE - }; - - return startSpan({ - name: description, - forceTransaction: true, - attributes - }, () => { - // TODO(bete): add proper error handling. Handle JSON RPC errors in the result - return callback(); - }); -} - -function createMcpNotificationSpan( - jsonRpcMessage: JsonRpcNotification, - transport: MCPTransport, - extra: ExtraHandlerData, - callback: () => unknown -) { - const { method, params } = jsonRpcMessage; - - const description = getNotificationDescription(method, params as Record); - - const sessionId = transport.sessionId; - - // Extract client information - const clientAddress = extractClientAddress(extra); - const clientPort = extractClientPort(extra); - - // Determine transport types - const { mcpTransport, networkTransport } = getTransportTypes(transport); - - const notificationAttribs = getNotificationAttributes(method, params as Record); - - const attributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'client_to_server', // Incoming notification - - ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), - ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), - ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), - [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, - [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, - [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', - - // Notification-specific attributes - ...notificationAttribs, - - // Sentry-specific attributes - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_NOTIFICATION_ORIGIN_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE - }; - - return startSpan({ - name: description, - forceTransaction: true, - attributes - }, () => { - const result = callback(); - return result; - }); -} - -function createMcpOutgoingNotificationSpan( - jsonRpcMessage: JsonRpcNotification, - transport: MCPTransport, - options: Record, - callback: () => unknown -) { - const { method, params } = jsonRpcMessage; - - const description = getNotificationDescription(method, params as Record); - - const sessionId = transport.sessionId; - - // Determine transport types - const { mcpTransport, networkTransport } = getTransportTypes(transport); - - const notificationAttribs = getNotificationAttributes(method, params as Record); - - const attributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'server_to_client', // Outgoing notification - - ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), - [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, - [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, - [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', - - // Notification-specific attributes - ...notificationAttribs, - - // Sentry-specific attributes - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MCP_NOTIFICATION_ORIGIN_VALUE, - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE - }; - - return startSpan({ - name: description, - forceTransaction: true, - attributes - }, () => { - const result = callback(); - return result; - }); -} - // ============================================================================= // SESSION AND REQUEST CORRELATION (Legacy support) // ============================================================================= -interface ExtraHandlerDataWithRequestId { - sessionId: SessionId; - requestId: RequestId; -} - -const sessionAndRequestToRequestParentSpanMap = new Map>(); +const sessionAndRequestToRequestParentSpanMap = new Map>(); function handleTransportOnClose(sessionId: SessionId): void { sessionAndRequestToRequestParentSpanMap.delete(sessionId); } // TODO(bete): refactor this and associateContextWithRequestSpan to use the new span API. -function handleTransportOnMessage(sessionId: SessionId, requestId: RequestId): void { - const activeSpan = getActiveSpan(); - if (activeSpan) { - const requestIdToSpanMap = sessionAndRequestToRequestParentSpanMap.get(sessionId) ?? new Map(); - requestIdToSpanMap.set(requestId, activeSpan); - sessionAndRequestToRequestParentSpanMap.set(sessionId, requestIdToSpanMap); - } -} - -function associateContextWithRequestSpan( - extraHandlerData: ExtraHandlerDataWithRequestId | undefined, - cb: () => T, -): T { - if (extraHandlerData) { - const { sessionId, requestId } = extraHandlerData; - const requestIdSpanMap = sessionAndRequestToRequestParentSpanMap.get(sessionId); - - if (!requestIdSpanMap) { - return cb(); - } - - const span = requestIdSpanMap.get(requestId); - if (!span) { - return cb(); - } - - // remove the span from the map so it can be garbage collected - requestIdSpanMap.delete(requestId); - return withActiveSpan(span, () => { - return cb(); - }); - } - - return cb(); -} +// function handleTransportOnMessage(sessionId: SessionId, requestId: string): void { +// const activeSpan = getActiveSpan(); +// if (activeSpan) { +// const requestIdToSpanMap = sessionAndRequestToRequestParentSpanMap.get(sessionId) ?? new Map(); +// requestIdToSpanMap.set(requestId, activeSpan); +// sessionAndRequestToRequestParentSpanMap.set(sessionId, requestIdToSpanMap); +// } +// } + +// function associateContextWithRequestSpan( +// extraHandlerData: { sessionId: SessionId; requestId: string } | undefined, +// cb: () => T, +// ): T { +// if (extraHandlerData) { +// const { sessionId, requestId } = extraHandlerData; +// const requestIdSpanMap = sessionAndRequestToRequestParentSpanMap.get(sessionId); + +// if (!requestIdSpanMap) { +// return cb(); +// } + +// const span = requestIdSpanMap.get(requestId); +// if (!span) { +// return cb(); +// } + +// // remove the span from the map so it can be garbage collected +// requestIdSpanMap.delete(requestId); +// return withActiveSpan(span, () => { +// return cb(); +// }); +// } + +// return cb(); +// } diff --git a/packages/core/src/utils/mcp-server/attributes.ts b/packages/core/src/utils/mcp-server/attributes.ts index bd848ede0b9f..7cd7da852c1f 100644 --- a/packages/core/src/utils/mcp-server/attributes.ts +++ b/packages/core/src/utils/mcp-server/attributes.ts @@ -1,6 +1,6 @@ /** * Essential MCP attribute constants for Sentry instrumentation - * + * * Based on OpenTelemetry MCP semantic conventions * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md */ @@ -115,4 +115,4 @@ export const MCP_NOTIFICATION_ORIGIN_VALUE = 'auto.mcp.notification'; /** * Sentry source value for MCP route spans */ -export const MCP_ROUTE_SOURCE_VALUE = 'route'; \ No newline at end of file +export const MCP_ROUTE_SOURCE_VALUE = 'route'; diff --git a/packages/core/src/utils/mcp-server/types.ts b/packages/core/src/utils/mcp-server/types.ts index b008d088bb27..61082116fd30 100644 --- a/packages/core/src/utils/mcp-server/types.ts +++ b/packages/core/src/utils/mcp-server/types.ts @@ -50,17 +50,17 @@ export interface MCPTransport { * The first argument is a JSON RPC message */ onmessage?: (...args: unknown[]) => void; - + /** * Close handler for transport lifecycle */ onclose?: (...args: unknown[]) => void; - + /** * Send method for outgoing messages */ send?: (message: JsonRpcMessage, options?: Record) => Promise; - + /** * Optional session identifier */ @@ -72,7 +72,6 @@ export interface MCPTransport { */ export type JsonRpcMessage = JsonRpcRequest | JsonRpcNotification | JsonRpcResponse; - /** * MCP server instance interface */ @@ -82,19 +81,19 @@ export interface MCPServerInstance { * The first arg is always a name, the last arg should always be a callback function (ie a handler). */ resource: (name: string, ...args: unknown[]) => void; - + /** * Register a tool handler * The first arg is always a name, the last arg should always be a callback function (ie a handler). */ tool: (name: string, ...args: unknown[]) => void; - + /** * Register a prompt handler * The first arg is always a name, the last arg should always be a callback function (ie a handler). */ prompt: (name: string, ...args: unknown[]) => void; - + /** * Connect the server to a transport */ @@ -112,4 +111,4 @@ export interface ExtraHandlerData { } export type SessionId = string; -export type RequestId = string | number; +export type RequestId = string | number; diff --git a/packages/core/src/utils/mcp-server/utils.ts b/packages/core/src/utils/mcp-server/utils.ts new file mode 100644 index 000000000000..7821ba039c14 --- /dev/null +++ b/packages/core/src/utils/mcp-server/utils.ts @@ -0,0 +1,504 @@ +/** + * Essential utility functions for MCP server instrumentation + */ + +import { DEBUG_BUILD } from '../../debug-build'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '../../semanticAttributes'; +import { startSpan } from '../../tracing'; +import { logger } from '../logger'; +import { + CLIENT_ADDRESS_ATTRIBUTE, + CLIENT_PORT_ATTRIBUTE, + MCP_FUNCTION_ORIGIN_VALUE, + MCP_METHOD_NAME_ATTRIBUTE, + MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, + MCP_NOTIFICATION_ORIGIN_VALUE, + MCP_REQUEST_ID_ATTRIBUTE, + MCP_ROUTE_SOURCE_VALUE, + MCP_SERVER_OP_VALUE, + MCP_SESSION_ID_ATTRIBUTE, + MCP_TRANSPORT_ATTRIBUTE, + NETWORK_PROTOCOL_VERSION_ATTRIBUTE, + NETWORK_TRANSPORT_ATTRIBUTE, +} from './attributes'; +import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, MCPTransport } from './types'; + +// ============================================================================= +// TYPE GUARDS +// ============================================================================= + +/** + * + */ +export function isJsonRpcRequest(message: unknown): message is JsonRpcRequest { + return ( + typeof message === 'object' && + message !== null && + 'jsonrpc' in message && + (message as JsonRpcRequest).jsonrpc === '2.0' && + 'method' in message && + 'id' in message + ); +} + +/** + * + */ +export function isJsonRpcNotification(message: unknown): message is JsonRpcNotification { + return ( + typeof message === 'object' && + message !== null && + 'jsonrpc' in message && + (message as JsonRpcNotification).jsonrpc === '2.0' && + 'method' in message && + !('id' in message) + ); +} + +/** + * + */ +export function validateMcpServerInstance(instance: unknown): boolean { + if ( + typeof instance === 'object' && + instance !== null && + 'resource' in instance && + 'tool' in instance && + 'prompt' in instance && + 'connect' in instance + ) { + return true; + } + DEBUG_BUILD && logger.warn('Did not patch MCP server. Interface is incompatible.'); + return false; +} + +// ============================================================================= +// ATTRIBUTE EXTRACTION +// ============================================================================= + +/** + * + */ +export function extractTarget(method: string, params: Record): string | undefined { + switch (method) { + case 'tools/call': + return typeof params?.name === 'string' ? params.name : undefined; + case 'resources/read': + case 'resources/subscribe': + case 'resources/unsubscribe': + return typeof params?.uri === 'string' ? params.uri : undefined; + case 'prompts/get': + return typeof params?.name === 'string' ? params.name : undefined; + default: + return undefined; + } +} + +/** + * + */ +export function getTargetAttributes(method: string, target: string): Record { + switch (method) { + case 'tools/call': + return { 'mcp.tool.name': target }; + case 'resources/read': + case 'resources/subscribe': + case 'resources/unsubscribe': + return { 'mcp.resource.uri': target }; + case 'prompts/get': + return { 'mcp.prompt.name': target }; + default: + return {}; + } +} + +/** + * + */ +export function getRequestArguments(method: string, params: Record): Record { + const args: Record = {}; + + // Argument capture for different methods + switch (method) { + case 'tools/call': + if (params?.arguments && typeof params.arguments === 'object') { + for (const [key, value] of Object.entries(params.arguments as Record)) { + args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); + } + } + break; + case 'resources/read': + if (params?.uri) { + args['mcp.request.argument.uri'] = JSON.stringify(params.uri); + } + break; + case 'prompts/get': + if (params?.name) { + args['mcp.request.argument.name'] = JSON.stringify(params.name); + } + if (params?.arguments && typeof params.arguments === 'object') { + for (const [key, value] of Object.entries(params.arguments as Record)) { + args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); + } + } + break; + } + + return args; +} + +// ============================================================================= +// TRANSPORT DETECTION +// ============================================================================= + +/** + * + */ +export function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { + const transportName = transport.constructor?.name?.toLowerCase() || ''; + + if (transportName.includes('sse')) return { mcpTransport: 'sse', networkTransport: 'tcp' }; + if (transportName.includes('websocket')) return { mcpTransport: 'websocket', networkTransport: 'tcp' }; + if (transportName.includes('stdio')) return { mcpTransport: 'stdio', networkTransport: 'pipe' }; + + return { mcpTransport: 'http', networkTransport: 'tcp' }; +} + +// ============================================================================= +// NOTIFICATION HANDLING +// ============================================================================= + +/** + * + */ +export function getNotificationDescription(method: string, params: Record): string { + // Enhanced description with target information + switch (method) { + case 'notifications/message': + // For logging messages, include logger in description + if (params?.logger && typeof params.logger === 'string') { + return `${method} logger:${params.logger}`; + } + return method; + case 'notifications/cancelled': + // For cancelled notifications, include request ID if available + if (params?.requestId) { + return `${method} request:${params.requestId}`; + } + return method; + case 'notifications/progress': + // For progress notifications, include progress token if available + if (params?.progressToken) { + return `${method} token:${params.progressToken}`; + } + return method; + case 'notifications/resources/updated': + // For resource updates, include URI + if (params?.uri && typeof params.uri === 'string') { + return `${method} ${params.uri}`; + } + return method; + default: + return method; + } +} + +/** + * + */ +export function getNotificationAttributes( + method: string, + params: Record, +): Record { + const attributes: Record = {}; + + // Comprehensive notification attributes + switch (method) { + case 'notifications/cancelled': + if (params?.requestId) { + attributes['mcp.cancelled.request_id'] = String(params.requestId); + } + if (params?.reason) { + attributes['mcp.cancelled.reason'] = String(params.reason); + } + break; + + case 'notifications/message': + // Logging-specific attributes + if (params?.level) { + attributes['mcp.logging.level'] = String(params.level); + } + if (params?.logger) { + attributes['mcp.logging.logger'] = String(params.logger); + } + if (params?.data !== undefined) { + attributes['mcp.logging.data_type'] = typeof params.data; + // Store the actual message content + if (typeof params.data === 'string') { + attributes['mcp.logging.message'] = params.data; + } else { + attributes['mcp.logging.message'] = JSON.stringify(params.data); + } + } + break; + + case 'notifications/progress': + if (params?.progressToken) { + attributes['mcp.progress.token'] = String(params.progressToken); + } + if (typeof params?.progress === 'number') { + attributes['mcp.progress.current'] = params.progress; + } + if (typeof params?.total === 'number') { + attributes['mcp.progress.total'] = params.total; + if (typeof params?.progress === 'number') { + attributes['mcp.progress.percentage'] = (params.progress / params.total) * 100; + } + } + if (params?.message) { + attributes['mcp.progress.message'] = String(params.message); + } + break; + + case 'notifications/resources/updated': + if (params?.uri) { + attributes['mcp.resource.uri'] = String(params.uri); + // Extract protocol from URI + try { + const url = new URL(String(params.uri)); + attributes['mcp.resource.protocol'] = url.protocol; + } catch { + // Ignore invalid URIs + } + } + break; + + case 'notifications/initialized': + // Mark as lifecycle event + attributes['mcp.lifecycle.phase'] = 'initialization_complete'; + attributes['mcp.protocol.ready'] = 1; + break; + } + + return attributes; +} + +// ============================================================================= +// CLIENT INFO EXTRACTION +// ============================================================================= + +/** + * + */ +export function extractClientAddress(extra: ExtraHandlerData): string | undefined { + return ( + extra?.requestInfo?.remoteAddress || + extra?.clientAddress || + extra?.request?.ip || + extra?.request?.connection?.remoteAddress + ); +} + +/** + * + */ +export function extractClientPort(extra: ExtraHandlerData): number | undefined { + return extra?.requestInfo?.remotePort || extra?.clientPort || extra?.request?.connection?.remotePort; +} + +// ============================================================================= +// SPAN NAMING +// ============================================================================= + +/** + * + */ +export function createSpanName(method: string, target?: string): string { + return target ? `${method} ${target}` : method; +} + +// ============================================================================= +// SPAN ATTRIBUTE BUILDERS +// ============================================================================= + +/** + * Builds base span attributes common to all MCP span types + */ +export function buildBaseSpanAttributes( + transport: MCPTransport, + extra?: ExtraHandlerData, +): Record { + // Session ID should come from the transport itself, not the RPC message + const sessionId = transport.sessionId; + + // Extract client information from extra/request data (if provided) + const clientAddress = extra ? extractClientAddress(extra) : undefined; + const clientPort = extra ? extractClientPort(extra) : undefined; + + // Determine transport types + const { mcpTransport, networkTransport } = getTransportTypes(transport); + + return { + ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), + ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), + ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, // Application level: "http", "sse", "stdio", "websocket" + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, // Network level: "tcp", "pipe", "udp", "quic" + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', // JSON-RPC version + }; +} + +/** + * Builds Sentry-specific span attributes + */ +export function buildSentrySpanAttributes(origin: string): Record { + return { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE, + }; +} + +// ============================================================================= +// SPAN CREATION FUNCTIONS +// ============================================================================= + +/** + * Creates a span for MCP server request handling + */ +export function createMcpServerSpan( + jsonRpcMessage: JsonRpcRequest, + transport: MCPTransport, + extra: ExtraHandlerData, + callback: () => unknown, +): unknown { + const { method, id: requestId, params } = jsonRpcMessage; + + // Extract target from method and params for proper description + const target = extractTarget(method, params as Record); + const description = createSpanName(method, target); + + // Build base attributes using shared builder + const baseAttributes = buildBaseSpanAttributes(transport, extra); + + // Build request-specific attributes + const requestAttributes: Record = { + [MCP_METHOD_NAME_ATTRIBUTE]: method, + ...(requestId !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(requestId) }), + ...(target && getTargetAttributes(method, target)), + // Opt-in: Tool arguments (if enabled) + ...getRequestArguments(method, params as Record), + }; + + // Build Sentry-specific attributes using shared builder + const sentryAttributes = buildSentrySpanAttributes(MCP_FUNCTION_ORIGIN_VALUE); + + return startSpan( + { + name: description, + forceTransaction: true, + attributes: { + ...baseAttributes, + ...requestAttributes, + ...sentryAttributes, + }, + }, + () => { + // TODO(bete): add proper error handling. Handle JSON RPC errors in the result + return callback(); + }, + ); +} + +/** + * Creates a span for incoming MCP notifications + */ +export function createMcpNotificationSpan( + jsonRpcMessage: JsonRpcNotification, + transport: MCPTransport, + extra: ExtraHandlerData, + callback: () => unknown, +): unknown { + const { method, params } = jsonRpcMessage; + + const description = getNotificationDescription(method, params as Record); + + // Build base attributes using shared builder + const baseAttributes = buildBaseSpanAttributes(transport, extra); + + // Build notification-specific attributes + const notificationAttributes: Record = { + [MCP_METHOD_NAME_ATTRIBUTE]: method, + [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'client_to_server', // Incoming notification + // Notification-specific attributes + ...getNotificationAttributes(method, params as Record), + }; + + // Build Sentry-specific attributes using shared builder + const sentryAttributes = buildSentrySpanAttributes(MCP_NOTIFICATION_ORIGIN_VALUE); + + return startSpan( + { + name: description, + forceTransaction: true, + attributes: { + ...baseAttributes, + ...notificationAttributes, + ...sentryAttributes, + }, + }, + () => { + const result = callback(); + return result; + }, + ); +} + +/** + * Creates a span for outgoing MCP notifications + */ +export function createMcpOutgoingNotificationSpan( + jsonRpcMessage: JsonRpcNotification, + transport: MCPTransport, + options: Record, + callback: () => unknown, +): unknown { + const { method, params } = jsonRpcMessage; + + const description = getNotificationDescription(method, params as Record); + + // Build base attributes using shared builder (no client info for outgoing notifications) + const baseAttributes = buildBaseSpanAttributes(transport); + + // Build notification-specific attributes + const notificationAttributes: Record = { + [MCP_METHOD_NAME_ATTRIBUTE]: method, + [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'server_to_client', // Outgoing notification + // Notification-specific attributes + ...getNotificationAttributes(method, params as Record), + }; + + // Build Sentry-specific attributes using shared builder + const sentryAttributes = buildSentrySpanAttributes(MCP_NOTIFICATION_ORIGIN_VALUE); + + return startSpan( + { + name: description, + forceTransaction: true, + attributes: { + ...baseAttributes, + ...notificationAttributes, + ...sentryAttributes, + }, + }, + () => { + const result = callback(); + return result; + }, + ); +} diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 1cfd0eecf2b6..48121d3dbb6e 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -4,7 +4,7 @@ import * as tracingModule from '../../src/tracing'; describe('wrapMcpServerWithSentry', () => { const startSpanSpy = vi.spyOn(tracingModule, 'startSpan'); - + beforeEach(() => { vi.clearAllMocks(); }); @@ -36,7 +36,7 @@ describe('wrapMcpServerWithSentry', () => { it('should not wrap the same instance twice', () => { const mockMcpServer = createMockMcpServer(); - + const wrappedOnce = wrapMcpServerWithSentry(mockMcpServer); const wrappedTwice = wrapMcpServerWithSentry(wrappedOnce); @@ -59,54 +59,54 @@ describe('wrapMcpServerWithSentry', () => { // We need to test this before connection, so create fresh instances const freshMockMcpServer = createMockMcpServer(); const originalConnect = freshMockMcpServer.connect; - + const freshWrappedMcpServer = wrapMcpServerWithSentry(freshMockMcpServer); - + expect(freshWrappedMcpServer.connect).not.toBe(originalConnect); }); it('should intercept transport onmessage handler', async () => { const originalOnMessage = mockTransport.onmessage; - + await wrappedMcpServer.connect(mockTransport); - + // onmessage should be wrapped after connection expect(mockTransport.onmessage).not.toBe(originalOnMessage); }); it('should intercept transport send handler', async () => { const originalSend = mockTransport.send; - + await wrappedMcpServer.connect(mockTransport); - - // send should be wrapped after connection + + // send should be wrapped after connection expect(mockTransport.send).not.toBe(originalSend); }); it('should intercept transport onclose handler', async () => { const originalOnClose = mockTransport.onclose; - + await wrappedMcpServer.connect(mockTransport); - + // onclose should be wrapped after connection expect(mockTransport.onclose).not.toBe(originalOnClose); }); it('should call original connect and preserve functionality', async () => { await wrappedMcpServer.connect(mockTransport); - + // Original connect should have been called expect(mockMcpServer.connect).toHaveBeenCalledWith(mockTransport); }); it('should create spans for incoming JSON-RPC requests', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/call', id: 'req-1', - params: { name: 'get-weather' } + params: { name: 'get-weather' }, }; // Simulate incoming message @@ -117,13 +117,13 @@ describe('wrapMcpServerWithSentry', () => { name: 'tools/call get-weather', forceTransaction: true, }), - expect.any(Function) + expect.any(Function), ); }); it('should create spans for incoming JSON-RPC notifications', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcNotification = { jsonrpc: '2.0', method: 'notifications/initialized', @@ -138,13 +138,13 @@ describe('wrapMcpServerWithSentry', () => { name: 'notifications/initialized', forceTransaction: true, }), - expect.any(Function) + expect.any(Function), ); }); it('should create spans for outgoing notifications', async () => { await wrappedMcpServer.connect(mockTransport); - + const outgoingNotification = { jsonrpc: '2.0', method: 'notifications/tools/list_changed', @@ -159,13 +159,13 @@ describe('wrapMcpServerWithSentry', () => { name: 'notifications/tools/list_changed', forceTransaction: true, }), - expect.any(Function) + expect.any(Function), ); }); it('should not create spans for non-JSON-RPC messages', async () => { await wrappedMcpServer.connect(mockTransport); - + // Simulate non-JSON-RPC message mockTransport.onmessage?.({ some: 'data' }, {}); @@ -175,7 +175,7 @@ describe('wrapMcpServerWithSentry', () => { it('should handle transport onclose events', async () => { await wrappedMcpServer.connect(mockTransport); mockTransport.sessionId = 'test-session-123'; - + // Trigger onclose - should not throw expect(() => mockTransport.onclose?.()).not.toThrow(); }); @@ -196,19 +196,19 @@ describe('wrapMcpServerWithSentry', () => { it('should create spans with correct MCP server semantic attributes for tool operations', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/call', id: 'req-1', - params: { name: 'get-weather', arguments: { location: 'Seattle, WA' }} + params: { name: 'get-weather', arguments: { location: 'Seattle, WA' } }, }; const extraWithClientInfo = { requestInfo: { remoteAddress: '192.168.1.100', - remotePort: 54321 - } + remotePort: 54321, + }, }; mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo); @@ -233,18 +233,18 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }, }, - expect.any(Function) + expect.any(Function), ); }); it('should create spans with correct attributes for resource operations', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcRequest = { jsonrpc: '2.0', method: 'resources/read', id: 'req-2', - params: { uri: 'file:///docs/api.md' } + params: { uri: 'file:///docs/api.md' }, }; mockTransport.onmessage?.(jsonRpcRequest, {}); @@ -267,18 +267,18 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }, }, - expect.any(Function) + expect.any(Function), ); }); it('should create spans with correct attributes for prompt operations', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcRequest = { jsonrpc: '2.0', method: 'prompts/get', id: 'req-3', - params: { name: 'analyze-code' } + params: { name: 'analyze-code' }, }; mockTransport.onmessage?.(jsonRpcRequest, {}); @@ -301,17 +301,17 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }, }, - expect.any(Function) + expect.any(Function), ); }); it('should create spans with correct attributes for notifications (no request id)', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcNotification = { jsonrpc: '2.0', method: 'notifications/tools/list_changed', - params: {} + params: {}, }; mockTransport.onmessage?.(jsonRpcNotification, {}); @@ -332,7 +332,7 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }, }, - expect.any(Function) + expect.any(Function), ); // Should not include mcp.request.id for notifications @@ -344,12 +344,12 @@ describe('wrapMcpServerWithSentry', () => { it('should create spans for list operations without target in name', async () => { await wrappedMcpServer.connect(mockTransport); - + const jsonRpcRequest = { jsonrpc: '2.0', method: 'tools/list', id: 'req-4', - params: {} + params: {}, }; mockTransport.onmessage?.(jsonRpcRequest, {}); @@ -362,7 +362,7 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.method.name': 'tools/list', 'mcp.request.id': 'req-4', 'mcp.session.id': 'test-session-123', - // Transport attributes + // Transport attributes 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', @@ -372,21 +372,21 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }), }), - expect.any(Function) + expect.any(Function), ); }); it('should create spans with logging attributes for notifications/message', async () => { await wrappedMcpServer.connect(mockTransport); - + const loggingNotification = { jsonrpc: '2.0', method: 'notifications/message', params: { level: 'info', logger: 'math-service', - data: 'Addition completed: 2 + 5 = 7' - } + data: 'Addition completed: 2 + 5 = 7', + }, }; mockTransport.onmessage?.(loggingNotification, {}); @@ -411,7 +411,7 @@ describe('wrapMcpServerWithSentry', () => { 'sentry.source': 'route', }, }, - expect.any(Function) + expect.any(Function), ); }); @@ -424,8 +424,8 @@ describe('wrapMcpServerWithSentry', () => { method: 'notifications/cancelled', params: { requestId: 'req-123', - reason: 'user_requested' - } + reason: 'user_requested', + }, }; mockTransport.onmessage?.(cancelledNotification, {}); @@ -440,7 +440,7 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.notification.direction': 'client_to_server', }), }), - expect.any(Function) + expect.any(Function), ); vi.clearAllMocks(); @@ -453,8 +453,8 @@ describe('wrapMcpServerWithSentry', () => { progressToken: 'token-456', progress: 75, total: 100, - message: 'Processing files...' - } + message: 'Processing files...', + }, }; mockTransport.onmessage?.(progressNotification, {}); @@ -472,7 +472,7 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.notification.direction': 'client_to_server', }), }), - expect.any(Function) + expect.any(Function), ); vi.clearAllMocks(); @@ -482,8 +482,8 @@ describe('wrapMcpServerWithSentry', () => { jsonrpc: '2.0', method: 'notifications/resources/updated', params: { - uri: 'file:///tmp/data.json' - } + uri: 'file:///tmp/data.json', + }, }; mockTransport.onmessage?.(resourceUpdatedNotification, {}); @@ -498,11 +498,9 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.notification.direction': 'client_to_server', }), }), - expect.any(Function) + expect.any(Function), ); }); - - }); }); From 08c39f1b3e18c8d645a1ef1cd80d75226d1533ad Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 13:34:40 +0200 Subject: [PATCH 16/22] remove unused import and comment legacy support --- packages/core/src/mcp-server.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index d5bf2921081a..5c6886d0d73d 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -1,9 +1,7 @@ -import type { Span } from './types-hoist/span'; import type { ExtraHandlerData, MCPServerInstance, MCPTransport, - SessionId, } from './utils/mcp-server/types'; import { createMcpNotificationSpan, @@ -92,9 +90,10 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): const originalOnClose = transport.onclose; transport.onclose = new Proxy(originalOnClose, { apply(onCloseTarget, onCloseThisArg, onCloseArgs) { - if (transport.sessionId) { - handleTransportOnClose(transport.sessionId); - } + //TODO(bete): session and request correlation (methods at the bottom of this file) + // if (transport.sessionId) { + // handleTransportOnClose(transport.sessionId); + // } return Reflect.apply(onCloseTarget, onCloseThisArg, onCloseArgs); }, }); @@ -111,11 +110,11 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): // SESSION AND REQUEST CORRELATION (Legacy support) // ============================================================================= -const sessionAndRequestToRequestParentSpanMap = new Map>(); +// const sessionAndRequestToRequestParentSpanMap = new Map>(); -function handleTransportOnClose(sessionId: SessionId): void { - sessionAndRequestToRequestParentSpanMap.delete(sessionId); -} +// function handleTransportOnClose(sessionId: SessionId): void { +// sessionAndRequestToRequestParentSpanMap.delete(sessionId); +// } // TODO(bete): refactor this and associateContextWithRequestSpan to use the new span API. // function handleTransportOnMessage(sessionId: SessionId, requestId: string): void { From fe2c865fa376b2f247b2fb69a91f6555bcb5b7ac Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 17:37:02 +0200 Subject: [PATCH 17/22] refactor(mcp-server): improve notification span handling and set attribute names to match OTEL draft semantic convention --- .../core/src/utils/mcp-server/attributes.ts | 22 +- packages/core/src/utils/mcp-server/utils.ts | 281 ++++++++---------- packages/core/test/lib/mcp-server.test.ts | 50 +++- 3 files changed, 179 insertions(+), 174 deletions(-) diff --git a/packages/core/src/utils/mcp-server/attributes.ts b/packages/core/src/utils/mcp-server/attributes.ts index 7cd7da852c1f..1df3952401d3 100644 --- a/packages/core/src/utils/mcp-server/attributes.ts +++ b/packages/core/src/utils/mcp-server/attributes.ts @@ -55,16 +55,6 @@ export const MCP_RESOURCE_URI_ATTRIBUTE = 'mcp.resource.uri'; */ export const MCP_PROMPT_NAME_ATTRIBUTE = 'mcp.prompt.name'; -// ============================================================================= -// NOTIFICATION ATTRIBUTES -// ============================================================================= - -/** - * Direction of the notification (client_to_server or server_to_client) - * @see https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md#notification-attributes - */ -export const MCP_NOTIFICATION_DIRECTION_ATTRIBUTE = 'mcp.notification.direction'; - // ============================================================================= // NETWORK ATTRIBUTES (OpenTelemetry Standard) // ============================================================================= @@ -102,6 +92,18 @@ export const CLIENT_PORT_ATTRIBUTE = 'client.port'; */ export const MCP_SERVER_OP_VALUE = 'mcp.server'; +/** + * Sentry operation value for client-to-server notifications + * Following OpenTelemetry MCP semantic conventions + */ +export const MCP_NOTIFICATION_CLIENT_TO_SERVER_OP_VALUE = 'mcp.notification.client_to_server'; + +/** + * Sentry operation value for server-to-client notifications + * Following OpenTelemetry MCP semantic conventions + */ +export const MCP_NOTIFICATION_SERVER_TO_CLIENT_OP_VALUE = 'mcp.notification.server_to_client'; + /** * Sentry origin value for MCP function spans */ diff --git a/packages/core/src/utils/mcp-server/utils.ts b/packages/core/src/utils/mcp-server/utils.ts index 7821ba039c14..6c30a0862304 100644 --- a/packages/core/src/utils/mcp-server/utils.ts +++ b/packages/core/src/utils/mcp-server/utils.ts @@ -15,8 +15,9 @@ import { CLIENT_PORT_ATTRIBUTE, MCP_FUNCTION_ORIGIN_VALUE, MCP_METHOD_NAME_ATTRIBUTE, - MCP_NOTIFICATION_DIRECTION_ATTRIBUTE, + MCP_NOTIFICATION_CLIENT_TO_SERVER_OP_VALUE, MCP_NOTIFICATION_ORIGIN_VALUE, + MCP_NOTIFICATION_SERVER_TO_CLIENT_OP_VALUE, MCP_REQUEST_ID_ATTRIBUTE, MCP_ROUTE_SOURCE_VALUE, MCP_SERVER_OP_VALUE, @@ -174,42 +175,15 @@ export function getTransportTypes(transport: MCPTransport): { mcpTransport: stri // ============================================================================= /** - * + * Get notification span name following OpenTelemetry conventions + * For notifications, we use the method name directly as per JSON-RPC conventions */ -export function getNotificationDescription(method: string, params: Record): string { - // Enhanced description with target information - switch (method) { - case 'notifications/message': - // For logging messages, include logger in description - if (params?.logger && typeof params.logger === 'string') { - return `${method} logger:${params.logger}`; - } - return method; - case 'notifications/cancelled': - // For cancelled notifications, include request ID if available - if (params?.requestId) { - return `${method} request:${params.requestId}`; - } - return method; - case 'notifications/progress': - // For progress notifications, include progress token if available - if (params?.progressToken) { - return `${method} token:${params.progressToken}`; - } - return method; - case 'notifications/resources/updated': - // For resource updates, include URI - if (params?.uri && typeof params.uri === 'string') { - return `${method} ${params.uri}`; - } - return method; - default: - return method; - } +export function getNotificationSpanName(method: string): string { + return method; } /** - * + * Extract additional attributes for specific notification types */ export function getNotificationAttributes( method: string, @@ -217,7 +191,6 @@ export function getNotificationAttributes( ): Record { const attributes: Record = {}; - // Comprehensive notification attributes switch (method) { case 'notifications/cancelled': if (params?.requestId) { @@ -229,7 +202,6 @@ export function getNotificationAttributes( break; case 'notifications/message': - // Logging-specific attributes if (params?.level) { attributes['mcp.logging.level'] = String(params.level); } @@ -279,7 +251,6 @@ export function getNotificationAttributes( break; case 'notifications/initialized': - // Mark as lifecycle event attributes['mcp.lifecycle.phase'] = 'initialization_complete'; attributes['mcp.protocol.ready'] = 1; break; @@ -323,49 +294,135 @@ export function createSpanName(method: string, target?: string): string { } // ============================================================================= -// SPAN ATTRIBUTE BUILDERS +// UNIFIED SPAN BUILDER // ============================================================================= +interface McpSpanConfig { + type: 'request' | 'notification-incoming' | 'notification-outgoing'; + message: JsonRpcRequest | JsonRpcNotification; + transport: MCPTransport; + extra?: ExtraHandlerData; + callback: () => unknown; +} + +/** + * Unified builder for creating MCP spans + * Follows OpenTelemetry semantic conventions for span naming + */ +function createMcpSpan(config: McpSpanConfig): unknown { + const { type, message, transport, extra, callback } = config; + const { method } = message; + const params = message.params as Record | undefined; + + // Determine span name based on type and OTEL conventions + let spanName: string; + if (type === 'request') { + const target = extractTarget(method, params || {}); + spanName = createSpanName(method, target); + } else { + // For notifications, use method name directly (OTEL convention) + spanName = getNotificationSpanName(method); + } + + // Build attributes + const attributes: Record = { + // Base attributes + ...buildTransportAttributes(transport, extra), + // Method name (required for all spans) + [MCP_METHOD_NAME_ATTRIBUTE]: method, + // Type-specific attributes + ...buildTypeSpecificAttributes(type, message, params), + // Sentry attributes + ...buildSentryAttributes(type), + }; + + return startSpan( + { + name: spanName, + forceTransaction: true, + attributes, + }, + callback, + ); +} + /** - * Builds base span attributes common to all MCP span types + * Build transport and network attributes */ -export function buildBaseSpanAttributes( +function buildTransportAttributes( transport: MCPTransport, extra?: ExtraHandlerData, ): Record { - // Session ID should come from the transport itself, not the RPC message const sessionId = transport.sessionId; - - // Extract client information from extra/request data (if provided) const clientAddress = extra ? extractClientAddress(extra) : undefined; const clientPort = extra ? extractClientPort(extra) : undefined; - - // Determine transport types const { mcpTransport, networkTransport } = getTransportTypes(transport); return { ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), - [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, // Application level: "http", "sse", "stdio", "websocket" - [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, // Network level: "tcp", "pipe", "udp", "quic" - [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', // JSON-RPC version + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', }; } /** - * Builds Sentry-specific span attributes + * Build type-specific attributes based on message type + */ +function buildTypeSpecificAttributes( + type: McpSpanConfig['type'], + message: JsonRpcRequest | JsonRpcNotification, + params?: Record, +): Record { + if (type === 'request') { + const request = message as JsonRpcRequest; + const target = extractTarget(request.method, params || {}); + + return { + ...(request.id !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(request.id) }), + ...(target && getTargetAttributes(request.method, target)), + ...getRequestArguments(request.method, params || {}), + }; + } + + // For notifications, only include notification-specific attributes + return getNotificationAttributes(message.method, params || {}); +} + +/** + * Build Sentry-specific attributes based on span type + * Uses specific operations for notification direction */ -export function buildSentrySpanAttributes(origin: string): Record { +function buildSentryAttributes(type: McpSpanConfig['type']): Record { + let op: string; + let origin: string; + + switch (type) { + case 'request': + op = MCP_SERVER_OP_VALUE; + origin = MCP_FUNCTION_ORIGIN_VALUE; + break; + case 'notification-incoming': + op = MCP_NOTIFICATION_CLIENT_TO_SERVER_OP_VALUE; + origin = MCP_NOTIFICATION_ORIGIN_VALUE; + break; + case 'notification-outgoing': + op = MCP_NOTIFICATION_SERVER_TO_CLIENT_OP_VALUE; + origin = MCP_NOTIFICATION_ORIGIN_VALUE; + break; + } + return { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: MCP_SERVER_OP_VALUE, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: MCP_ROUTE_SOURCE_VALUE, }; } // ============================================================================= -// SPAN CREATION FUNCTIONS +// PUBLIC API - SIMPLIFIED SPAN CREATION FUNCTIONS // ============================================================================= /** @@ -377,42 +434,13 @@ export function createMcpServerSpan( extra: ExtraHandlerData, callback: () => unknown, ): unknown { - const { method, id: requestId, params } = jsonRpcMessage; - - // Extract target from method and params for proper description - const target = extractTarget(method, params as Record); - const description = createSpanName(method, target); - - // Build base attributes using shared builder - const baseAttributes = buildBaseSpanAttributes(transport, extra); - - // Build request-specific attributes - const requestAttributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - ...(requestId !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(requestId) }), - ...(target && getTargetAttributes(method, target)), - // Opt-in: Tool arguments (if enabled) - ...getRequestArguments(method, params as Record), - }; - - // Build Sentry-specific attributes using shared builder - const sentryAttributes = buildSentrySpanAttributes(MCP_FUNCTION_ORIGIN_VALUE); - - return startSpan( - { - name: description, - forceTransaction: true, - attributes: { - ...baseAttributes, - ...requestAttributes, - ...sentryAttributes, - }, - }, - () => { - // TODO(bete): add proper error handling. Handle JSON RPC errors in the result - return callback(); - }, - ); + return createMcpSpan({ + type: 'request', + message: jsonRpcMessage, + transport, + extra, + callback, + }); } /** @@ -424,39 +452,13 @@ export function createMcpNotificationSpan( extra: ExtraHandlerData, callback: () => unknown, ): unknown { - const { method, params } = jsonRpcMessage; - - const description = getNotificationDescription(method, params as Record); - - // Build base attributes using shared builder - const baseAttributes = buildBaseSpanAttributes(transport, extra); - - // Build notification-specific attributes - const notificationAttributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'client_to_server', // Incoming notification - // Notification-specific attributes - ...getNotificationAttributes(method, params as Record), - }; - - // Build Sentry-specific attributes using shared builder - const sentryAttributes = buildSentrySpanAttributes(MCP_NOTIFICATION_ORIGIN_VALUE); - - return startSpan( - { - name: description, - forceTransaction: true, - attributes: { - ...baseAttributes, - ...notificationAttributes, - ...sentryAttributes, - }, - }, - () => { - const result = callback(); - return result; - }, - ); + return createMcpSpan({ + type: 'notification-incoming', + message: jsonRpcMessage, + transport, + extra, + callback, + }); } /** @@ -468,37 +470,10 @@ export function createMcpOutgoingNotificationSpan( options: Record, callback: () => unknown, ): unknown { - const { method, params } = jsonRpcMessage; - - const description = getNotificationDescription(method, params as Record); - - // Build base attributes using shared builder (no client info for outgoing notifications) - const baseAttributes = buildBaseSpanAttributes(transport); - - // Build notification-specific attributes - const notificationAttributes: Record = { - [MCP_METHOD_NAME_ATTRIBUTE]: method, - [MCP_NOTIFICATION_DIRECTION_ATTRIBUTE]: 'server_to_client', // Outgoing notification - // Notification-specific attributes - ...getNotificationAttributes(method, params as Record), - }; - - // Build Sentry-specific attributes using shared builder - const sentryAttributes = buildSentrySpanAttributes(MCP_NOTIFICATION_ORIGIN_VALUE); - - return startSpan( - { - name: description, - forceTransaction: true, - attributes: { - ...baseAttributes, - ...notificationAttributes, - ...sentryAttributes, - }, - }, - () => { - const result = callback(); - return result; - }, - ); + return createMcpSpan({ + type: 'notification-outgoing', + message: jsonRpcMessage, + transport, + callback, + }); } diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 48121d3dbb6e..6ce3b6c1837c 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -323,11 +323,10 @@ describe('wrapMcpServerWithSentry', () => { attributes: { 'mcp.method.name': 'notifications/tools/list_changed', 'mcp.session.id': 'test-session-123', - 'mcp.notification.direction': 'client_to_server', 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', - 'sentry.op': 'mcp.server', + 'sentry.op': 'mcp.notification.client_to_server', 'sentry.origin': 'auto.mcp.notification', 'sentry.source': 'route', }, @@ -393,12 +392,11 @@ describe('wrapMcpServerWithSentry', () => { expect(startSpanSpy).toHaveBeenCalledWith( { - name: 'notifications/message logger:math-service', + name: 'notifications/message', forceTransaction: true, attributes: { 'mcp.method.name': 'notifications/message', 'mcp.session.id': 'test-session-123', - 'mcp.notification.direction': 'client_to_server', 'mcp.transport': 'http', 'network.transport': 'tcp', 'network.protocol.version': '2.0', @@ -406,7 +404,7 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.logging.logger': 'math-service', 'mcp.logging.data_type': 'string', 'mcp.logging.message': 'Addition completed: 2 + 5 = 7', - 'sentry.op': 'mcp.server', + 'sentry.op': 'mcp.notification.client_to_server', 'sentry.origin': 'auto.mcp.notification', 'sentry.source': 'route', }, @@ -432,12 +430,14 @@ describe('wrapMcpServerWithSentry', () => { expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - name: 'notifications/cancelled request:req-123', + name: 'notifications/cancelled', attributes: expect.objectContaining({ 'mcp.method.name': 'notifications/cancelled', 'mcp.cancelled.request_id': 'req-123', 'mcp.cancelled.reason': 'user_requested', - 'mcp.notification.direction': 'client_to_server', + 'sentry.op': 'mcp.notification.client_to_server', + 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', }), }), expect.any(Function), @@ -461,7 +461,7 @@ describe('wrapMcpServerWithSentry', () => { expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - name: 'notifications/progress token:token-456', + name: 'notifications/progress', attributes: expect.objectContaining({ 'mcp.method.name': 'notifications/progress', 'mcp.progress.token': 'token-456', @@ -469,7 +469,9 @@ describe('wrapMcpServerWithSentry', () => { 'mcp.progress.total': 100, 'mcp.progress.percentage': 75, 'mcp.progress.message': 'Processing files...', - 'mcp.notification.direction': 'client_to_server', + 'sentry.op': 'mcp.notification.client_to_server', + 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', }), }), expect.any(Function), @@ -490,12 +492,38 @@ describe('wrapMcpServerWithSentry', () => { expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - name: 'notifications/resources/updated file:///tmp/data.json', + name: 'notifications/resources/updated', attributes: expect.objectContaining({ 'mcp.method.name': 'notifications/resources/updated', 'mcp.resource.uri': 'file:///tmp/data.json', 'mcp.resource.protocol': 'file:', - 'mcp.notification.direction': 'client_to_server', + 'sentry.op': 'mcp.notification.client_to_server', + 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', + }), + }), + expect.any(Function), + ); + }); + + it('should create spans with correct operation for outgoing notifications', async () => { + await wrappedMcpServer.connect(mockTransport); + + const outgoingNotification = { + jsonrpc: '2.0', + method: 'notifications/tools/list_changed', + }; + + await mockTransport.send?.(outgoingNotification); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/tools/list_changed', + attributes: expect.objectContaining({ + 'mcp.method.name': 'notifications/tools/list_changed', + 'sentry.op': 'mcp.notification.server_to_client', + 'sentry.origin': 'auto.mcp.notification', + 'sentry.source': 'route', }), }), expect.any(Function), From ec3cb6fa530f5904ab15c652a86ed185e5d09c67 Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 19:21:30 +0200 Subject: [PATCH 18/22] refactor(mcp-server): span and attribute creation --- packages/core/src/mcp-server.ts | 4 +- packages/core/src/utils/mcp-server/types.ts | 11 ++ packages/core/src/utils/mcp-server/utils.ts | 184 +++++++------------- 3 files changed, 72 insertions(+), 127 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 5c6886d0d73d..520ed5d143ce 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -71,11 +71,11 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): transport.send = new Proxy(originalSend, { async apply(sendTarget, sendThisArg, sendArgs) { - const [message, options] = sendArgs; + const [message] = sendArgs; // Instrument outgoing notifications (but not requests/responses) if (isJsonRpcNotification(message)) { - return createMcpOutgoingNotificationSpan(message, transport, options as Record, () => { + return createMcpOutgoingNotificationSpan(message, transport, () => { return Reflect.apply(sendTarget, sendThisArg, sendArgs); }); } diff --git a/packages/core/src/utils/mcp-server/types.ts b/packages/core/src/utils/mcp-server/types.ts index 61082116fd30..5491fdc2aa73 100644 --- a/packages/core/src/utils/mcp-server/types.ts +++ b/packages/core/src/utils/mcp-server/types.ts @@ -110,5 +110,16 @@ export interface ExtraHandlerData { }; } +/** + * Configuration for creating MCP spans + */ +export interface McpSpanConfig { + type: 'request' | 'notification-incoming' | 'notification-outgoing'; + message: JsonRpcRequest | JsonRpcNotification; + transport: MCPTransport; + extra?: ExtraHandlerData; + callback: () => unknown; +} + export type SessionId = string; export type RequestId = string | number; diff --git a/packages/core/src/utils/mcp-server/utils.ts b/packages/core/src/utils/mcp-server/utils.ts index 6c30a0862304..f29907d44fab 100644 --- a/packages/core/src/utils/mcp-server/utils.ts +++ b/packages/core/src/utils/mcp-server/utils.ts @@ -26,15 +26,9 @@ import { NETWORK_PROTOCOL_VERSION_ATTRIBUTE, NETWORK_TRANSPORT_ATTRIBUTE, } from './attributes'; -import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, MCPTransport } from './types'; +import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, McpSpanConfig, MCPTransport } from './types'; -// ============================================================================= -// TYPE GUARDS -// ============================================================================= - -/** - * - */ +/** Validates if a message is a JSON-RPC request */ export function isJsonRpcRequest(message: unknown): message is JsonRpcRequest { return ( typeof message === 'object' && @@ -46,9 +40,7 @@ export function isJsonRpcRequest(message: unknown): message is JsonRpcRequest { ); } -/** - * - */ +/** Validates if a message is a JSON-RPC notification */ export function isJsonRpcNotification(message: unknown): message is JsonRpcNotification { return ( typeof message === 'object' && @@ -60,9 +52,7 @@ export function isJsonRpcNotification(message: unknown): message is JsonRpcNotif ); } -/** - * - */ +/** Extracts target info from method and params based on method type */ export function validateMcpServerInstance(instance: unknown): boolean { if ( typeof instance === 'object' && @@ -78,50 +68,39 @@ export function validateMcpServerInstance(instance: unknown): boolean { return false; } -// ============================================================================= -// ATTRIBUTE EXTRACTION -// ============================================================================= +/** Extracts target info from method and params based on method type */ +function extractTargetInfo(method: string, params: Record): { + target?: string; + attributes: Record +} { + let target: string | undefined; + let attributeKey: string | undefined; -/** - * - */ -export function extractTarget(method: string, params: Record): string | undefined { switch (method) { case 'tools/call': - return typeof params?.name === 'string' ? params.name : undefined; + target = typeof params?.name === 'string' ? params.name : undefined; + attributeKey = 'mcp.tool.name'; + break; case 'resources/read': case 'resources/subscribe': case 'resources/unsubscribe': - return typeof params?.uri === 'string' ? params.uri : undefined; + target = typeof params?.uri === 'string' ? params.uri : undefined; + attributeKey = 'mcp.resource.uri'; + break; case 'prompts/get': - return typeof params?.name === 'string' ? params.name : undefined; - default: - return undefined; + target = typeof params?.name === 'string' ? params.name : undefined; + attributeKey = 'mcp.prompt.name'; + break; } -} -/** - * - */ -export function getTargetAttributes(method: string, target: string): Record { - switch (method) { - case 'tools/call': - return { 'mcp.tool.name': target }; - case 'resources/read': - case 'resources/subscribe': - case 'resources/unsubscribe': - return { 'mcp.resource.uri': target }; - case 'prompts/get': - return { 'mcp.prompt.name': target }; - default: - return {}; - } + return { + target, + attributes: target && attributeKey ? { [attributeKey]: target } : {} + }; } -/** - * - */ -export function getRequestArguments(method: string, params: Record): Record { +/** Extracts request arguments based on method type */ +function getRequestArguments(method: string, params: Record): Record { const args: Record = {}; // Argument capture for different methods @@ -153,14 +132,8 @@ export function getRequestArguments(method: string, params: Record, ): Record { @@ -259,51 +218,14 @@ export function getNotificationAttributes( return attributes; } -// ============================================================================= -// CLIENT INFO EXTRACTION -// ============================================================================= - -/** - * - */ -export function extractClientAddress(extra: ExtraHandlerData): string | undefined { - return ( - extra?.requestInfo?.remoteAddress || - extra?.clientAddress || - extra?.request?.ip || - extra?.request?.connection?.remoteAddress - ); -} - -/** - * - */ -export function extractClientPort(extra: ExtraHandlerData): number | undefined { - return extra?.requestInfo?.remotePort || extra?.clientPort || extra?.request?.connection?.remotePort; -} - -// ============================================================================= -// SPAN NAMING -// ============================================================================= /** - * + * Creates a span name based on the method and target */ -export function createSpanName(method: string, target?: string): string { +function createSpanName(method: string, target?: string): string { return target ? `${method} ${target}` : method; } -// ============================================================================= -// UNIFIED SPAN BUILDER -// ============================================================================= - -interface McpSpanConfig { - type: 'request' | 'notification-incoming' | 'notification-outgoing'; - message: JsonRpcRequest | JsonRpcNotification; - transport: MCPTransport; - extra?: ExtraHandlerData; - callback: () => unknown; -} /** * Unified builder for creating MCP spans @@ -317,11 +239,11 @@ function createMcpSpan(config: McpSpanConfig): unknown { // Determine span name based on type and OTEL conventions let spanName: string; if (type === 'request') { - const target = extractTarget(method, params || {}); - spanName = createSpanName(method, target); + const targetInfo = extractTargetInfo(method, params || {}); + spanName = createSpanName(method, targetInfo.target); } else { - // For notifications, use method name directly (OTEL convention) - spanName = getNotificationSpanName(method); + // For notifications, use method name directly per OpenTelemetry conventions + spanName = method; } // Build attributes @@ -354,14 +276,13 @@ function buildTransportAttributes( extra?: ExtraHandlerData, ): Record { const sessionId = transport.sessionId; - const clientAddress = extra ? extractClientAddress(extra) : undefined; - const clientPort = extra ? extractClientPort(extra) : undefined; + const clientInfo = extra ? extractClientInfo(extra) : {}; const { mcpTransport, networkTransport } = getTransportTypes(transport); return { ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), - ...(clientAddress && { [CLIENT_ADDRESS_ATTRIBUTE]: clientAddress }), - ...(clientPort && { [CLIENT_PORT_ATTRIBUTE]: clientPort }), + ...(clientInfo.address && { [CLIENT_ADDRESS_ATTRIBUTE]: clientInfo.address }), + ...(clientInfo.port && { [CLIENT_PORT_ATTRIBUTE]: clientInfo.port }), [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', @@ -378,11 +299,11 @@ function buildTypeSpecificAttributes( ): Record { if (type === 'request') { const request = message as JsonRpcRequest; - const target = extractTarget(request.method, params || {}); + const targetInfo = extractTargetInfo(request.method, params || {}); return { ...(request.id !== undefined && { [MCP_REQUEST_ID_ATTRIBUTE]: String(request.id) }), - ...(target && getTargetAttributes(request.method, target)), + ...targetInfo.attributes, ...getRequestArguments(request.method, params || {}), }; } @@ -421,10 +342,6 @@ function buildSentryAttributes(type: McpSpanConfig['type']): Record, callback: () => unknown, ): unknown { return createMcpSpan({ @@ -477,3 +393,21 @@ export function createMcpOutgoingNotificationSpan( callback, }); } + +/** + * Combine the two extraction functions into one + */ +function extractClientInfo(extra: ExtraHandlerData): { + address?: string; + port?: number +} { + return { + address: extra?.requestInfo?.remoteAddress || + extra?.clientAddress || + extra?.request?.ip || + extra?.request?.connection?.remoteAddress, + port: extra?.requestInfo?.remotePort || + extra?.clientPort || + extra?.request?.connection?.remotePort + }; +} From e19311845614c022ea2f82bef43be8b0e9fbaa34 Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 19:36:17 +0200 Subject: [PATCH 19/22] refactor(mcp-server): method configuration and argument extraction for improved instrumentation --- packages/core/src/utils/mcp-server/types.ts | 12 +++ packages/core/src/utils/mcp-server/utils.ts | 106 ++++++++++++-------- 2 files changed, 75 insertions(+), 43 deletions(-) diff --git a/packages/core/src/utils/mcp-server/types.ts b/packages/core/src/utils/mcp-server/types.ts index 5491fdc2aa73..a3d592b78257 100644 --- a/packages/core/src/utils/mcp-server/types.ts +++ b/packages/core/src/utils/mcp-server/types.ts @@ -2,6 +2,17 @@ * types for MCP server instrumentation */ + +/** Method configuration type */ +export type MethodConfig = { + targetField: string; + targetAttribute: string; + captureArguments?: boolean; + argumentsField?: string; + captureUri?: boolean; + captureName?: boolean; +}; + /** * JSON-RPC 2.0 request object */ @@ -121,5 +132,6 @@ export interface McpSpanConfig { callback: () => unknown; } + export type SessionId = string; export type RequestId = string | number; diff --git a/packages/core/src/utils/mcp-server/utils.ts b/packages/core/src/utils/mcp-server/utils.ts index f29907d44fab..abb8e39eef47 100644 --- a/packages/core/src/utils/mcp-server/utils.ts +++ b/packages/core/src/utils/mcp-server/utils.ts @@ -18,15 +18,18 @@ import { MCP_NOTIFICATION_CLIENT_TO_SERVER_OP_VALUE, MCP_NOTIFICATION_ORIGIN_VALUE, MCP_NOTIFICATION_SERVER_TO_CLIENT_OP_VALUE, + MCP_PROMPT_NAME_ATTRIBUTE, MCP_REQUEST_ID_ATTRIBUTE, + MCP_RESOURCE_URI_ATTRIBUTE, MCP_ROUTE_SOURCE_VALUE, MCP_SERVER_OP_VALUE, MCP_SESSION_ID_ATTRIBUTE, + MCP_TOOL_NAME_ATTRIBUTE, MCP_TRANSPORT_ATTRIBUTE, NETWORK_PROTOCOL_VERSION_ATTRIBUTE, NETWORK_TRANSPORT_ATTRIBUTE, } from './attributes'; -import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, McpSpanConfig, MCPTransport } from './types'; +import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, McpSpanConfig, MCPTransport, MethodConfig } from './types'; /** Validates if a message is a JSON-RPC request */ export function isJsonRpcRequest(message: unknown): message is JsonRpcRequest { @@ -68,65 +71,82 @@ export function validateMcpServerInstance(instance: unknown): boolean { return false; } +/** Configuration for MCP methods to extract targets and arguments */ +const METHOD_CONFIGS: Record = { + 'tools/call': { + targetField: 'name', + targetAttribute: MCP_TOOL_NAME_ATTRIBUTE, + captureArguments: true, + argumentsField: 'arguments', + }, + 'resources/read': { + targetField: 'uri', + targetAttribute: MCP_RESOURCE_URI_ATTRIBUTE, + captureUri: true, + }, + 'resources/subscribe': { + targetField: 'uri', + targetAttribute: MCP_RESOURCE_URI_ATTRIBUTE, + }, + 'resources/unsubscribe': { + targetField: 'uri', + targetAttribute: MCP_RESOURCE_URI_ATTRIBUTE, + }, + 'prompts/get': { + targetField: 'name', + targetAttribute: MCP_PROMPT_NAME_ATTRIBUTE, + captureName: true, + captureArguments: true, + argumentsField: 'arguments', + }, +}; + /** Extracts target info from method and params based on method type */ function extractTargetInfo(method: string, params: Record): { target?: string; attributes: Record } { - let target: string | undefined; - let attributeKey: string | undefined; - - switch (method) { - case 'tools/call': - target = typeof params?.name === 'string' ? params.name : undefined; - attributeKey = 'mcp.tool.name'; - break; - case 'resources/read': - case 'resources/subscribe': - case 'resources/unsubscribe': - target = typeof params?.uri === 'string' ? params.uri : undefined; - attributeKey = 'mcp.resource.uri'; - break; - case 'prompts/get': - target = typeof params?.name === 'string' ? params.name : undefined; - attributeKey = 'mcp.prompt.name'; - break; + const config = METHOD_CONFIGS[method as keyof typeof METHOD_CONFIGS]; + if (!config) { + return { attributes: {} }; } + const target = config.targetField && typeof params?.[config.targetField] === 'string' + ? params[config.targetField] as string + : undefined; + return { target, - attributes: target && attributeKey ? { [attributeKey]: target } : {} + attributes: target && config.targetAttribute ? { [config.targetAttribute]: target } : {} }; } /** Extracts request arguments based on method type */ function getRequestArguments(method: string, params: Record): Record { const args: Record = {}; + const config = METHOD_CONFIGS[method as keyof typeof METHOD_CONFIGS]; + + if (!config) { + return args; + } - // Argument capture for different methods - switch (method) { - case 'tools/call': - if (params?.arguments && typeof params.arguments === 'object') { - for (const [key, value] of Object.entries(params.arguments as Record)) { - args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); - } + // Capture arguments from the configured field + if (config.captureArguments && config.argumentsField && params?.[config.argumentsField]) { + const argumentsObj = params[config.argumentsField]; + if (typeof argumentsObj === 'object' && argumentsObj !== null) { + for (const [key, value] of Object.entries(argumentsObj as Record)) { + args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); } - break; - case 'resources/read': - if (params?.uri) { - args['mcp.request.argument.uri'] = JSON.stringify(params.uri); - } - break; - case 'prompts/get': - if (params?.name) { - args['mcp.request.argument.name'] = JSON.stringify(params.name); - } - if (params?.arguments && typeof params.arguments === 'object') { - for (const [key, value] of Object.entries(params.arguments as Record)) { - args[`mcp.request.argument.${key.toLowerCase()}`] = JSON.stringify(value); - } - } - break; + } + } + + // Capture specific fields as arguments + if (config.captureUri && params?.uri) { + args['mcp.request.argument.uri'] = JSON.stringify(params.uri); + } + + if (config.captureName && params?.name) { + args['mcp.request.argument.name'] = JSON.stringify(params.name); } return args; From 347422cedb52d62a8f5b2ee0d66c367ae21212b1 Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 20:25:44 +0200 Subject: [PATCH 20/22] refactor(mcp-server): improve transport type handling and add tests for stdio and SSE transports --- packages/core/src/utils/mcp-server/utils.ts | 25 +++- packages/core/test/lib/mcp-server.test.ts | 158 +++++++++++++++++++- 2 files changed, 172 insertions(+), 11 deletions(-) diff --git a/packages/core/src/utils/mcp-server/utils.ts b/packages/core/src/utils/mcp-server/utils.ts index abb8e39eef47..33383542101f 100644 --- a/packages/core/src/utils/mcp-server/utils.ts +++ b/packages/core/src/utils/mcp-server/utils.ts @@ -55,7 +55,7 @@ export function isJsonRpcNotification(message: unknown): message is JsonRpcNotif ); } -/** Extracts target info from method and params based on method type */ +/** Validates MCP server instance with comprehensive type checking */ export function validateMcpServerInstance(instance: unknown): boolean { if ( typeof instance === 'object' && @@ -156,11 +156,26 @@ function getRequestArguments(method: string, params: Record): R function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { const transportName = transport.constructor?.name?.toLowerCase() || ''; - if (transportName.includes('sse')) return { mcpTransport: 'sse', networkTransport: 'tcp' }; - if (transportName.includes('websocket')) return { mcpTransport: 'websocket', networkTransport: 'tcp' }; - if (transportName.includes('stdio')) return { mcpTransport: 'stdio', networkTransport: 'pipe' }; + // Standard MCP transports per specification + if (transportName.includes('stdio')) { + return { mcpTransport: 'stdio', networkTransport: 'pipe' }; + } + + // Streamable HTTP is the standard HTTP-based transport + // The official SDK uses 'StreamableHTTPServerTransport' / 'StreamableHTTPClientTransport' + if (transportName.includes('streamablehttp') || transportName.includes('streamable')) { + return { mcpTransport: 'http', networkTransport: 'tcp' }; + } + + // SSE is the deprecated HTTP+SSE transport (backwards compatibility) + // Note: Modern Streamable HTTP can use SSE internally, but SSE transport is deprecated + if (transportName.includes('sse')) { + return { mcpTransport: 'sse', networkTransport: 'tcp' }; + } - return { mcpTransport: 'http', networkTransport: 'tcp' }; + // For custom transports, mark as unknown + // TODO(bete): Add support for custom transports + return { mcpTransport: 'unknown', networkTransport: 'unknown' }; } /** Extracts additional attributes for specific notification types */ diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 6ce3b6c1837c..2503223b9586 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -530,6 +530,123 @@ describe('wrapMcpServerWithSentry', () => { ); }); }); + + describe('Stdio Transport Tests', () => { + let mockMcpServer: ReturnType; + let wrappedMcpServer: ReturnType; + let mockStdioTransport: ReturnType; + + beforeEach(() => { + mockMcpServer = createMockMcpServer(); + wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + mockStdioTransport = createMockStdioTransport(); + mockStdioTransport.sessionId = 'stdio-session-456'; + }); + + it('should detect stdio transport and set correct attributes', async () => { + await wrappedMcpServer.connect(mockStdioTransport); + + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/call', + id: 'req-stdio-1', + params: { name: 'process-file', arguments: { path: '/tmp/data.txt' } }, + }; + + mockStdioTransport.onmessage?.(jsonRpcRequest, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + { + name: 'tools/call process-file', + forceTransaction: true, + attributes: { + 'mcp.method.name': 'tools/call', + 'mcp.tool.name': 'process-file', + 'mcp.request.id': 'req-stdio-1', + 'mcp.session.id': 'stdio-session-456', + 'mcp.transport': 'stdio', // Should be stdio, not http + 'network.transport': 'pipe', // Should be pipe, not tcp + 'network.protocol.version': '2.0', + 'mcp.request.argument.path': '"/tmp/data.txt"', + 'sentry.op': 'mcp.server', + 'sentry.origin': 'auto.function.mcp_server', + 'sentry.source': 'route', + }, + }, + expect.any(Function), + ); + }); + + it('should handle stdio transport notifications correctly', async () => { + await wrappedMcpServer.connect(mockStdioTransport); + + const notification = { + jsonrpc: '2.0', + method: 'notifications/message', + params: { + level: 'debug', + data: 'Processing stdin input', + }, + }; + + mockStdioTransport.onmessage?.(notification, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'notifications/message', + attributes: expect.objectContaining({ + 'mcp.method.name': 'notifications/message', + 'mcp.session.id': 'stdio-session-456', + 'mcp.transport': 'stdio', + 'network.transport': 'pipe', + 'mcp.logging.level': 'debug', + 'mcp.logging.message': 'Processing stdin input', + }), + }), + expect.any(Function), + ); + }); + }); + + describe('SSE Transport Tests (Backwards Compatibility)', () => { + let mockMcpServer: ReturnType; + let wrappedMcpServer: ReturnType; + let mockSseTransport: ReturnType; + + beforeEach(() => { + mockMcpServer = createMockMcpServer(); + wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); + mockSseTransport = createMockSseTransport(); + mockSseTransport.sessionId = 'sse-session-789'; + }); + + it('should detect SSE transport for backwards compatibility', async () => { + await wrappedMcpServer.connect(mockSseTransport); + + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'resources/read', + id: 'req-sse-1', + params: { uri: 'https://api.example.com/data' }, + }; + + mockSseTransport.onmessage?.(jsonRpcRequest, {}); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'resources/read https://api.example.com/data', + attributes: expect.objectContaining({ + 'mcp.method.name': 'resources/read', + 'mcp.resource.uri': 'https://api.example.com/data', + 'mcp.transport': 'sse', // Deprecated but supported + 'network.transport': 'tcp', + 'mcp.session.id': 'sse-session-789', + }), + }), + expect.any(Function), + ); + }); + }); }); // Test helpers @@ -546,10 +663,39 @@ function createMockMcpServer() { } function createMockTransport() { - return { - onmessage: vi.fn(), - onclose: vi.fn(), - send: vi.fn().mockResolvedValue(undefined), - sessionId: 'test-session-123', - }; + // exact naming pattern from the official SDK + class StreamableHTTPServerTransport { + onmessage = vi.fn(); + onclose = vi.fn(); + send = vi.fn().mockResolvedValue(undefined); + sessionId = 'test-session-123'; + } + + return new StreamableHTTPServerTransport(); +} + +function createMockStdioTransport() { + // Create a mock that mimics StdioServerTransport + // Using the exact naming pattern from the official SDK + class StdioServerTransport { + onmessage = vi.fn(); + onclose = vi.fn(); + send = vi.fn().mockResolvedValue(undefined); + sessionId = 'stdio-session-456'; + } + + return new StdioServerTransport(); +} + +function createMockSseTransport() { + // Create a mock that mimics the deprecated SSEServerTransport + // For backwards compatibility testing + class SSEServerTransport { + onmessage = vi.fn(); + onclose = vi.fn(); + send = vi.fn().mockResolvedValue(undefined); + sessionId = 'sse-session-789'; + } + + return new SSEServerTransport(); } From 9776402c57a5eb36272ae23483967c2176a7f356 Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 3 Jul 2025 20:30:16 +0200 Subject: [PATCH 21/22] fix lint --- packages/core/src/mcp-server.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 520ed5d143ce..5aa30bf1ce99 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -32,7 +32,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): const serverInstance = mcpServerInstance as MCPServerInstance; // Wrap connect() to intercept AFTER Protocol sets up transport handlers - const originalConnect = serverInstance.connect; + const originalConnect = serverInstance.connect.bind(serverInstance); serverInstance.connect = new Proxy(originalConnect, { async apply(target, thisArg, argArray) { const [transport, ...restArgs] = argArray as [MCPTransport, ...unknown[]]; @@ -42,7 +42,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): // Intercept incoming messages via onmessage if (transport.onmessage) { - const protocolOnMessage = transport.onmessage; + const protocolOnMessage = transport.onmessage.bind(transport); transport.onmessage = new Proxy(protocolOnMessage, { apply(onMessageTarget, onMessageThisArg, onMessageArgs) { @@ -67,7 +67,7 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): // Intercept outgoing messages via send if (transport.send) { - const originalSend = transport.send; + const originalSend = transport.send.bind(transport); transport.send = new Proxy(originalSend, { async apply(sendTarget, sendThisArg, sendArgs) { @@ -87,10 +87,10 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): // Handle transport lifecycle events if (transport.onclose) { - const originalOnClose = transport.onclose; + const originalOnClose = transport.onclose.bind(transport); transport.onclose = new Proxy(originalOnClose, { apply(onCloseTarget, onCloseThisArg, onCloseArgs) { - //TODO(bete): session and request correlation (methods at the bottom of this file) + // TODO(bete): session and request correlation (methods at the bottom of this file) // if (transport.sessionId) { // handleTransportOnClose(transport.sessionId); // } From d4c74a9f5f4598f44685f77236e58f5094091580 Mon Sep 17 00:00:00 2001 From: betegon Date: Fri, 4 Jul 2025 20:16:53 +0200 Subject: [PATCH 22/22] refactor(mcp-server): use fill for method wrapping for transport handlers --- packages/core/src/mcp-server.ts | 70 ++++++++--------------- packages/core/test/lib/mcp-server.test.ts | 7 ++- 2 files changed, 27 insertions(+), 50 deletions(-) diff --git a/packages/core/src/mcp-server.ts b/packages/core/src/mcp-server.ts index 5aa30bf1ce99..85b1e03ec621 100644 --- a/packages/core/src/mcp-server.ts +++ b/packages/core/src/mcp-server.ts @@ -11,6 +11,7 @@ import { isJsonRpcRequest, validateMcpServerInstance, } from './utils/mcp-server/utils'; +import { fill } from './utils/object'; const wrappedMcpServerInstances = new WeakSet(); @@ -31,75 +32,50 @@ export function wrapMcpServerWithSentry(mcpServerInstance: S): const serverInstance = mcpServerInstance as MCPServerInstance; - // Wrap connect() to intercept AFTER Protocol sets up transport handlers - const originalConnect = serverInstance.connect.bind(serverInstance); - serverInstance.connect = new Proxy(originalConnect, { - async apply(target, thisArg, argArray) { - const [transport, ...restArgs] = argArray as [MCPTransport, ...unknown[]]; + fill(serverInstance, 'connect', (originalConnect) => { + return async function(this: MCPServerInstance, transport: MCPTransport, ...restArgs: unknown[]) { + const result = await originalConnect.call(this, transport, ...restArgs); - // Call the original connect first to let Protocol set up its handlers - const result = await Reflect.apply(target, thisArg, [transport, ...restArgs]); - - // Intercept incoming messages via onmessage if (transport.onmessage) { - const protocolOnMessage = transport.onmessage.bind(transport); - - transport.onmessage = new Proxy(protocolOnMessage, { - apply(onMessageTarget, onMessageThisArg, onMessageArgs) { - const [jsonRpcMessage, extra] = onMessageArgs; - - // Instrument both requests and notifications + fill(transport, 'onmessage', (originalOnMessage) => { + return function(this: MCPTransport, jsonRpcMessage: unknown, extra?: unknown) { if (isJsonRpcRequest(jsonRpcMessage)) { - return createMcpServerSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { - return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); + return createMcpServerSpan(jsonRpcMessage, this, extra as ExtraHandlerData, () => { + return originalOnMessage.call(this, jsonRpcMessage, extra); }); } if (isJsonRpcNotification(jsonRpcMessage)) { - return createMcpNotificationSpan(jsonRpcMessage, transport, extra as ExtraHandlerData, () => { - return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); + return createMcpNotificationSpan(jsonRpcMessage, this, extra as ExtraHandlerData, () => { + return originalOnMessage.call(this, jsonRpcMessage, extra); }); } - - return Reflect.apply(onMessageTarget, onMessageThisArg, onMessageArgs); - }, + return originalOnMessage.call(this, jsonRpcMessage, extra); + }; }); } - // Intercept outgoing messages via send if (transport.send) { - const originalSend = transport.send.bind(transport); - - transport.send = new Proxy(originalSend, { - async apply(sendTarget, sendThisArg, sendArgs) { - const [message] = sendArgs; - - // Instrument outgoing notifications (but not requests/responses) + fill(transport, 'send', (originalSend) => { + return async function(this: MCPTransport, message: unknown) { if (isJsonRpcNotification(message)) { - return createMcpOutgoingNotificationSpan(message, transport, () => { - return Reflect.apply(sendTarget, sendThisArg, sendArgs); + return createMcpOutgoingNotificationSpan(message, this, () => { + return originalSend.call(this, message); }); } - - return Reflect.apply(sendTarget, sendThisArg, sendArgs); - }, + return originalSend.call(this, message); + }; }); } - // Handle transport lifecycle events if (transport.onclose) { - const originalOnClose = transport.onclose.bind(transport); - transport.onclose = new Proxy(originalOnClose, { - apply(onCloseTarget, onCloseThisArg, onCloseArgs) { - // TODO(bete): session and request correlation (methods at the bottom of this file) - // if (transport.sessionId) { - // handleTransportOnClose(transport.sessionId); - // } - return Reflect.apply(onCloseTarget, onCloseThisArg, onCloseArgs); - }, + fill(transport, 'onclose', (originalOnClose) => { + return function(this: MCPTransport, ...args: unknown[]) { + return originalOnClose.call(this, ...args); + }; }); } return result; - }, + }; }); wrappedMcpServerInstances.add(mcpServerInstance); diff --git a/packages/core/test/lib/mcp-server.test.ts b/packages/core/test/lib/mcp-server.test.ts index 2503223b9586..91b1410b8209 100644 --- a/packages/core/test/lib/mcp-server.test.ts +++ b/packages/core/test/lib/mcp-server.test.ts @@ -47,12 +47,13 @@ describe('wrapMcpServerWithSentry', () => { let mockMcpServer: ReturnType; let wrappedMcpServer: ReturnType; let mockTransport: ReturnType; + let originalConnect: any; beforeEach(() => { mockMcpServer = createMockMcpServer(); + originalConnect = mockMcpServer.connect; wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer); mockTransport = createMockTransport(); - // Don't connect transport here. let individual tests control when connection happens }); it('should proxy the connect method', () => { @@ -95,8 +96,8 @@ describe('wrapMcpServerWithSentry', () => { it('should call original connect and preserve functionality', async () => { await wrappedMcpServer.connect(mockTransport); - // Original connect should have been called - expect(mockMcpServer.connect).toHaveBeenCalledWith(mockTransport); + // Check the original spy was called + expect(originalConnect).toHaveBeenCalledWith(mockTransport); }); it('should create spans for incoming JSON-RPC requests', async () => {