fix(instrumentation): only patch core modules if enabled

Otherwise, disabling the instrumentation module before actually requiring the to-be-instrumented module doesn't work, and the module is patched.
open-telemetry · May 26, 2022 · 68e1caa · 68e1caa
1 parent bfd04b0
commit 68e1caa
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 1 deletion.
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(instrumentation): only patch core modules if enabled #2993 @santigimeno
 
 ### :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
@@ -95,7 +95,9 @@ export abstract class InstrumentationBase<T = any>
     if (!baseDir) {
       if (typeof module.patch === 'function') {
         module.moduleExports = exports;
-        return module.patch(exports);
+        if (this._enabled) {
+          return module.patch(exports);
+        }
       }
       return exports;
     }

diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts
@@ -23,6 +23,7 @@ const MODULE_FILE_NAME = 'test-module-file';
 const MODULE_VERSION = '0.1.0';
 const WILDCARD_VERSION = '*';
 const MODULE_DIR = '/random/dir';
+const CORE_MODULE = 'random_core';
 
 class TestInstrumentation extends InstrumentationBase {
   constructor() {
@@ -33,6 +34,61 @@ class TestInstrumentation extends InstrumentationBase {
 }
 
 describe('InstrumentationBase', () => {
+  describe('_onRequire - core module', () => {
+    let instrumentation: TestInstrumentation;
+    let modulePatchSpy: sinon.SinonSpy;
+    beforeEach(() => {
+      instrumentation = new TestInstrumentation();
+      modulePatchSpy = sinon.spy();
+    });
+
+    describe('AND module is not enabled', () => {
+      it('should not patch the module', () => {
+        // @ts-expect-error access internal property for testing
+        instrumentation._enabled = false;
+        const moduleExports = {};
+        const instrumentationModule = {
+          name: CORE_MODULE,
+          patch: modulePatchSpy as unknown,
+        } as InstrumentationModuleDefinition<unknown>;
+
+        // @ts-expect-error access internal property for testing
+        instrumentation._onRequire<unknown>(
+          instrumentationModule,
+          moduleExports,
+          CORE_MODULE,
+          undefined
+        );
+
+        assert.strictEqual(instrumentationModule.moduleExports, moduleExports);
+        sinon.assert.notCalled(modulePatchSpy);
+      });
+    });
+
+    describe('AND module is enabled', () => {
+      it('should patch the module', () => {
+        // @ts-expect-error access internal property for testing
+        instrumentation._enabled = true;
+        const moduleExports = {};
+        const instrumentationModule = {
+          name: CORE_MODULE,
+          patch: modulePatchSpy as unknown,
+        } as InstrumentationModuleDefinition<unknown>;
+
+        // @ts-expect-error access internal property for testing
+        instrumentation._onRequire<unknown>(
+          instrumentationModule,
+          moduleExports,
+          CORE_MODULE,
+          undefined
+        );
+
+        assert.strictEqual(instrumentationModule.moduleExports, moduleExports);
+        sinon.assert.calledOnceWithExactly(modulePatchSpy, moduleExports);
+      });
+    });
+  });
+
   describe('_onRequire - module version is not available', () => {
     // For all of these cases, there is no indication of the actual module version,
     // so we require there to be a wildcard supported version.