Skip to content

Commit 03ffd25

Browse files
authored
fix(cloudflare): Don't track negatively sampled spans (#21367)
supersedes #21349 for the time being. This does not fix the case where a `spanStart` event is emitted not a `spanEnd` event isn't. It only locally fixes an occurance of the symptom. We're gonna merge this first to resolve the cloudflare SDK bug but we'll need a more reliable fix than the one I hacked together in #21349. See #21349 (comment)
1 parent 7c19ead commit 03ffd25

2 files changed

Lines changed: 38 additions & 5 deletions

File tree

packages/cloudflare/src/client.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core';
2-
import { applySdkMetadata, debug, ServerRuntimeClient } from '@sentry/core';
2+
import { applySdkMetadata, debug, ServerRuntimeClient, spanIsSampled } from '@sentry/core';
33
import { DEBUG_BUILD } from './debug-build';
44
import type { makeFlushLock } from './flush';
55
import type { CloudflareTransportOptions } from './transport';
@@ -44,6 +44,15 @@ export class CloudflareClient extends ServerRuntimeClient {
4444
this._unsubscribeSpanStart = this.on('spanStart', span => {
4545
const spanId = span.spanContext().spanId;
4646
DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId);
47+
48+
// Negatively sampled spans never emit spanEnd,
49+
// so tracking them would cause _pendingSpans to grow unboundedly.
50+
// We should fix the inconsistent behavior for NonRecordingSpans in the future but
51+
// for now, we just ignore them.
52+
if (!spanIsSampled(span)) {
53+
return;
54+
}
55+
4756
this._pendingSpans.add(spanId);
4857

4958
if (!this._spanCompletionPromise) {

packages/cloudflare/test/client.test.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async';
33
import { CloudflareClient, type CloudflareClientOptions } from '../src/client';
44
import { makeFlushLock } from '../src/flush';
55

6+
const TRACE_FLAG_SAMPLED = 0x1;
7+
68
const MOCK_CLIENT_OPTIONS: CloudflareClientOptions = {
79
dsn: 'https://public@dsn.ingest.sentry.io/1337',
810
stackParser: () => [],
@@ -232,7 +234,7 @@ describe('CloudflareClient', () => {
232234

233235
// Emit spanStart
234236
const mockSpan = {
235-
spanContext: () => ({ spanId: 'test-span-id' }),
237+
spanContext: () => ({ spanId: 'test-span-id', traceFlags: TRACE_FLAG_SAMPLED }),
236238
};
237239
client.emit('spanStart', mockSpan as any);
238240

@@ -249,7 +251,7 @@ describe('CloudflareClient', () => {
249251
};
250252

251253
const mockSpan = {
252-
spanContext: () => ({ spanId: 'test-span-id' }),
254+
spanContext: () => ({ spanId: 'test-span-id', traceFlags: TRACE_FLAG_SAMPLED }),
253255
};
254256

255257
// Start span
@@ -269,8 +271,12 @@ describe('CloudflareClient', () => {
269271
_spanCompletionPromise: Promise<void> | null;
270272
};
271273

272-
const mockSpan1 = { spanContext: () => ({ spanId: 'span-1' }) };
273-
const mockSpan2 = { spanContext: () => ({ spanId: 'span-2' }) };
274+
const mockSpan1 = {
275+
spanContext: () => ({ spanId: 'span-1', traceFlags: TRACE_FLAG_SAMPLED }),
276+
};
277+
const mockSpan2 = {
278+
spanContext: () => ({ spanId: 'span-2', traceFlags: TRACE_FLAG_SAMPLED }),
279+
};
274280

275281
// Start both spans
276282
client.emit('spanStart', mockSpan1 as any);
@@ -291,6 +297,24 @@ describe('CloudflareClient', () => {
291297
await expect(completionPromise).resolves.toBeUndefined();
292298
});
293299

300+
it('does not track negatively sampled spans', () => {
301+
const client = new CloudflareClient(MOCK_CLIENT_OPTIONS);
302+
303+
const privateClient = client as unknown as {
304+
_pendingSpans: Set<string>;
305+
_spanCompletionPromise: Promise<void> | null;
306+
};
307+
308+
const nonRecordingSpan = {
309+
spanContext: () => ({ spanId: 'non-recording-span-id', traceFlags: 0 }),
310+
};
311+
312+
client.emit('spanStart', nonRecordingSpan as any);
313+
314+
expect(privateClient._pendingSpans.has('non-recording-span-id')).toBe(false);
315+
expect(privateClient._spanCompletionPromise).toBeNull();
316+
});
317+
294318
it('does not track spans after dispose', () => {
295319
const client = new CloudflareClient(MOCK_CLIENT_OPTIONS);
296320

0 commit comments

Comments
 (0)