Skip to content

Commit c0aa40e

Browse files
committed
fixup! ref: Remove the option to enable them again
1 parent 689e90c commit c0aa40e

7 files changed

Lines changed: 108 additions & 16 deletions

File tree

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ module.exports = [
480480
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
481481
gzip: false,
482482
brotli: false,
483-
limit: '445 KiB',
483+
limit: '448 KiB',
484484
disablePlugins: ['@size-limit/webpack'],
485485
webpack: false,
486486
modifyEsbuildConfig: function (config) {

dev-packages/e2e-tests/test-applications/cloudflare-agent/tests/callable.test.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,20 @@ test('@callable() methods work correctly with Sentry instrumentDurableObjectWith
3434
},
3535
spans: expect.arrayContaining([
3636
expect.objectContaining({
37-
op: 'db.query',
38-
origin: 'auto.db.cloudflare.durable_object.sql',
39-
description: expect.stringMatching(/^SELECT /),
40-
data: expect.objectContaining({
41-
'db.system.name': 'cloudflare-durable-object-sql',
42-
'db.operation.name': 'exec',
43-
'db.query.summary': expect.any(String),
44-
'db.query.text': expect.any(String),
45-
'sentry.op': 'db.query',
46-
'sentry.origin': 'auto.db.cloudflare.durable_object.sql',
47-
}),
37+
data: {
38+
'db.operation.name': 'get',
39+
'db.system.name': 'cloudflare.durable_object.storage',
40+
'sentry.op': 'db',
41+
'sentry.origin': 'auto.db.cloudflare.durable_object',
42+
},
43+
description: 'durable_object_storage_get',
44+
op: 'db',
45+
origin: 'auto.db.cloudflare.durable_object',
46+
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
47+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
48+
start_timestamp: expect.any(Number),
49+
timestamp: expect.any(Number),
50+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
4851
}),
4952
]),
5053
start_timestamp: expect.any(Number),

packages/cloudflare/src/client.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,30 @@ interface BaseCloudflareOptions {
216216
*/
217217
enableRpcTracePropagation?: boolean;
218218

219+
/**
220+
* Table names that should stay instrumented even though they match the reserved `cf_` prefix used
221+
* by Durable Object frameworks (`agents`, `partyserver`, ...) for their internal SQLite tables.
222+
*
223+
* By default, `exec` queries against `cf_`-prefixed tables are treated as framework noise and no
224+
* `db.query` span is created for them. If one of your own tables happens to use this prefix, add it
225+
* here to opt it back into instrumentation. Entries are matched against each table name in the
226+
* query summary — strings must match exactly, while regular expressions give you prefix/pattern
227+
* matching.
228+
*
229+
* @default []
230+
* @example
231+
* ```ts
232+
* export default Sentry.withSentry(
233+
* (env) => ({
234+
* dsn: env.SENTRY_DSN,
235+
* durableObjectSqlSpanAllowlist: ['cf_my_table', /^cf_reports_/],
236+
* }),
237+
* handler,
238+
* );
239+
* ```
240+
*/
241+
durableObjectSqlSpanAllowlist?: Array<string | RegExp>;
242+
219243
/**
220244
* @deprecated Use `enableRpcTracePropagation` instead. This option will be removed in a future major version.
221245
*

packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ import type { SqlStorage } from '@cloudflare/workers-types';
22
import {
33
_INTERNAL_getSqlQuerySummary,
44
_INTERNAL_sanitizeSqlQuery,
5+
getClient,
56
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
67
startSpan,
78
} from '@sentry/core';
9+
import type { CloudflareClientOptions } from '../client';
810
import { targetsCloudflareInternalTable } from '../utils/internalSqlQuery';
911

1012
/**
@@ -28,7 +30,10 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
2830
const sanitizedQuery = _INTERNAL_sanitizeSqlQuery(query);
2931
const querySummary = _INTERNAL_getSqlQuerySummary(sanitizedQuery);
3032

31-
if (targetsCloudflareInternalTable(querySummary)) {
33+
const allowlist = (getClient()?.getOptions() as CloudflareClientOptions | undefined)
34+
?.durableObjectSqlSpanAllowlist;
35+
36+
if (targetsCloudflareInternalTable(querySummary, allowlist)) {
3237
return (original as (...a: unknown[]) => ReturnType<SqlStorage['exec']>).apply(target, args);
3338
}
3439

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { stringMatchesSomePattern } from '@sentry/core';
2+
13
/**
24
* Cloudflare frameworks that build on Durable Objects (`agents`, `partyserver`, ...) manage their
35
* own internal SQLite tables, all namespaced with a `cf_` prefix — e.g. `cf_agents_schedules`,
@@ -6,18 +8,31 @@
68
* with dozens of zero-signal `db.query` spans per request. The exact set of tables even varies
79
* between framework versions, so we match the reserved prefix rather than an enumerated list.
810
*
9-
* User tables never use this prefix, so skipping their spans is safe.
11+
* The `cf_` prefix is a reserved convention for framework-managed tables, so user tables should not
12+
* use it. In case a user table does collide with the prefix, the `durableObjectSqlSpanAllowlist`
13+
* option lets them opt those tables back into instrumentation.
1014
*
1115
* The check operates on the query summary produced by `getSqlQuerySummary` (`{operation} {table} ...`,
1216
* the same value used as the span name), so table targets are already isolated from the rest of the
1317
* query.
1418
*/
15-
export function targetsCloudflareInternalTable(querySummary: string | undefined): boolean {
19+
export function targetsCloudflareInternalTable(
20+
querySummary: string | undefined,
21+
allowlist?: Array<string | RegExp>,
22+
): boolean {
1623
if (!querySummary) {
1724
return false;
1825
}
1926

2027
const [, ...tables] = querySummary.split(' ');
2128

22-
return tables.some(table => table.toLowerCase().startsWith('cf_'));
29+
return tables.some(table => {
30+
if (!table.toLowerCase().startsWith('cf_')) {
31+
return false;
32+
}
33+
34+
// A table on the allowlist is treated as a user table and stays instrumented, even though it
35+
// matches the reserved prefix.
36+
return !allowlist?.length || !stringMatchesSomePattern(table, allowlist, true);
37+
});
2338
}

packages/cloudflare/test/instrumentSqlStorage.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,20 @@ describe('instrumentSqlStorage', () => {
168168

169169
expect(startSpanSpy).toHaveBeenCalledTimes(1);
170170
});
171+
172+
it('creates a span for a cf_ table on the durableObjectSqlSpanAllowlist', () => {
173+
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');
174+
vi.spyOn(sentryCore, 'getClient').mockReturnValue({
175+
getOptions: () => ({ durableObjectSqlSpanAllowlist: ['cf_my_table'] }),
176+
} as unknown as ReturnType<typeof sentryCore.getClient>);
177+
178+
const mockSql = createMockSqlStorage();
179+
const instrumented = instrumentSqlStorage(mockSql);
180+
181+
instrumented.exec('SELECT * FROM cf_my_table WHERE id = ?', 1);
182+
183+
expect(startSpanSpy).toHaveBeenCalledTimes(1);
184+
});
171185
});
172186
});
173187

packages/cloudflare/test/utils/internalSqlQuery.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,37 @@ describe('targetsCloudflareInternalTable', () => {
6060
});
6161
});
6262

63+
describe('allowlist (opt a cf_ table back into instrumentation)', () => {
64+
it('returns false for an allowlisted table matched by exact string', () => {
65+
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_my_table'), ['cf_my_table'])).toBe(false);
66+
});
67+
68+
it('returns false for an allowlisted table matched by regex', () => {
69+
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_reports_daily'), [/^cf_reports_/])).toBe(false);
70+
});
71+
72+
it('requires an exact match for string entries', () => {
73+
// Substring matches must not opt a table back in, otherwise `cf_` would allowlist everything.
74+
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), ['cf_agents'])).toBe(true);
75+
});
76+
77+
it('still skips genuine internal tables that are not allowlisted', () => {
78+
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), ['cf_my_table'])).toBe(true);
79+
});
80+
81+
it('still skips when an internal table is joined with an allowlisted table', () => {
82+
expect(
83+
targetsCloudflareInternalTable(summarize('SELECT * FROM cf_my_table t JOIN cf_agents_state s ON s.id = t.id'), [
84+
'cf_my_table',
85+
]),
86+
).toBe(true);
87+
});
88+
89+
it('ignores an empty allowlist', () => {
90+
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), [])).toBe(true);
91+
});
92+
});
93+
6394
describe('summaries without a resolvable table target (safe default: instrument)', () => {
6495
it.each([
6596
['undefined', undefined],

0 commit comments

Comments
 (0)