From 28885a9a1c16d3c545aad7a3ad6b7c56b55ba3ab 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 | 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); + }); + }); +});