sentry-javascript
sentry-javascript copied to clipboard
MCP Server Support/Instrumentation
Description
We want to have instrumentation for @modelcontextprotocol/sdk and maybe mcp-framework.
TODOs:
- [ ] Auto instrumentation (blocked by https://github.com/nodejs/import-in-the-middle/issues/187 in iitm)
- [X] Associate handler spans with POST requests (blocked by https://github.com/modelcontextprotocol/typescript-sdk/pull/358)
ref https://github.com/open-telemetry/semantic-conventions/pull/2083
Even with the fix is still broken. @timfish and I had a look at this one week ago but didn't work. For now we're working on instrumentating the MCP by wrapping it with wrapMcpServerWithSentry. Auto instrumentation will be looked at after that is released.
Thanks @timfish for your help 🙏
Did we identify what was specifically broken? Let's try to get a minimal repro just so we can escalate this afterwards.
Did we identify what was specifically broken?
Not that I know. I debugged it for a while, requested some help from @timfish and he kept debugging, but I don't think he found the cause.
Let me get a repro!
How to repro:
- Clone this repo (sentry javascript SDK)
- Build
nodepackage inpackages/nodewithyarn && yarn build - Link it:
yarn link - Clone this MCP Server example: https://github.com/betegon/express-mcp-server
- Go to the root directory and link the node package:
yarn link "@sentry/node" - Setup your Sentry DSN in
instrument.mjs - Run the server:
node --import ./instrument.mjs mcp-express.mjs
The result will be that nothing crashes and you can see traces/spans in sentry UI but without the MCP instrumentation, so it's like you didn't wrap the server with our instrumentation wrapMcpServerWithSentry.
It looked like the changes I made to import-in-the-middle were not quite enough to allow instrumenting inside the module. Even when I made an extra change the limiting factor seemed to be otel's InstrumentationBase. I would politely describe it as an undocumented web of years of legacy additions. It looks like maybe it only allows instrumenting inside modules via require-in-the-middle but I spent a few hours debugging and couldn't really work it out!
I think we decided it might be easier to make our own simple wrapper for import-in-the-middle/require-in-the-middle? Even if I can work out the changes required in the Node otel code, it would likely be breaking because it would change the behaviour.