instrumentation-pg requestHook isn't called for connection events

[drob]

What version of OpenTelemetry are you using?

@opentelemetry/api 1.6.0 @opentelemetry/instrumentation-pg 0.36.1

What version of Node are you using?

18.18.0

What did you do?

You can reproduce this with a simple instrumentation-pg implementation.

const pgInstrumentation = new PgInstrumentation({
  requestHook: (span: Span, requestInfo: PgRequestHookInformation) => {
    // This will appear on query spans, but not connection spans.
    span.setAttribute('library', 'pg');
  },
});

...

registerInstrumentations({
  tracerProvider: provider,
  instrumentations: [
    pgInstrumentation,
  ],
});

What did you expect to see?

requestHook to be called for all pg spans

What did you see instead?

requestHook is called for pg queries but not connection events

[drob]

I'd be happy to send a PR for this. Just let me know how you'd want it to work? The request hook for connection events needs to have different args, since the usual request hook knows the query that the client is running, whereas for connection events there isn't a query.

So we could either make the query part optional, like this:

  export interface PgRequestHookInformation {
-   query: {
+   query?: {
      text: string;
      name?: string;
      values?: unknown[];
    };
    connection: {
      database?: string;
      host?: string;
      port?: number;
      user?: string;
    };
  }

Or, we could create a separate type and extend the instrumentation config, like this:

  export interface PgInstrumentationConfig extends InstrumentationConfig {
    ...
+   /**
+    * Hook that allows adding custom span attributes based on the data about a
+    * connection request.
+    *
+    * @default undefined
+    */
+   connectionRequestHook?: PgInstrumentationConnectRequestHook;
    ...
  }

+ export interface PgConnectRequestHookInformation {
+   connection: {
+     database?: string;
+     host?: string;
+     port?: number;
+     user?: string;
+   };
+ }
+
+ export interface PgInstrumentationConnectRequestHook {
+   (span: api.Span, connectionInfo: PgConnectRequestHookInformation): void;
+ }

The latter preserves backcompat and is probably more in line with what a typical user would want, so I'd lean towards that, but I'm happy to implement either.

[dyladan]

I think this isn't a bug. The connection was never meant to be called by the hook. Reclassified as an enhancement request.

[drob]

Makes sense. As I mentioned above I'm happy to send a PR for this if you'd be open to it. Just let me know which interface you'd prefer.