Skip to content

Commit

Permalink
feat: warn user when a instrumented package was already required open…
Browse files Browse the repository at this point in the history
…-telemetry#636 (open-telemetry#654)

* feat: warn user when a instrumented package was already required open-telemetry#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>
  • Loading branch information
vmarchaud and mayurkale22 authored Feb 10, 2020
1 parent 82723ef commit b6a2ccd
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 0 deletions.
19 changes: 19 additions & 0 deletions packages/opentelemetry-node/src/instrumentation/PluginLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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()', () => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b6a2ccd

Please sign in to comment.