From c6d754b9fff3033512e6e8f59eaf8999ffce46b6 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 27 Jul 2026 13:04:52 +0200 Subject: [PATCH] feat(cloudflare): Filter framework-internal Durable Object storage spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KV entries managed by Durable Object frameworks themselves (`agents`, `partyserver`) — `cf_`-prefixed state keys, `__ps_` partyserver keys and MCP OAuth client state under `/` — are bookkeeping rather than user work and flooded traces with `durable_object_storage_*` spans. Skip spans for those keys, mirroring how `cf_`-prefixed SQL tables are filtered, and add a `durableObjectStorageSpanAllowlist` option to opt colliding user keys back into instrumentation. Co-Authored-By: Claude Opus 4.8 --- .../cloudflare-agent/tests/callable.test.ts | 90 +++++++++++++++---- .../cloudflare-agent/worker/index.ts | 13 +++ packages/cloudflare/src/client.ts | 25 ++++++ .../instrumentDurableObjectStorage.ts | 13 ++- .../src/utils/internalStorageKey.ts | 75 ++++++++++++++++ .../instrumentDurableObjectStorage.test.ts | 74 +++++++++++++++ .../test/utils/internalStorageKey.test.ts | 86 ++++++++++++++++++ 7 files changed, 357 insertions(+), 19 deletions(-) create mode 100644 packages/cloudflare/src/utils/internalStorageKey.ts create mode 100644 packages/cloudflare/test/utils/internalStorageKey.test.ts diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-agent/tests/callable.test.ts b/dev-packages/e2e-tests/test-applications/cloudflare-agent/tests/callable.test.ts index bdd5bd22b8c0..d23b9f4c3870 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-agent/tests/callable.test.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-agent/tests/callable.test.ts @@ -9,6 +9,16 @@ test('@callable() methods work correctly with Sentry instrumentDurableObjectWith ); }); + // The greet() call goes over the websocket, so its storage spans land in a webSocketMessage + // transaction. Filter for the one carrying our put span — control messages produce their own + // webSocketMessage transactions without storage spans. + const storageTransactionPromise = waitForTransaction('cloudflare-agent', transactionEvent => { + return ( + transactionEvent.transaction === 'webSocketMessage' && + (transactionEvent.spans ?? []).some(span => span.description === 'durable_object_storage_put') + ); + }); + await page.goto(baseURL!); await expect(page.getByText('Connected')).toBeVisible(); @@ -32,24 +42,7 @@ test('@callable() methods work correctly with Sentry instrumentDurableObjectWith culture: { timezone: expect.any(String) }, runtime: { name: 'cloudflare' }, }, - spans: expect.arrayContaining([ - expect.objectContaining({ - data: { - 'db.operation.name': 'get', - 'db.system.name': 'cloudflare.durable_object.storage', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.cloudflare.durable_object', - }, - description: 'durable_object_storage_get', - op: 'db', - origin: 'auto.db.cloudflare.durable_object', - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - }), - ]), + spans: [], start_timestamp: expect.any(Number), timestamp: expect.any(Number), transaction: 'GET /agents/my-agent/user-123', @@ -72,4 +65,65 @@ test('@callable() methods work correctly with Sentry instrumentDurableObjectWith packages: expect.any(Array), }, }); + + // greet() touches 6 storage keys: 2 user ops + 3 framework-internal keys (cf_, __ps_, /) that + // must be filtered + 1 allowlisted cf_ key. Spans carry no key attribute, so filtering can only + // be verified by count — exactly these 3 storage spans (in execution order) should survive, and + // any framework-internal span leaking through shows up as an extra entry here. + const storageTransaction = await storageTransactionPromise; + + const storageSpans = (storageTransaction.spans ?? []).filter( + span => span.origin === 'auto.db.cloudflare.durable_object', + ); + + expect(storageSpans).toEqual([ + expect.objectContaining({ + data: { + 'db.operation.name': 'put', + 'db.system.name': 'cloudflare.durable_object.storage', + 'sentry.op': 'db', + 'sentry.origin': 'auto.db.cloudflare.durable_object', + }, + description: 'durable_object_storage_put', + op: 'db', + origin: 'auto.db.cloudflare.durable_object', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }), + expect.objectContaining({ + data: { + 'db.operation.name': 'get', + 'db.system.name': 'cloudflare.durable_object.storage', + 'sentry.op': 'db', + 'sentry.origin': 'auto.db.cloudflare.durable_object', + }, + description: 'durable_object_storage_get', + op: 'db', + origin: 'auto.db.cloudflare.durable_object', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }), + expect.objectContaining({ + data: { + 'db.operation.name': 'get', + 'db.system.name': 'cloudflare.durable_object.storage', + 'sentry.op': 'db', + 'sentry.origin': 'auto.db.cloudflare.durable_object', + }, + description: 'durable_object_storage_get', + op: 'db', + origin: 'auto.db.cloudflare.durable_object', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }), + ]); }); diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-agent/worker/index.ts b/dev-packages/e2e-tests/test-applications/cloudflare-agent/worker/index.ts index fc92d0e2de74..677bff3028d7 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-agent/worker/index.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-agent/worker/index.ts @@ -4,6 +4,18 @@ import { routeAgentRequest, Agent, callable } from 'agents'; class MyBaseAgent extends Agent { @callable() async greet(name: string): Promise { + // User keys — instrumented, spans expected + await this.ctx.storage.put('test', 'any value'); + await this.ctx.storage.get('test'); + + // Framework-internal keys (agents/partyserver/MCP OAuth conventions) — filtered, no spans expected + await this.ctx.storage.put('cf_e2e_internal', 'bookkeeping'); + await this.ctx.storage.get('__ps_name'); + await this.ctx.storage.get('/oauth/client/token'); + + // Allowlisted cf_ key — span expected + await this.ctx.storage.get('cf_user_key'); + return `Hello, ${name}!`; } } @@ -15,6 +27,7 @@ export const MyAgent = Sentry.instrumentDurableObjectWithSentry( tunnel: `http://localhost:3031/`, tracesSampleRate: 1, enableRpcTracePropagation: true, + durableObjectStorageSpanAllowlist: ['cf_user_key'], }), MyBaseAgent, ); diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index f14d1719f962..087fb9f63373 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -240,6 +240,31 @@ interface BaseCloudflareOptions { */ durableObjectSqlSpanAllowlist?: Array; + /** + * KV keys that should stay instrumented even though they match a reserved prefix used by Durable + * Object frameworks (`agents`, `partyserver`, ...) for their internal storage entries. + * + * By default, KV reads/writes (`get`, `put`, `delete`, `list`) of `cf_`- or `__ps_`-prefixed keys + * are treated as framework noise and no `durable_object_storage_*` span is created for them, + * mirroring how `cf_`-prefixed SQL tables are handled (see {@link durableObjectSqlSpanAllowlist}). + * If one of your own keys happens to use such a prefix, add it here to opt it back into + * instrumentation. Strings must match exactly, while regular expressions give you prefix/pattern + * matching. + * + * @default [] + * @example + * ```ts + * export default Sentry.withSentry( + * (env) => ({ + * dsn: env.SENTRY_DSN, + * durableObjectStorageSpanAllowlist: ['cf_my_key', /^cf_reports_/], + * }), + * handler, + * ); + * ``` + */ + durableObjectStorageSpanAllowlist?: Array; + /** * @deprecated Use `enableRpcTracePropagation` instead. This option will be removed in a future major version. * diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts index 9413b313b4b0..e468512dd722 100644 --- a/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts @@ -1,5 +1,7 @@ import type { DurableObjectStorage, SyncKvStorage, SqlStorage } from '@cloudflare/workers-types'; -import { isThenable, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { getClient, isThenable, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import type { CloudflareClientOptions } from '../client'; +import { getStorageKeys, targetsCloudflareInternalKey } from '../utils/internalStorageKey'; import { storeSpanContext } from '../utils/traceLinks'; import { instrumentDurableObjectSyncKvStorage } from './instrumentDurableObjectSyncKvStorage'; import { instrumentSqlStorage } from './instrumentSqlStorage'; @@ -55,6 +57,15 @@ export function instrumentDurableObjectStorage( } return function (this: unknown, ...args: unknown[]) { + // KV entries managed by the DO framework itself (agents/partyserver state) are bookkeeping + // rather than user work — skip the span, mirroring how `cf_` SQL tables are treated. + const allowlist = (getClient()?.getOptions() as CloudflareClientOptions | undefined) + ?.durableObjectStorageSpanAllowlist; + const keys = getStorageKeys(methodName, args); + if (keys && keys.length > 0 && keys.every(key => targetsCloudflareInternalKey(key, allowlist))) { + return (original as (...a: unknown[]) => unknown).apply(target, args); + } + return startSpan( { // Use underscore naming to match Cloudflare's native instrumentation (e.g., "durable_object_storage_get") diff --git a/packages/cloudflare/src/utils/internalStorageKey.ts b/packages/cloudflare/src/utils/internalStorageKey.ts new file mode 100644 index 000000000000..9706bc424b5e --- /dev/null +++ b/packages/cloudflare/src/utils/internalStorageKey.ts @@ -0,0 +1,75 @@ +import { stringMatchesSomePattern } from '@sentry/core'; + +/** + * Cloudflare frameworks that build on Durable Objects (`agents`, `partyserver`, ...) also manage + * their own internal KV entries alongside their internal SQLite tables, namespaced with a reserved + * prefix — e.g. `cf_agents_state`, `cf_agents_mcp_servers`, `__ps_name`. Reads/writes of these + * (message persistence, MCP connection bookkeeping, name hydration) are framework implementation + * details that otherwise flood traces with dozens of zero-signal `durable_object_storage_*` spans + * per request, so we match the reserved prefixes rather than an enumerated list. This mirrors the + * `cf_` convention used for internal SQL tables (see `targetsCloudflareInternalTable`). + * + * The prefixes are a reserved convention for framework-managed entries, so user keys should not use + * them. In case a user key does collide, the `durableObjectStorageSpanAllowlist` option lets them + * opt those keys back into instrumentation. + */ +export function targetsCloudflareInternalKey(key: string | undefined, allowlist?: Array): boolean { + if (!key) { + return false; + } + + // Framework-managed KV namespaces: + // - `cf_` — agents / ai-chat internal state (mirrors the internal SQL table convention) + // - `__ps_` — partyserver internals (e.g. `__ps_name`) + // - `/` — MCP OAuth client state (`///{token,client_info,state,...}`), + // read on every MCP tool call. User keys on an Agent rarely use a leading slash; if one does, + // the allowlist opts it back in. + const isFrameworkKey = key.startsWith('cf_') || key.startsWith('__ps_') || key.startsWith('/'); + if (!isFrameworkKey) { + return false; + } + + // A key on the allowlist is treated as a user key and stays instrumented, even though it matches + // a reserved prefix. + return !allowlist?.length || !stringMatchesSomePattern(key, allowlist, true); +} + +/** + * Extracts the KV keys a Durable Object storage call targets, so the caller can decide whether the + * operation only touches framework-internal entries. Returns `undefined` when the keys can't be + * determined from the arguments (e.g. `list()` without a prefix), in which case the call is treated + * as user work and stays instrumented. + */ +export function getStorageKeys(methodName: string, args: unknown[]): string[] | undefined { + const [first] = args; + + if (methodName === 'get' || methodName === 'delete') { + // get(key) / get(keys[]) / delete(key) / delete(keys[]) + if (typeof first === 'string') { + return [first]; + } + if (Array.isArray(first)) { + return first.filter((k): k is string => typeof k === 'string'); + } + return undefined; + } + + if (methodName === 'put') { + // put(key, value) or put({ key: value, ... }) + if (typeof first === 'string') { + return [first]; + } + if (first && typeof first === 'object' && !Array.isArray(first)) { + return Object.keys(first); + } + return undefined; + } + + if (methodName === 'list') { + // list({ prefix }) + const prefix = first && typeof first === 'object' ? (first as { prefix?: unknown }).prefix : undefined; + return typeof prefix === 'string' ? [prefix] : undefined; + } + + return undefined; +} diff --git a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts index ee4cdc66360b..ec311fe66188 100644 --- a/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts +++ b/packages/cloudflare/test/instrumentDurableObjectStorage.test.ts @@ -324,6 +324,80 @@ describe('instrumentDurableObjectStorage', () => { ); }); + describe('framework-internal KV keys', () => { + it('does not create a span for a cf_-prefixed get', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.get('cf_agents_state'); + + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('does not create a span for a __ps_-prefixed get', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.get('__ps_name'); + + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('does not create a span for a cf_-prefixed put with object entries', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.put({ cf_agents_a: 1, cf_agents_b: 2 }); + + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('does not create a span for a cf_-prefixed delete with an array of keys', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.delete(['cf_agents_a', 'cf_agents_b']); + + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('does not create a span for a list with a cf_ prefix', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.list({ prefix: 'cf_agents_' }); + + expect(startSpanSpy).not.toHaveBeenCalled(); + }); + + it('still creates a span when a batch mixes framework and user keys', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.get(['cf_agents_state', 'myKey']); + + expect(startSpanSpy).toHaveBeenCalled(); + }); + + it('still creates a span for a list without a prefix', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.list(); + + expect(startSpanSpy).toHaveBeenCalled(); + }); + + it('still creates a span for a user key', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); + const instrumented = instrumentDurableObjectStorage(createMockStorage()); + + await instrumented.get('myKey'); + + expect(startSpanSpy).toHaveBeenCalled(); + }); + }); + describe('non-instrumented methods', () => { it('does not instrument deleteAll, sync, transaction', async () => { const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); diff --git a/packages/cloudflare/test/utils/internalStorageKey.test.ts b/packages/cloudflare/test/utils/internalStorageKey.test.ts new file mode 100644 index 000000000000..b26379b8ea69 --- /dev/null +++ b/packages/cloudflare/test/utils/internalStorageKey.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest'; +import { getStorageKeys, targetsCloudflareInternalKey } from '../../src/utils/internalStorageKey'; + +describe('targetsCloudflareInternalKey', () => { + it('matches cf_-prefixed keys', () => { + expect(targetsCloudflareInternalKey('cf_agents_state')).toBe(true); + expect(targetsCloudflareInternalKey('cf_mcp_servers')).toBe(true); + }); + + it('matches __ps_-prefixed keys', () => { + expect(targetsCloudflareInternalKey('__ps_name')).toBe(true); + }); + + it('matches MCP OAuth client-state keys', () => { + expect(targetsCloudflareInternalKey('/sentry/abc123/def456/token')).toBe(true); + expect(targetsCloudflareInternalKey('/github-inspector/abc123/state/nonce')).toBe(true); + expect(targetsCloudflareInternalKey('/sentry/abc123/def456/client_info/')).toBe(true); + }); + + it('does not match user keys', () => { + expect(targetsCloudflareInternalKey('myKey')).toBe(false); + expect(targetsCloudflareInternalKey('user_settings')).toBe(false); + }); + + it('does not match keys that merely contain a reserved substring', () => { + expect(targetsCloudflareInternalKey('my_cf_key')).toBe(false); + }); + + it('returns false for undefined or empty keys', () => { + expect(targetsCloudflareInternalKey(undefined)).toBe(false); + expect(targetsCloudflareInternalKey('')).toBe(false); + }); + + it('respects an exact-string allowlist entry', () => { + expect(targetsCloudflareInternalKey('cf_my_key', ['cf_my_key'])).toBe(false); + expect(targetsCloudflareInternalKey('cf_other', ['cf_my_key'])).toBe(true); + }); + + it('respects a regex allowlist entry', () => { + expect(targetsCloudflareInternalKey('cf_reports_daily', [/^cf_reports_/])).toBe(false); + expect(targetsCloudflareInternalKey('cf_agents_state', [/^cf_reports_/])).toBe(true); + }); +}); + +describe('getStorageKeys', () => { + it('extracts a single string key for get/delete', () => { + expect(getStorageKeys('get', ['myKey'])).toEqual(['myKey']); + expect(getStorageKeys('delete', ['myKey'])).toEqual(['myKey']); + }); + + it('extracts an array of keys for get/delete', () => { + expect(getStorageKeys('get', [['a', 'b']])).toEqual(['a', 'b']); + expect(getStorageKeys('delete', [['a', 'b']])).toEqual(['a', 'b']); + }); + + it('filters non-string entries from key arrays', () => { + expect(getStorageKeys('get', [['a', 1, 'b']])).toEqual(['a', 'b']); + }); + + it('extracts a single key for put(key, value)', () => { + expect(getStorageKeys('put', ['myKey', 'myValue'])).toEqual(['myKey']); + }); + + it('extracts all keys for put(entries)', () => { + expect(getStorageKeys('put', [{ a: 1, b: 2 }])).toEqual(['a', 'b']); + }); + + it('extracts the prefix for list({ prefix })', () => { + expect(getStorageKeys('list', [{ prefix: 'cf_agents_' }])).toEqual(['cf_agents_']); + }); + + it('returns undefined for list() without a prefix', () => { + expect(getStorageKeys('list', [])).toBeUndefined(); + expect(getStorageKeys('list', [{}])).toBeUndefined(); + }); + + it('returns undefined for alarm methods', () => { + expect(getStorageKeys('setAlarm', [Date.now()])).toBeUndefined(); + expect(getStorageKeys('deleteAlarm', [])).toBeUndefined(); + expect(getStorageKeys('getAlarm', [])).toBeUndefined(); + }); + + it('returns undefined for unknown methods', () => { + expect(getStorageKeys('deleteAll', [])).toBeUndefined(); + }); +});