Skip to content

Commit c6f790b

Browse files
andreiborzaclaude
andauthored
fix(node): Prevent PostgresJs integration from emitting duplicate spans per query (#21364)
postgres.js calls handle() from then/catch/finally, but only the first invocation executes SQL (guarded by this.executed). The patched handle was creating a new span on every call, inflating span count 2-3x. Fixes #21355 --------- Co-authored-by: Claude claude-opus-4-6 <noreply@anthropic.com>
1 parent d645349 commit c6f790b

3 files changed

Lines changed: 46 additions & 4 deletions

File tree

‎packages/core/src/integrations/postgresjs.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,10 @@ function _wrapSingleQueryHandle(
217217

218218
// IMPORTANT: We must replace the handle function directly, not use a Proxy,
219219
// because Query.then() internally calls this.handle(), which would bypass a Proxy wrapper.
220-
const wrappedHandle = async function (this: unknown, ...args: unknown[]): Promise<unknown> {
221-
if (!_shouldCreateSpans(options)) {
220+
const wrappedHandle = async function (this: { executed?: boolean }, ...args: unknown[]): Promise<unknown> {
221+
// postgres.js calls handle() from then/catch/finally — only the first call executes SQL,
222+
// subsequent calls are no-ops (guarded by this.executed). Skip span creation for no-ops.
223+
if (this.executed || !_shouldCreateSpans(options)) {
222224
return originalHandle.apply(this, args);
223225
}
224226

‎packages/core/test/lib/integrations/postgresjs.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
22
import { _reconstructQuery, _sanitizeSqlQuery, instrumentPostgresJsSql } from '../../../src/integrations/postgresjs';
3+
import * as tracing from '../../../src/tracing';
34
import * as spanUtils from '../../../src/utils/spanUtils';
45

56
describe('PostgresJs portable instrumentation', () => {
@@ -554,6 +555,42 @@ describe('PostgresJs portable instrumentation', () => {
554555
// handle was wrapped
555556
expect((mockQuery.handle as any).__sentryWrapped).toBe(true);
556557
});
558+
559+
it('only creates one span even when handle() is called multiple times', async () => {
560+
const mockSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), end: vi.fn() };
561+
const startSpanManualSpy = vi
562+
.spyOn(tracing, 'startSpanManual')
563+
.mockImplementation((_opts, callback) => callback(mockSpan as any, () => {}));
564+
565+
const originalHandle = vi.fn().mockResolvedValue([]);
566+
const mockQuery = {
567+
handle: originalHandle,
568+
strings: ['SELECT 1'],
569+
resolve: vi.fn(),
570+
reject: vi.fn(),
571+
executed: false,
572+
};
573+
const mockSql = vi.fn().mockReturnValue(mockQuery);
574+
575+
const instrumented = instrumentPostgresJsSql(mockSql, { requireParentSpan: false });
576+
instrumented(['SELECT 1']);
577+
578+
const wrappedHandle = mockQuery.handle as (...args: unknown[]) => Promise<unknown>;
579+
580+
// First call — executed is false, should create a span
581+
await wrappedHandle.call(mockQuery);
582+
expect(startSpanManualSpy).toHaveBeenCalledTimes(1);
583+
584+
// Simulate postgres.js setting executed = true after first handle()
585+
mockQuery.executed = true;
586+
587+
// Second and third calls (from .then/.catch/.finally) — should NOT create more spans
588+
await wrappedHandle.call(mockQuery);
589+
await wrappedHandle.call(mockQuery);
590+
expect(startSpanManualSpy).toHaveBeenCalledTimes(1);
591+
592+
startSpanManualSpy.mockRestore();
593+
});
557594
});
558595

559596
it('does not wrap non-query results from sql call', () => {

‎packages/node/src/integrations/tracing/postgresjs.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,11 +281,14 @@ export class PostgresJsInstrumentation extends InstrumentationBase<PostgresJsIns
281281
resolve: unknown;
282282
reject: unknown;
283283
strings?: string[];
284+
executed?: boolean;
284285
},
285286
...args: unknown[]
286287
): Promise<unknown> {
287-
// Skip if this query came from an instrumented sql instance (already handled by wrapper)
288-
if ((this as Record<symbol, unknown>)[QUERY_FROM_INSTRUMENTED_SQL]) {
288+
// Skip if this query came from an instrumented sql instance (already handled by wrapper),
289+
// or if handle() was already called (postgres.js calls handle() from then/catch/finally —
290+
// only the first call executes SQL, subsequent calls are no-ops).
291+
if (this.executed || (this as Record<symbol, unknown>)[QUERY_FROM_INSTRUMENTED_SQL]) {
289292
return originalHandle.apply(this, args);
290293
}
291294

0 commit comments

Comments
 (0)