sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

MCP Server Support/Instrumentation

Open lforst opened this issue 7 months ago • 1 comments

Description

We want to have instrumentation for @modelcontextprotocol/sdk and maybe mcp-framework.

TODOs:

lforst avatar Apr 14 '25 14:04 lforst

ref https://github.com/open-telemetry/semantic-conventions/pull/2083

AbhiPrasad avatar Apr 14 '25 15:04 AbhiPrasad

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 🙏

betegon avatar Jul 07 '25 16:07 betegon

Did we identify what was specifically broken? Let's try to get a minimal repro just so we can escalate this afterwards.

AbhiPrasad avatar Jul 07 '25 16:07 AbhiPrasad

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!

betegon avatar Jul 07 '25 16:07 betegon

How to repro:

  1. Clone this repo (sentry javascript SDK)
  2. Build node package in packages/node with yarn && yarn build
  3. Link it: yarn link
  4. Clone this MCP Server example: https://github.com/betegon/express-mcp-server
  5. Go to the root directory and link the node package: yarn link "@sentry/node"
  6. Setup your Sentry DSN in instrument.mjs
  7. 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.

betegon avatar Jul 07 '25 18:07 betegon

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.

timfish avatar Jul 07 '25 20:07 timfish