fix(instrumentation): use all provided filepatches (open-telemetry#2963)

Co-authored-by: Valentin Marchaud <[email protected]> Co-authored-by: Daniel Dyla <[email protected]>
weyert · Jun 22, 2022 · b891509 · b891509
1 parent cb642d7
commit b891509
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 11 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -63,6 +63,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(instrumentation): only patch core modules if enabled #2993 @santigimeno
 * fix(otlp-transformer): include esm and esnext in package files and update README #2992 @pichlermarc
 * fix(metrics): specification compliant default metric unit #2983 @andyfleming
+* fix(opentelemetry-instrumentation): use all provided patches for the same file [#2963](https://github.com/open-telemetry/opentelemetry-js/pull/2963) @Ugzuzg
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
@@ -116,18 +116,23 @@ export abstract class InstrumentationBase<T = any>
           }
         }
       }
-    } else {
-      // internal file
-      const files = module.files ?? [];
-      const file = files.find(f => f.name === name);
-      if (file && isSupported(file.supportedVersions, version, module.includePrerelease)) {
-        file.moduleExports = exports;
+      return exports;
+    }
+    // internal file
+    const files = module.files ?? [];
+    const supportedFileInstrumentations = files
+      .filter(f => f.name === name)
+      .filter(f => isSupported(f.supportedVersions, version, module.includePrerelease));
+    return supportedFileInstrumentations.reduce<T>(
+      (patchedExports, file) => {
+        file.moduleExports = patchedExports;
         if (this._enabled) {
-          return file.patch(exports, module.moduleVersion);
+          return file.patch(patchedExports, module.moduleVersion);
         }
-      }
-    }
-    return exports;
+        return patchedExports;
+      },
+      exports,
+    );
   }
 
   public enable(): void {

diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts
@@ -155,7 +155,7 @@ describe('InstrumentationBase', () => {
       let filePatchSpy: sinon.SinonSpy;
 
       beforeEach(() => {
-        filePatchSpy = sinon.spy();
+        filePatchSpy = sinon.stub().callsFake(exports => exports);
       });
 
       describe('AND there is no wildcard supported version', () => {
@@ -217,6 +217,41 @@ describe('InstrumentationBase', () => {
           sinon.assert.calledOnceWithExactly(filePatchSpy, moduleExports, undefined);
         });
       });
+
+      describe('AND there is multiple patches for the same file', () => {
+        it('should patch the same file twice', () => {
+          const moduleExports = {};
+          const supportedVersions = [`^${MODULE_VERSION}`, WILDCARD_VERSION];
+          const instrumentationModule = {
+            supportedVersions,
+            name: MODULE_NAME,
+            patch: modulePatchSpy as unknown,
+            files: [{
+              name: MODULE_FILE_NAME,
+              supportedVersions,
+              patch: filePatchSpy as unknown
+            }, {
+              name: MODULE_FILE_NAME,
+              supportedVersions,
+              patch: filePatchSpy as unknown
+            }]
+          } as InstrumentationModuleDefinition<unknown>;
+
+          // @ts-expect-error access internal property for testing
+          instrumentation._onRequire<unknown>(
+            instrumentationModule,
+            moduleExports,
+            MODULE_FILE_NAME,
+            MODULE_DIR
+          );
+
+          assert.strictEqual(instrumentationModule.moduleVersion, undefined);
+          assert.strictEqual(instrumentationModule.files[0].moduleExports, moduleExports);
+          assert.strictEqual(instrumentationModule.files[1].moduleExports, moduleExports);
+          sinon.assert.notCalled(modulePatchSpy);
+          sinon.assert.calledTwice(filePatchSpy);
+        });
+      });
     });
   });
 });