From 86205fd89eab08cc92640b7cc318a31d0ceb2601 Mon Sep 17 00:00:00 2001 From: Arya Date: Sat, 18 Oct 2025 22:05:07 +0530 Subject: [PATCH 1/2] fix(pg-instrumentation): record exceptions as span events --- .../instrumentation-pg/src/instrumentation.ts | 13 +- packages/instrumentation-pg/src/utils.ts | 10 +- .../instrumentation-pg/test/pg-pool.test.ts | 114 +++++++++++++++ packages/instrumentation-pg/test/pg.test.ts | 137 +++++++++++++----- 4 files changed, 238 insertions(+), 36 deletions(-) diff --git a/packages/instrumentation-pg/src/instrumentation.ts b/packages/instrumentation-pg/src/instrumentation.ts index dd92882454..45ce33b557 100644 --- a/packages/instrumentation-pg/src/instrumentation.ts +++ b/packages/instrumentation-pg/src/instrumentation.ts @@ -339,8 +339,8 @@ export class PgInstrumentation extends InstrumentationBase { return new Promise((_, reject) => { + if (error instanceof Error) { + span.recordException(error); + } span.setStatus({ code: SpanStatusCode.ERROR, message: error.message, @@ -612,6 +618,9 @@ function handleConnectResult(span: Span, connectResult: unknown) { return result; }) .catch((error: unknown) => { + if (error instanceof Error) { + span.recordException(error); + } span.setStatus({ code: SpanStatusCode.ERROR, message: utils.getErrorMessage(error), diff --git a/packages/instrumentation-pg/src/utils.ts b/packages/instrumentation-pg/src/utils.ts index bde98c9aed..2d8d9cd253 100644 --- a/packages/instrumentation-pg/src/utils.ts +++ b/packages/instrumentation-pg/src/utils.ts @@ -342,7 +342,9 @@ export function patchCallback( if (Object.prototype.hasOwnProperty.call(err, 'code')) { attributes[ATTR_ERROR_TYPE] = (err as any)['code']; } - + if (err instanceof Error) { + span.recordException(err); + } span.setStatus({ code: SpanStatusCode.ERROR, message: err.message, @@ -412,6 +414,9 @@ export function patchCallbackPGPool( done: any ) { if (err) { + if (err instanceof Error) { + span.recordException(err); + } span.setStatus({ code: SpanStatusCode.ERROR, message: err.message, @@ -428,6 +433,9 @@ export function patchClientConnectCallback(span: Span, cb: Function): Function { err: Error ) { if (err) { + if (err instanceof Error) { + span.recordException(err); + } span.setStatus({ code: SpanStatusCode.ERROR, message: err.message, diff --git a/packages/instrumentation-pg/test/pg-pool.test.ts b/packages/instrumentation-pg/test/pg-pool.test.ts index 0e3cba4302..4c98b65877 100644 --- a/packages/instrumentation-pg/test/pg-pool.test.ts +++ b/packages/instrumentation-pg/test/pg-pool.test.ts @@ -530,6 +530,120 @@ describe('pg-pool', () => { }); }); + describe('exception event recording', () => { + const queryText = 'SELECT foo FROM nonexistent_table'; + + it('should record exceptions as events on spans for a query to a nonexistent table (callback)', done => { + const parentSpan = provider + .getTracer('test-pg-pool') + .startSpan('test span'); + context.with(trace.setSpan(context.active(), parentSpan), () => { + pool.query(queryText, err => { + assert.notEqual(err, null, 'Expected query to throw an error'); + + const spans = memoryExporter.getFinishedSpans(); + + const querySpan = spans.find( + s => + s.attributes?.[ATTR_DB_STATEMENT] && + String(s.attributes[ATTR_DB_STATEMENT]).includes( + 'nonexistent_table' + ) + ); + assert.ok( + querySpan, + 'Expected a span for the nonexistent table query' + ); + + const exceptionEvents = querySpan.events.filter( + e => e.name === 'exception' + ); + assert.ok( + exceptionEvents.length > 0, + 'Expected at least one exception event' + ); + + exceptionEvents.forEach(e => { + const attrs = e.attributes!; + assert.strictEqual( + attrs['exception.type'], + '42P01', + 'exception.type should match Postgres error code' + ); + assert.ok( + attrs['exception.message'], + 'exception.message should exist' + ); + assert.ok( + attrs['exception.stacktrace'], + 'exception.stacktrace should exist' + ); + }); + + memoryExporter.reset(); + done(); + }); + }); + }); + + it('should record exceptions as events on spans for a query to a nonexistent table (async-await)', async () => { + const parentSpan = provider + .getTracer('test-pg-pool') + .startSpan('test span'); + + await context.with( + trace.setSpan(context.active(), parentSpan), + async () => { + try { + await pool.query(queryText); + assert.fail('Expected query to throw an error'); + } catch { + const spans = memoryExporter.getFinishedSpans(); + + const querySpan = spans.find( + s => + s.attributes?.[ATTR_DB_STATEMENT] && + String(s.attributes[ATTR_DB_STATEMENT]).includes( + 'nonexistent_table' + ) + ); + assert.ok( + querySpan, + 'Expected a span for the nonexistent table query' + ); + + const exceptionEvents = querySpan.events.filter( + e => e.name === 'exception' + ); + assert.ok( + exceptionEvents.length > 0, + 'Expected at least one exception event' + ); + + exceptionEvents.forEach(e => { + const attrs = e.attributes!; + assert.strictEqual( + attrs['exception.type'], + '42P01', + 'exception.type should match Postgres error code' + ); + assert.ok( + attrs['exception.message'], + 'exception.message should exist' + ); + assert.ok( + attrs['exception.stacktrace'], + 'exception.stacktrace should exist' + ); + }); + + memoryExporter.reset(); + } + } + ); + }); + }); + describe('pg metrics', () => { let metricReader: testUtils.TestMetricReader; diff --git a/packages/instrumentation-pg/test/pg.test.ts b/packages/instrumentation-pg/test/pg.test.ts index 05e687e094..a5789a1888 100644 --- a/packages/instrumentation-pg/test/pg.test.ts +++ b/packages/instrumentation-pg/test/pg.test.ts @@ -161,6 +161,8 @@ describe('pg', () => { }); beforeEach(() => { + memoryExporter.reset(); + contextManager = new AsyncLocalStorageContextManager().enable(); context.setGlobalContextManager(contextManager); @@ -192,47 +194,54 @@ describe('pg', () => { return /node_modules[/\\]pg/.test(src); }; - assert.throws( - () => { - (client as any).query(); + const errorThrowCases = [ + { fn: () => (client as any).query(), desc: 'no args provided' }, + { fn: () => (client as any).query(null), desc: 'null as only arg' }, + { + fn: () => (client as any).query(undefined), + desc: 'undefined as only arg', }, - assertPgError, - 'pg should throw when no args provided' - ); - runCallbackTest(null, DEFAULT_ATTRIBUTES, [], errorStatus); - memoryExporter.reset(); + ]; - assert.throws( - () => { - (client as any).query(null); - }, - assertPgError, - 'pg should throw when null provided as only arg' - ); - runCallbackTest(null, DEFAULT_ATTRIBUTES, [], errorStatus); - memoryExporter.reset(); + errorThrowCases.forEach(({ fn, desc }) => { + assert.throws(fn, assertPgError, `pg should throw when ${desc}`); + const spans = memoryExporter.getFinishedSpans(); + assert.ok(spans.length > 0, 'No spans recorded'); + + const exceptionEvents = spans[0].events.filter( + e => e.name === 'exception' + ); + assert.strictEqual( + exceptionEvents.length, + 1, + 'Expected one exception event' + ); + + const event = exceptionEvents[0]; + assert.strictEqual( + event.attributes!['exception.message'], + 'Client was passed a null or undefined query' + ); + assert.strictEqual(event.attributes!['exception.type'], 'TypeError'); + assert.ok(event.time.length === 2, 'Event time should be a HrTime array'); - assert.throws( - () => { - (client as any).query(undefined); - }, - assertPgError, - 'pg should throw when undefined provided as only arg' - ); - runCallbackTest(null, DEFAULT_ATTRIBUTES, [], errorStatus); - memoryExporter.reset(); + memoryExporter.reset(); + }); assert.doesNotThrow( () => (client as any).query({ foo: 'bar' }, undefined, () => { - runCallbackTest( - null, - { - ...DEFAULT_ATTRIBUTES, - }, - [], - errorStatus + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const exceptionEvents = spans[0].events.filter( + e => e.name === 'exception' ); + assert.strictEqual( + exceptionEvents.length, + 1, + 'Expected 1 exception event' + ); + memoryExporter.reset(); }), 'pg should not throw when invalid config args are provided' ); @@ -982,6 +991,68 @@ describe('pg', () => { }); }); + describe('exception event recording', () => { + const assertExceptionEvents = (pgSpan: any) => { + assert.strictEqual( + pgSpan.status.code, + SpanStatusCode.ERROR, + 'Span should have ERROR status' + ); + + const exceptionEvents = pgSpan.events.filter( + (e: { name: string }) => e.name === 'exception' + ); + assert.ok( + exceptionEvents.length > 0, + 'Expected at least one exception event' + ); + + exceptionEvents.forEach((err: { attributes: any }) => { + const attrs = err.attributes!; + assert.ok(attrs['exception.message'], 'Expected exception.message'); + assert.strictEqual( + attrs['exception.type'], + '42P01', + 'exception.type should match Postgres error code' + ); + assert.ok( + attrs['exception.stacktrace'], + 'Expected exception.stacktrace' + ); + }); + }; + + it('should record exceptions as events on spans for a query to a nonexistent table (callback)', done => { + client.query('SELECT foo FROM nonexistent_table', err => { + assert.notEqual(err, null, 'Expected query to throw an error'); + + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1, 'Expected one finished span'); + const pgSpan = spans[0]; + + assertExceptionEvents(pgSpan); + + memoryExporter.reset(); + done(); + }); + }); + + it('should record exceptions as events on spans for a query to a nonexistent table (promise)', async () => { + try { + await client.query('SELECT foo FROM nonexistent_table'); + assert.fail('Expected query to throw an error'); + } catch { + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1, 'Expected one finished span'); + const pgSpan = spans[0]; + + assertExceptionEvents(pgSpan); + + memoryExporter.reset(); + } + }); + }); + describe('pg metrics', () => { let metricReader: testUtils.TestMetricReader; From 8a0c23b69fc72da83d553133bd6103227efed1c2 Mon Sep 17 00:00:00 2001 From: Arya Date: Sat, 18 Oct 2025 22:18:31 +0530 Subject: [PATCH 2/2] test: added an additional status code assertion --- packages/instrumentation-pg/test/pg.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/instrumentation-pg/test/pg.test.ts b/packages/instrumentation-pg/test/pg.test.ts index a5789a1888..4b48eab145 100644 --- a/packages/instrumentation-pg/test/pg.test.ts +++ b/packages/instrumentation-pg/test/pg.test.ts @@ -211,6 +211,9 @@ describe('pg', () => { const exceptionEvents = spans[0].events.filter( e => e.name === 'exception' ); + + assert.strictEqual(spans[0].status.code, errorStatus.code); + assert.strictEqual( exceptionEvents.length, 1,