feat(cloudflare): Add trace propagation for RPC method calls#20343
feat(cloudflare): Add trace propagation for RPC method calls#20343
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // instrumentPrototypeMethods can be boolean or string[], convert to boolean | ||
| return ( | ||
| instrumentPrototypeMethods === true || | ||
| (Array.isArray(instrumentPrototypeMethods) && instrumentPrototypeMethods.length > 0) | ||
| ); |
There was a problem hiding this comment.
Bug: The instrumentPrototypeMethods option no longer supports selective instrumentation. Providing an array of method names incorrectly instruments all methods, not just the specified ones.
Severity: MEDIUM
Suggested Fix
Update the proxy's get trap in durableobject.ts to check if instrumentPrototypeMethods is an array. If it is, only wrap methods whose names are included in that array. The boolean conversion in rpcOptions.ts should be removed, and the array of method names should be passed through to the instrumentation logic.
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/utils/rpcOptions.ts#L37-L41
Potential issue: The implementation for `instrumentPrototypeMethods` has a regression.
When an array of method names is provided to selectively instrument specific RPC
methods, the code at `rpcOptions.ts` incorrectly converts the array to a boolean,
discarding the method names. Consequently, the proxy logic in `durableobject.ts`
instruments all prototype methods instead of only the ones specified in the array. This
is a silent behavioral change that violates the documented API contract, which still
claims selective instrumentation is supported. Users expecting only certain methods to
be instrumented will have all of them instrumented instead.
Also affects:
packages/cloudflare/src/durableobject.ts:140~174
Did we get this right? 👍 / 👎 to inform future reviews.
![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 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7e5c662. Configure here.
| return ( | ||
| instrumentPrototypeMethods === true || | ||
| (Array.isArray(instrumentPrototypeMethods) && instrumentPrototypeMethods.length > 0) | ||
| ); |
There was a problem hiding this comment.
Deprecated array option silently instruments all methods
Medium Severity
The deprecated instrumentPrototypeMethods option accepted string[] for selective method instrumentation. getEffectiveRpcPropagation collapses any non-empty array to true, and the new Proxy in durableobject.ts wraps ALL prototype methods unconditionally. Users relying on instrumentPrototypeMethods: ['specificMethod'] now silently get every method instrumented. The documentation in client.ts still claims "only the specified method names will be instrumented."
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 7e5c662. Configure here.
|
|
||
| while (current && current !== Object.prototype) { | ||
| Object.getOwnPropertyNames(current).forEach(name => { | ||
| if (name !== 'constructor' && typeof (current as Record<string, unknown>)[name] === 'function') { |
There was a problem hiding this comment.
Object.prototype methods wrapped as RPC methods
Low Severity
The RPC Proxy's get trap only excludes methods in BUILT_IN_DO_METHODS. Inherited Object.prototype methods like toString, valueOf, and hasOwnProperty are not own properties and not in the exclusion set, so they pass all filters and get wrapped with wrapMethodWithSentry as RPC methods. Accessing them (e.g., via string coercion or logging) would create unwanted rpc spans and initialize a Sentry client.
Reviewed by Cursor Bugbot for commit 7e5c662. Configure here.
size-limit report 📦
|
|
Moved to draft, as tests are failing and I have to change 1-2 things that could reduce the amount lines added |
…pagation (#20345) follow up to #19991 It is better to release it first with an option to be enabled, that would then also be in line with #20343, otherwise `.fetch()` RPC calls would work without any option and the actual Cap'n'Proto RPC calls wouldn't work without. That would be an odd experience. ### New option: `enableRpcTracePropagation` > `instrumentPrototypeMethods` has been deprecated in favor of `enableRpcTracePropagation` Replaces the deprecated `instrumentPrototypeMethods` option with a clearer name that describes what it actually does. This option must be enabled on **both** the caller (Worker) and receiver (Durable Object) sides for trace propagation to work. It is also worth to mention that the implementation of "instrumenting prototype methods" has changed to a Proxy. ```ts // Worker side export default Sentry.withSentry( (env) => ({ dsn: env.SENTRY_DSN, enableRpcTracePropagation: true, }), handler, ); // Durable Object side export const MyDurableObject = Sentry.instrumentDurableObjectWithSentry( (env) => ({ dsn: env.SENTRY_DSN, enableRpcTracePropagation: true, }), MyDurableObjectBase, ); ```
1 participant
closes #19327
closes JS-1715
closes #16898
closes JS-680
closes #16760
closes JS-622
Summary
Adds trace propagation for Cloudflare Workers RPC method calls to Durable Objects.
This is admittedly a bit of a hack: Cap'n Proto (which powers Cloudflare RPC) has no native support for headers or metadata. To work around this, we append our trace data (sentry-trace + baggage) as a trailing argument object
{ __sentry: { trace, baggage } }to every RPC call. On the receiving DO side, we strip this argument before the user's method is invoked, so it's completely transparent.Caveat: If the Durable Object is not instrumented with Sentry, the trailing
__sentryargument will remain in the args array and be passed to the user's method. I would count this as ok since:...argsto retrieve all argumentsOtherwise, trace propagation should be seamless across Worker → DO and Worker → Worker → DO call chains.
New option:
enableRpcTracePropagationReplaces the deprecated
instrumentPrototypeMethodsoption with a clearer name that describes what it actually does. This option must be enabled on both the caller (Worker) and receiver (Durable Object) sides for trace propagation to work.It is also worth to mention that the implementation of "instrumenting prototype methods" has changed to a Proxy.
How it works
As mentioned above a Sentry trace object is appended on each call