diff --git a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts b/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts index e7ae5e5d173f..706960562f88 100644 --- a/packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts +++ b/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 => + (original as (...a: unknown[]) => ReturnType).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).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, ); }; }, }); } + +/** + * 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); +} 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/test/instrumentDurableObjectStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts index 27d393ddb6ee..af5161e5bcde 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(); @@ -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); diff --git a/packages/cloudflare/test/instrumentSqlStorage.test.ts b/packages/cloudflare/test/instrumentSqlStorage.test.ts index 3436d0eb9032..8d76316460f3 100644 --- a/packages/cloudflare/test/instrumentSqlStorage.test.ts +++ b/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(); + }); + }); + }); }); /** 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/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); + }); +});