From 7cc6e5f3633fccb075dcc4794e128a258ef7ae37 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 23 Sep 2026 14:39:42 +0200 Subject: [PATCH] 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 | 15 ++++++- .../instrumentDurableObjectSyncKvStorage.ts | 7 +++- .../instrumentations/instrumentSqlStorage.ts | 7 +++- .../worker/instrumentQueueProducer.ts | 12 +++++- .../instrumentations/worker/instrumentR2.ts | 29 +++++++++++++- .../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 ++++++++++++- 11 files changed, 241 insertions(+), 8 deletions(-) diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts index 976412bae83d..c5b7836f2b16 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts @@ -1,7 +1,14 @@ import type { DurableObjectStorage, SyncKvStorage, SqlStorage } from '@cloudflare/workers-types'; 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 { + getActiveSpan, + getClient, + isThenable, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + spanIsSampled, + startSpan, +} from '@sentry/core'; import type { CloudflareClientOptions } from '../client'; import { getStorageKeys, targetsCloudflareInternalKey } from '../utils/internalStorageKey'; import { storeSpanContext } from '../utils/traceLinks'; @@ -59,6 +66,12 @@ export function instrumentDurableObjectStorage( } return function (this: unknown, ...args: unknown[]) { + // `setAlarm` keeps its span, because the alarm links back to the span context stored here. + const activeSpan = getActiveSpan(); + if (methodName !== 'setAlarm' && activeSpan && !spanIsSampled(activeSpan)) { + 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..3cbfdd79f82b 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts @@ -1,7 +1,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 { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanIsSampled, startSpan } from '@sentry/core'; const SYNC_KV_METHODS_TO_INSTRUMENT = ['get', 'put', 'delete', 'list'] as const; @@ -23,6 +23,11 @@ export function instrumentDurableObjectSyncKvStorage(syncKv: SyncKvStorage): Syn } return function (this: unknown, ...args: unknown[]) { + const activeSpan = getActiveSpan(); + if (activeSpan && !spanIsSampled(activeSpan)) { + 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..18cb39c9b89a 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -1,7 +1,7 @@ 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, 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'; @@ -24,6 +24,11 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { return function (this: unknown, ...args: unknown[]) { const [query, ...bindings] = args as [string, ...unknown[]]; + const activeSpan = getActiveSpan(); + if (activeSpan && !spanIsSampled(activeSpan)) { + 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 293b027d5441..7c3f333a7f0f 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentQueueProducer.ts @@ -1,7 +1,7 @@ import type { MessageSendRequest, Queue, QueueSendBatchOptions, QueueSendOptions } from '@cloudflare/workers-types'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { QUEUE_PUBLISH } from '@sentry/conventions/op'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanIsSampled, startSpan } from '@sentry/core'; const ORIGIN = 'auto.faas.cloudflare.queue'; @@ -71,6 +71,11 @@ 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 { + const activeSpan = getActiveSpan(); + if (activeSpan && !spanIsSampled(activeSpan)) { + return Reflect.apply(original, target, [message, options]); + } + return startPublishSpan({ bindingName, bodySize: getBodySize(message) }, () => Reflect.apply(original, target, [message, options]), ); @@ -84,6 +89,11 @@ export function instrumentQueueProducer(queue: T, bindingName: messages: Iterable, options?: QueueSendBatchOptions, ): ReturnType { + const activeSpan = getActiveSpan(); + if (activeSpan && !spanIsSampled(activeSpan)) { + 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..0a5df2af0fb5 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentR2.ts @@ -19,7 +19,7 @@ import { OBJECT_PUT, OBJECT_UPLOAD_PART, } from '@sentry/conventions/op'; -import { isObjectLike, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { getActiveSpan, isObjectLike, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanIsSampled, startSpan } from '@sentry/core'; const ORIGIN = 'auto.faas.cloudflare.r2'; @@ -72,6 +72,11 @@ function createSpanOptions(bindingName: string, r2Op: R2OperationKey, key?: stri }; } +function isUnsampled(): boolean { + const activeSpan = getActiveSpan(); + return !!activeSpan && !spanIsSampled(activeSpan); +} + function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: string): R2MultipartUpload { const { key } = upload; @@ -81,6 +86,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isUnsampled()) { + return Reflect.apply(original, target, args); + } + const [partNumber] = args; const spanOptions = createSpanOptions(bindingName, 'uploadPart', key); @@ -101,6 +110,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown) { + if (isUnsampled()) { + return Reflect.apply(original, target, []); + } + return startSpan(createSpanOptions(bindingName, 'abortMultipartUpload', key), () => Reflect.apply(original, target, []), ); @@ -111,6 +124,10 @@ function instrumentR2MultipartUpload(upload: R2MultipartUpload, bindingName: str const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isUnsampled()) { + return Reflect.apply(original, target, args); + } + return startSpan(createSpanOptions(bindingName, 'completeMultipartUpload', key), () => Reflect.apply(original, target, args), ); @@ -135,6 +152,10 @@ export function instrumentR2Bucket(bucket: T, bindingName: s const original = Reflect.get(target, prop, receiver); return function (this: unknown, ...args: Parameters) { + if (isUnsampled()) { + return Reflect.apply(original, target, args); + } + const [key] = args; return startSpan(createSpanOptions(bindingName, prop, key), () => Reflect.apply(original, target, args)); @@ -145,6 +166,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 (isUnsampled()) { + 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/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 5b291bea7e3e..c10a8189c10d 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 { @@ -178,4 +179,30 @@ describe('instrumentQueueProducer', () => { const wrapped = instrumentQueueProducer(queue, 'MY_QUEUE') as Queue & { 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 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; +}