Skip to content

Commit 66599aa

Browse files
JPeer264sentry-junior[bot]
JPeer264
authored and
sentry-junior[bot]
committed
fixup! feat(cloudflare): Continue the Cloudflare trace of the invocation
Co-Authored-By: Jan Peer Stöcklmair <jan.peer@sentry.io>
1 parent fe52c29 commit 66599aa

7 files changed

Lines changed: 190 additions & 36 deletions

File tree

‎dev-packages/cloudflare-integration-tests/suites/tracing/propagation/worker-workerentrypoint-rpc/test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect, it } from 'vitest';
22
import type { Event, SerializedStreamedSpan } from '@sentry/core';
3+
import { SHORT_UUID_MATCHER } from '../../../../expect';
34
import { createRunner } from '../../../../runner';
5+
import { supports } from '../../../../runtime';
46
import { getSpanOp, getSpansFromEnvelope } from '../../../../spanUtils';
57

68
// Every route here is a raw URL, so the streamed segment name keeps the method only and the route
@@ -225,3 +227,32 @@ it('captures errors from loopback WorkerEntrypoint RPC without trace propagation
225227

226228
await runner.completed();
227229
});
230+
231+
it.runIf(supports({ minWorkerd: '1.20261009.1' }))(
232+
'attaches errors from loopback WorkerEntrypoint RPC without trace propagation to the Cloudflare trace',
233+
async ({ signal }) => {
234+
const errorEvents: Event[] = [];
235+
const segmentSpans: SerializedStreamedSpan[] = [];
236+
237+
const runner = createRunner(__dirname)
238+
.expect(envelope => {
239+
expect(envelope[1][0][0].type).toBe('event');
240+
errorEvents.push(envelope[1][0][1] as Event);
241+
})
242+
.expect(envelope => {
243+
expect(envelope[1][0][0].type).toBe('span');
244+
segmentSpans.push(...getSpansFromEnvelope(envelope).filter(span => span.is_segment));
245+
})
246+
.unordered()
247+
.start(signal);
248+
249+
await runner.makeRequest<string>('get', '/call-loopback-rpc-error');
250+
await runner.completed();
251+
252+
expect(errorEvents).toHaveLength(1);
253+
expect(segmentSpans).toHaveLength(1);
254+
// The RPC call carries no Sentry trace data, so only the Cloudflare trace connects the error to the caller.
255+
expect(errorEvents[0]?.contexts?.trace?.trace_id).toBe(segmentSpans[0]?.trace_id);
256+
expect(errorEvents[0]?.contexts?.trace?.parent_span_id).toEqual(SHORT_UUID_MATCHER);
257+
},
258+
);

‎packages/cloudflare/src/utils/cloudflareTrace.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { _INTERNAL_safeMathRandom, getCurrentScope } from '@sentry/core';
1+
import { _INTERNAL_safeMathRandom, getCurrentScope, isContinuingTrace } from '@sentry/core';
22

33
interface CloudflareSpanContext {
44
traceId: string;
@@ -23,13 +23,12 @@ const SPAN_ID_REGEXP = /^(?!0{16})[0-9a-f]{16}$/;
2323
* Sets the propagation context of the current scope to the Cloudflare trace of the invocation, so
2424
* errors, logs and spans get the trace id of the spans that Cloudflare exports.
2525
*
26-
* Does nothing when the request `headers` have a `sentry-trace` header: an incoming Sentry trace
27-
* keeps precedence, so traces from instrumented clients stay connected. Also does nothing when
28-
* Cloudflare does not trace the invocation, or when the runtime does not expose
26+
* Does nothing when the current scope already continues a trace, for example an incoming Sentry
27+
* trace, when Cloudflare does not trace the invocation, or when the runtime does not expose
2928
* `Span.spanContext()`. The sampling decision is left to the SDK.
3029
*/
31-
export function continueCloudflareTrace(context: unknown, headers?: Headers): void {
32-
if (headers?.get('sentry-trace')) {
30+
export function continueCloudflareTrace(context: unknown): void {
31+
if (isContinuingTrace(getCurrentScope().getPropagationContext())) {
3332
return;
3433
}
3534

‎packages/cloudflare/src/wrapMethodWithSentry.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type { ExecutionContextCompat } from './executionContext';
1616
import { flushAndDispose, getOriginalWaitUntil } from './flush';
1717
import { ensureInstrumented } from './instrument';
1818
import { init } from './sdk';
19+
import { continueCloudflareTrace } from './utils/cloudflareTrace';
1920
import { getInvocationState, getInvocationWaitUntil } from './utils/invocationContext';
2021
import { withInvocationIsolationScope } from './utils/invocationScope';
2122
import { extractRpcMeta } from './utils/rpcMeta';
@@ -183,6 +184,8 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
183184
}
184185
};
185186

187+
continueCloudflareTrace(context);
188+
186189
if (!spanName) {
187190
try {
188191
if (callback) {
@@ -272,10 +275,10 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
272275

273276
if (rpcMeta) {
274277
return runWithTeardown(() =>
275-
continueTrace(
276-
{ sentryTrace: rpcMeta['sentry-trace'] || '', baggage: rpcMeta.baggage || '' },
277-
executeSpan,
278-
),
278+
continueTrace({ sentryTrace: rpcMeta['sentry-trace'] || '', baggage: rpcMeta.baggage || '' }, () => {
279+
continueCloudflareTrace(context);
280+
return executeSpan();
281+
}),
279282
);
280283
}
281284

‎packages/cloudflare/src/wrapRequestHandlerWithInit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export function wrapRequestHandlerWithInit(
129129
origin: 'auto.http.cloudflare',
130130
mechanism: captureErrors ? { handled: false, type: getRequestErrorMechanismType(context) } : undefined,
131131
attributes,
132-
onTraceContinued: () => continueCloudflareTrace(context, request.headers),
132+
onTraceContinued: () => continueCloudflareTrace(context),
133133
isStreamingResponse: response => {
134134
const classification = safeCallback(
135135
DEBUG_BUILD ? 'Error in `isStreamingResponse`, using the default classification:' : '',

‎packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,100 @@ describe('instrumentWorkerEntrypoint', () => {
574574

575575
expect(initAndBind).toHaveBeenCalledTimes(5);
576576
});
577+
578+
describe('with a Cloudflare trace', () => {
579+
const CLOUDFLARE_TRACE_ID = 'a1b2c3d4e5f60718293a4b5c6d7e8f90';
580+
const CLOUDFLARE_SPAN_ID = 'b1c2d3e4f5a60718';
581+
const INCOMING_TRACE_ID = '12312012123120121231201212312012';
582+
const INCOMING_SPAN_ID = '1121201211212012';
583+
const CLOUDFLARE_TRACING = {
584+
getActiveSpan: () => ({
585+
isTraced: true,
586+
spanContext: () => ({ traceId: CLOUDFLARE_TRACE_ID, spanId: CLOUDFLARE_SPAN_ID, traceFlags: 1 }),
587+
}),
588+
};
589+
590+
it('attaches errors of RPC calls without RPC metadata to the Cloudflare trace', async () => {
591+
const events: Event[] = [];
592+
const waits: Promise<unknown>[] = [];
593+
const context = { ...createMockExecutionContext(), tracing: CLOUDFLARE_TRACING } as unknown as ExecutionContext;
594+
context.waitUntil = vi.fn(promise => {
595+
waits.push(promise);
596+
});
597+
const TestClass = class extends WorkerEntrypoint {
598+
async fail(): Promise<never> {
599+
throw new Error('rpc failure');
600+
}
601+
};
602+
const obj = Reflect.construct(
603+
instrumentWorkerEntrypoint(
604+
() => ({
605+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
606+
beforeSend(event) {
607+
events.push(event);
608+
return null;
609+
},
610+
}),
611+
TestClass as unknown as WorkerEntrypointConstructor,
612+
),
613+
[context, {}],
614+
);
615+
616+
await expect(obj.fail()).rejects.toThrow('rpc failure');
617+
await Promise.all(waits);
618+
619+
expect(events).toHaveLength(1);
620+
expect(events[0]?.contexts?.trace?.trace_id).toBe(CLOUDFLARE_TRACE_ID);
621+
expect(events[0]?.contexts?.trace?.parent_span_id).toBe(CLOUDFLARE_SPAN_ID);
622+
});
623+
624+
it('makes the RPC span a child of the Cloudflare span when the SDK does not continue the RPC trace', () => {
625+
let rpcSpan: ReturnType<typeof SentryCore.spanToJSON> | undefined;
626+
const TestClass = class extends WorkerEntrypoint {
627+
read() {
628+
rpcSpan = SentryCore.spanToJSON(SentryCore.getActiveSpan()!);
629+
}
630+
};
631+
const obj = Reflect.construct(
632+
instrumentWorkerEntrypoint(
633+
() => ({ dsn: 'https://public@o1.ingest.sentry.io/1337', tracesSampleRate: 1 }),
634+
TestClass as unknown as WorkerEntrypointConstructor,
635+
),
636+
[{ ...createMockExecutionContext(), tracing: CLOUDFLARE_TRACING }, {}],
637+
);
638+
639+
obj.read({
640+
__sentry_rpc_meta__: {
641+
'sentry-trace': `${INCOMING_TRACE_ID}-${INCOMING_SPAN_ID}-1`,
642+
baggage: `sentry-trace_id=${INCOMING_TRACE_ID},sentry-org_id=2`,
643+
},
644+
});
645+
646+
expect(rpcSpan?.trace_id).toBe(CLOUDFLARE_TRACE_ID);
647+
expect(rpcSpan?.parent_span_id).toBe(CLOUDFLARE_SPAN_ID);
648+
});
649+
650+
it('prefers the Sentry trace of the RPC metadata', () => {
651+
let rpcSpan: ReturnType<typeof SentryCore.spanToJSON> | undefined;
652+
const TestClass = class extends WorkerEntrypoint {
653+
read() {
654+
rpcSpan = SentryCore.spanToJSON(SentryCore.getActiveSpan()!);
655+
}
656+
};
657+
const obj = Reflect.construct(
658+
instrumentWorkerEntrypoint(
659+
() => ({ dsn: 'https://public@o1.ingest.sentry.io/1337', tracesSampleRate: 1 }),
660+
TestClass as unknown as WorkerEntrypointConstructor,
661+
),
662+
[{ ...createMockExecutionContext(), tracing: CLOUDFLARE_TRACING }, {}],
663+
);
664+
665+
obj.read({ __sentry_rpc_meta__: { 'sentry-trace': `${INCOMING_TRACE_ID}-${INCOMING_SPAN_ID}-1` } });
666+
667+
expect(rpcSpan?.trace_id).toBe(INCOMING_TRACE_ID);
668+
expect(rpcSpan?.parent_span_id).toBe(INCOMING_SPAN_ID);
669+
});
670+
});
577671
});
578672

579673
describe('env instrumentation', () => {

‎packages/cloudflare/test/request.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,51 @@ describe('withSentry', () => {
783783

784784
expect(sentryEvent.contexts?.trace?.trace_id).toBe('12312012123120121231201212312012');
785785
});
786+
787+
test.each([
788+
{
789+
reason: 'the org ids do not match',
790+
headers: {
791+
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
792+
baggage: 'sentry-trace_id=12312012123120121231201212312012,sentry-org_id=2',
793+
},
794+
options: {},
795+
},
796+
{
797+
reason: 'strictTraceContinuation is set and the baggage has no org id',
798+
headers: { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1' },
799+
options: { strictTraceContinuation: true },
800+
},
801+
{
802+
reason: 'the sentry-trace header is malformed',
803+
headers: { 'sentry-trace': 'not-a-sentry-trace' },
804+
options: {},
805+
},
806+
])('uses the Cloudflare trace when $reason', async ({ headers, options }) => {
807+
let sentryEvent: Event = {};
808+
await wrapRequestHandler(
809+
{
810+
options: {
811+
dsn: 'https://public@o1.ingest.sentry.io/1337',
812+
cacheClient: false,
813+
...options,
814+
beforeSend(event) {
815+
sentryEvent = event;
816+
return null;
817+
},
818+
},
819+
request: new Request('https://example.com', { headers }),
820+
context: { ...createMockExecutionContext(), tracing: CLOUDFLARE_TRACING } as unknown as ExecutionContext,
821+
},
822+
() => {
823+
SentryCore.captureException(new Error('test'));
824+
return new Response('test');
825+
},
826+
);
827+
828+
expect(sentryEvent.contexts?.trace?.trace_id).toBe(CLOUDFLARE_TRACE_ID);
829+
expect(sentryEvent.contexts?.trace?.parent_span_id).toBe(CLOUDFLARE_SPAN_ID);
830+
});
786831
});
787832
});
788833

‎packages/cloudflare/test/utils/cloudflareTrace.test.ts‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ describe('continueCloudflareTrace()', () => {
2727
});
2828
});
2929

30-
it('continues the trace when the sentry-trace header is empty', () => {
30+
it('keeps the propagation context when the scope continues a trace', () => {
3131
const context = {
3232
tracing: {
3333
getActiveSpan: () => ({
@@ -36,34 +36,16 @@ describe('continueCloudflareTrace()', () => {
3636
}),
3737
},
3838
};
39-
const headers = new Headers({ 'sentry-trace': '' });
40-
41-
withScope(() => {
42-
continueCloudflareTrace(context, headers);
43-
44-
expect(getCurrentScope().getPropagationContext()).toEqual({
45-
traceId: CLOUDFLARE_TRACE_ID,
46-
parentSpanId: CLOUDFLARE_SPAN_ID,
47-
sampleRand: expect.any(Number),
48-
});
49-
});
50-
});
51-
52-
it('keeps the propagation context when the request has a sentry-trace header', () => {
53-
const context = {
54-
tracing: {
55-
getActiveSpan: () => ({
56-
isTraced: true,
57-
spanContext: () => ({ traceId: CLOUDFLARE_TRACE_ID, spanId: CLOUDFLARE_SPAN_ID, traceFlags: 1 }),
58-
}),
59-
},
60-
};
61-
const headers = new Headers({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1' });
6239

6340
withScope(scope => {
41+
scope.setPropagationContext({
42+
traceId: '12345678901234567890123456789012',
43+
parentSpanId: '1234567890123456',
44+
sampleRand: 0.5,
45+
});
6446
const propagationContext = scope.getPropagationContext();
6547

66-
continueCloudflareTrace(context, headers);
48+
continueCloudflareTrace(context);
6749

6850
expect(scope.getPropagationContext()).toBe(propagationContext);
6951
});

0 commit comments

Comments
 (0)