From 3a59d22828427e65481289b35a48430539daca2c Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 23 Sep 2026 14:39:42 +0200 Subject: [PATCH 1/4] fix(cloudflare): Skip binding instrumentation work when spans cannot be sent The binding instrumentations (DO SQL, DO KV, sync KV, D1, R2, queue producer, agent callable RPC) built span data on every call, even when the SDK was disabled, had no DSN, had no tracing configured, or ran under an unsampled parent. On SQL-heavy Durable Objects the per-query sanitizing alone showed up as a large CPU regression with `tracesSampleRate: 0`. They now call the original method directly in these cases. D1 keeps adding breadcrumbs while the SDK is enabled, and sanitizes the query only when a statement runs. `setAlarm` keeps its span path, because it stores the span context for the alarm trace link. Co-Authored-By: Claude Opus 5.5 --- .../instrumentDurableObjectStorage.ts | 6 +++ .../instrumentDurableObjectSyncKvStorage.ts | 5 +++ .../instrumentations/instrumentSqlStorage.ts | 5 +++ .../worker/instrumentQueueProducer.ts | 9 +++++ .../instrumentations/worker/instrumentR2.ts | 23 +++++++++++ .../cloudflare/src/utils/isInUnsampledSpan.ts | 9 +++++ .../instrumentDurableObjectStorage.test.ts | 39 +++++++++++++++++++ ...strumentDurableObjectSyncKvStorage.test.ts | 26 +++++++++++++ .../test/instrumentSqlStorage.test.ts | 29 ++++++++++++++ .../worker/instrumentQueueProducer.test.ts | 29 +++++++++++++- .../worker/instrumentR2.test.ts | 29 +++++++++++++- packages/cloudflare/test/testUtils.ts | 27 ++++++++++++- .../test/utils/isInUnsampledSpan.test.ts | 32 +++++++++++++++ 13 files changed, 265 insertions(+), 3 deletions(-) create mode 100644 packages/cloudflare/src/utils/isInUnsampledSpan.ts create mode 100644 packages/cloudflare/test/utils/isInUnsampledSpan.test.ts diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts index 976412bae83d..764a1976edde 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts @@ -3,6 +3,7 @@ import { SENTRY_OP } from '@sentry/conventions/attributes'; import { DB } from '@sentry/conventions/op'; import { getClient, isThenable, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; import type { CloudflareClientOptions } from '../client'; +import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; import { getStorageKeys, targetsCloudflareInternalKey } from '../utils/internalStorageKey'; import { storeSpanContext } from '../utils/traceLinks'; import { instrumentDurableObjectSyncKvStorage } from './instrumentDurableObjectSyncKvStorage'; @@ -59,6 +60,11 @@ export function instrumentDurableObjectStorage( } return function (this: unknown, ...args: unknown[]) { + // `setAlarm` keeps its span, because the alarm links back to the span context stored here. + if (methodName !== 'setAlarm' && isInUnsampledSpan()) { + return (original as (...a: unknown[]) => unknown).apply(target, args); + } + // KV entries managed by the DO framework itself (agents/partyserver state) are bookkeeping // rather than user work — skip the span, mirroring how `cf_` SQL tables are treated. // oxlint-disable-next-line typescript/no-unnecessary-type-assertion -- rule false positive: the cast reaches the Cloudflare-only `durableObjectStorageSpanAllowlist`; tsc errors without it diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts index c8b94d14fef4..9203db88f338 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts @@ -2,6 +2,7 @@ import type { SyncKvStorage } from '@cloudflare/workers-types'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { DB } from '@sentry/conventions/op'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; const SYNC_KV_METHODS_TO_INSTRUMENT = ['get', 'put', 'delete', 'list'] as const; @@ -23,6 +24,10 @@ export function instrumentDurableObjectSyncKvStorage(syncKv: SyncKvStorage): Syn } return function (this: unknown, ...args: unknown[]) { + if (isInUnsampledSpan()) { + return (original as (...args: unknown[]) => unknown).apply(target, args); + } + return startSpan( { name: `durable_object_storage_kv_${methodName}`, diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index e7ae5e5d173f..ba3293f124c5 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -5,6 +5,7 @@ import { getClient, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/ import { getSqlQuerySummary, sanitizeSqlQuery } from '@sentry/server-utils'; import type { CloudflareClientOptions } from '../client'; import { targetsCloudflareInternalTable } from '../utils/internalSqlQuery'; +import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; /** * Instruments the Durable Object SqlStorage `exec` method with Sentry spans. @@ -24,6 +25,10 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { return function (this: unknown, ...args: unknown[]) { const [query, ...bindings] = args as [string, ...unknown[]]; + if (isInUnsampledSpan()) { + return (original as (...a: unknown[]) => ReturnType).apply(target, args); + } + const sanitizedQuery = sanitizeSqlQuery(query); const querySummary = getSqlQuerySummary(sanitizedQuery); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts b/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts index d1b3de4ba107..0996d716318f 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts @@ -2,6 +2,7 @@ import type { MessageSendRequest, Queue, QueueSendBatchOptions, QueueSendOptions import { SENTRY_OP } from '@sentry/conventions/attributes'; import { QUEUE_PUBLISH } from '@sentry/conventions/op'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { isInUnsampledSpan } from '../../utils/isInUnsampledSpan'; const ORIGIN = 'auto.faas.cloudflare.queue'; @@ -71,6 +72,10 @@ export function instrumentQueueProducer(queue: T, bindingName: const original = Reflect.get(target, prop, receiver) as Queue['send']; return function (this: unknown, message: unknown, options?: QueueSendOptions): ReturnType { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, [message, options]); + } + return startPublishSpan({ bindingName, bodySize: getBodySize(message) }, () => Reflect.apply(original, target, [message, options]), ); @@ -84,6 +89,10 @@ export function instrumentQueueProducer(queue: T, bindingName: messages: Iterable, options?: QueueSendBatchOptions, ): ReturnType { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, [messages, options]); + } + const messageArray = Array.from(messages); const totalBodySize = messageArray.reduce((acc, m) => { const size = getBodySize(m.body); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts b/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts index 7d6ad24ae3e2..0d2cd145ec02 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts @@ -20,6 +20,7 @@ import { OBJECT_UPLOAD_PART, } from '@sentry/conventions/op'; import { isObjectLike, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { isInUnsampledSpan } from '../../utils/isInUnsampledSpan'; const ORIGIN = 'auto.faas.cloudflare.r2'; @@ -81,6 +82,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, args); + } + const [partNumber] = args; const spanOptions = createSpanOptions(bindingName, 'uploadPart', key); @@ -101,6 +106,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown) { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, []); + } + return startSpan(createSpanOptions(bindingName, 'abortMultipartUpload', key), () => Reflect.apply(original, target, []), ); @@ -111,6 +120,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, args); + } + return startSpan(createSpanOptions(bindingName, 'completeMultipartUpload', key), () => Reflect.apply(original, target, args), ); @@ -135,6 +148,10 @@ export function instrumentR2Bucket(bucket: T, bindingName: s const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, args); + } + const [key] = args; return startSpan(createSpanOptions(bindingName, prop, key), () => Reflect.apply(original, target, args)); @@ -145,6 +162,12 @@ export function instrumentR2Bucket(bucket: T, bindingName: s const original = Reflect.get(target, prop, receiver) as R2Bucket['createMultipartUpload']; return function (this: unknown, ...args: Parameters) { + if (isInUnsampledSpan()) { + return Reflect.apply(original, target, args).then(upload => + instrumentR2MultipartUpload(upload, bindingName), + ); + } + const [key] = args; return startSpan(createSpanOptions(bindingName, 'createMultipartUpload', key), async () => { diff --git a/packages/cloudflare/src/utils/isInUnsampledSpan.ts b/packages/cloudflare/src/utils/isInUnsampledSpan.ts new file mode 100644 index 000000000000..94653abb0309 --- /dev/null +++ b/packages/cloudflare/src/utils/isInUnsampledSpan.ts @@ -0,0 +1,9 @@ +import { getActiveSpan, spanIsSampled } from '@sentry/core'; + +/** + * Returns `true` when an active span exists and is not sampled. Returns `false` when there is no active span. + */ +export function isInUnsampledSpan(): boolean { + const activeSpan = getActiveSpan(); + return !!activeSpan && !spanIsSampled(activeSpan); +} diff --git a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts index 27d393ddb6ee..7cb1974885e9 100644 --- a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts @@ -3,6 +3,7 @@ import * as sentryCore from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { instrumentDurableObjectStorage } from '../src/instrumentations/instrumentDurableObjectStorage'; import * as traceLinks from '../src/utils/traceLinks'; +import { initTestClient, resetSdk } from './testUtils'; vi.mock('../src/utils/traceLinks', async importOriginal => { const actual = await importOriginal(); @@ -474,6 +475,44 @@ describe('instrumentDurableObjectStorage', () => { await expect(instrumented.get('myKey')).rejects.toThrow('Storage error'); }); }); + + describe('inside a parent span', () => { + afterEach(() => { + resetSdk(); + }); + + it.each([ + [1, true], + [0, false], + ])('with tracesSampleRate %s, starts a span for KV methods: %s', (tracesSampleRate, startsSpan) => { + initTestClient({ tracesSampleRate }); + const mockStorage = createMockStorage(); + const instrumented = instrumentDurableObjectStorage(mockStorage); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + + void instrumented.get('myKey'); + + expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + }); + + expect(mockStorage.get).toHaveBeenCalledWith('myKey'); + }); + + it('with tracesSampleRate 0, still starts a span for setAlarm', () => { + initTestClient({ tracesSampleRate: 0 }); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + + void instrumented.setAlarm(Date.now() + 1000); + + expect(startSpanSpy).toHaveBeenCalledTimes(1); + }); + }); + }); }); function createMockStorage(): any { diff --git a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts index f6135c2f919c..4040586c3341 100644 --- a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts @@ -2,6 +2,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import * as sentryCore from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { instrumentDurableObjectSyncKvStorage } from '../src/instrumentations/instrumentDurableObjectSyncKvStorage'; +import { initTestClient, resetSdk } from './testUtils'; describe('instrumentDurableObjectSyncKvStorage', () => { afterEach(() => { @@ -185,6 +186,31 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(() => instrumented.get('myKey')).toThrow('Storage error'); }); }); + + describe('inside a parent span', () => { + afterEach(() => { + resetSdk(); + }); + + it.each([ + [1, true], + [0, false], + ])('with tracesSampleRate %s, starts a span: %s', (tracesSampleRate, startsSpan) => { + initTestClient({ tracesSampleRate }); + const mockKv = createMockSyncKv(); + const instrumented = instrumentDurableObjectSyncKvStorage(mockKv); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + + instrumented.get('myKey'); + + expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + }); + + expect(mockKv.get).toHaveBeenCalledWith('myKey'); + }); + }); }); function createMockSyncKv(): any { diff --git a/packages/cloudflare/test/instrumentSqlStorage.test.ts b/packages/cloudflare/test/instrumentSqlStorage.test.ts index 3436d0eb9032..9b23725df356 100644 --- a/packages/cloudflare/test/instrumentSqlStorage.test.ts +++ b/packages/cloudflare/test/instrumentSqlStorage.test.ts @@ -1,7 +1,9 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import * as sentryCore from '@sentry/core'; +import * as serverUtils from '@sentry/server-utils'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { instrumentSqlStorage } from '../src/instrumentations/instrumentSqlStorage'; +import { initTestClient, resetSdk } from './testUtils'; describe('instrumentSqlStorage', () => { afterEach(() => { @@ -259,6 +261,33 @@ describe('instrumentSqlStorage', () => { }); }); }); + + describe('inside a parent span', () => { + afterEach(() => { + resetSdk(); + }); + + it.each([ + [1, true], + [0, false], + ])('with tracesSampleRate %s, sanitizes the query and starts a span: %s', (tracesSampleRate, startsSpan) => { + initTestClient({ tracesSampleRate }); + const sanitizeSpy = vi.spyOn(serverUtils, 'sanitizeSqlQuery'); + const mockSql = createMockSqlStorage(); + const instrumented = instrumentSqlStorage(mockSql); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + + instrumented.exec('SELECT * FROM users WHERE id = ?', 42); + + expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + }); + + expect(sanitizeSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + expect(mockSql.exec).toHaveBeenCalledWith('SELECT * FROM users WHERE id = ?', 42); + }); + }); }); /** diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts index f176d92e53db..68f969ba4818 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts @@ -1,7 +1,8 @@ import type { Queue } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentQueueProducer } from '../../../src/instrumentations/worker/instrumentQueueProducer'; +import { initTestClient, resetSdk } from '../../testUtils'; function createMockQueue(): Queue { return { @@ -193,4 +194,30 @@ describe('instrumentQueueProducer', () => { await expect(wrapped.metrics()).resolves.toEqual(metrics); }); + + describe('inside a parent span', () => { + afterEach(() => { + resetSdk(); + }); + + test.each([ + [1, true], + [0, false], + ])('with tracesSampleRate %s, starts a span: %s', async (tracesSampleRate, startsSpan) => { + initTestClient({ tracesSampleRate }); + const queue = createMockQueue(); + const wrapped = instrumentQueueProducer(queue, 'MY_QUEUE'); + + await SentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); + + const result = wrapped.send({ hello: 'world' }); + + expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + return result; + }); + + expect(queue.send).toHaveBeenLastCalledWith({ hello: 'world' }, undefined); + }); + }); }); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts index 73aa3d2bf72c..e3c2b0585ee6 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts @@ -1,7 +1,8 @@ import type { R2Bucket, R2MultipartUpload } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentR2Bucket } from '../../../src/instrumentations/worker/instrumentR2'; +import { initTestClient, resetSdk } from '../../testUtils'; const MOCK_R2_OBJECT = { key: 'my-file.txt', @@ -342,4 +343,30 @@ describe('instrumentR2Bucket', () => { const wrapped = instrumentR2Bucket(bucket, 'MY_BUCKET') as R2Bucket & { customMethod: () => string }; expect(wrapped.customMethod()).toBe('hi'); }); + + describe('inside a parent span', () => { + afterEach(() => { + resetSdk(); + }); + + test.each([ + [1, true], + [0, false], + ])('with tracesSampleRate %s, starts a span: %s', async (tracesSampleRate, startsSpan) => { + initTestClient({ tracesSampleRate }); + const bucket = createMockR2Bucket(); + const wrapped = instrumentR2Bucket(bucket, 'MY_BUCKET'); + + await SentryCore.startSpan({ name: 'parent' }, () => { + startSpanSpy.mockClear(); + + const result = wrapped.get('my-file.txt'); + + expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + return result; + }); + + expect(bucket.get).toHaveBeenCalledWith('my-file.txt'); + }); + }); }); diff --git a/packages/cloudflare/test/testUtils.ts b/packages/cloudflare/test/testUtils.ts index 79bdfa403365..5956935f8eba 100644 --- a/packages/cloudflare/test/testUtils.ts +++ b/packages/cloudflare/test/testUtils.ts @@ -1,5 +1,7 @@ import { context, propagation, trace } from '@opentelemetry/api'; -import { getMainCarrier } from '@sentry/core'; +import { getMainCarrier, setCurrentClient } from '@sentry/core'; +import { vi } from 'vitest'; +import { CloudflareClient, type CloudflareClientOptions } from '../src/client'; import { _clearGlobalClientCache } from '../src/clientCache'; function resetGlobals(): void { @@ -18,3 +20,26 @@ export function resetSdk(): void { cleanupOtel(); _clearGlobalClientCache(); } + +/** + * Resets the SDK and sets a `CloudflareClient` with a DSN and a mock transport as the current client. + * `options` are passed to the client, for example `tracesSampleRate`. + */ +export function initTestClient(options: Partial = {}): CloudflareClient { + resetSdk(); + + const client = new CloudflareClient({ + dsn: 'https://123@sentry.io/42', + stackParser: () => [], + integrations: [], + transport: () => ({ + send: vi.fn().mockResolvedValue({}), + flush: vi.fn().mockResolvedValue(true), + }), + ...options, + }); + setCurrentClient(client); + client.init(); + + return client; +} diff --git a/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts b/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts new file mode 100644 index 000000000000..821aef2fb5ad --- /dev/null +++ b/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts @@ -0,0 +1,32 @@ +import { startSpan } from '@sentry/core'; +import { afterEach, describe, expect, it } from 'vitest'; +import { isInUnsampledSpan } from '../../src/utils/isInUnsampledSpan'; +import { initTestClient, resetSdk } from '../testUtils'; + +describe('isInUnsampledSpan', () => { + afterEach(() => { + resetSdk(); + }); + + it('returns false when there is no active span', () => { + initTestClient({ tracesSampleRate: 0 }); + + expect(isInUnsampledSpan()).toBe(false); + }); + + it('returns false inside a sampled span', () => { + initTestClient({ tracesSampleRate: 1 }); + + startSpan({ name: 'parent' }, () => { + expect(isInUnsampledSpan()).toBe(false); + }); + }); + + it('returns true inside an unsampled span', () => { + initTestClient({ tracesSampleRate: 0 }); + + startSpan({ name: 'parent' }, () => { + expect(isInUnsampledSpan()).toBe(true); + }); + }); +}); From 48b1d3e8bd9542c3aef9889db1cfe0a053a8d995 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Tue, 29 Sep 2026 15:03:26 +0200 Subject: [PATCH 2/4] fixup! fix(cloudflare): Skip binding instrumentation work when spans cannot be sent --- .../instrumentDurableObjectStorage.ts | 6 -- .../instrumentDurableObjectSyncKvStorage.ts | 5 -- .../instrumentations/instrumentSqlStorage.ts | 39 +++++++--- .../worker/instrumentQueueProducer.ts | 9 --- .../instrumentations/worker/instrumentR2.ts | 23 ------ .../cloudflare/src/utils/internalSqlQuery.ts | 10 +++ .../cloudflare/src/utils/isInUnsampledSpan.ts | 9 --- .../instrumentDurableObjectStorage.test.ts | 40 +--------- ...strumentDurableObjectSyncKvStorage.test.ts | 26 ------- .../test/instrumentSqlStorage.test.ts | 78 ++++++++++++++++--- .../worker/instrumentQueueProducer.test.ts | 29 +------ .../worker/instrumentR2.test.ts | 29 +------ .../test/utils/internalSqlQuery.test.ts | 22 +++++- .../test/utils/isInUnsampledSpan.test.ts | 32 -------- 14 files changed, 131 insertions(+), 226 deletions(-) delete mode 100644 packages/cloudflare/src/utils/isInUnsampledSpan.ts delete mode 100644 packages/cloudflare/test/utils/isInUnsampledSpan.test.ts diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts index 764a1976edde..976412bae83d 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts @@ -3,7 +3,6 @@ import { SENTRY_OP } from '@sentry/conventions/attributes'; import { DB } from '@sentry/conventions/op'; import { getClient, isThenable, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; import type { CloudflareClientOptions } from '../client'; -import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; import { getStorageKeys, targetsCloudflareInternalKey } from '../utils/internalStorageKey'; import { storeSpanContext } from '../utils/traceLinks'; import { instrumentDurableObjectSyncKvStorage } from './instrumentDurableObjectSyncKvStorage'; @@ -60,11 +59,6 @@ export function instrumentDurableObjectStorage( } return function (this: unknown, ...args: unknown[]) { - // `setAlarm` keeps its span, because the alarm links back to the span context stored here. - if (methodName !== 'setAlarm' && isInUnsampledSpan()) { - return (original as (...a: unknown[]) => unknown).apply(target, args); - } - // KV entries managed by the DO framework itself (agents/partyserver state) are bookkeeping // rather than user work — skip the span, mirroring how `cf_` SQL tables are treated. // oxlint-disable-next-line typescript/no-unnecessary-type-assertion -- rule false positive: the cast reaches the Cloudflare-only `durableObjectStorageSpanAllowlist`; tsc errors without it diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts index 9203db88f338..c8b94d14fef4 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts @@ -2,7 +2,6 @@ import type { SyncKvStorage } from '@cloudflare/workers-types'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { DB } from '@sentry/conventions/op'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; -import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; const SYNC_KV_METHODS_TO_INSTRUMENT = ['get', 'put', 'delete', 'list'] as const; @@ -24,10 +23,6 @@ export function instrumentDurableObjectSyncKvStorage(syncKv: SyncKvStorage): Syn } return function (this: unknown, ...args: unknown[]) { - if (isInUnsampledSpan()) { - return (original as (...args: unknown[]) => unknown).apply(target, args); - } - return startSpan( { name: `durable_object_storage_kv_${methodName}`, diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index ba3293f124c5..295d9753d326 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -1,11 +1,24 @@ import type { SqlStorage } from '@cloudflare/workers-types'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { DB_QUERY } from '@sentry/conventions/op'; -import { getClient, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { + getActiveSpan, + getClient, + hasSpansEnabled, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + spanIsSampled, + startSpan, +} from '@sentry/core'; import { getSqlQuerySummary, sanitizeSqlQuery } from '@sentry/server-utils'; import type { CloudflareClientOptions } from '../client'; -import { targetsCloudflareInternalTable } from '../utils/internalSqlQuery'; -import { isInUnsampledSpan } from '../utils/isInUnsampledSpan'; +import { mayTargetCloudflareInternalTable, targetsCloudflareInternalTable } from '../utils/internalSqlQuery'; + +const SPAN_ATTRIBUTES = { + [SENTRY_OP]: DB_QUERY, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object.sql', + 'db.system.name': 'cloudflare-durable-object-sql', + 'db.operation.name': 'exec', +}; /** * Instruments the Durable Object SqlStorage `exec` method with Sentry spans. @@ -23,10 +36,17 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { } return function (this: unknown, ...args: unknown[]) { + const callOriginal = (): ReturnType => + (original as (...a: unknown[]) => ReturnType).apply(target, args); + const [query, ...bindings] = args as [string, ...unknown[]]; - if (isInUnsampledSpan()) { - return (original as (...a: unknown[]) => ReturnType).apply(target, args); + const activeSpan = getActiveSpan(); + const spanIsNeverSent = !hasSpansEnabled() || (!!activeSpan && !spanIsSampled(activeSpan)); + if (spanIsNeverSent && !mayTargetCloudflareInternalTable(query)) { + // The query needs no sanitizing. `startSpan` still runs, so each query starts a span with and + // without spans enabled, and an unsampled span records its dropped span outcome. + return startSpan({ name: 'exec', attributes: SPAN_ATTRIBUTES }, callOriginal); } const sanitizedQuery = sanitizeSqlQuery(query); @@ -37,23 +57,20 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { ?.durableObjectSqlSpanAllowlist; if (targetsCloudflareInternalTable(querySummary, allowlist, sanitizedQuery)) { - return (original as (...a: unknown[]) => ReturnType).apply(target, args); + return callOriginal(); } return startSpan( { name: querySummary || sanitizedQuery, attributes: { - [SENTRY_OP]: DB_QUERY, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object.sql', - 'db.system.name': 'cloudflare-durable-object-sql', - 'db.operation.name': 'exec', + ...SPAN_ATTRIBUTES, 'db.query.text': sanitizedQuery, 'db.query.summary': querySummary, 'cloudflare.durable_object.query.bindings': bindings.length, }, }, - () => (original as (...a: unknown[]) => ReturnType).apply(target, args), + callOriginal, ); }; }, diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts b/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts index 0996d716318f..d1b3de4ba107 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts @@ -2,7 +2,6 @@ import type { MessageSendRequest, Queue, QueueSendBatchOptions, QueueSendOptions import { SENTRY_OP } from '@sentry/conventions/attributes'; import { QUEUE_PUBLISH } from '@sentry/conventions/op'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; -import { isInUnsampledSpan } from '../../utils/isInUnsampledSpan'; const ORIGIN = 'auto.faas.cloudflare.queue'; @@ -72,10 +71,6 @@ export function instrumentQueueProducer(queue: T, bindingName: const original = Reflect.get(target, prop, receiver) as Queue['send']; return function (this: unknown, message: unknown, options?: QueueSendOptions): ReturnType { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, [message, options]); - } - return startPublishSpan({ bindingName, bodySize: getBodySize(message) }, () => Reflect.apply(original, target, [message, options]), ); @@ -89,10 +84,6 @@ export function instrumentQueueProducer(queue: T, bindingName: messages: Iterable, options?: QueueSendBatchOptions, ): ReturnType { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, [messages, options]); - } - const messageArray = Array.from(messages); const totalBodySize = messageArray.reduce((acc, m) => { const size = getBodySize(m.body); diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts b/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts index 0d2cd145ec02..7d6ad24ae3e2 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts @@ -20,7 +20,6 @@ import { OBJECT_UPLOAD_PART, } from '@sentry/conventions/op'; import { isObjectLike, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; -import { isInUnsampledSpan } from '../../utils/isInUnsampledSpan'; const ORIGIN = 'auto.faas.cloudflare.r2'; @@ -82,10 +81,6 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, args); - } - const [partNumber] = args; const spanOptions = createSpanOptions(bindingName, 'uploadPart', key); @@ -106,10 +101,6 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown) { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, []); - } - return startSpan(createSpanOptions(bindingName, 'abortMultipartUpload', key), () => Reflect.apply(original, target, []), ); @@ -120,10 +111,6 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, args); - } - return startSpan(createSpanOptions(bindingName, 'completeMultipartUpload', key), () => Reflect.apply(original, target, args), ); @@ -148,10 +135,6 @@ export function instrumentR2Bucket(bucket: T, bindingName: s const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, args); - } - const [key] = args; return startSpan(createSpanOptions(bindingName, prop, key), () => Reflect.apply(original, target, args)); @@ -162,12 +145,6 @@ export function instrumentR2Bucket(bucket: T, bindingName: s const original = Reflect.get(target, prop, receiver) as R2Bucket['createMultipartUpload']; return function (this: unknown, ...args: Parameters) { - if (isInUnsampledSpan()) { - return Reflect.apply(original, target, args).then(upload => - instrumentR2MultipartUpload(upload, bindingName), - ); - } - const [key] = args; return startSpan(createSpanOptions(bindingName, 'createMultipartUpload', key), async () => { diff --git a/packages/cloudflare/src/utils/internalSqlQuery.ts b/packages/cloudflare/src/utils/internalSqlQuery.ts index 7c67415d4da9..70d45a5af9d8 100644 --- a/packages/cloudflare/src/utils/internalSqlQuery.ts +++ b/packages/cloudflare/src/utils/internalSqlQuery.ts @@ -37,6 +37,16 @@ export function targetsCloudflareInternalTable( return tables.some(table => isCloudflareInternalTable(table, allowlist)); } +/** + * Returns `false` when the raw `query` has no word that starts with `cf_`, so it cannot target a + * Cloudflare internal table. Needs no sanitizing, which makes it cheap enough to run on every query. + */ +export function mayTargetCloudflareInternalTable(query: string): boolean { + return CF_PREFIX_RE.test(query); +} + +const CF_PREFIX_RE = /\bcf_/i; + // `CREATE [UNIQUE] INDEX [IF NOT EXISTS] ON ` — the IF EXISTS shape mirrors DDL_RE // in @sentry/core. const CREATE_INDEX_TABLE_RE = diff --git a/packages/cloudflare/src/utils/isInUnsampledSpan.ts b/packages/cloudflare/src/utils/isInUnsampledSpan.ts deleted file mode 100644 index 94653abb0309..000000000000 --- a/packages/cloudflare/src/utils/isInUnsampledSpan.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { getActiveSpan, spanIsSampled } from '@sentry/core'; - -/** - * Returns `true` when an active span exists and is not sampled. Returns `false` when there is no active span. - */ -export function isInUnsampledSpan(): boolean { - const activeSpan = getActiveSpan(); - return !!activeSpan && !spanIsSampled(activeSpan); -} diff --git a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts index 7cb1974885e9..af5161e5bcde 100644 --- a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts @@ -16,6 +16,7 @@ vi.mock('../src/utils/traceLinks', async importOriginal => { describe('instrumentDurableObjectStorage', () => { afterEach(() => { vi.restoreAllMocks(); + resetSdk(); }); describe('get', () => { @@ -302,6 +303,7 @@ describe('instrumentDurableObjectStorage', () => { }); it('instruments sql exec', () => { + initTestClient({ tracesSampleRate: 1 }); const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); const mockStorage = createMockStorage(); const instrumented = instrumentDurableObjectStorage(mockStorage); @@ -475,44 +477,6 @@ describe('instrumentDurableObjectStorage', () => { await expect(instrumented.get('myKey')).rejects.toThrow('Storage error'); }); }); - - describe('inside a parent span', () => { - afterEach(() => { - resetSdk(); - }); - - it.each([ - [1, true], - [0, false], - ])('with tracesSampleRate %s, starts a span for KV methods: %s', (tracesSampleRate, startsSpan) => { - initTestClient({ tracesSampleRate }); - const mockStorage = createMockStorage(); - const instrumented = instrumentDurableObjectStorage(mockStorage); - - sentryCore.startSpan({ name: 'parent' }, () => { - const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); - - void instrumented.get('myKey'); - - expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); - }); - - expect(mockStorage.get).toHaveBeenCalledWith('myKey'); - }); - - it('with tracesSampleRate 0, still starts a span for setAlarm', () => { - initTestClient({ tracesSampleRate: 0 }); - const instrumented = instrumentDurableObjectStorage(createMockStorage()); - - sentryCore.startSpan({ name: 'parent' }, () => { - const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); - - void instrumented.setAlarm(Date.now() + 1000); - - expect(startSpanSpy).toHaveBeenCalledTimes(1); - }); - }); - }); }); function createMockStorage(): any { diff --git a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts index 4040586c3341..f6135c2f919c 100644 --- a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts @@ -2,7 +2,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import * as sentryCore from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { instrumentDurableObjectSyncKvStorage } from '../src/instrumentations/instrumentDurableObjectSyncKvStorage'; -import { initTestClient, resetSdk } from './testUtils'; describe('instrumentDurableObjectSyncKvStorage', () => { afterEach(() => { @@ -186,31 +185,6 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(() => instrumented.get('myKey')).toThrow('Storage error'); }); }); - - describe('inside a parent span', () => { - afterEach(() => { - resetSdk(); - }); - - it.each([ - [1, true], - [0, false], - ])('with tracesSampleRate %s, starts a span: %s', (tracesSampleRate, startsSpan) => { - initTestClient({ tracesSampleRate }); - const mockKv = createMockSyncKv(); - const instrumented = instrumentDurableObjectSyncKvStorage(mockKv); - - sentryCore.startSpan({ name: 'parent' }, () => { - const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); - - instrumented.get('myKey'); - - expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); - }); - - expect(mockKv.get).toHaveBeenCalledWith('myKey'); - }); - }); }); function createMockSyncKv(): any { diff --git a/packages/cloudflare/test/instrumentSqlStorage.test.ts b/packages/cloudflare/test/instrumentSqlStorage.test.ts index 9b23725df356..8d76316460f3 100644 --- a/packages/cloudflare/test/instrumentSqlStorage.test.ts +++ b/packages/cloudflare/test/instrumentSqlStorage.test.ts @@ -1,13 +1,18 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import * as sentryCore from '@sentry/core'; import * as serverUtils from '@sentry/server-utils'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { instrumentSqlStorage } from '../src/instrumentations/instrumentSqlStorage'; import { initTestClient, resetSdk } from './testUtils'; describe('instrumentSqlStorage', () => { + beforeEach(() => { + initTestClient({ tracesSampleRate: 1 }); + }); + afterEach(() => { vi.restoreAllMocks(); + resetSdk(); }); it('instruments exec with summary as span name and sanitized query as db.query.text', () => { @@ -262,31 +267,84 @@ describe('instrumentSqlStorage', () => { }); }); + it('starts a span without sanitizing the query when tracing is not configured', () => { + initTestClient(); + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const sanitizeSpy = vi.spyOn(serverUtils, 'sanitizeSqlQuery'); + const mockSql = createMockSqlStorage(); + const instrumented = instrumentSqlStorage(mockSql); + + instrumented.exec('SELECT * FROM users WHERE id = ?', 42); + + expect(startSpanSpy).toHaveBeenCalledWith( + { + name: 'exec', + attributes: { + 'sentry.op': 'db.query', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object.sql', + 'db.system.name': 'cloudflare-durable-object-sql', + 'db.operation.name': 'exec', + }, + }, + expect.any(Function), + ); + expect(sanitizeSpy).not.toHaveBeenCalled(); + expect(mockSql.exec).toHaveBeenCalledWith('SELECT * FROM users WHERE id = ?', 42); + }); + describe('inside a parent span', () => { - afterEach(() => { - resetSdk(); + it('with tracesSampleRate 1, sanitizes the query and names the span after it', () => { + const sanitizeSpy = vi.spyOn(serverUtils, 'sanitizeSqlQuery'); + const instrumented = instrumentSqlStorage(createMockSqlStorage()); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + + instrumented.exec('SELECT * FROM users WHERE id = ?', 42); + + expect(startSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ name: 'SELECT users' }), + expect.any(Function), + ); + }); + + expect(sanitizeSpy).toHaveBeenCalledTimes(1); }); - it.each([ - [1, true], - [0, false], - ])('with tracesSampleRate %s, sanitizes the query and starts a span: %s', (tracesSampleRate, startsSpan) => { - initTestClient({ tracesSampleRate }); + it('with tracesSampleRate 0, records the dropped span without sanitizing the query', () => { + const client = initTestClient({ tracesSampleRate: 0 }); const sanitizeSpy = vi.spyOn(serverUtils, 'sanitizeSqlQuery'); const mockSql = createMockSqlStorage(); const instrumented = instrumentSqlStorage(mockSql); sentryCore.startSpan({ name: 'parent' }, () => { const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); instrumented.exec('SELECT * FROM users WHERE id = ?', 42); - expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + expect(startSpanSpy).toHaveBeenCalledTimes(1); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('sample_rate', 'span'); }); - expect(sanitizeSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); + expect(sanitizeSpy).not.toHaveBeenCalled(); expect(mockSql.exec).toHaveBeenCalledWith('SELECT * FROM users WHERE id = ?', 42); }); + + it('with tracesSampleRate 0, records no dropped span for Cloudflare-internal queries', () => { + const client = initTestClient({ tracesSampleRate: 0 }); + const instrumented = instrumentSqlStorage(createMockSqlStorage()); + + sentryCore.startSpan({ name: 'parent' }, () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + + instrumented.exec('SELECT * FROM cf_agents_state WHERE id = ?', 'foo'); + + expect(startSpanSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts index 68f969ba4818..f176d92e53db 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentQueueProducer.test.ts @@ -1,8 +1,7 @@ import type { Queue } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentQueueProducer } from '../../../src/instrumentations/worker/instrumentQueueProducer'; -import { initTestClient, resetSdk } from '../../testUtils'; function createMockQueue(): Queue { return { @@ -194,30 +193,4 @@ describe('instrumentQueueProducer', () => { await expect(wrapped.metrics()).resolves.toEqual(metrics); }); - - describe('inside a parent span', () => { - afterEach(() => { - resetSdk(); - }); - - test.each([ - [1, true], - [0, false], - ])('with tracesSampleRate %s, starts a span: %s', async (tracesSampleRate, startsSpan) => { - initTestClient({ tracesSampleRate }); - const queue = createMockQueue(); - const wrapped = instrumentQueueProducer(queue, 'MY_QUEUE'); - - await SentryCore.startSpan({ name: 'parent' }, () => { - const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); - - const result = wrapped.send({ hello: 'world' }); - - expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); - return result; - }); - - expect(queue.send).toHaveBeenLastCalledWith({ hello: 'world' }, undefined); - }); - }); }); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts index e3c2b0585ee6..73aa3d2bf72c 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentR2.test.ts @@ -1,8 +1,7 @@ import type { R2Bucket, R2MultipartUpload } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentR2Bucket } from '../../../src/instrumentations/worker/instrumentR2'; -import { initTestClient, resetSdk } from '../../testUtils'; const MOCK_R2_OBJECT = { key: 'my-file.txt', @@ -343,30 +342,4 @@ describe('instrumentR2Bucket', () => { const wrapped = instrumentR2Bucket(bucket, 'MY_BUCKET') as R2Bucket & { customMethod: () => string }; expect(wrapped.customMethod()).toBe('hi'); }); - - describe('inside a parent span', () => { - afterEach(() => { - resetSdk(); - }); - - test.each([ - [1, true], - [0, false], - ])('with tracesSampleRate %s, starts a span: %s', async (tracesSampleRate, startsSpan) => { - initTestClient({ tracesSampleRate }); - const bucket = createMockR2Bucket(); - const wrapped = instrumentR2Bucket(bucket, 'MY_BUCKET'); - - await SentryCore.startSpan({ name: 'parent' }, () => { - startSpanSpy.mockClear(); - - const result = wrapped.get('my-file.txt'); - - expect(startSpanSpy).toHaveBeenCalledTimes(startsSpan ? 1 : 0); - return result; - }); - - expect(bucket.get).toHaveBeenCalledWith('my-file.txt'); - }); - }); }); diff --git a/packages/cloudflare/test/utils/internalSqlQuery.test.ts b/packages/cloudflare/test/utils/internalSqlQuery.test.ts index 72d01d58ad9e..0c9d6c641dde 100644 --- a/packages/cloudflare/test/utils/internalSqlQuery.test.ts +++ b/packages/cloudflare/test/utils/internalSqlQuery.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { targetsCloudflareInternalTable } from '../../src/utils/internalSqlQuery'; +import { mayTargetCloudflareInternalTable, targetsCloudflareInternalTable } from '../../src/utils/internalSqlQuery'; // Behavioural coverage of the filter lives in `instrumentSqlStorage.test.ts`, which drives real // queries through the instrumented `exec`. What remains here are the signature-level contracts that @@ -23,3 +23,23 @@ describe('targetsCloudflareInternalTable', () => { expect(targetsCloudflareInternalTable('CREATE INDEX idx_agents_state_id')).toBe(false); }); }); + +describe('mayTargetCloudflareInternalTable', () => { + it.each([ + ['a cf_ table', 'SELECT * FROM cf_agents_state WHERE id = ?'], + ['an uppercase CF_ table', 'select * from CF_AGENTS_STATE'], + ['a quoted cf_ table', 'SELECT * FROM "cf_agents_state"'], + ['a schema-qualified cf_ table', 'SELECT * FROM main.cf_agents_state'], + ['a cf_ table in a CREATE INDEX ON clause', 'CREATE INDEX idx_agents_state_id ON cf_agents_state (id)'], + ])('returns true for %s', (_label, query) => { + expect(mayTargetCloudflareInternalTable(query)).toBe(true); + }); + + it.each([ + ['a user table', 'SELECT * FROM users WHERE id = ?'], + ['a table with cf in the middle', 'SELECT * FROM my_cf_table'], + ['a table starting with cfg', 'SELECT * FROM cfg_settings'], + ])('returns false for %s', (_label, query) => { + expect(mayTargetCloudflareInternalTable(query)).toBe(false); + }); +}); diff --git a/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts b/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts deleted file mode 100644 index 821aef2fb5ad..000000000000 --- a/packages/cloudflare/test/utils/isInUnsampledSpan.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { startSpan } from '@sentry/core'; -import { afterEach, describe, expect, it } from 'vitest'; -import { isInUnsampledSpan } from '../../src/utils/isInUnsampledSpan'; -import { initTestClient, resetSdk } from '../testUtils'; - -describe('isInUnsampledSpan', () => { - afterEach(() => { - resetSdk(); - }); - - it('returns false when there is no active span', () => { - initTestClient({ tracesSampleRate: 0 }); - - expect(isInUnsampledSpan()).toBe(false); - }); - - it('returns false inside a sampled span', () => { - initTestClient({ tracesSampleRate: 1 }); - - startSpan({ name: 'parent' }, () => { - expect(isInUnsampledSpan()).toBe(false); - }); - }); - - it('returns true inside an unsampled span', () => { - initTestClient({ tracesSampleRate: 0 }); - - startSpan({ name: 'parent' }, () => { - expect(isInUnsampledSpan()).toBe(true); - }); - }); -}); From 7aa9bbb7f46c8db0a8e4b26991133b9c82b9dcba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Peer=20St=C3=B6cklmair?= Date: Wed, 30 Sep 2026 10:03:19 +0200 Subject: [PATCH 3/4] Update packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts Co-authored-by: isaacs --- .../cloudflare/src/instrumentations/instrumentSqlStorage.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index 295d9753d326..a30d0c265340 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -44,8 +44,10 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { const activeSpan = getActiveSpan(); const spanIsNeverSent = !hasSpansEnabled() || (!!activeSpan && !spanIsSampled(activeSpan)); if (spanIsNeverSent && !mayTargetCloudflareInternalTable(query)) { - // The query needs no sanitizing. `startSpan` still runs, so each query starts a span with and - // without spans enabled, and an unsampled span records its dropped span outcome. + // This span is never sent, so skip the costly sanitize and summary + // steps. We still start the span so it records its dropped span + // outcome. A query that may target a `cf_` table takes the full path, + // because an internal query must start no span. return startSpan({ name: 'exec', attributes: SPAN_ATTRIBUTES }, callOriginal); } From 186832da281e80e0a2059d6241fdd12d7561358d Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Thu, 1 Oct 2026 15:05:27 +0200 Subject: [PATCH 4/4] fixup! fix(cloudflare): Skip binding instrumentation work when spans cannot be sent Co-Authored-By: Claude Opus 5.5 --- .../instrumentations/instrumentSqlStorage.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index a30d0c265340..706960562f88 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -41,9 +41,7 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { const [query, ...bindings] = args as [string, ...unknown[]]; - const activeSpan = getActiveSpan(); - const spanIsNeverSent = !hasSpansEnabled() || (!!activeSpan && !spanIsSampled(activeSpan)); - if (spanIsNeverSent && !mayTargetCloudflareInternalTable(query)) { + if (childSpanWillNotBeRecorded() && !mayTargetCloudflareInternalTable(query)) { // This span is never sent, so skip the costly sanitize and summary // steps. We still start the span so it records its dropped span // outcome. A query that may target a `cf_` table takes the full path, @@ -78,3 +76,16 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { }, }); } + +/** + * Returns `true` when `startSpan` creates a non-recording span: spans are disabled, or the active span + * is not sampled. Must agree with `createChildOrRootSpan` and `_startChildSpan` in `@sentry/core`. + */ +function childSpanWillNotBeRecorded(): boolean { + if (!hasSpansEnabled()) { + return true; + } + + const activeSpan = getActiveSpan(); + return !!activeSpan && !spanIsSampled(activeSpan); +}