Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,6 @@ Sentry.init({
);
```

- The `enableRpcTracePropagation` option now defaults to `true`. Trace context is propagated across RPC calls (service bindings, Durable Objects, WorkerEntrypoints) unless you explicitly set `enableRpcTracePropagation: false`.

- The `instrumentPrototypeMethods` option of `instrumentDurableObjectWithSentry` was removed. Use `enableRpcTracePropagation` instead, which was introduced as its replacement in v10.

```diff
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const MyDurableObject = Sentry.instrumentDurableObjectWithSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
MyDurableObjectBase,
);
Expand All @@ -28,7 +27,6 @@ export default Sentry.withSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
{
async fetch(request, env) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const MyDurableObject = Sentry.instrumentDurableObjectWithSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
MyDurableObjectBase,
);
Expand All @@ -31,7 +30,6 @@ export default Sentry.withSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
{
async fetch(request, env) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export const NoPropagationEntrypoint = Sentry.withSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
transportOptions: { fetch: fetch.bind(globalThis) },
}),
MySubWorkerEntrypointBase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ class LoopbackEntrypointBase extends WorkerEntrypoint<Env> {
}

export const LoopbackEntrypoint = Sentry.withSentry(
(env: Env) => ({
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 0,
enableRpcTracePropagation: false,
}),
(env: Env) => ({ dsn: env.SENTRY_DSN, traceLifecycle: 'static', tracesSampleRate: 0 }),
LoopbackEntrypointBase,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const MyDurableObject = Sentry.instrumentDurableObjectWithSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
MyDurableObjectBase,
);
Expand All @@ -47,7 +46,6 @@ export default Sentry.withSentry(
dsn: env.SENTRY_DSN,
traceLifecycle: 'static',
tracesSampleRate: 1.0,
enableRpcTracePropagation: false,
}),
MyWorkerEntrypointBase,
);
24 changes: 19 additions & 5 deletions packages/cloudflare/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,34 @@ interface BaseCloudflareOptions {
* - Create spans for each RPC method invocation
* - Capture errors thrown by RPC methods
*
* **Important:** This option is enabled by default. Set it to `false` to opt out, e.g. if you
* do not want trace context to leave your Worker via RPC calls.
* **Important:** This option should be enabled on **both sides** for full trace propagation.
*
* @default true
* @default false
* @example
* ```ts
* // Opt out of RPC trace propagation
* // Worker side (caller)
* export default Sentry.withSentry(
* (env) => ({
* dsn: env.SENTRY_DSN,
* enableRpcTracePropagation: false,
* enableRpcTracePropagation: true,
* }),
* handler,
* );
*
* // Durable Object side (receiver)
* export const MyDO = Sentry.instrumentDurableObjectWithSentry(
* (env) => ({
* dsn: env.SENTRY_DSN,
* enableRpcTracePropagation: true,
* }),
* MyDOBase,
* );
*
* // WorkerEntrypoint side (receiver)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The migration guide incorrectly states enableRpcTracePropagation defaults to true. The PR changes the default to false, but the documentation was not updated to reflect this.
Severity: HIGH

Suggested Fix

Update the migration documentation in docs/migration/v11-end-state.md. Correct the statement about enableRpcTracePropagation to reflect that it now defaults to false and is an opt-in feature. Emphasize that users must explicitly set enableRpcTracePropagation: true on both the caller and receiver to enable full trace propagation.

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/client.ts#L226

Potential issue: The PR reverts the default value of `enableRpcTracePropagation` from
`true` back to `false`. However, the migration documentation in
`docs/migration/v11-end-state.md` was not updated to reflect this change. The guide
still claims the option defaults to `true`. This discrepancy will cause users following
the migration guide to believe RPC trace propagation is enabled by default, leading to a
silent loss of tracing data when it is actually disabled. They will not see RPC spans in
their traces and may only discover this when debugging production issues.

Did we get this right? 👍 / 👎 to inform future reviews.

* export const MyEntrypoint = Sentry.withSentry(
* env => ({ dsn: env.SENTRY_DSN, enableRpcTracePropagation: true }),
* MyEntrypointBase,
* );
* ```
*/
enableRpcTracePropagation?: boolean;
Expand Down
8 changes: 4 additions & 4 deletions packages/cloudflare/src/durableobject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ export function finalizeWithRpcInstrumentation<T extends object>(
context: InstrumentedDurableObjectContext,
excludedMethods?: ReadonlySet<string>,
): T {
// Skip RPC instrumentation only when explicitly opted out (enabled by default)
if (options.enableRpcTracePropagation === false) {
// Skip RPC instrumentation if not enabled
if (!options.enableRpcTracePropagation) {
return obj;
}

Expand Down Expand Up @@ -403,8 +403,7 @@ function createRpcPrototypeWrapper(methodName: string, originalMethod: Unchecked
* - webSocketClose
* - webSocketError
*
* RPC methods (prototype methods) are instrumented by default. Set `enableRpcTracePropagation`
* to `false` to opt out.
* To instrument RPC methods (prototype methods), enable the `enableRpcTracePropagation` option.
*
* @param optionsCallback Function that returns the options for the SDK initialization.
* @param DurableObjectClass The Durable Object class to instrument.
Expand Down Expand Up @@ -483,6 +482,7 @@ export function instrumentDurableObjectWithSentry<
* env => ({
* dsn: env.SENTRY_DSN,
* tracesSampleRate: 1.0,
* enableRpcTracePropagation: true,
* }),
* MyAgentBase,
* );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function instrumentMethod(
true,
);

if (options.enableRpcTracePropagation === false) {
if (!options.enableRpcTracePropagation) {
return captureMethod;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function instrumentEnv<Env extends Record<string, unknown>>(env: Env, opt
return instrumented;
}

if (options?.enableRpcTracePropagation === false) {
if (!options?.enableRpcTracePropagation) {
return item;
}

Expand Down
21 changes: 2 additions & 19 deletions packages/cloudflare/test/durableobject.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,37 +317,20 @@ describe('instrumentDurableObjectWithSentry', () => {
expect(getInstrumented(obj.alarm)).toBeTruthy();
});

it('Does not instrument RPC methods when enableRpcTracePropagation is false', () => {
it('Does not instrument RPC methods when enableRpcTracePropagation is not set', () => {
const testClass = class {
rpcMethod() {
return 'result';
}
};
const instrumented = instrumentDurableObjectWithSentry(
vi.fn().mockReturnValue({ enableRpcTracePropagation: false }),
testClass as any,
);
const instrumented = instrumentDurableObjectWithSentry(vi.fn().mockReturnValue({}), testClass as any);
const obj = Reflect.construct(instrumented, []);

// RPC method should not be wrapped
expect(getInstrumented(obj.rpcMethod)).toBeFalsy();
expect(obj.rpcMethod()).toBe('result');
});

it('instruments RPC methods by default when enableRpcTracePropagation is not set', () => {
const testClass = class {
rpcMethod() {
return 'result';
}
};
const instrumented = instrumentDurableObjectWithSentry(vi.fn().mockReturnValue({}), testClass as any);
const obj = Reflect.construct(instrumented, []);

// RPC method should be wrapped on the prototype by default
expect(getInstrumented(obj.rpcMethod)).toBeTruthy();
expect(obj.rpcMethod()).toBe('result');
});

it('skips non-configurable prototype methods instead of failing construction', () => {
const testClass = class {
sealedMethod() {
Expand Down
27 changes: 6 additions & 21 deletions packages/cloudflare/test/instrumentations/instrumentEnv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,13 @@ describe('instrumentEnv', () => {
newUniqueId: vi.fn(),
};
const env = { COUNTER: doNamespace };
const instrumented = instrumentEnv(env, { enableRpcTracePropagation: false });
const instrumented = instrumentEnv(env);

// DO bindings pass through untouched when RPC propagation is disabled
expect(instrumented.COUNTER).toBe(doNamespace);
expect(instrumentDurableObjectNamespace).not.toHaveBeenCalled();
});

it('detects and instruments DurableObjectNamespace bindings by default', () => {
const doNamespace = {
idFromName: vi.fn(),
idFromString: vi.fn(),
get: vi.fn(),
newUniqueId: vi.fn(),
};
const env = { COUNTER: doNamespace };
const instrumented = instrumentEnv(env, {});

const result = instrumented.COUNTER;
expect(instrumentDurableObjectNamespace).toHaveBeenCalledWith(doNamespace);
expect((result as any).__instrumented).toBe(true);
});

it('detects and instruments DurableObjectNamespace bindings when enableRpcTracePropagation is enabled', () => {
const doNamespace = {
idFromName: vi.fn(),
Expand Down Expand Up @@ -175,7 +160,7 @@ describe('instrumentEnv', () => {
},
);
const env = { SERVICE: jsrpcProxy };
const instrumented = instrumentEnv(env, { enableRpcTracePropagation: false });
const instrumented = instrumentEnv(env);

const result = instrumented.SERVICE;
// Should be the same reference — not wrapped when propagation is disabled
Expand Down Expand Up @@ -361,7 +346,7 @@ describe('instrumentEnv', () => {
const mockFetch = vi.fn();
const mtlsFetcher = createMtlsFetcherProxy(mockFetch);
const env = { MY_CERT: mtlsFetcher };
const instrumented = instrumentEnv(env, { enableRpcTracePropagation: false });
const instrumented = instrumentEnv(env);

expect(instrumented.MY_CERT).toBe(mtlsFetcher);
});
Expand Down Expand Up @@ -393,7 +378,7 @@ describe('instrumentEnv', () => {
});

describe('JSRPC RPC method instrumentation', () => {
it('does not inject Sentry RPC meta when enableRpcTracePropagation is disabled', () => {
it('does not inject Sentry RPC meta by default (enableRpcTracePropagation not set)', () => {
vi.spyOn(SentryCore, 'getTraceData').mockReturnValue({
'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
baggage: 'sentry-environment=production',
Expand All @@ -412,11 +397,11 @@ describe('instrumentEnv', () => {
},
);
const env = { SERVICE: jsrpcProxy };
const instrumented = instrumentEnv(env, { enableRpcTracePropagation: false });
const instrumented = instrumentEnv(env);

instrumented.SERVICE.myRpcMethod('arg1', 42);

// With enableRpcTracePropagation disabled, no metadata should be injected
// Without enableRpcTracePropagation, no metadata should be injected
expect(rpcMethod).toHaveBeenCalledWith('arg1', 42);
});

Expand Down
Loading