From 1c1265dca5bdd2ef2fdd32df232c263dd54f99a9 Mon Sep 17 00:00:00 2001 From: Naseem Date: Thu, 23 Apr 2020 01:05:43 -0400 Subject: [PATCH] feat: merge user supplied and default plugin configs 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 --- packages/opentelemetry-node/package.json | 1 + .../src/NodeTracerProvider.ts | 31 +++++++++++++++++-- .../test/NodeTracerProvider.test.ts | 15 +++++++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-node/package.json b/packages/opentelemetry-node/package.json index e2eb356c3da..2cc60af0691 100644 --- a/packages/opentelemetry-node/package.json +++ b/packages/opentelemetry-node/package.json @@ -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", diff --git a/packages/opentelemetry-node/src/NodeTracerProvider.ts b/packages/opentelemetry-node/src/NodeTracerProvider.ts index 11a0874e5e8..f919ac5d495 100644 --- a/packages/opentelemetry-node/src/NodeTracerProvider.ts +++ b/packages/opentelemetry-node/src/NodeTracerProvider.ts @@ -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. @@ -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() { diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts index d38dda53196..5be55f15154 100644 --- a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts @@ -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']; @@ -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', () => {