feat(instrumentation): add support for esm module

open-telemetry · Apr 18, 2022 · 68aa810 · 68aa810
1 parent ed2f033
commit 68aa810
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 23 deletions.
diff --git a/experimental/packages/opentelemetry-instrumentation/package.json b/experimental/packages/opentelemetry-instrumentation/package.json
@@ -69,6 +69,7 @@
   },
   "dependencies": {
     "@opentelemetry/api-metrics": "0.27.0",
+    "import-in-the-middle": "^1.2.0",
     "require-in-the-middle": "^5.0.3",
     "semver": "^7.3.2",
     "shimmer": "^1.2.1"

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -17,19 +17,26 @@
 import * as types from '../../types';
 import * as path from 'path';
 import * as RequireInTheMiddle from 'require-in-the-middle';
+import * as ImportInTheMiddle from 'import-in-the-middle';
 import { satisfies } from 'semver';
 import { InstrumentationAbstract } from '../../instrumentation';
 import { InstrumentationModuleDefinition } from './types';
 import { diag } from '@opentelemetry/api';
 
+/**
+ * We are forced to re-type there because ImportInTheMiddle is exported as normal CJS
+ * in the JS files but transpiled ESM (with a default export) in its typing.
+ */
+const ESMHook = ImportInTheMiddle as unknown as typeof ImportInTheMiddle.default;
+
 /**
  * Base abstract class for instrumenting node plugins
  */
 export abstract class InstrumentationBase<T = any>
   extends InstrumentationAbstract
   implements types.Instrumentation {
   private _modules: InstrumentationModuleDefinition<T>[];
-  private _hooks: RequireInTheMiddle.Hooked[] = [];
+  private _moduleHooks: { esm: ImportInTheMiddle.default, cjs: RequireInTheMiddle.Hooked }[] = [];
   private _enabled = false;
 
   constructor(
@@ -71,7 +78,7 @@ export abstract class InstrumentationBase<T = any>
     return undefined;
   }
 
-  private _onRequire<T>(
+  private _onRequireOrImport<T>(
     module: InstrumentationModuleDefinition<T>,
     exports: T,
     name: string,
@@ -120,7 +127,7 @@ export abstract class InstrumentationBase<T = any>
     this._enabled = true;
 
     // already hooked, just call patch again
-    if (this._hooks.length > 0) {
+    if (this._moduleHooks.length > 0) {
       for (const module of this._modules) {
         if (typeof module.patch === 'function' && module.moduleExports) {
           module.patch(module.moduleExports, module.moduleVersion);
@@ -135,22 +142,30 @@ export abstract class InstrumentationBase<T = any>
     }
 
     for (const module of this._modules) {
-      this._hooks.push(
-        RequireInTheMiddle(
-          [module.name],
-          { internals: true },
-          (exports, name, baseDir) => {
-            return this._onRequire<typeof exports>(
-              (module as unknown) as InstrumentationModuleDefinition<
-                typeof exports
-              >,
-              exports,
-              name,
-              baseDir
-            );
-          }
-        )
-      );
+      // Note that for some reasons we can't share the hook callback, it mess up
+      // the context in which onRequireOrImport is called
+      this._moduleHooks.push({
+        cjs: RequireInTheMiddle([module.name], { internals: true }, (exports, name, baseDir) => {
+          return this._onRequireOrImport<typeof exports>(
+            (module as unknown) as InstrumentationModuleDefinition<
+            typeof exports
+          >,
+            exports,
+            name,
+            baseDir
+          );
+        }),
+        esm: new ESMHook([module.name], { internals: true }, (exports, name, baseDir) => {
+          return this._onRequireOrImport<typeof exports>(
+            (module as unknown) as InstrumentationModuleDefinition<
+            typeof exports
+          >,
+            exports,
+            name,
+            typeof baseDir === 'string' ? baseDir : undefined
+          );
+        })
+      });
     }
   }
 

diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts
@@ -58,7 +58,7 @@ describe('InstrumentationBase', () => {
           } as InstrumentationModuleDefinition<unknown>;
 
           // @ts-expect-error access internal property for testing
-          instrumentation._onRequire<unknown>(
+          instrumentation._onRequireOrImport<unknown>(
             instrumentationModule,
             moduleExports,
             MODULE_NAME,
@@ -81,7 +81,7 @@ describe('InstrumentationBase', () => {
           } as InstrumentationModuleDefinition<unknown>;
 
           // @ts-expect-error access internal property for testing
-          instrumentation._onRequire<unknown>(
+          instrumentation._onRequireOrImport<unknown>(
             instrumentationModule,
             moduleExports,
             MODULE_NAME,
@@ -118,7 +118,7 @@ describe('InstrumentationBase', () => {
           } as InstrumentationModuleDefinition<unknown>;
 
           // @ts-expect-error access internal property for testing
-          instrumentation._onRequire<unknown>(
+          instrumentation._onRequireOrImport<unknown>(
             instrumentationModule,
             moduleExports,
             MODULE_FILE_NAME,
@@ -148,7 +148,7 @@ describe('InstrumentationBase', () => {
           } as InstrumentationModuleDefinition<unknown>;
 
           // @ts-expect-error access internal property for testing
-          instrumentation._onRequire<unknown>(
+          instrumentation._onRequireOrImport<unknown>(
             instrumentationModule,
             moduleExports,
             MODULE_FILE_NAME,