From 81e3f7cebf1e0d7db12845f3c4243b163ff0250f Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Sun, 29 Dec 2019 21:48:10 +0100 Subject: [PATCH] fix: do not load plugin when they patch a different module than defined in config #626 --- .../src/trace/instrumentation/BasePlugin.ts | 2 +- .../src/instrumentation/PluginLoader.ts | 10 +++++++++- .../test/instrumentation/PluginLoader.test.ts | 17 +++++++++++++++++ .../plugin-http-module/http-module.js | 1 + .../plugin-notsupported-module/simple-module.js | 1 + .../plugin-simple-module/simple-module.js | 1 + .../plugin-supported-module/simple-module.js | 1 + .../node_modules/random-module/index.js | 4 ++++ .../node_modules/random-module/package.json | 4 ++++ .../src/trace/instrumentation/Plugin.ts | 5 +++++ .../opentelemetry-web/test/WebTracer.test.ts | 1 + 11 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 packages/opentelemetry-node/test/instrumentation/node_modules/random-module/index.js create mode 100644 packages/opentelemetry-node/test/instrumentation/node_modules/random-module/package.json diff --git a/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts b/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts index 04ccc60b26c..f25beb2ba77 100644 --- a/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts +++ b/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts @@ -29,7 +29,7 @@ import * as path from 'path'; /** This class represent the base to patch plugin. */ export abstract class BasePlugin implements Plugin { supportedVersions?: string[]; - readonly moduleName?: string; // required for internalFilesExports + abstract readonly moduleName: string; // required for internalFilesExports readonly version?: string; // required for internalFilesExports protected readonly _basedir?: string; // required for internalFilesExports diff --git a/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts b/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts index 99d2abf487d..1541b303af1 100644 --- a/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts +++ b/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts @@ -113,8 +113,16 @@ export class PluginLoader { // Expecting a plugin from module; try { const plugin: Plugin = require(modulePath).plugin; - if (!utils.isSupportedVersion(version, plugin.supportedVersions)) { + this.logger.error( + `PluginLoader#load: Plugin ${name} only support module ${plugin.moduleName} with the versions: ${plugin.supportedVersions}` + ); + return exports; + } + if (plugin.moduleName !== name) { + this.logger.error( + `PluginLoader#load: Entry ${name} use a plugin that instrument ${plugin.moduleName}` + ); return exports; } diff --git a/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts b/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts index 93440eb2942..376c7da383a 100644 --- a/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts +++ b/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts @@ -85,6 +85,13 @@ const notSupportedVersionPlugins: Plugins = { }, }; +const differentNamePlugins: Plugins = { + 'random-module': { + enabled: true, + path: '@opentelemetry/plugin-http-module', + }, +}; + describe('PluginLoader', () => { const registry = new NoopTracerRegistry(); const logger = new NoopLogger(); @@ -218,6 +225,16 @@ describe('PluginLoader', () => { assert.strictEqual(require('simple-module').value(), 0); pluginLoader.unload(); }); + + it('should not load a plugin that patch a different module that the one configured', () => { + const pluginLoader = new PluginLoader(registry, logger); + assert.strictEqual(pluginLoader['_plugins'].length, 0); + pluginLoader.load(differentNamePlugins); + // @ts-ignore only to trigger the loading of the plugin + const randomModule = require('random-module'); + assert.strictEqual(pluginLoader['_plugins'].length, 0); + pluginLoader.unload(); + }); }); describe('.unload()', () => { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-http-module/http-module.js b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-http-module/http-module.js index 06f69e50f63..2c46ac30ee2 100644 --- a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-http-module/http-module.js +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-http-module/http-module.js @@ -5,6 +5,7 @@ const shimmer = require("shimmer"); class HttpModulePlugin extends core_1.BasePlugin { constructor() { super(); + this.moduleName = 'http'; } patch() { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-notsupported-module/simple-module.js b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-notsupported-module/simple-module.js index 86a27bc32d9..92bb7d4e807 100644 --- a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-notsupported-module/simple-module.js +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-notsupported-module/simple-module.js @@ -5,6 +5,7 @@ const shimmer = require("shimmer"); class SimpleModulePlugin extends core_1.BasePlugin { constructor() { super(); + this.moduleName = 'notsupported-module'; } patch() { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-simple-module/simple-module.js b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-simple-module/simple-module.js index 142b861d222..67e8f838d46 100644 --- a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-simple-module/simple-module.js +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-simple-module/simple-module.js @@ -5,6 +5,7 @@ const shimmer = require("shimmer"); class SimpleModulePlugin extends core_1.BasePlugin { constructor() { super(); + this.moduleName = 'simple-module'; } patch() { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-supported-module/simple-module.js b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-supported-module/simple-module.js index fb2a965e7f3..6395bd261af 100644 --- a/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-supported-module/simple-module.js +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/@opentelemetry/plugin-supported-module/simple-module.js @@ -5,6 +5,7 @@ const shimmer = require("shimmer"); class SimpleModulePlugin extends core_1.BasePlugin { constructor() { super(); + this.moduleName = 'supported-module'; } patch() { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/index.js b/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/index.js new file mode 100644 index 00000000000..35a4110c28e --- /dev/null +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/index.js @@ -0,0 +1,4 @@ +module.exports = { + name: () => 'random-module', + value: () => 0, +}; diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/package.json b/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/package.json new file mode 100644 index 00000000000..a5c840081be --- /dev/null +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/random-module/package.json @@ -0,0 +1,4 @@ +{ + "name": "random-module", + "version": "0.1.0" +} diff --git a/packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts b/packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts index 3462a5df097..9fce0adc706 100644 --- a/packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts +++ b/packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts @@ -28,6 +28,11 @@ export interface Plugin { */ supportedVersions?: string[]; + /** + * Name of the module that the plugin instrument. + */ + moduleName: string; + /** * Method that enables the instrumentation patch. * @param moduleExports The value of the `module.exports` property that would diff --git a/packages/opentelemetry-web/test/WebTracer.test.ts b/packages/opentelemetry-web/test/WebTracer.test.ts index 140571405c8..c5ecb6a6e99 100644 --- a/packages/opentelemetry-web/test/WebTracer.test.ts +++ b/packages/opentelemetry-web/test/WebTracer.test.ts @@ -27,6 +27,7 @@ class DummyPlugin extends BasePlugin { constructor() { super('dummy'); } + patch() {} unpatch() {} }