feat(instrumentation-pg): implementation of metric operation duration

plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts

@@ -26,6 +26,11 @@
   Span,
   SpanStatusCode,
   SpanKind,
+  Histogram,
+  MeterProvider,
+  ValueType,
+  Attributes,
+  HrTime,
 } from '@opentelemetry/api';
 import type * as pgTypes from 'pg';
 import type * as pgPoolTypes from 'pg-pool';
@@ -41,10 +46,49 @@
 import { addSqlCommenterComment } from '@opentelemetry/sql-common';
 import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
 import { SpanNames } from './enums/SpanNames';
+import {
+  hrTime,
+  hrTimeDuration,
+  hrTimeToMilliseconds,
+} from '@opentelemetry/core';
+import {
+  DBSYSTEMVALUES_POSTGRESQL,
+  SEMATTRS_DB_SYSTEM,
+} from '@opentelemetry/semantic-conventions';
+
+// TODO: Replace these constants once a new version of the semantic conventions
+// package is created
+const SEMATTRS_DB_NAMESPACE = 'db.namespace';
+const SEMATTRS_ERROR_TYPE = 'error.type';
+const SEMATTRS_SERVER_PORT = 'server.port';
+const SEMATTRS_SERVER_ADDRESS = 'server.address';
 
 export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConfig> {
+  private _operationDuration!: Histogram;
   constructor(config: PgInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
+    this._setMetricInstruments();
+  }
+
+  override setMeterProvider(meterProvider: MeterProvider) {
+    super.setMeterProvider(meterProvider);
+    this._setMetricInstruments();
+  }
+
+  private _setMetricInstruments() {
+    this._operationDuration = this.meter.createHistogram(
+      'db.client.operation.duration',
+      {
+        description: 'Duration of database client operations.',
+        unit: 's',
+        valueType: ValueType.DOUBLE,
+        advice: {
+          explicitBucketBoundaries: [
+            0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10,
+          ],
+        },
+      }
+    );
   }
 
   protected init() {
@@ -146,6 +190,29 @@
     };
   }
 
+  private recordOperationDuration(attributes: Attributes, startTime: HrTime) {
+    // Time to record metrics
+    const metricsAttributes: Attributes = {};
+    // Get the attribs already in span attributes
+    const keysToCopy = [
+      SEMATTRS_DB_SYSTEM,
+      SEMATTRS_DB_NAMESPACE,
+      SEMATTRS_ERROR_TYPE,
+      SEMATTRS_SERVER_PORT,
+      SEMATTRS_SERVER_ADDRESS,
+    ];
+
+    keysToCopy.forEach(key => {
+      if (key in attributes) {
+        metricsAttributes[key] = attributes[key];
+      }
+    });
+
+    const durationSeconds =
+      hrTimeToMilliseconds(hrTimeDuration(startTime, hrTime())) / 1000;
+    this._operationDuration.record(durationSeconds, metricsAttributes);
+  }
+
   private _getClientQueryPatch() {
     const plugin = this;
     return (original: typeof pgTypes.Client.prototype.query) => {
@@ -154,6 +221,7 @@
         if (utils.shouldSkipInstrumentation(plugin.getConfig())) {
           return original.apply(this, args as never);
         }
+        const startTime = hrTime();
 
         // client.query(text, cb?), client.query(text, values, cb?), and
         // client.query(configObj, cb?) are all valid signatures. We construct
@@ -179,6 +247,16 @@
           ? (arg0 as utils.ObjectWithText)
           : undefined;
 
+        const attributes: Attributes = {
+          [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_POSTGRESQL,
+          [SEMATTRS_DB_NAMESPACE]: this.database,
+          [SEMATTRS_SERVER_PORT]: this.connectionParameters.port,
+          [SEMATTRS_SERVER_ADDRESS]: this.connectionParameters.host,
+        };
+        this.on('end', () => {
+          plugin.recordOperationDuration(attributes, startTime);
+        });
+
         const instrumentationConfig = plugin.getConfig();
 
         const span = utils.handleConfigQuery.call(
@@ -209,7 +287,8 @@
             args[args.length - 1] = utils.patchCallback(
               instrumentationConfig,
               span,
-              args[args.length - 1] as PostgresCallback // nb: not type safe.
+              args[args.length - 1] as PostgresCallback, // nb: not type safe.
+              attributes
             );
 
             // If a parent span exists, bind the callback
@@ -224,7 +303,8 @@
             let callback = utils.patchCallback(
               plugin.getConfig(),
               span,
-              queryConfig.callback as PostgresCallback // nb: not type safe.
+              queryConfig.callback as PostgresCallback, // nb: not type safe.
+              attributes
             );
 
             // If a parent span existed, bind the callback
@@ -282,12 +362,12 @@
         try {
           result = original.apply(this, args as never);
         } catch (e: unknown) {
-          // span.recordException(e);
           span.setStatus({
             code: SpanStatusCode.ERROR,
             message: utils.getErrorMessage(e),
           });
           span.end();
+          attributes[SEMATTRS_ERROR_TYPE] = utils.getErrorName(e);
           throw e;
         }
 
@@ -309,6 +389,7 @@
                   message: error.message,
                 });
                 span.end();
+                attributes[SEMATTRS_ERROR_TYPE] = error.name;
                 reject(error);
               });
             });

plugins/node/opentelemetry-instrumentation-pg/src/utils.ts

@@ -22,6 +22,7 @@
   Tracer,
   SpanKind,
   diag,
+  Attributes,
 } from '@opentelemetry/api';
 import { AttributeNames } from './enums/AttributeNames';
 import {
@@ -236,15 +237,16 @@
 export function patchCallback(
   instrumentationConfig: PgInstrumentationConfig,
   span: Span,
-  cb: PostgresCallback
+  cb: PostgresCallback,
+  attributes: Attributes
 ): PostgresCallback {
   return function patchedCallback(
     this: PgClientExtended,
     err: Error,
     res: object
   ) {
     if (err) {
-      // span.recordException(err);
+      attributes['error.type'] = err.message;
       span.setStatus({
         code: SpanStatusCode.ERROR,
         message: err.message,
@@ -306,6 +308,17 @@
     : undefined;
 }
 
+/**
+ * Attempt to get a name string from a thrown value, while being quite
+ * defensive, to recognize the fact that, in JS, any kind of value (even
+ * primitives) can be thrown.
+ */
+export function getErrorName(e: unknown) {
+  return typeof e === 'object' && e !== null && 'name' in e
+    ? String((e as { name?: unknown }).name)
+    : undefined;
+}
+
 export function isObjectWithTextString(it: unknown): it is ObjectWithText {
   return (
     typeof it === 'object' &&