Uh oh!
There was an error while loading. Please reload this page.
feat(core): MCP server instrumentation - #16807
Conversation
…improved test isolation
…racing and monitoring
…r MCP server instrumentation
…ibute names to match OTEL draft semantic convention
…r improved instrumentation
…or stdio and SSE transports
AbhiPrasad
left a comment
There was a problem hiding this comment.
Generally this is looks to be the right direction! The only comment I would make is to do mv core/src/mcp-server.ts core/src/integrations/mcp-server/index.ts and move all of the utils/mcp-server files into that integrations folder. This matches our existing structure.
I do have some low prio items, but that I can get to once this PR gets out of draft.
| attributes['mcp.resource.uri'] = String(params.uri); | ||
| // Extract protocol from URI | ||
| try { | ||
| const url = new URL(String(params.uri)); |
There was a problem hiding this comment.
we have a url parsing helper in the core package that we should use here. It also handles relative URLs.
Thanks @AbhiPrasad! I'm closing this in favor of #16817, that avoids I'll apply your suggestions on #16817 |
…6817) Closes#16826, #16654, #16666#16978 Different approach from #16807 . Using `Proxy` was causing issues in cloudflare #16182. Now using `fill` we shouldn't have those problems as `fill` doesn't create a new wrapper object with a different identity, so now: 1. `fill` just replaces the method on the existing object 2. The transport object keeps its original identity 3. When `transport.start()` runs and accesses private fields, this is still the original transport object 4. The `WeakMap` recognizes it as the same object that owns the private fields ## What's inside - Support for new MCP SDK methods (`mcpServerInstance.tool()`, `mcpServerInstance.resource()`, etc.) - Tracing instrumentation - Error handling ## Tracing It follows [OTEL semantic conventions for MCP](https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md) and adds more attributes we thought are useful. It also handles PII based on user setting of `sendDefaultPii`. ### Tracing flow 1. **Transport receives tools/call request (`id: 2`)** 2. **Create INACTIVE `mcp.server` span** 3. **Store span in**`requestIdToSpanMap[2] = { span, method: "tools/call", startTime }` 4. **Execute handler with span context** *(handler gets requestId: 2)* 5. **Handler finds span using** `requestId: 2` 6. **Tool execution happens within span context** 7. **Response sent with tool results** 8. `completeSpanWithToolResults(2, result)` **enriches and completes span** ## Error handling 1. **error capture** - `errorCapture.ts` - Non-invasive error reporting that never interferes with MCP service operation - Error context with MCP-specific metadata - PII filtering respects `sendDefaultPii` settings - Resilient to Sentry failures (wrapped in try-catch) 2. **Tool execution error capturing** - `handlers.ts` - Captures exceptions thrown during tool execution - Preserves normal MCP behaviour (errors converted to `isError: true`) - Includes tool name, arguments, and request context - Handles both sync and async tool handlers 3. **Transport error instrumentation** - `transport.ts` - Captures connection errors and network failures - Intercepts JSON-RPC error responses - Includes transport type and session information - Handles error responses being sent back to clients
…6817) Closes#16826, #16654, #16666#16978 Different approach from #16807 . Using `Proxy` was causing issues in cloudflare #16182. Now using `fill` we shouldn't have those problems as `fill` doesn't create a new wrapper object with a different identity, so now: 1. `fill` just replaces the method on the existing object 2. The transport object keeps its original identity 3. When `transport.start()` runs and accesses private fields, this is still the original transport object 4. The `WeakMap` recognizes it as the same object that owns the private fields - Support for new MCP SDK methods (`mcpServerInstance.tool()`, `mcpServerInstance.resource()`, etc.) - Tracing instrumentation - Error handling It follows [OTEL semantic conventions for MCP](https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md) and adds more attributes we thought are useful. It also handles PII based on user setting of `sendDefaultPii`. 1. **Transport receives tools/call request (`id: 2`)** 2. **Create INACTIVE `mcp.server` span** 3. **Store span in**`requestIdToSpanMap[2] = { span, method: "tools/call", startTime }` 4. **Execute handler with span context** *(handler gets requestId: 2)* 5. **Handler finds span using** `requestId: 2` 6. **Tool execution happens within span context** 7. **Response sent with tool results** 8. `completeSpanWithToolResults(2, result)` **enriches and completes span** 1. **error capture** - `errorCapture.ts` - Non-invasive error reporting that never interferes with MCP service operation - Error context with MCP-specific metadata - PII filtering respects `sendDefaultPii` settings - Resilient to Sentry failures (wrapped in try-catch) 2. **Tool execution error capturing** - `handlers.ts` - Captures exceptions thrown during tool execution - Preserves normal MCP behaviour (errors converted to `isError: true`) - Includes tool name, arguments, and request context - Handles both sync and async tool handlers 3. **Transport error instrumentation** - `transport.ts` - Captures connection errors and network failures - Intercepts JSON-RPC error responses - Includes transport type and session information - Handles error responses being sent back to clients
…6817) Closes#16826, #16654, #16666#16978 Different approach from #16807 . Using `Proxy` was causing issues in cloudflare #16182. Now using `fill` we shouldn't have those problems as `fill` doesn't create a new wrapper object with a different identity, so now: 1. `fill` just replaces the method on the existing object 2. The transport object keeps its original identity 3. When `transport.start()` runs and accesses private fields, this is still the original transport object 4. The `WeakMap` recognizes it as the same object that owns the private fields - Support for new MCP SDK methods (`mcpServerInstance.tool()`, `mcpServerInstance.resource()`, etc.) - Tracing instrumentation - Error handling It follows [OTEL semantic conventions for MCP](https://github.com/open-telemetry/semantic-conventions/blob/3097fb0af5b9492b0e3f55dc5f6c21a3dc2be8df/docs/gen-ai/mcp.md) and adds more attributes we thought are useful. It also handles PII based on user setting of `sendDefaultPii`. 1. **Transport receives tools/call request (`id: 2`)** 2. **Create INACTIVE `mcp.server` span** 3. **Store span in**`requestIdToSpanMap[2] = { span, method: "tools/call", startTime }` 4. **Execute handler with span context** *(handler gets requestId: 2)* 5. **Handler finds span using** `requestId: 2` 6. **Tool execution happens within span context** 7. **Response sent with tool results** 8. `completeSpanWithToolResults(2, result)` **enriches and completes span** 1. **error capture** - `errorCapture.ts` - Non-invasive error reporting that never interferes with MCP service operation - Error context with MCP-specific metadata - PII filtering respects `sendDefaultPii` settings - Resilient to Sentry failures (wrapped in try-catch) 2. **Tool execution error capturing** - `handlers.ts` - Captures exceptions thrown during tool execution - Preserves normal MCP behaviour (errors converted to `isError: true`) - Includes tool name, arguments, and request context - Handles both sync and async tool handlers 3. **Transport error instrumentation** - `transport.ts` - Captures connection errors and network failures - Intercepts JSON-RPC error responses - Includes transport type and session information - Handles error responses being sent back to clients
An improvement on MCP server tracing. It follows:
This is WIP, and will later add:
Project with this new spans:
yarn lint) & (yarn test).