Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions packages/cloudflare/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,14 @@ interface BaseCloudflareOptions {
rpcTracePropagationBindings?: TracePropagationTargets;

/**
* Table names that should stay instrumented even though they match the reserved `cf_` prefix used
* by Durable Object frameworks (`agents`, `partyserver`, ...) for their internal SQLite tables.
* Table names that should stay instrumented even though they match a reserved prefix used by
* Durable Object frameworks for their internal SQLite tables: `cf_` (`agents`, `partyserver`, ...)
* and `pi_` (pi-durable in the `PiHarness` of `agents`).
*
* By default, `exec` queries against `cf_`-prefixed tables are treated as framework noise and no
* `db.query` span is created for them. If one of your own tables happens to use this prefix, add it
* here to opt it back into instrumentation. Entries are matched against each table name in the
* By default, `exec` queries against tables with these prefixes are treated as framework noise and
* no `db.query` span is created for them. If one of your own tables happens to use such a prefix,
* add it here to opt it back into instrumentation, or add `/^pi_/` to see the statements of
* pi-durable. Entries are matched against each table name in the
* query summary — strings must match exactly, while regular expressions give you prefix/pattern
* matching.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
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,
// 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);
}
Expand Down
19 changes: 12 additions & 7 deletions packages/cloudflare/src/utils/internalSqlQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import { stringMatchesSomePattern } from '@sentry/core';
* between framework versions, so we match the reserved prefix rather than an enumerated list.
*
* The `cf_` prefix is a reserved convention for framework-managed tables, so user tables should not
* use it. In case a user table does collide with the prefix, the `durableObjectSqlSpanAllowlist`
* option lets them opt those tables back into instrumentation.
* use it. The same goes for `pi_`: the `PiHarness` of `agents` keeps the state of pi-durable in
* tables with that prefix, and one run writes close to 200 statements. In case a user table does
* collide with a prefix, the `durableObjectSqlSpanAllowlist` option lets them opt those tables back
* into instrumentation.
*
* The check operates on the query summary produced by `getSqlQuerySummary` (`{operation} {table} ...`,
* the same value used as the span name), so table targets are already isolated from the rest of the
Expand Down Expand Up @@ -38,22 +40,25 @@ export function targetsCloudflareInternalTable(
}

/**
* 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.
* Returns `false` when the raw `query` has no word that starts with `cf_` or `pi_`, 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);
return INTERNAL_PREFIX_RE.test(query);
}

const CF_PREFIX_RE = /\bcf_/i;
const INTERNAL_TABLE_PREFIXES = ['cf_', 'pi_'];
const INTERNAL_PREFIX_RE = /\b(?:cf|pi)_/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 =
/^\s*CREATE\s+(?:UNIQUE\s+)?INDEX(?:\s+IF\s+(?:NOT\s+)?EXISTS)?\s+[^\s(,;)]+\s+ON\s+(?<table>[^\s(,;)]+)/i;

function isCloudflareInternalTable(table: string, allowlist?: Array<string | RegExp>): boolean {
if (!table.toLowerCase().startsWith('cf_')) {
const name = table.toLowerCase();
if (!INTERNAL_TABLE_PREFIXES.some(prefix => name.startsWith(prefix))) {
return false;
}

Expand Down
25 changes: 24 additions & 1 deletion packages/cloudflare/test/instrumentSqlStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,26 @@ describe('instrumentSqlStorage', () => {
});
});

// The Agents SDK `PiHarness` keeps the state of pi-durable in its own tables, all with the `pi_`
// prefix of its session store.
describe('pi-durable tables (pi_ prefix) are skipped', () => {
it.each([
['SELECT', 'SELECT record FROM pi_conversations WHERE id = ?'],
[
'upsert',
`INSERT INTO pi_tasks (id, conversation_id, kind, status, record) VALUES (?, ?, ?, ?, ?)
ON CONFLICT(id) DO UPDATE SET status = excluded.status, record = excluded.record`,
],
['INSERT OR IGNORE', 'INSERT OR IGNORE INTO pi_record_ids (id, record_type) VALUES (?, ?)'],
['UPDATE', 'UPDATE pi_durable_metadata SET next_id = ?, next_seq = ? WHERE singleton = ?'],
['DELETE', 'DELETE FROM pi_document_revisions WHERE document_id = ?'],
['CREATE TABLE', 'CREATE TABLE pi_durable_schema (version INTEGER NOT NULL) STRICT'],
['CREATE INDEX', 'CREATE INDEX pi_tasks_by_status ON pi_tasks (status, id)'],
])('skips %s', (_label, query) => {
expect(execCreatesSpan(query)).toBe(false);
});
});

describe('user queries stay instrumented', () => {
it.each([
['SELECT', 'SELECT * FROM users WHERE id = ?'],
Expand All @@ -228,6 +248,8 @@ describe('instrumentSqlStorage', () => {
['CREATE INDEX', 'CREATE INDEX idx_name ON users (name)'],
['table with cf in the middle', 'SELECT * FROM my_cf_table'],
['table starting with cfg', 'SELECT * FROM cfg_settings'],
['table with pi_ in the middle', 'SELECT * FROM api_keys'],
['table starting with pi', 'SELECT * FROM pipelines'],
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO users (id, name) VALUES (?, ?)'],
['REPLACE INTO', 'REPLACE INTO sessions (id, token) VALUES (?, ?)'],
['UPDATE OR IGNORE', 'UPDATE OR IGNORE products SET price = ? WHERE id = ?'],
Expand All @@ -241,10 +263,11 @@ describe('instrumentSqlStorage', () => {
});
});

describe('durableObjectSqlSpanAllowlist (opt a cf_ table back into instrumentation)', () => {
describe('durableObjectSqlSpanAllowlist (opt a cf_ or pi_ table back into instrumentation)', () => {
it.each([
['exact string', 'SELECT * FROM cf_my_table', ['cf_my_table']],
['regex', 'SELECT * FROM cf_reports_daily', [/^cf_reports_/]],
['regex for the pi-durable tables', 'SELECT record FROM pi_conversations WHERE id = ?', [/^pi_/]],
['upsert target', 'INSERT OR REPLACE INTO cf_my_table (id) VALUES (?)', ['cf_my_table']],
['CREATE INDEX target', 'CREATE INDEX idx_mine ON cf_my_table (id)', ['cf_my_table']],
])('instruments an allowlisted table matched by %s', (_label, query, allowlist) => {
Expand Down
2 changes: 2 additions & 0 deletions packages/cloudflare/test/utils/internalSqlQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('mayTargetCloudflareInternalTable', () => {
['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)'],
['a pi_ table', 'SELECT record FROM pi_tasks WHERE id = ?'],
])('returns true for %s', (_label, query) => {
expect(mayTargetCloudflareInternalTable(query)).toBe(true);
});
Expand All @@ -39,6 +40,7 @@ describe('mayTargetCloudflareInternalTable', () => {
['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'],
['a table with pi_ in the middle', 'SELECT * FROM api_keys'],
])('returns false for %s', (_label, query) => {
expect(mayTargetCloudflareInternalTable(query)).toBe(false);
});
Expand Down
Loading