From 6e4a2b44f40c2c290b6888141a66bee46e215d58 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Sun, 29 Dec 2019 22:12:51 +0100 Subject: [PATCH] feat: warn user when a instrumented package was already required #636 --- .../src/instrumentation/PluginLoader.ts | 10 ++++++++ .../test/instrumentation/PluginLoader.test.ts | 24 +++++++++++++++++++ .../already-require-module/index.js | 4 ++++ .../already-require-module/package.json | 4 ++++ 4 files changed, 42 insertions(+) create mode 100644 packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/index.js create mode 100644 packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/package.json diff --git a/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts b/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts index 99d2abf487d..29c5e34fccf 100644 --- a/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts +++ b/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts @@ -83,6 +83,16 @@ export class PluginLoader { return this; } + const alreadyRequiredModules = Object.keys(require.cache); + const requiredModulesToHook = modulesToHook.filter(name => + alreadyRequiredModules.some(cached => cached.includes(name)) + ); + if (requiredModulesToHook.length > 0) { + this.logger.warn( + `Some modules (${requiredModulesToHook}) were already required when their respective plugin were loaded, some plugins might not work. Make sure the SDK is setup before you require in other modules.` + ); + } + // Enable the require hook. hook(modulesToHook, (exports, name, baseDir) => { if (this._hookState !== HookState.ENABLED) return exports; diff --git a/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts b/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts index 93440eb2942..8103955f85c 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 alreadyRequiredPlugins: Plugins = { + 'already-require-module': { + enabled: true, + path: '@opentelemetry/plugin-supported-module', + }, +}; + describe('PluginLoader', () => { const registry = new NoopTracerRegistry(); const logger = new NoopLogger(); @@ -218,6 +225,23 @@ describe('PluginLoader', () => { assert.strictEqual(require('simple-module').value(), 0); pluginLoader.unload(); }); + + it(`should warn when module was already loaded`, callback => { + const verifyWarnLogger = { + error: logger.error, + info: logger.info, + debug: logger.debug, + warn: (message: string, ...args: unknown[]) => { + assert(message.match(/were already required when/)); + assert(message.match(/(already-require-module)/)); + return callback(); + }, + }; + require('already-require-module'); + const pluginLoader = new PluginLoader(tracer, verifyWarnLogger); + pluginLoader.load(alreadyRequiredPlugins); + pluginLoader.unload(); + }); }); describe('.unload()', () => { diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/index.js b/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/index.js new file mode 100644 index 00000000000..18c0f69a3b9 --- /dev/null +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/index.js @@ -0,0 +1,4 @@ +module.exports = { + name: () => 'already-module', + value: () => 0, +}; diff --git a/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/package.json b/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/package.json new file mode 100644 index 00000000000..7ae0ab8f096 --- /dev/null +++ b/packages/opentelemetry-node/test/instrumentation/node_modules/already-require-module/package.json @@ -0,0 +1,4 @@ +{ + "name": "already-module", + "version": "0.1.0" +}