fix(cloudflare): Fix instrumentDurableObjectWithSentry breaking Cloudflare Agents#21101
fix(cloudflare): Fix instrumentDurableObjectWithSentry breaking Cloudflare Agents#21101JPeer264 wants to merge 1 commit into
Conversation
…flare Agents The fix binds ALL methods to the original object, ensuring private fields work correctly. Additionally, spans are now only created when Sentry RPC metadata (`__sentry_rpc_meta__`) is present in the arguments, which is only the case for actual RPC calls that have trace propagation enabled on the calling side. This prevents creating spans for internal framework method calls like those made by the Agents SDK over WebSocket. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
JPeer264
requested review from
andreiborza and
mydea
and removed request for
a team
| TestDurableObjectBase, | ||
| ); | ||
|
|
||
| export default { |
There was a problem hiding this comment.
note: This is needed, as without it we now don't generate rpc traces. So this was fixed as well
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7234674. Configure here.
| const rpcMethodCache = new Map<string, UncheckedMethod>(); | ||
| // When using the deprecated `instrumentPrototypeMethods` option, always create spans. | ||
| // When using the new `enableRpcTracePropagation`, only create spans when RPC metadata is present. | ||
| const alwaysTrace = options.instrumentPrototypeMethods !== undefined; |
There was a problem hiding this comment.
alwaysTrace ignores enableRpcTracePropagation
High Severity
alwaysTrace is true whenever instrumentPrototypeMethods is present in options, including false. With enableRpcTracePropagation enabled, RPC prototype methods return the always-traced wrapper instead of the metadata-gated one, so internal calls without __sentry_rpc_meta__ still create RPC spans—reintroducing the Agents regression this PR fixes.
Reviewed by Cursor Bugbot for commit 7234674. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const rpcMethodCache = new Map<string, UncheckedMethod>(); | ||
| // When using the deprecated `instrumentPrototypeMethods` option, always create spans. | ||
| // When using the new `enableRpcTracePropagation`, only create spans when RPC metadata is present. | ||
| const alwaysTrace = options.instrumentPrototypeMethods !== undefined; |
There was a problem hiding this comment.
Bug: The alwaysTrace flag is incorrectly determined by the presence of the deprecated instrumentPrototypeMethods option, ignoring the new enableRpcTracePropagation option and leading to incorrect tracing behavior.
Severity: HIGH
Suggested Fix
The logic for setting the alwaysTrace flag in durableobject.ts should be updated to be consistent with the logic in getEffectiveRpcPropagation. It should prioritize the enableRpcTracePropagation option and correctly handle cases where both the new and deprecated options are present.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/cloudflare/src/durableobject.ts#L136
Potential issue: The logic for determining whether to always create spans for RPC
methods is flawed. The `alwaysTrace` flag is set to `true` if the deprecated
`instrumentPrototypeMethods` option is defined, regardless of its value (`true` or
`false`) and without considering the new `enableRpcTracePropagation` option. This leads
to incorrect behavior in two scenarios. First, if a user sets both
`enableRpcTracePropagation: true` and `instrumentPrototypeMethods: true` during
migration, tracing will be unconditionally enabled, bypassing the intended fix. Second,
if a user sets `instrumentPrototypeMethods: false` to disable it, tracing is still
unconditionally enabled for all methods, which is the opposite of the intended effect.
Did we get this right? 👍 / 👎 to inform future reviews.
size-limit report 📦
|
|
Some e2e tests are not passing, moving back to draft |
1 participant
By investigating #20099 it seemed that Cloudflare Agents were not working when
enableRpcTracePropagation: truewas set. The main reason for that was that the original target was not bound to the call.By doing that I changed the logic a little to also make it more resistant. As the
rpctrace was always everywhere attached, even though it could be that it was not directly a real RPC call. No therpctrace is only attached when in the arguments the__sentry_rpc_meta__is there as last parameter - which gives an extra layer of safety.Also
BUILT_IN_DO_METHODSis not needed and got removed, as we actually ignore these with the other checks (test was added for it)The deprecated
instrumentPrototypeMethodsis still using the previous behavior.This PR also proofs that we can instrument Agents on Cloudflare (
cloudflare-agente2e test)AI text on top:
The fix binds ALL methods to the original object, ensuring private fields
work correctly. Additionally, spans are now only created when Sentry RPC
metadata (
__sentry_rpc_meta__) is present in the arguments, which is onlythe case for actual RPC calls that have trace propagation enabled on the
calling side. This prevents creating spans for internal framework method
calls like those made by the Agents SDK over WebSocket.