Skip to content

Commit

Permalink
feat: merge user supplied and default plugin configs
Browse files Browse the repository at this point in the history
By merging user supplied config two levels deep, a user can now only configure the diff
from the default auto enablement of installed plugins, rather than
having to explicitly re enable all plugins if at least one of their
configurations require are edited.

Furthermore, user supplied plugins not listed in default plugins are
implicitly enabled.

Signed-off-by: Naseem <[email protected]>
  • Loading branch information
Naseem committed Apr 30, 2020
1 parent 23677a1 commit f4aeb6e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
31 changes: 29 additions & 2 deletions packages/opentelemetry-node/src/NodeTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
SDKRegistrationConfig,
} from '@opentelemetry/tracing';
import { DEFAULT_INSTRUMENTATION_PLUGINS, NodeTracerConfig } from './config';
import { PluginLoader } from './instrumentation/PluginLoader';
import { PluginLoader, Plugins } from './instrumentation/PluginLoader';

/**
* Register this TracerProvider for use with the OpenTelemetry API.
Expand All @@ -39,7 +39,34 @@ export class NodeTracerProvider extends BasicTracerProvider {
super(config);

this._pluginLoader = new PluginLoader(this, this.logger);
this._pluginLoader.load(config.plugins || DEFAULT_INSTRUMENTATION_PLUGINS);

/**
* For user supplied config of plugin(s) that are loaded by default,
* merge the user supplied and default configs of said plugin(s)
*/
let mergedUserSuppliedPlugins: Plugins = {};

for (const pluginName in config.plugins) {
if (DEFAULT_INSTRUMENTATION_PLUGINS.hasOwnProperty(pluginName)) {
mergedUserSuppliedPlugins[pluginName] = {
...DEFAULT_INSTRUMENTATION_PLUGINS[pluginName],
...config.plugins[pluginName],
};
} else {
mergedUserSuppliedPlugins[pluginName] = config.plugins[pluginName];
// enable user-supplied plugins unless explicitly disabled
if (mergedUserSuppliedPlugins[pluginName].enabled === undefined) {
mergedUserSuppliedPlugins[pluginName].enabled = true;
}
}
}

const mergedPlugins: Plugins = {
...DEFAULT_INSTRUMENTATION_PLUGINS,
...mergedUserSuppliedPlugins,
};

this._pluginLoader.load(mergedPlugins);
}

stop() {
Expand Down
15 changes: 12 additions & 3 deletions packages/opentelemetry-node/test/NodeTracerProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,26 @@ describe('NodeTracerProvider', () => {
assert.ok(provider instanceof NodeTracerProvider);
});

it('should load user configured plugins', () => {
it('should load a merge of user configured and default plugins and implictly enable non-default plugins', () => {
provider = new NodeTracerProvider({
logger: new NoopLogger(),
plugins: {
'simple-module': {
enabled: true,
path: '@opentelemetry/plugin-simple-module',
},
'supported-module': {
enabled: true,
path: '@opentelemetry/plugin-supported-module',
enhancedDatabaseReporting: false,
ignoreMethods: [],
ignoreUrls: [],
},
'random-module': {
enabled: false,
path: '@opentelemetry/random-module',
},
http: {
path: '@opentelemetry/plugin-http-module',
},
},
});
const pluginLoader = provider['_pluginLoader'];
Expand All @@ -104,6 +109,10 @@ describe('NodeTracerProvider', () => {
assert.strictEqual(pluginLoader['_plugins'].length, 1);
require('supported-module');
assert.strictEqual(pluginLoader['_plugins'].length, 2);
require('random-module');
assert.strictEqual(pluginLoader['_plugins'].length, 2);
require('http');
assert.strictEqual(pluginLoader['_plugins'].length, 3);
});

it('should construct an instance with default attributes', () => {
Expand Down

0 comments on commit f4aeb6e

Please sign in to comment.