fix(cloudflare): Skip binding instrumentation work when spans are not sent

packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts

@@ -1,10 +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 { 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.
@@ -22,8 +36,19 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
       }
 
       return function (this: unknown, ...args: unknown[]) {
+        const callOriginal = (): ReturnType<SqlStorage['exec']> =>
+          (original as (...a: unknown[]) => ReturnType<SqlStorage['exec']>).apply(target, args);
+
         const [query, ...bindings] = args as [string, ...unknown[]];
 
+        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,
+          // because an internal query must start no span.
+          return startSpan({ name: 'exec', attributes: SPAN_ATTRIBUTES }, callOriginal);
+        }
+
         const sanitizedQuery = sanitizeSqlQuery(query);
         const querySummary = getSqlQuerySummary(sanitizedQuery);
 
@@ -32,25 +57,35 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
           ?.durableObjectSqlSpanAllowlist;
 
         if (targetsCloudflareInternalTable(querySummary, allowlist, sanitizedQuery)) {
-          return (original as (...a: unknown[]) => ReturnType<SqlStorage['exec']>).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<SqlStorage['exec']>).apply(target, args),
+          callOriginal,
         );
       };
     },
   });
 }
+
+/**
+ * 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);
+}

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] <name> ON <table>` — the IF EXISTS shape mirrors DDL_RE
 // in @sentry/core.
 const CREATE_INDEX_TABLE_RE =

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<typeof traceLinks>();
@@ -15,6 +16,7 @@ vi.mock('../src/utils/traceLinks', async importOriginal => {
 describe('instrumentDurableObjectStorage', () => {
   afterEach(() => {
     vi.restoreAllMocks();
+    resetSdk();
   });
 
   describe('get', () => {
@@ -301,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);

packages/cloudflare/test/instrumentSqlStorage.test.ts

@@ -1,11 +1,18 @@
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
 import * as sentryCore from '@sentry/core';
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import * as serverUtils from '@sentry/server-utils';
+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', () => {
@@ -259,6 +266,86 @@ 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', () => {
+    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('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(1);
+        expect(recordDroppedEventSpy).toHaveBeenCalledWith('sample_rate', 'span');
+      });
+
+      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();
+      });
+    });
+  });
 });
 
 /**

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<CloudflareClientOptions> = {}): 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;
+}

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