From d391a452d004876f96a7839326504f6307ed6bd6 Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 27 Jun 2025 13:40:33 -0400 Subject: [PATCH 1/3] add experimental flag on plugins to disable them by default --- packages/datadog-plugin-prisma/src/index.js | 1 + packages/dd-trace/src/plugin_manager.js | 2 +- packages/dd-trace/test/plugin_manager.spec.js | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/datadog-plugin-prisma/src/index.js b/packages/datadog-plugin-prisma/src/index.js index 7fefd0ab7ca..c6a95656efb 100644 --- a/packages/datadog-plugin-prisma/src/index.js +++ b/packages/datadog-plugin-prisma/src/index.js @@ -6,6 +6,7 @@ const PrismaClientPlugin = require('./client') const PrismaEnginePlugin = require('./engine') class PrismaPlugin extends CompositePlugin { + static experimental = true static get id () { return 'prisma' } static get plugins () { return { diff --git a/packages/dd-trace/src/plugin_manager.js b/packages/dd-trace/src/plugin_manager.js index ed4639d5cdc..a4e0827be8a 100644 --- a/packages/dd-trace/src/plugin_manager.js +++ b/packages/dd-trace/src/plugin_manager.js @@ -74,7 +74,7 @@ module.exports = class PluginManager { this._pluginsByName[name] = new Plugin(this._tracer, this._tracerConfig) } const pluginConfig = this._configsByName[name] || { - enabled: this._tracerConfig.plugins !== false + enabled: this._tracerConfig.plugins !== false && !Plugin.experimental } // extracts predetermined configuration from tracer and combines it with plugin-specific config diff --git a/packages/dd-trace/test/plugin_manager.spec.js b/packages/dd-trace/test/plugin_manager.spec.js index 9abcf70e1d4..f4ee303c30c 100644 --- a/packages/dd-trace/test/plugin_manager.spec.js +++ b/packages/dd-trace/test/plugin_manager.spec.js @@ -16,6 +16,7 @@ describe('Plugin Manager', () => { let Four let Five let Six + let Eight let pm beforeEach(() => { @@ -53,7 +54,11 @@ describe('Plugin Manager', () => { return 'six' } }, - seven: {} + seven: {}, + eight: class Eight extends FakePlugin { + static experimental = true + static id = 'eight' + } } Two = plugins.two @@ -67,6 +72,9 @@ describe('Plugin Manager', () => { Six = plugins.six Six.prototype.configure = sinon.spy() + Eight = plugins.eight + Eight.prototype.configure = sinon.spy() + process.env.DD_TRACE_DISABLED_PLUGINS = 'five,six,seven' PluginManager = proxyquire.noPreserveCache()('../src/plugin_manager', { @@ -273,6 +281,14 @@ describe('Plugin Manager', () => { }) }) + describe('with an experimental plugin', () => { + it('should disable the plugin by default', () => { + pm.configure() + loadChannel.publish({ name: 'eight' }) + expect(Eight.prototype.configure).to.have.been.calledWithMatch({ enabled: false }) + }) + }) + it('instantiates plugin classes', () => { pm.configure() loadChannel.publish({ name: 'two' }) From 7707fc2b89bf2b2dcd307a66a18843fac859636c Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 27 Jun 2025 13:56:46 -0400 Subject: [PATCH 2/3] fix support for environment variable --- packages/dd-trace/src/plugin_manager.js | 12 ++++++++---- packages/dd-trace/test/plugin_manager.spec.js | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/dd-trace/src/plugin_manager.js b/packages/dd-trace/src/plugin_manager.js index a4e0827be8a..78302d5f22a 100644 --- a/packages/dd-trace/src/plugin_manager.js +++ b/packages/dd-trace/src/plugin_manager.js @@ -1,7 +1,7 @@ 'use strict' const { channel } = require('dc-polyfill') -const { isFalse, normalizePluginEnvName } = require('./util') +const { isFalse, isTrue, normalizePluginEnvName } = require('./util') const plugins = require('./plugins') const log = require('./log') const { getEnvironmentVariable } = require('../../dd-trace/src/config-helper') @@ -32,8 +32,7 @@ loadChannel.subscribe(({ name }) => { function maybeEnable (Plugin) { if (!Plugin || typeof Plugin !== 'function') return if (!pluginClasses[Plugin.id]) { - const envName = `DD_TRACE_${Plugin.id.toUpperCase()}_ENABLED` - const enabled = getEnvironmentVariable(normalizePluginEnvName(envName)) + const enabled = getEnvEnabled(Plugin) // TODO: remove the need to load the plugin class in order to disable the plugin if (isFalse(enabled) || disabledPlugins.has(Plugin.id)) { @@ -46,6 +45,11 @@ function maybeEnable (Plugin) { } } +function getEnvEnabled (Plugin) { + const envName = `DD_TRACE_${Plugin.id.toUpperCase()}_ENABLED` + return getEnvironmentVariable(normalizePluginEnvName(envName)) +} + // TODO this must always be a singleton. module.exports = class PluginManager { constructor (tracer) { @@ -74,7 +78,7 @@ module.exports = class PluginManager { this._pluginsByName[name] = new Plugin(this._tracer, this._tracerConfig) } const pluginConfig = this._configsByName[name] || { - enabled: this._tracerConfig.plugins !== false && !Plugin.experimental + enabled: isTrue(getEnvEnabled(Plugin)) || (this._tracerConfig.plugins !== false && !Plugin.experimental) } // extracts predetermined configuration from tracer and combines it with plugin-specific config diff --git a/packages/dd-trace/test/plugin_manager.spec.js b/packages/dd-trace/test/plugin_manager.spec.js index f4ee303c30c..3b0bc9f5025 100644 --- a/packages/dd-trace/test/plugin_manager.spec.js +++ b/packages/dd-trace/test/plugin_manager.spec.js @@ -91,6 +91,7 @@ describe('Plugin Manager', () => { afterEach(() => { delete process.env.DD_TRACE_DISABLED_PLUGINS + delete process.env.DD_TRACE_EIGHT_ENABLED pm.destroy() }) @@ -287,6 +288,20 @@ describe('Plugin Manager', () => { loadChannel.publish({ name: 'eight' }) expect(Eight.prototype.configure).to.have.been.calledWithMatch({ enabled: false }) }) + + it('should enable the plugin when configured programmatically', () => { + pm.configure() + pm.configurePlugin('eight') + loadChannel.publish({ name: 'eight' }) + expect(Eight.prototype.configure).to.have.been.calledWithMatch({ enabled: true }) + }) + + it('should enable the plugin when configured with an environment variable', () => { + process.env.DD_TRACE_EIGHT_ENABLED = 'true' + pm.configure() + loadChannel.publish({ name: 'eight' }) + expect(Eight.prototype.configure).to.have.been.calledWithMatch({ enabled: true }) + }) }) it('instantiates plugin classes', () => { From 2e3f1b866db7ac4a9f606eea8195923e50849826 Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 27 Jun 2025 14:02:26 -0400 Subject: [PATCH 3/3] change ordering to prioritize programmatic config --- packages/dd-trace/src/plugin_manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dd-trace/src/plugin_manager.js b/packages/dd-trace/src/plugin_manager.js index 78302d5f22a..8305fcf89bc 100644 --- a/packages/dd-trace/src/plugin_manager.js +++ b/packages/dd-trace/src/plugin_manager.js @@ -78,7 +78,7 @@ module.exports = class PluginManager { this._pluginsByName[name] = new Plugin(this._tracer, this._tracerConfig) } const pluginConfig = this._configsByName[name] || { - enabled: isTrue(getEnvEnabled(Plugin)) || (this._tracerConfig.plugins !== false && !Plugin.experimental) + enabled: this._tracerConfig.plugins !== false && (!Plugin.experimental || isTrue(getEnvEnabled(Plugin))) } // extracts predetermined configuration from tracer and combines it with plugin-specific config