Skip to content

Commit

Permalink
feat: add plugin config options (open-telemetry#229)
Browse files Browse the repository at this point in the history 
* feat: add plugin config options

* fix: resolve conflicts

* fix: review comments
  • Loading branch information
@mayurkale22
mayurkale22 authored Sep 10, 2019
1 parent ea28887 commit 8279133
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 35 deletions.
6 changes: 4 additions & 2 deletions packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,26 @@
* limitations under the License.
*/

import { Tracer, Plugin, Logger } from '@opentelemetry/types';
import { Tracer, Plugin, Logger, PluginConfig } from '@opentelemetry/types';

/** This class represent the base to patch plugin. */
export abstract class BasePlugin<T> implements Plugin<T> {
protected _moduleExports!: T;
protected _tracer!: Tracer;
protected _logger!: Logger;
protected _config!: PluginConfig;
supportedVersions?: string[];

enable(
moduleExports: T,
tracer: Tracer,
logger: Logger,
config?: { [key: string]: unknown }
config?: PluginConfig
): T {
this._moduleExports = moduleExports;
this._tracer = tracer;
this._logger = logger;
if (config) this._config = config;
return this.patch();
}

Expand Down
48 changes: 24 additions & 24 deletions packages/opentelemetry-node-tracer/src/instrumentation/PluginLoader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Logger, Plugin, Tracer } from '@opentelemetry/types';
import { Logger, Plugin, Tracer, PluginConfig } from '@opentelemetry/types';
import * as hook from 'require-in-the-middle';
import * as utils from './utils';

Expand All @@ -25,13 +25,20 @@ export enum HookState {
DISABLED,
}

interface PluginNames {
[pluginName: string]: string;
export interface Plugins {
[pluginName: string]: PluginConfig;
}

interface PluginConfig {
// TODO: Consider to add configuration options
[pluginName: string]: boolean;
/**
* Returns the Plugins object that meet the below conditions.
* Valid criterias: 1. It should be enabled. 2. Should have non-empty path.
*/
function filterPlugins(plugins: Plugins): Plugins {
const keys = Object.keys(plugins);
return keys.reduce((acc: Plugins, key: string) => {
if (plugins[key].enabled && plugins[key].path) acc[key] = plugins[key];
return acc;
}, {});
}

/**
Expand All @@ -55,21 +62,13 @@ export class PluginLoader {
* {@link Plugin} interface and export an instance named as 'plugin'. This
* function will attach a hook to be called the first time the module is
* loaded.
* @param pluginConfig an object whose keys are plugin names and whose
* boolean values indicate whether to enable the plugin.
* @param Plugins an object whose keys are plugin names and whose
* {@link PluginConfig} values indicate several configuration options.
*/
load(pluginConfig: PluginConfig): PluginLoader {
load(plugins: Plugins): PluginLoader {
if (this._hookState === HookState.UNINITIALIZED) {
const plugins = Object.keys(pluginConfig).reduce(
(plugins: PluginNames, moduleName: string) => {
if (pluginConfig[moduleName]) {
plugins[moduleName] = utils.defaultPackageName(moduleName);
}
return plugins;
},
{} as PluginNames
);
const modulesToHook = Object.keys(plugins);
const pluginsToLoad = filterPlugins(plugins);
const modulesToHook = Object.keys(pluginsToLoad);
// Do not hook require when no module is provided. In this case it is
// not necessary. With skipping this step we lower our footprint in
// customer applications and require-in-the-middle won't show up in CPU
Expand All @@ -83,7 +82,8 @@ export class PluginLoader {
hook(modulesToHook, (exports, name, baseDir) => {
if (this._hookState !== HookState.ENABLED) return exports;

const moduleName = plugins[name];
const config = pluginsToLoad[name];
const modulePath = config.path!;
// Get the module version.
const version = utils.getPackageVersion(this.logger, baseDir as string);
this.logger.info(
Expand All @@ -93,23 +93,23 @@ export class PluginLoader {
if (!version) return exports;

this.logger.debug(
`PluginLoader#load: applying patch to ${name}@${version} using ${moduleName} module`
`PluginLoader#load: applying patch to ${name}@${version} using ${modulePath} module`
);

// Expecting a plugin from module;
try {
const plugin: Plugin = require(moduleName).plugin;
const plugin: Plugin = require(modulePath).plugin;

if (!utils.isSupportedVersion(version, plugin.supportedVersions)) {
return exports;
}

this._plugins.push(plugin);
// Enable each supported plugin.
return plugin.enable(exports, this.tracer, this.logger);
return plugin.enable(exports, this.tracer, this.logger, config);
} catch (e) {
this.logger.error(
`PluginLoader#load: could not load plugin ${moduleName} of module ${name}. Error: ${e.message}`
`PluginLoader#load: could not load plugin ${modulePath} of module ${name}. Error: ${e.message}`
);
return exports;
}
Expand Down
78 changes: 70 additions & 8 deletions packages/opentelemetry-node-tracer/test/instrumentation/PluginLoader.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,61 @@ import {
HookState,
PluginLoader,
searchPathForTest,
Plugins,
} from '../../src/instrumentation/PluginLoader';

const INSTALLED_PLUGINS_PATH = path.join(__dirname, 'node_modules');

const simplePlugins: Plugins = {
'simple-module': {
enabled: true,
path: '@opentelemetry/plugin-simple-module',
ignoreMethods: [],
ignoreUrls: [],
},
};

const disablePlugins: Plugins = {
'simple-module': {
enabled: false,
path: '@opentelemetry/plugin-simple-module',
},
nonexistent: {
enabled: false,
path: '@opentelemetry/plugin-nonexistent-module',
},
};

const nonexistentPlugins: Plugins = {
nonexistent: {
enabled: true,
path: '@opentelemetry/plugin-nonexistent-module',
},
};

const missingPathPlugins: Plugins = {
'simple-module': {
enabled: true,
},
nonexistent: {
enabled: true,
},
};

const supportedVersionPlugins: Plugins = {
'supported-module': {
enabled: true,
path: '@opentelemetry/plugin-supported-module',
},
};

const notSupportedVersionPlugins: Plugins = {
'notsupported-module': {
enabled: true,
path: 'notsupported-module',
},
};

describe('PluginLoader', () => {
const tracer = new NoopTracer();
const logger = new NoopLogger();
Expand All @@ -47,14 +98,14 @@ describe('PluginLoader', () => {

it('transitions from UNINITIALIZED to ENABLED', () => {
const pluginLoader = new PluginLoader(tracer, logger);
pluginLoader.load({ 'simple-module': true });
pluginLoader.load(simplePlugins);
assert.strictEqual(pluginLoader['_hookState'], HookState.ENABLED);
pluginLoader.unload();
});

it('transitions from ENABLED to DISABLED', () => {
const pluginLoader = new PluginLoader(tracer, logger);
pluginLoader.load({ 'simple-module': true }).unload();
pluginLoader.load(simplePlugins).unload();
assert.strictEqual(pluginLoader['_hookState'], HookState.DISABLED);
});
});
Expand All @@ -80,7 +131,7 @@ describe('PluginLoader', () => {
it('should load a plugin and patch the target modules', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'simple-module': true });
pluginLoader.load(simplePlugins);
// The hook is only called the first time the module is loaded.
const simpleModule = require('simple-module');
assert.strictEqual(pluginLoader['_plugins'].length, 1);
Expand All @@ -92,7 +143,7 @@ describe('PluginLoader', () => {
it('should not load the plugin when supported versions does not match', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'notsupported-module': true });
pluginLoader.load(notSupportedVersionPlugins);
// The hook is only called the first time the module is loaded.
require('notsupported-module');
assert.strictEqual(pluginLoader['_plugins'].length, 0);
Expand All @@ -102,7 +153,7 @@ describe('PluginLoader', () => {
it('should load a plugin and patch the target modules when supported versions match', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'supported-module': true });
pluginLoader.load(supportedVersionPlugins);
// The hook is only called the first time the module is loaded.
const simpleModule = require('supported-module');
assert.strictEqual(pluginLoader['_plugins'].length, 1);
Expand All @@ -114,7 +165,18 @@ describe('PluginLoader', () => {
it('should not load a plugin when value is false', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'simple-module': false });
pluginLoader.load(disablePlugins);
const simpleModule = require('simple-module');
assert.strictEqual(pluginLoader['_plugins'].length, 0);
assert.strictEqual(simpleModule.value(), 0);
assert.strictEqual(simpleModule.name(), 'simple-module');
pluginLoader.unload();
});

it('should not load a plugin when value is true but path is missing', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load(missingPathPlugins);
const simpleModule = require('simple-module');
assert.strictEqual(pluginLoader['_plugins'].length, 0);
assert.strictEqual(simpleModule.value(), 0);
Expand All @@ -125,7 +187,7 @@ describe('PluginLoader', () => {
it('should not load a non existing plugin', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'nonexistent-module': true });
pluginLoader.load(nonexistentPlugins);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.unload();
});
Expand All @@ -142,7 +204,7 @@ describe('PluginLoader', () => {
it('should unload the plugins and unpatch the target module when unloads', () => {
const pluginLoader = new PluginLoader(tracer, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load({ 'simple-module': true });
pluginLoader.load(simplePlugins);
// The hook is only called the first time the module is loaded.
const simpleModule = require('simple-module');
assert.strictEqual(pluginLoader['_plugins'].length, 1);
Expand Down
53 changes: 52 additions & 1 deletion packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,60 @@ export interface Plugin<T = any> {
moduleExports: T,
tracer: Tracer,
logger: Logger,
config?: { [key: string]: unknown }
config?: PluginConfig
): T;

/** Method to disable the instrumentation */
disable(): void;
}

export interface PluginConfig {
/**
* Whether to enable the plugin.
* @default true
*/
enabled?: boolean;

/**
* Path of the trace plugin to load.
* @default '@opentelemetry/plugin-http' in case of http.
*/
path?: string;

/**
* Request methods that match any string in ignoreMethods will not be traced.
*/
ignoreMethods?: string[];

/**
* URLs that partially match any regex in ignoreUrls will not be traced.
* In addition, URLs that are _exact matches_ of strings in ignoreUrls will
* also not be traced.
*/
ignoreUrls?: Array<string | RegExp>;

/**
* List of internal files that need patch and are not exported by
* default.
*/
internalFilesExports?: PluginInternalFiles;

/**
* If true, additional information about query parameters and
* results will be attached (as `attributes`) to spans representing
* database operations.
*/
enhancedDatabaseReporting?: boolean;
}

export interface PluginInternalFilesVersion {
[pluginName: string]: string;
}

/**
* Each key should be the name of the module to trace, and its value
* a mapping of a property name to a internal plugin file name.
*/
export interface PluginInternalFiles {
[versions: string]: PluginInternalFilesVersion;
}

0 comments on commit 8279133

Please sign in to comment.