Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import * as Sentry from '@sentry/cloudflare';
import { DurableObject } from 'cloudflare:workers';

interface Env {
SENTRY_DSN: string;
TEST_DURABLE_OBJECT: DurableObjectNamespace;
}

class SqlDurableObjectBase extends DurableObject<Env> {
public constructor(ctx: DurableObjectState, env: Env) {
super(ctx, env);
}

async fetch(request: Request): Promise<Response> {
const url = new URL(request.url);

if (url.pathname === '/exec') {
this.ctx.storage.sql.exec('CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, name TEXT)');
this.ctx.storage.sql.exec('INSERT INTO users (name) VALUES (?)', 'Alice');
const cursor = this.ctx.storage.sql.exec('SELECT * FROM users');
const rows = cursor.toArray();

return Response.json({ rows });
}

return new Response('OK');
}
}

export const TestDurableObject = Sentry.instrumentDurableObjectWithSentry(
(env: Env) => ({
dsn: env.SENTRY_DSN,
tracesSampleRate: 1.0,
}),
SqlDurableObjectBase,
);

export default Sentry.withSentry(
(env: Env) => ({
dsn: env.SENTRY_DSN,
tracesSampleRate: 1.0,
}),
{
async fetch(request: Request, env: Env): Promise<Response> {
const url = new URL(request.url);

if (url.pathname === '/flush-marker') {
Sentry.captureMessage('flush-marker');
return new Response(JSON.stringify({ ok: true }), { headers: { 'Content-Type': 'application/json' } });
}

const id = env.TEST_DURABLE_OBJECT.idFromName('test');
const stub = env.TEST_DURABLE_OBJECT.get(id);
return stub.fetch(request);
},
} satisfies ExportedHandler<Env>,
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import type { Envelope } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import { expect, it } from 'vitest';
import { createRunner } from '../../../runner';

const flushMarkerMatcher = (envelope: Envelope): void => {
const [, items] = envelope;
const [itemHeader, itemBody] = items[0] as [{ type: string }, Record<string, unknown>];

expect(itemHeader.type).toBe('event');
expect(itemBody.message).toBe('flush-marker');
};

it('instruments SQL exec operations on Durable Object storage', async ({ signal }) => {
const runner = createRunner(__dirname)
.expect(envelope => {
const transactionEvent = envelope[1]?.[0]?.[1];
const spans = transactionEvent?.spans ?? [];

expect(transactionEvent).toEqual(
expect.objectContaining({
type: 'transaction',
transaction: 'GET /exec',
}),
);

const sqlSpans = (spans as Array<Record<string, unknown>>).filter(
s => s.origin === 'auto.db.cloudflare.durable_object.sql',
);

expect(sqlSpans).toHaveLength(3);
expect(sqlSpans).toEqual(
expect.arrayContaining([
expect.objectContaining({
description: 'CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, name TEXT)',
op: 'db.query',
origin: 'auto.db.cloudflare.durable_object.sql',
data: expect.objectContaining({
[SEMANTIC_ATTRIBUTE_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',
'db.query.text': 'CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, name TEXT)',
'cloudflare.durable_object.query.bindings': 0,
'cloudflare.durable_object.response.rows_read': expect.any(Number),
'cloudflare.durable_object.response.rows_written': expect.any(Number),
}),
}),
expect.objectContaining({
description: 'INSERT INTO users (name) VALUES (?)',
op: 'db.query',
origin: 'auto.db.cloudflare.durable_object.sql',
data: expect.objectContaining({
'db.system.name': 'cloudflare-durable-object-sql',
'db.operation.name': 'exec',
'db.query.text': 'INSERT INTO users (name) VALUES (?)',
'cloudflare.durable_object.query.bindings': 1,
'cloudflare.durable_object.response.rows_written': expect.any(Number),
}),
}),
expect.objectContaining({
description: 'SELECT * FROM users',
op: 'db.query',
origin: 'auto.db.cloudflare.durable_object.sql',
data: expect.objectContaining({
'db.system.name': 'cloudflare-durable-object-sql',
'db.operation.name': 'exec',
'db.query.text': 'SELECT * FROM users',
'cloudflare.durable_object.query.bindings': 0,
'cloudflare.durable_object.response.rows_read': expect.any(Number),
}),
}),
]),
);
})
.expect(flushMarkerMatcher)
.unordered()
.start(signal);

await runner.makeRequest('get', '/exec');
await runner.makeRequest('get', '/flush-marker');
await runner.completed();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"name": "worker-name",
"main": "index.ts",
"compatibility_date": "2025-06-17",
"migrations": [
{
"new_sqlite_classes": ["TestDurableObject"],
"tag": "v1",
},
],
"durable_objects": {
"bindings": [
{
"class_name": "TestDurableObject",
"name": "TEST_DURABLE_OBJECT",
},
],
},
"compatibility_flags": ["nodejs_als"],
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { DurableObjectStorage, SyncKvStorage } from '@cloudflare/workers-types';
import type { DurableObjectStorage, SyncKvStorage, SqlStorage } from '@cloudflare/workers-types';
import { isThenable, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core';
import { storeSpanContext } from '../utils/traceLinks';
import { instrumentDurableObjectSyncKvStorage } from './instrumentDurableObjectSyncKvStorage';
import { instrumentSqlStorage } from './instrumentSqlStorage';

const STORAGE_METHODS_TO_INSTRUMENT = ['get', 'put', 'delete', 'list', 'setAlarm', 'getAlarm', 'deleteAlarm'] as const;

Expand Down Expand Up @@ -40,6 +41,10 @@ export function instrumentDurableObjectStorage(
return instrumentDurableObjectSyncKvStorage(original as SyncKvStorage);
}

if (prop === 'sql' && original != null && 'databaseSize' in original && 'exec' in original) {
return instrumentSqlStorage(original as SqlStorage);
}

if (typeof original !== 'function') {
return original;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import type { SqlStorage, SqlStorageCursor, SqlStorageValue } from '@cloudflare/workers-types';
import { _INTERNAL_sanitizeSqlQuery, addBreadcrumb, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core';

/**
* Instruments the Durable Object SqlStorage `exec` method with Sentry spans.
*
* @param sql - The SqlStorage instance to instrument
* @returns An instrumented SqlStorage instance
*/
export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
return new Proxy(sql, {
get(target, prop, _receiver) {
const original = Reflect.get(target, prop, target);

if (prop !== 'exec' || typeof original !== 'function') {
return original;
}

return function (this: unknown, ...args: unknown[]) {
const [query, ...bindings] = args as [string, ...unknown[]];
const sanitizedQuery = _INTERNAL_sanitizeSqlQuery(query);

return startSpan(
{
op: 'db.query',
name: sanitizedQuery,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object.sql',
'db.system.name': 'cloudflare-durable-object-sql',
'db.operation.name': 'exec',
'db.query.text': sanitizedQuery,
'cloudflare.durable_object.query.bindings': bindings.length,
},
},
span => {
const cursor: SqlStorageCursor<Record<string, SqlStorageValue>> = (
original as (...a: unknown[]) => SqlStorageCursor<Record<string, SqlStorageValue>>
).apply(target, args);

span.setAttributes({
'cloudflare.durable_object.response.rows_read': cursor.rowsRead,
'cloudflare.durable_object.response.rows_written': cursor.rowsWritten,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: cursor.rowsRead and cursor.rowsWritten are read before the cursor is iterated, causing SQL metrics to be incorrectly reported as 0.
Severity: MEDIUM

Suggested Fix

To fix this, the cursor's iteration methods (like next(), all(), etc.) should be wrapped. The rowsRead and rowsWritten properties should only be read and set on the span after these iteration methods have been called and the cursor has been at least partially consumed. This ensures the metrics reflect the actual state of the cursor after data access.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts#L40-L43

Potential issue: The instrumentation for Cloudflare's `SqlStorageCursor` reads the
`cursor.rowsRead` and `cursor.rowsWritten` properties immediately after the cursor is
created. According to official documentation, the cursor is a lazy iterator, and these
values are only populated as the cursor is iterated over. Because the instrumentation
captures these metrics before returning the cursor to the caller for consumption, the
span attributes and breadcrumbs will always record `0` for
`cloudflare.durable_object.response.rows_read` and
`cloudflare.durable_object.response.rows_written`. This renders the row tracking metrics
non-functional, as they will consistently be inaccurate in any real-world usage.

Also affects:

  • packages/cloudflare/src/instrumentations/instrumentSqlStorage.ts:48~51

Did we get this right? 👍 / 👎 to inform future reviews.


addBreadcrumb({
category: 'query',
message: sanitizedQuery,
data: {
'cloudflare.durable_object.response.rows_read': cursor.rowsRead,
'cloudflare.durable_object.response.rows_written': cursor.rowsWritten,
},
});

return cursor;
Comment thread
cursor[bot] marked this conversation as resolved.
},
);
};
},
});
}
31 changes: 23 additions & 8 deletions packages/cloudflare/test/instrumentDurableObjectStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,29 @@ describe('instrumentDurableObjectStorage', () => {
});
});

it('instruments sql exec', () => {
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');
const mockStorage = createMockStorage();
const instrumented = instrumentDurableObjectStorage(mockStorage);

instrumented.sql.exec('SELECT 1');

expect(startSpanSpy).toHaveBeenCalledWith(
{
name: 'SELECT ?',
op: 'db.query',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.cloudflare.durable_object.sql',
'db.system.name': 'cloudflare-durable-object-sql',
'db.operation.name': 'exec',
'db.query.text': 'SELECT ?',
'cloudflare.durable_object.query.bindings': 0,
},
},
expect.any(Function),
);
});

describe('non-instrumented methods', () => {
it('does not instrument deleteAll, sync, transaction', async () => {
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');
Expand All @@ -312,14 +335,6 @@ describe('instrumentDurableObjectStorage', () => {

expect(startSpanSpy).not.toHaveBeenCalled();
});

it('does not instrument sql property', () => {
const mockStorage = createMockStorage();
const instrumented = instrumentDurableObjectStorage(mockStorage);

// sql is a property, not a method we instrument
expect(instrumented.sql).toBe(mockStorage.sql);
});
});

describe('sync KV instrumentation', () => {
Expand Down
Loading
Loading