feat(pg-instrumentation): record exceptions as span events (#3182)

Author: aryamohanan

packages/instrumentation-pg/src/instrumentation.ts

@@ -339,8 +339,8 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
               values: Array.isArray(args[1]) ? args[1] : undefined,
             }
           : firstArgIsQueryObjectWithText
-          ? (arg0 as utils.ObjectWithText)
-          : undefined;
+            ? (arg0 as utils.ObjectWithText)
+            : undefined;
 
         const attributes: Attributes = {
           [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL,
@@ -469,6 +469,9 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
         try {
           result = original.apply(this, args as never);
         } catch (e: unknown) {
+          if (e instanceof Error) {
+            span.recordException(utils.sanitizedErrorMessage(e));
+          }
           span.setStatus({
             code: SpanStatusCode.ERROR,
             message: utils.getErrorMessage(e),
@@ -491,6 +494,9 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
             })
             .catch((error: Error) => {
               return new Promise((_, reject) => {
+                if (error instanceof Error) {
+                  span.recordException(utils.sanitizedErrorMessage(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(utils.sanitizedErrorMessage(error));
+        }
         span.setStatus({
           code: SpanStatusCode.ERROR,
           message: utils.getErrorMessage(error),

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(sanitizedErrorMessage(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(sanitizedErrorMessage(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(sanitizedErrorMessage(err));
+      }
       span.setStatus({
         code: SpanStatusCode.ERROR,
         message: err.message,
@@ -460,3 +468,14 @@ export type ObjectWithText = {
   text: string;
   [k: string]: unknown;
 };
+
+/**
+ * Generates a sanitized message for the error.
+ * Only includes the error type and PostgreSQL error code, omitting any sensitive details.
+ */
+export function sanitizedErrorMessage(error: unknown): string {
+  const name = (error as any)?.name ?? 'PostgreSQLError';
+  const code = (error as any)?.code ?? 'UNKNOWN';
+
+  return `PostgreSQL error of type '${name}' occurred (code: ${code})`;
+}

packages/instrumentation-pg/test/pg-pool.test.ts

@@ -530,6 +530,127 @@ 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!;
+            const code = '42P01';
+
+            const message = attrs['exception.message'];
+            console.log('exception message:', message);
+            assert.ok(message, 'exception.message should exist');
+            assert.strictEqual(
+              typeof message,
+              'string',
+              'exception.message should be a string'
+            );
+
+            if (typeof message === 'string') {
+              assert.ok(
+                message.includes(code),
+                `exception.message should include the Postgres error code ${code}`
+              );
+            }
+          });
+
+          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!;
+              const code = '42P01';
+
+              const message = attrs['exception.message'];
+              assert.ok(message, 'exception.message should exist');
+              assert.strictEqual(
+                typeof message,
+                'string',
+                'exception.message should be a string'
+              );
+
+              if (typeof message === 'string') {
+                assert.ok(
+                  message.includes(code),
+                  `exception.message should include the Postgres error code ${code}`
+                );
+              }
+            });
+
+            memoryExporter.reset();
+          }
+        }
+      );
+    });
+  });
+
   describe('pg metrics', () => {
     let metricReader: testUtils.TestMetricReader;
 

packages/instrumentation-pg/test/pg.test.ts

@@ -83,9 +83,6 @@ const DEFAULT_ATTRIBUTES = {
 const unsetStatus: SpanStatus = {
   code: SpanStatusCode.UNSET,
 };
-const errorStatus: SpanStatus = {
-  code: SpanStatusCode.ERROR,
-};
 
 const runCallbackTest = (
   span: Span | null,
@@ -161,6 +158,8 @@ describe('pg', () => {
   });
 
   beforeEach(() => {
+    memoryExporter.reset();
+
     contextManager = new AsyncLocalStorageContextManager().enable();
     context.setGlobalContextManager(contextManager);
 
@@ -192,47 +191,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');
 
-    assert.throws(
-      () => {
-        (client as any).query(undefined);
-      },
-      assertPgError,
-      'pg should throw when undefined provided as only arg'
-    );
-    runCallbackTest(null, DEFAULT_ATTRIBUTES, [], errorStatus);
-    memoryExporter.reset();
+      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'],
+        "PostgreSQL error of type 'TypeError' occurred (code: UNKNOWN)"
+      );
+      assert.ok(event.time.length === 2, 'Event time should be a HrTime array');
+
+      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 +988,64 @@ 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');
+        const code = '42P01';
+        assert.ok(
+          attrs['exception.message'].includes(code),
+          `Expected exception.message to include error code ${code}`
+        );
+      });
+    };
+
+    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;