fix: pluginLoader for core modules (open-telemetry#312)

closes open-telemetry#311

Signed-off-by: Olivier Albertini <[email protected]>

Author: OlivierAlbertini

packages/opentelemetry-node-sdk/package.json

@@ -48,7 +48,7 @@
     "nyc": "^14.1.1",
     "shimmer": "^1.2.0",
     "tslint-microsoft-contrib": "^6.2.0",
-    "tslint-consistent-codestyle":"^1.15.1",
+    "tslint-consistent-codestyle": "^1.15.1",
     "ts-mocha": "^6.0.0",
     "ts-node": "^8.0.0",
     "typescript": "^3.4.5"

packages/opentelemetry-node-sdk/src/instrumentation/PluginLoader.ts

@@ -81,13 +81,22 @@ export class PluginLoader {
       // Enable the require hook.
       hook(modulesToHook, (exports, name, baseDir) => {
         if (this._hookState !== HookState.ENABLED) return exports;
-
         const config = pluginsToLoad[name];
         const modulePath = config.path!;
-        // Get the module version.
-        const version = utils.getPackageVersion(this.logger, baseDir as string);
+        let version = null;
+
+        if (!baseDir) {
+          // basedir is the directory where the module is located,
+          // or undefined for core modules.
+          // lets plugins restrict what they support for core modules (see plugin.supportedVersions)
+          version = process.versions.node;
+        } else {
+          // Get the module version.
+          version = utils.getPackageVersion(this.logger, baseDir);
+        }
+
         this.logger.info(
-          `PluginLoader#load: trying loading ${name}.${version}`
+          `PluginLoader#load: trying loading ${name}@${version}`
         );
 
         if (!version) return exports;

packages/opentelemetry-node-sdk/src/instrumentation/utils.ts

@@ -21,14 +21,12 @@ import * as semver from 'semver';
 /**
  * Gets the package version.
  * @param logger The logger to use.
- * @param [basedir] The base directory.
+ * @param basedir The base directory.
  */
 export function getPackageVersion(
   logger: Logger,
-  basedir?: string
+  basedir: string
 ): string | null {
-  if (!basedir) return null;
-
   const pjsonPath = path.join(basedir, 'package.json');
   try {
     const version = require(pjsonPath).version;

packages/opentelemetry-node-sdk/test/instrumentation/PluginLoader.test.ts

@@ -35,6 +35,15 @@ const simplePlugins: Plugins = {
   },
 };
 
+const httpPlugins: Plugins = {
+  http: {
+    enabled: true,
+    path: '@opentelemetry/plugin-http-module',
+    ignoreMethods: [],
+    ignoreUrls: [],
+  },
+};
+
 const disablePlugins: Plugins = {
   'simple-module': {
     enabled: false,
@@ -139,6 +148,17 @@ describe('PluginLoader', () => {
       assert.strictEqual(simpleModule.name(), 'patched-simple-module');
       pluginLoader.unload();
     });
+
+    it('should load a plugin and patch the core module', () => {
+      const pluginLoader = new PluginLoader(tracer, logger);
+      assert.strictEqual(pluginLoader['_plugins'].length, 0);
+      pluginLoader.load(httpPlugins);
+      // The hook is only called the first time the module is loaded.
+      const httpModule = require('http');
+      assert.strictEqual(pluginLoader['_plugins'].length, 1);
+      assert.strictEqual(httpModule.get(), 'patched');
+      pluginLoader.unload();
+    });
     // @TODO: simplify this test once we can load module with custom path
     it('should not load the plugin when supported versions does not match', () => {
       const pluginLoader = new PluginLoader(tracer, logger);

packages/opentelemetry-node-sdk/test/instrumentation/node_modules/@opentelemetry/plugin-http-module/http-module.js

@@ -43,10 +43,6 @@ describe('Instrumentation#utils', () => {
   });
 
   describe('getPackageVersion', () => {
-    it('should handle when undefined basedir', () => {
-      assert.strictEqual(utils.getPackageVersion(logger), null);
-    });
-
     TEST_MODULES.forEach(testCase => {
       it(`should return ${testCase.version} for ${testCase.name}`, () => {
         assert.strictEqual(