fix(postgres): pass 'arguments' to the connect callback (open-telemet…

…ry#1395) * fix(postgres): make the connect callback patched function to return the client object * fix(postgres): lint * fix(postgres): remove PgErrorCallback * fix(postgres): remove PgErrorCallback * fix(postgres): lint * fix(postgres): remove PgErrorCallback from connect() too * fix(postgres): chenge declaration of PgClientConnect --------- Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
dynatrace-oss-contrib · Feb 16, 2023 · b02775f · b02775f
1 parent 61255e2
commit b02775f
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 16 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -33,7 +33,6 @@ import type * as pgPoolTypes from 'pg-pool';
 import {
   PgClientConnect,
   PgClientExtended,
-  PgErrorCallback,
   PostgresCallback,
   PgPoolExtended,
   PgPoolCallback,
@@ -133,10 +132,7 @@ export class PgInstrumentation extends InstrumentationBase {
   private _getClientConnectPatch() {
     const plugin = this;
     return (original: PgClientConnect) => {
-      return function connect(
-        this: pgTypes.Client,
-        callback?: PgErrorCallback
-      ) {
+      return function connect(this: pgTypes.Client, callback?: Function) {
         if (utils.shouldSkipInstrumentation(plugin.getConfig())) {
           return original.call(this, callback);
         }

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-pg/src/internal-types.ts
@@ -46,8 +46,6 @@ export type PgPoolCallback = (
   done: (release?: any) => void
 ) => void;
 
-export type PgErrorCallback = (err: Error) => void;
-
 export interface PgPoolOptionsParams {
   database: string;
   host: string;
@@ -61,6 +59,4 @@ export interface PgPoolExtended extends pgPoolTypes<pgTypes.Client> {
   options: PgPoolOptionsParams;
 }
 
-export type PgClientConnect = (
-  callback?: (err: Error) => void
-) => Promise<void> | void;
+export type PgClientConnect = (callback?: Function) => Promise<void> | void;
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -34,7 +34,6 @@ import {
 import {
   PgClientExtended,
   PostgresCallback,
-  PgErrorCallback,
   PgPoolCallback,
   PgPoolExtended,
   PgParsedConnectionParams,
@@ -239,10 +238,7 @@ export function patchCallbackPGPool(
   };
 }
 
-export function patchClientConnectCallback(
-  span: Span,
-  cb: PgErrorCallback
-): PgErrorCallback {
+export function patchClientConnectCallback(span: Span, cb: Function): Function {
   return function patchedClientConnectCallback(
     this: pgTypes.Client,
     err: Error
@@ -254,7 +250,7 @@ export function patchClientConnectCallback(
       });
     }
     span.end();
-    cb.call(this, err);
+    cb.apply(this, arguments);
   };
 }
 

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -247,6 +247,17 @@ describe('pg', () => {
       assert.strictEqual(res, undefined, 'No promise is returned');
     });
 
+    it('should pass the client connection object in the callback function', done => {
+      connClient.connect(function (err: Error) {
+        // Even though the documented signature for connect() callback is `(err) => void`
+        // `pg` actually also passes the client if the connection was successful and some
+        // packages(`knex`) might rely on that
+        // https://github.com/brianc/node-postgres/blob/master/packages/pg/lib/client.js#L282
+        assert.strictEqual(arguments[1], connClient);
+        done();
+      });
+    });
+
     it('should return a promise if callback is not provided', done => {
       const resPromise = connClient.connect();
       resPromise