fix(sentry): stop tracing GET requests to the MCP streamable routes - #2100
Merged
Merged
Conversation
The MCP server is stateless, so the GET that streamable-HTTP clients send to open a server-to-client stream is not a supported route: nothing is ever pushed on it. It is still about a quarter of MCP traffic (roughly 180k-214k of ~750k requests per day), and each one became a Sentry transaction that stayed open for the life of the held connection (avg ~5 min, p95 15 min). Sentry usage is already near its limits, which is why #2082 recently cut trace sampling from 100% to 20%. There is nothing to learn from traces of an unsupported route, so tracesSampler now returns 0 for GET on /mcp, /mcp/:id and the /mcp-oauth* mounts. POST traffic on the same routes, the legacy /sse/:id transport and the /.well-known discovery routes keep the existing sample rates. Pairs with the change that answers these GETs with 405. Deploy this one after that change has been verified in Sentry, because the verification reads the GET transactions this commit removes. Testing: - Called the real tracesSampler with request contexts shaped like the http instrumentation passes them (method and URL via normalizedRequest and via span attributes only): GET on the MCP routes returns 0; POST on the same routes 0.2; unrelated GETs 0.2; analytics 0.01; legacy /sse 0.2; /.well-known/.../mcp-oauth discovery 0.2; /mcpfoo 0.2. - Ran the backend against a Sentry development environment and sent 140+ GETs and 60+ POSTs to /mcp, /mcp/:id and /mcp-oauth-claude: zero GET transactions recorded, POST /mcp and POST /mcp/:id recorded as before. Same result on SDK 10.45.0 and 10.56.0. - Backend type-check passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Observability fix (backend and orchestrator, Sentry init).
tracesSamplerinlibraries/nestjs-libraries/src/sentry/initialize.sentry.tsnow returns 0 for GET requests to the MCP streamable-HTTP routes (/mcp,/mcp/:idand the/mcp-oauth*mounts), so they no longer produce Sentry transactions. POST traffic on the same routes, the legacy/sse/:idtransport, the/.well-knownOAuth discovery routes and every other route keep the existing sample rates (20%, and 1% for public analytics). No request handling changes.Why was this change needed?
The MCP server is stateless, so the GET that streamable-HTTP clients send to open a server-to-client stream is not a supported route: nothing is ever pushed on it. It is still about a quarter of MCP traffic, roughly 180k to 214k of about 750k requests per day, and each GET became a Sentry transaction that stayed open for the life of the held connection (about 5 minutes on average, 15 minutes at p95).
Sentry usage is already close to its limits, which is why #2082 recently cut trace sampling from 100% to 20%. Traces of an unsupported route tell us nothing, so there is no reason to keep spending quota on them. While sampling was at 100%, these long-lived transactions (plus a profiler each) were also a large part of what made the MCP service's heap climb to the V8 limit about once a day; the climb stopped when #2082 deployed.
Other information:
This pairs with #1850, which answers these GETs with 405. Please merge and deploy this PR after #1850 has been verified in production, because that verification reads the GET transactions in Sentry (grouped by user agent and status code) and this PR removes them.
The route match is anchored at the start of the path, so
/.well-known/oauth-protected-resource/mcp-oauthand unrelated paths such as/mcpfooare not affected.QA
/mcp,/mcp/<api key>and/mcp-oauth-claude(with and without an Authorization header)tools/listrequests to/mcp(bearer token) and/mcp/<api key>is_transaction:true transaction:*mcp*for that environment, grouped bytransactionandhttp.request.methodPOST /mcpandPOST /mcp/:idtransactions to be present (at the 20% sample rate) and no transaction withhttp.request.method:GETfor any MCP route/.well-known/oauth-protected-resource/mcp-oauthand any regular API route a few dozen times and confirm their GET transactions still appearResults of the testing already done:
tracesSamplerwith request contexts shaped like Sentry's http instrumentation passes them (method and URL vianormalizedRequest, and via span attributes only). GET on/mcp,/mcp/:id,/mcp-oauth-claudeand/mcp?x=1returns 0. POST on the same routes returns 0.2. Unrelated GETs return 0.2, public analytics 0.01, legacy/sse/:id0.2,/.well-known/.../mcp-oauth0.2,/mcpfoo0.2./mcp,/mcp/:idand/mcp-oauth-claude: zero GET transactions were recorded, whilePOST /mcpandPOST /mcp/:idwere recorded as before. Same result on SDK 10.45.0 (the locked version) and 10.56.0.Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
🤖 Generated with Claude Code