From b6a2ccdf29383d1d9402d15df22cb5413d768cba Mon Sep 17 00:00:00 2001
From: Valentin Marchaud <contact@vmarchaud.fr>
Date: Mon, 10 Feb 2020 18:43:43 +0100
Subject: [PATCH] feat: warn user when a instrumented package was already
 required #636 (#654)

* feat: warn user when a instrumented package was already required #636

* chore: address PR comments

* chore: extract package name from require.cache

* chore: use find instead of some

* chore: use require.resolve to find already required modules

* chore: try/catch require.resolve

Co-authored-by: Mayur Kale <mayurkale@google.com>
---
 .../src/instrumentation/PluginLoader.ts       | 19 +++++++++++++++
 .../test/instrumentation/PluginLoader.test.ts | 24 +++++++++++++++++++
 .../already-require-module/index.js           |  4 ++++
 .../already-require-module/package.json       |  4 ++++
 4 files changed, 51 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 d9d271acfc9..e5fb3a83a79 100644
--- a/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts
+++ b/packages/opentelemetry-node/src/instrumentation/PluginLoader.ts
@@ -83,6 +83,25 @@ export class PluginLoader {
         return this;
       }
 
+      const alreadyRequiredModules = Object.keys(require.cache);
+      const requiredModulesToHook = modulesToHook.filter(
+        name =>
+          alreadyRequiredModules.find(cached => {
+            try {
+              return require.resolve(name) === cached;
+            } catch (err) {
+              return false;
+            }
+          }) !== undefined
+      );
+      if (requiredModulesToHook.length > 0) {
+        this.logger.warn(
+          `Some modules (${requiredModulesToHook.join(
+            ', '
+          )}) were already required when their respective plugin was 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 ad4edb7863a..c762a5c2873 100644
--- a/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts
+++ b/packages/opentelemetry-node/test/instrumentation/PluginLoader.test.ts
@@ -86,6 +86,13 @@ const notSupportedVersionPlugins: Plugins = {
   },
 };
 
+const alreadyRequiredPlugins: Plugins = {
+  'already-require-module': {
+    enabled: true,
+    path: '@opentelemetry/plugin-supported-module',
+  },
+};
+
 describe('PluginLoader', () => {
   const provider = new NoopTracerProvider();
   const logger = new NoopLogger();
@@ -219,6 +226,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(provider, 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"
+}