pg: cannot instrument an ESM-imported pg

[mjsalinger]

What version of OpenTelemetry are you using?

honeycomb/opentelemetry-node 0.5.0
opentelemetry/auto-instrumentations-node 0.39.2
opentelemetry/api 1.4.1

What version of Node are you using?

16.20.2

What did you do?

Started telemetry with auto-instrumentation enabled
Started node with

node --experimental-loader=@opentelemetry/instrumentation/hook.mjs ./src/index.js",

What did you expect to see?

Node start and instrumentation produced for pg module

What did you see instead?

/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/@opentelemetry/instrumentation-pg/build/src/instrumentation.js:33
            if ((0, instrumentation_1.isWrapped)(moduleExports.Client.prototype.query)) {
                                                                      ^

TypeError: Cannot read properties of undefined (reading 'prototype')
    at InstrumentationNodeModuleDefinition.patch (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/@opentelemetry/instrumentation-pg/build/src/instrumentation.js:33:71)
    at PgInstrumentation._onRequire (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/@opentelemetry/instrumentation/build/src/platform/node/instrumentation.js:153:39)
    at hookFn (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/@opentelemetry/instrumentation/build/src/platform/node/instrumentation.js:194:29)
    at callHookFn (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/import-in-the-middle/index.js:28:22)
    at Hook._iitmHook (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/import-in-the-middle/index.js:76:11)
    at /Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/import-in-the-middle/lib/register.js:29:31
    at Array.forEach (<anonymous>)
    at register (/Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/import-in-the-middle/lib/register.js:29:15)
    at file:///Users/mjsalinger/Development/platform/api/backend-api-service/node_modules/pg/lib/index.js?iitm=true:13:1
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)

Additional context

It appears that the pg instrumentation may need to be updated to properly handle ESM imports. Seems possibly similar to #1692

[mjsalinger]

Is there anyone who can take a look at this?

[Flarna]

Looks very similar as #1692

There is already #1694 to fix this for ioredis.
Maybe you could try the same approach for pg?

[trentm]

I can reproduce with this:

% cd ~/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg

% cat example.mjs
import {PgInstrumentation} from "./build/src/index.js";
new PgInstrumentation()
import 'pg'
setTimeout(() => 0, 1000)

% node --experimental-loader=@opentelemetry/instrumentation/hook.mjs example.mjs
(node:74985) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/build/src/instrumentation.js:33
            if ((0, instrumentation_1.isWrapped)(moduleExports.Client.prototype.query)) {
                                                                      ^

TypeError: Cannot read properties of undefined (reading 'prototype')
    at InstrumentationNodeModuleDefinition.patch (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/build/src/instrumentation.js:33:71)
    at PgInstrumentation._onRequire (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/@opentelemetry/instrumentation/build/src/platform/node/instrumentation.js:153:39)
    at hookFn (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/@opentelemetry/instrumentation/build/src/platform/node/instrumentation.js:194:29)
    at callHookFn (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/import-in-the-middle/index.js:28:22)
    at Hook._iitmHook (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/import-in-the-middle/index.js:76:11)
    at /Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/import-in-the-middle/index.js:17:41
    at Array.forEach (<anonymous>)
    at addHook (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/import-in-the-middle/index.js:17:10)
    at new Hook (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/import-in-the-middle/index.js:84:3)
    at PgInstrumentation.enable (/Users/trentm/tm/opentelemetry-js-contrib2/plugins/node/opentelemetry-instrumentation-pg/node_modules/@opentelemetry/instrumentation/build/src/platform/node/instrumentation.js:206:29)

A starter fix is:

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
index f47b1e81..b42da2b8 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -66,7 +66,12 @@ export class PgInstrumentation extends InstrumentationBase {
     const modulePG = new InstrumentationNodeModuleDefinition<typeof pgTypes>(
       'pg',
       ['8.*'],
-      moduleExports => {
+      // XXX type any, do better
+      (module: any) => {
+        const moduleExports: typeof pgTypes =
+          module[Symbol.toStringTag] === 'Module'
+            ? module.default // ESM
+            : module; // CommonJS
         if (isWrapped(moduleExports.Client.prototype.query)) {
           this._unwrap(moduleExports.Client.prototype, 'query');
         }
@@ -87,9 +92,14 @@ export class PgInstrumentation extends InstrumentationBase {
           this._getClientConnectPatch() as any
         );

-        return moduleExports;
+        return module;
       },
-      moduleExports => {
+      // XXX type any, do better
+      (module: any) => {
+        const moduleExports: typeof pgTypes =
+          module[Symbol.toStringTag] === 'Module'
+            ? module.default // ESM
+            : module; // CommonJS
         if (isWrapped(moduleExports.Client.prototype.query)) {
           this._unwrap(moduleExports.Client.prototype, 'query');
         }

I have some TypeScript fighting to work through and, as with #1694, a decision to make on testing. I'll start a PR.