From da7b68556404892e5cbd1cb4b6e652cb9102a05f Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 29 Oct 2026 12:23:03 +0500 Subject: [PATCH] fix(cloudflare): Require a parent span for storage and D1 operations Co-Authored-By: Claude Opus 5.5 --- .../instrumentDurableObjectStorage.ts | 1 + .../instrumentDurableObjectSyncKvStorage.ts | 1 + .../instrumentations/instrumentSqlStorage.ts | 3 +- .../instrumentations/worker/instrumentD1.ts | 2 ++ .../instrumentations/worker/instrumentKV.ts | 1 + .../instrumentDurableObjectStorage.test.ts | 24 ++++++++++++++++ ...strumentDurableObjectSyncKvStorage.test.ts | 18 ++++++++++++ .../test/instrumentSqlStorage.test.ts | 16 +++++++++++ .../worker/instrumentD1.test.ts | 28 +++++++++++++++++++ .../worker/instrumentKV.test.ts | 25 ++++++++++++++++- 10 files changed, 117 insertions(+), 2 deletions(-) diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts index e1d1a3fc1025..d67d189e9cf0 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts @@ -73,6 +73,7 @@ export function instrumentDurableObjectStorage( { // Use underscore naming to match Cloudflare's native instrumentation (e.g., "durable_object_storage_get") name: `durable_object_storage_${methodName}`, + onlyIfParent: true, attributes: { [SENTRY_OP]: DB, [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts index eb21b57a2fbf..ff26fbf431a6 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectSyncKvStorage.ts @@ -26,6 +26,7 @@ export function instrumentDurableObjectSyncKvStorage(syncKv: SyncKvStorage): Syn return startSpan( { name: `durable_object_storage_kv_${methodName}`, + onlyIfParent: true, attributes: { [SENTRY_OP]: DB, [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index 8147a2a2bdbb..c11653bf970d 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts @@ -39,7 +39,7 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { // steps. We still start the span so it records its dropped span // outcome. A query that may target a `cf_` or `pi_` table takes the full path, // because an internal query must start no span. - return startSpan({ name: 'exec', attributes: SPAN_ATTRIBUTES }, callOriginal); + return startSpan({ name: 'exec', onlyIfParent: true, attributes: SPAN_ATTRIBUTES }, callOriginal); } const sanitizedQuery = sanitizeSqlQuery(query); @@ -56,6 +56,7 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage { return startSpan( { name: querySummary || sanitizedQuery, + onlyIfParent: true, attributes: { ...SPAN_ATTRIBUTES, 'db.query.text': sanitizedQuery, diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentD1.ts b/packages/cloudflare/src/instrumentations/worker/instrumentD1.ts index 42b01428816c..5ba05274abff 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentD1.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentD1.ts @@ -135,6 +135,7 @@ function createStartSpanOptions(queryText: string | undefined, type: D1QueryType return { name, + onlyIfParent: true, attributes: { [SENTRY_OP]: DB_QUERY, 'db.system.name': 'cloudflare-d1', @@ -174,6 +175,7 @@ function instrumentBatch( return startSpan( { name: 'D1 batch', + onlyIfParent: true, attributes: { [SENTRY_OP]: DB_QUERY, 'db.system.name': 'cloudflare-d1', diff --git a/packages/cloudflare/src/instrumentations/worker/instrumentKV.ts b/packages/cloudflare/src/instrumentations/worker/instrumentKV.ts index 685b9fc46726..5a19cedb84ff 100644 --- a/packages/cloudflare/src/instrumentations/worker/instrumentKV.ts +++ b/packages/cloudflare/src/instrumentations/worker/instrumentKV.ts @@ -44,6 +44,7 @@ function createSpanOptions(bindingName: string, operation: KVOperation, args: un const keys = getCacheKeys(operation, args); return { name: cacheOperation, + onlyIfParent: true, attributes: { [SENTRY_OP]: cacheOperation, [CACHE_OPERATION]: cacheOperationName, diff --git a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts index a2295ceff624..d31ccbfc316f 100644 --- a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts @@ -30,6 +30,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_get', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -51,6 +52,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_get', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -74,6 +76,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_put', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -95,6 +98,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_put', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -118,6 +122,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_delete', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -139,6 +144,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_delete', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -162,6 +168,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_list', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -185,6 +192,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_setAlarm', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -269,6 +277,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_getAlarm', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -290,6 +299,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_deleteAlarm', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -313,6 +323,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'SELECT', + onlyIfParent: true, attributes: { 'sentry.op': 'db.query', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object_sql', @@ -327,6 +338,18 @@ describe('instrumentDurableObjectStorage', () => { ); }); + it('only creates spans when there is an active parent span', async () => { + const client = initTestClient({ tracesSampleRate: 1 }); + const startedSpans: string[] = []; + client.on('spanStart', span => startedSpans.push(sentryCore.spanToJSON(span).name)); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.get('outside'); + await sentryCore.startSpan({ name: 'parent' }, () => instrumented.get('inside')); + + expect(startedSpans).toEqual(['parent', 'durable_object_storage_get']); + }); + describe('framework-internal KV keys', () => { it('does not create a span for a cf_-prefixed get', async () => { const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); @@ -437,6 +460,7 @@ describe('instrumentDurableObjectStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_kv_get', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', diff --git a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts index b75cc0e0e245..97d27d1bbf06 100644 --- a/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectSyncKvStorage.test.ts @@ -2,10 +2,12 @@ import { SENTRY_ORIGIN } from '@sentry/conventions/attributes'; 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(() => { vi.restoreAllMocks(); + resetSdk(); }); describe('get', () => { @@ -19,6 +21,7 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_kv_get', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -62,6 +65,7 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_kv_put', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -94,6 +98,7 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_kv_delete', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -127,6 +132,7 @@ describe('instrumentDurableObjectSyncKvStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'durable_object_storage_kv_list', + onlyIfParent: true, attributes: { 'sentry.op': 'db', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object', @@ -162,6 +168,18 @@ describe('instrumentDurableObjectSyncKvStorage', () => { }); }); + it('only creates spans when there is an active parent span', () => { + const client = initTestClient({ tracesSampleRate: 1 }); + const startedSpans: string[] = []; + client.on('spanStart', span => startedSpans.push(sentryCore.spanToJSON(span).name)); + const instrumented = instrumentDurableObjectSyncKvStorage(createMockSyncKv()); + + instrumented.get('outside'); + sentryCore.startSpan({ name: 'parent' }, () => instrumented.get('inside')); + + expect(startedSpans).toEqual(['parent', 'durable_object_storage_kv_get']); + }); + describe('non-instrumented properties', () => { it('passes through unknown properties without instrumentation', () => { const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); diff --git a/packages/cloudflare/test/instrumentSqlStorage.test.ts b/packages/cloudflare/test/instrumentSqlStorage.test.ts index af9cff36cfec..50c2c61fb745 100644 --- a/packages/cloudflare/test/instrumentSqlStorage.test.ts +++ b/packages/cloudflare/test/instrumentSqlStorage.test.ts @@ -25,6 +25,7 @@ describe('instrumentSqlStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'SELECT users', + onlyIfParent: true, attributes: { 'sentry.op': 'db.query', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object_sql', @@ -49,6 +50,7 @@ describe('instrumentSqlStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'SELECT users', + onlyIfParent: true, attributes: { 'sentry.op': 'db.query', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object_sql', @@ -82,6 +84,7 @@ describe('instrumentSqlStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'INSERT users', + onlyIfParent: true, attributes: { 'sentry.op': 'db.query', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object_sql', @@ -152,6 +155,18 @@ describe('instrumentSqlStorage', () => { expect(mockSql.exec).toHaveBeenCalledTimes(2); }); + it('only creates spans when there is an active parent span', () => { + const client = initTestClient({ tracesSampleRate: 1 }); + const startedSpans: string[] = []; + client.on('spanStart', span => startedSpans.push(sentryCore.spanToJSON(span).name)); + const instrumented = instrumentSqlStorage(createMockSqlStorage()); + + instrumented.exec('SELECT * FROM users'); + sentryCore.startSpan({ name: 'parent' }, () => instrumented.exec('SELECT * FROM users')); + + expect(startedSpans).toEqual(['parent', 'SELECT users']); + }); + describe('internal storage queries', () => { it('does not create a span for Cloudflare-internal queries', () => { const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); @@ -302,6 +317,7 @@ describe('instrumentSqlStorage', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: 'exec', + onlyIfParent: true, attributes: { 'sentry.op': 'db.query', [SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object_sql', diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentD1.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentD1.test.ts index 7c6a6478853e..2aad38b340c0 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentD1.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentD1.test.ts @@ -2,6 +2,7 @@ import type { D1Database, D1DatabaseSession, D1PreparedStatement } from '@cloudf import * as SentryCore from '@sentry/core'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentD1 } from '../../../src/instrumentations/worker/instrumentD1'; +import { initTestClient, resetSdk } from '../../testUtils'; const MOCK_FIRST_RETURN_VALUE = { id: 1, name: 'Foo' }; @@ -57,6 +58,10 @@ describe('instrumentD1', () => { vi.clearAllMocks(); }); + afterEach(() => { + resetSdk(); + }); + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); const addBreadcrumbSpy = vi.spyOn(SentryCore, 'addBreadcrumb'); @@ -83,6 +88,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'SELECT * FROM users', + onlyIfParent: true, }, expect.any(Function), ); @@ -185,6 +191,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'INSERT INTO users (name) VALUES (?)', + onlyIfParent: true, }, expect.any(Function), ); @@ -239,6 +246,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'INSERT INTO users (name) VALUES (?)', + onlyIfParent: true, }, expect.any(Function), ); @@ -293,6 +301,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'SELECT * FROM users', + onlyIfParent: true, }, expect.any(Function), ); @@ -344,6 +353,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'D1 batch', + onlyIfParent: true, }, expect.any(Function), ); @@ -383,6 +393,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'D1 batch', + onlyIfParent: true, }, expect.any(Function), ); @@ -412,6 +423,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'CREATE TABLE users (id INTEGER PRIMARY KEY)', + onlyIfParent: true, }, expect.any(Function), ); @@ -450,6 +462,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'SELECT * FROM users', + onlyIfParent: true, }, expect.any(Function), ); @@ -487,6 +500,7 @@ describe('instrumentD1', () => { 'sentry.origin': 'auto.db.cloudflare.d1', }, name: 'D1 batch', + onlyIfParent: true, }, expect.any(Function), ); @@ -508,6 +522,20 @@ describe('instrumentD1', () => { }); }); + test('only creates spans when there is an active parent span', async () => { + const client = initTestClient({ tracesSampleRate: 1 }); + const startedSpans: string[] = []; + client.on('spanStart', span => startedSpans.push(SentryCore.spanToJSON(span).name)); + const instrumentedDb = instrumentD1(createMockD1Database()); + + await instrumentedDb.prepare('SELECT * FROM users').first(); + await instrumentedDb.batch([instrumentedDb.prepare('SELECT 1')]); + await instrumentedDb.exec('SELECT 1'); + await SentryCore.startSpan({ name: 'parent' }, () => instrumentedDb.prepare('SELECT * FROM users').first()); + + expect(startedSpans).toEqual(['parent', 'SELECT users']); + }); + describe('double instrumentation prevention', () => { test('does not double-instrument the same database', async () => { const db = createMockD1Database(); diff --git a/packages/cloudflare/test/instrumentations/worker/instrumentKV.test.ts b/packages/cloudflare/test/instrumentations/worker/instrumentKV.test.ts index 50e08dc1b69b..43bca9652a08 100644 --- a/packages/cloudflare/test/instrumentations/worker/instrumentKV.test.ts +++ b/packages/cloudflare/test/instrumentations/worker/instrumentKV.test.ts @@ -10,8 +10,9 @@ import { } from '@sentry/conventions/attributes'; import { CACHE_GET, CACHE_PUT, CACHE_REMOVE } from '@sentry/conventions/op'; import * as SentryCore from '@sentry/core'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { instrumentKV } from '../../../src/instrumentations/worker/instrumentKV'; +import { initTestClient, resetSdk } from '../../testUtils'; function createMockKVNamespace(): KVNamespace { return { @@ -28,6 +29,10 @@ describe('instrumentKV', () => { vi.clearAllMocks(); }); + afterEach(() => { + resetSdk(); + }); + const startSpanSpy = vi.spyOn(SentryCore, 'startSpan'); test('forwards get with the type argument', async () => { @@ -49,6 +54,7 @@ describe('instrumentKV', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: CACHE_GET, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_GET, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -82,6 +88,7 @@ describe('instrumentKV', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: CACHE_GET, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_GET, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -106,6 +113,7 @@ describe('instrumentKV', () => { expect(startSpanSpy).toHaveBeenCalledWith( { name: CACHE_GET, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_GET, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -133,6 +141,7 @@ describe('instrumentKV', () => { 1, { name: CACHE_PUT, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_PUT, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -149,6 +158,7 @@ describe('instrumentKV', () => { 2, { name: CACHE_REMOVE, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_REMOVE, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -165,6 +175,7 @@ describe('instrumentKV', () => { 3, { name: CACHE_GET, + onlyIfParent: true, attributes: { [SENTRY_OP]: CACHE_GET, [SENTRY_ORIGIN]: 'auto.cache.cloudflare.kv', @@ -178,6 +189,18 @@ describe('instrumentKV', () => { ); }); + test('only creates spans when there is an active parent span', async () => { + const client = initTestClient({ tracesSampleRate: 1 }); + const startedSpans: string[] = []; + client.on('spanStart', span => startedSpans.push(SentryCore.spanToJSON(span).name)); + const wrapped = instrumentKV(createMockKVNamespace(), 'MY_KV'); + + await wrapped.get('outside'); + await SentryCore.startSpan({ name: 'parent' }, () => wrapped.get('inside')); + + expect(startedSpans).toEqual(['parent', CACHE_GET]); + }); + test('forwards unknown property accesses transparently', () => { const kv = Object.assign(createMockKVNamespace(), { customMethod: vi.fn().mockReturnValue('ok'),