fix(instrumentation): use all provided filepatches

Fixes #2355
open-telemetry · May 25, 2022 · c0d9629 · c0d9629
1 parent bfd04b0
commit c0d9629
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 12 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -26,6 +26,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(opentelemetry-instrumentation-http): use correct origin when port is `null` #2948 @danielgblanco
 * fix(otlp-exporter-base): include esm and esnext in package files #2952 @dyladan
 * fix(otlp-http-exporter): update endpoint to match spec #2895 @svetlanabrennan
+* 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
@@ -114,18 +114,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;
-        if (this._enabled) {
-          return file.patch(exports, module.moduleVersion);
-        }
-      }
     }
-    return exports;
+    // internal file
+    const files = module.files ?? [];
+    const matchingFileInstrumentations = files.filter(f => f.name === name);
+    // will return unmodified `exports` if the matching list is empty
+    return matchingFileInstrumentations.reduce<T>(
+      (patchedExports, file) => {
+        if (isSupported(file.supportedVersions, version, module.includePrerelease)) {
+          file.moduleExports = patchedExports;
+          if (this._enabled) {
+            return file.patch(patchedExports, module.moduleVersion);
+          }
+        }
+        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
@@ -99,7 +99,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', () => {
@@ -161,6 +161,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);
+        });
+      });
     });
   });
 });