Skip to content

Conversation

@JPeer264
Copy link
Member

@JPeer264 JPeer264 commented Nov 21, 2025

Events which hold attributes such as organization.slug or mcp.* were not flushed correctly.

attributes: {
"mcp.tool.name": tool.name,
"mcp.server.name": MCP_SERVER_NAME,
"mcp.server.version": LIB_VERSION,
...extractMcpParameters(params || {}),
},

With this it will re-add these missing events, as it will wait with waitUntil that these events are flushed within the tool call.

Example of a tool call event

Screenshot 2025-11-21 at 11 03 16

@JPeer264 JPeer264 requested review from betegon and dcramer November 21, 2025 10:04
Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I wonder tho why this/what changed compared to before

Copy link
Member

@betegon betegon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@JPeer264 JPeer264 merged commit bcb4c96 into main Nov 21, 2025
13 checks passed
@JPeer264 JPeer264 deleted the jp/fix-tool-calls branch November 21, 2025 11:15
@dcramer
Copy link
Member

dcramer commented Nov 21, 2025

Shouldn’t this be be resolved upstream in Sentry?

@JPeer264
Copy link
Member Author

Shouldn’t this be be resolved upstream in Sentry?

You're totally right. I'll see why this is actually needed.

JPeer264 added a commit to getsentry/sentry-javascript that referenced this pull request Dec 2, 2025
(closes #18360)

## Problem

In getsentry/sentry-mcp#652 we had to manually
wait for the flush to ensure that async operations completed before the
response was sent.

This was caused by the MCP server streaming data after the actual
request has been finished. When using the Cloudflare SDK with MCP
servers (or other streaming APIs), spans created during the streaming
phase were not being flushed properly because the `flush()` method would
complete before these async spans finished.

## Solution

During the request, when a span is created for the streaming call, we
now register listeners on the CloudflareClient for `spanStart` and
`spanEnd` events. The client tracks all pending spans and waits for them
to complete before flushing.

Specifically:
- Added `_pendingSpans` Set to track active span IDs
- Created a promise-based mechanism (`_spanCompletionPromise`) to wait
for all spans to complete
- Modified the `flush()` method to wait for pending spans before calling
`super.flush()`
- Added a timeout (default 5 seconds) to prevent indefinite waiting

## Test

Added a comprehensive E2E test application (`cloudflare-mcp`) that:
- Sets up a Cloudflare Worker with an MCP server
- Registers a tool that creates spans during execution
- Verifies that both the HTTP request span and the MCP tool call span
are properly sent to Sentry
- Validates span relationships (parent/child) and trace IDs

The test confirms that spans are now flushed correctly without manual
intervention.
dcramer pushed a commit that referenced this pull request Dec 2, 2025
In #652 we had to manually add the flush, which should actually be done
by the Cloudflare SDK. This has now landed in `10.28.0` (via [this
PR](getsentry/sentry-javascript#18334)) and no
manual flushing has to be done anymore.

Just to be sure sure I verified on top if everything has been sent
locally. This is a trace from my local deployment:

https://sentry-sdks.sentry.io/explore/traces/trace/434f1b1e8655444c875bce5c66645376/?fov=0%2C291&node=span-84a7a24ae593ee49&project=4510380305022976&source=traces&statsPeriod=15m&targetId=a505468e10176600&timestamp=1764689933

I recommend to review this PR [without whitespaces
enabled](https://github.com/getsentry/sentry-mcp/pull/663/files?w=1).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants