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 b80a7bc
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/opentelemetry-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
},
"devDependencies": {
"@opentelemetry/context-base": "^0.7.0",
"@opentelemetry/plugin-http": "^0.7.0",
"@opentelemetry/resources": "^0.7.0",
"@types/mocha": "^7.0.0",
"@types/node": "^12.6.8",
Expand Down
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 b80a7bc

Please sign in to comment.