Skip to content

Commit

Permalink
fix: do not load plugin when they patch a different module than defin…
Browse files Browse the repository at this point in the history
…ed in config #626
  • Loading branch information
vmarchaud committed Jan 3, 2020
1 parent 6c98ab9 commit 7dd415c
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import * as path from 'path';
/** This class represent the base to patch plugin. */
export abstract class BasePlugin<T> implements Plugin<T> {
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

Expand Down
10 changes: 9 additions & 1 deletion packages/opentelemetry-node/src/instrumentation/PluginLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ const notSupportedVersionPlugins: Plugins = {
},
};

const differentNamePlugins: Plugins = {
'random-module': {
enabled: true,
path: '@opentelemetry/plugin-http-module',
},
};

describe('PluginLoader', () => {
const tracer = new NoopTracer();
const logger = new NoopLogger();
Expand Down Expand Up @@ -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(tracer, 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()', () => {
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.

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.

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.

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export interface Plugin<T = any> {
*/
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
Expand Down
2 changes: 2 additions & 0 deletions packages/opentelemetry-web/test/WebTracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { WebTracer } from '../src/WebTracer';
import { ZoneScopeManager } from '@opentelemetry/scope-zone';

class DummyPlugin extends BasePlugin<unknown> {
moduleName = 'dummy';

patch() {}

unpatch() {}
Expand Down

0 comments on commit 7dd415c

Please sign in to comment.