From 7d3a4175d35764b03bb0b10739119c3758bc6b33 Mon Sep 17 00:00:00 2001 From: Bryan English Date: Thu, 19 Dec 2024 07:05:22 -0500 Subject: [PATCH 01/10] env var to disable all middleware spans --- .../datadog-plugin-express/test/index.spec.js | 313 +++++++++--------- packages/datadog-plugin-web/src/index.js | 2 +- packages/dd-trace/src/config.js | 3 + packages/dd-trace/src/plugins/util/web.js | 19 +- 4 files changed, 175 insertions(+), 162 deletions(-) diff --git a/packages/datadog-plugin-express/test/index.spec.js b/packages/datadog-plugin-express/test/index.spec.js index 8899c34ecb3..e61ba3b0945 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1566,209 +1566,214 @@ describe('Plugin', () => { }) }) - describe('with configuration for middleware disabled', () => { - before(() => { - return agent.load(['express', 'http'], [{ - middleware: false - }, { client: false }]) - }) - - after(() => { - return agent.close({ ritmReset: false }) - }) + middlewareDisabledTest('plugin', [{ middleware: false }, { client: false }], {}) + middlewareDisabledTest('tracer', [{}, { client: false }], { middleware: false }) - beforeEach(() => { - express = require(`../../../versions/express@${version}`).get() - }) - - it('should not activate a scope per middleware', done => { - const app = express() - - let span - - app.use((req, res, next) => { - span = tracer.scope().active() - next() + function middlewareDisabledTest(description, pluginConfig, tracerConfig) { + describe(`with configuration for middleware disabled via ${description} config`, () => { + before(() => { + return agent.load(['express', 'http'], pluginConfig, tracerConfig) }) - app.get('/user', (req, res) => { - res.status(200).send() - try { - expect(tracer.scope().active()).to.equal(span).and.to.not.be.null - done() - } catch (e) { - done(e) - } + after(() => { + return agent.close({ ritmReset: false }) }) - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - axios.get(`http://localhost:${port}/user`) - .catch(done) + beforeEach(() => { + express = require(`../../../versions/express@${version}`).get() + console.log('loaded express', description) }) - }) - it('should not do automatic instrumentation on middleware', done => { - const app = express() + it('should not activate a scope per middleware', done => { + const app = express() - app.use((req, res, next) => { - next() - }) + let span - app.get('/user', (req, res, next) => { - res.status(200).send() - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port + app.use((req, res, next) => { + span = tracer.scope().active() + next() + }) - agent - .use(traces => { - const spans = sort(traces[0]) + app.get('/user', (req, res) => { + res.status(200).send() + try { + expect(tracer.scope().active()).to.equal(span).and.to.not.be.null + done() + } catch (e) { + done(e) + } + }) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(traces.length).to.equal(1) - }) - .then(done) - .catch(done) + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port - axios.get(`http://localhost:${port}/user`) - .catch(done) + axios.get(`http://localhost:${port}/user`) + .catch(done) + }) }) - }) - it('should handle error status codes', done => { - const app = express() + it('should not do automatic instrumentation on middleware', done => { + const app = express() - app.use((req, res, next) => { - next() - }) + app.use((req, res, next) => { + next() + }) - app.get('/user', (req, res) => { - res.status(500).send() - }) + app.get('/user', (req, res, next) => { + res.status(200).send() + }) - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port - agent.use(traces => { - const spans = sort(traces[0]) + agent + .use(traces => { + const spans = sort(traces[0]) - expect(spans[0]).to.have.property('error', 1) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(traces.length).to.equal(1) + }) + .then(done) + .catch(done) - done() + axios.get(`http://localhost:${port}/user`) + .catch(done) }) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) }) - }) - it('should only handle errors for configured status codes', done => { - const app = express() + it('should handle error status codes', done => { + const app = express() - app.use((req, res, next) => { - next() - }) + app.use((req, res, next) => { + next() + }) - app.get('/user', (req, res) => { - res.statusCode = 400 - throw new Error('boom') - }) + app.get('/user', (req, res) => { + res.status(500).send() + }) - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port - agent - .use(traces => { + agent.use(traces => { const spans = sort(traces[0]) - expect(spans[0]).to.have.property('error', 0) + expect(spans[0]).to.have.property('error', 1) expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '400') + expect(spans[0].meta).to.have.property('http.status_code', '500') expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 400 + done() }) - .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) }) - }) - it('should handle middleware errors', done => { - const app = express() - const error = new Error('boom') + it('should only handle errors for configured status codes', done => { + const app = express() - app.use((req, res) => { throw error }) - // eslint-disable-next-line n/handle-callback-err - app.use((error, req, res, next) => res.status(500).send()) + app.use((req, res, next) => { + next() + }) - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port + app.get('/user', (req, res) => { + res.statusCode = 400 + throw new Error('boom') + }) - agent - .use(traces => { - const spans = sort(traces[0]) + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) + agent + .use(traces => { + const spans = sort(traces[0]) - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) + expect(spans[0]).to.have.property('error', 0) + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(spans[0].meta).to.have.property('http.status_code', '400') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 400 + }) + .catch(done) + }) }) - }) - it('should handle request errors', done => { - const app = express() - const error = new Error('boom') + it('should handle middleware errors', done => { + const app = express() + const error = new Error('boom') - app.use(() => { throw error }) + app.use((req, res) => { throw error }) + // eslint-disable-next-line n/handle-callback-err + app.use((error, req, res, next) => res.status(500).send()) - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port - agent - .use(traces => { - const spans = sort(traces[0]) + agent + .use(traces => { + const spans = sort(traces[0]) - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + + it('should handle request errors', done => { + const app = express() + const error = new Error('boom') + + app.use(() => { throw error }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('http.status_code', '500') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) }) }) - }) + } + }) }) }) diff --git a/packages/datadog-plugin-web/src/index.js b/packages/datadog-plugin-web/src/index.js index 688d5dc2781..21996c952d6 100644 --- a/packages/datadog-plugin-web/src/index.js +++ b/packages/datadog-plugin-web/src/index.js @@ -9,7 +9,7 @@ class WebPlugin extends Plugin { } configure (config) { - return super.configure(web.normalizeConfig(config)) + return super.configure(web.normalizeConfig(config, this._tracerConfig)) } setFramework (req, name, config) { diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 8dd63cccdf6..6a167822e81 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -520,6 +520,7 @@ class Config { this._setValue(defaults, 'lookup', undefined) this._setValue(defaults, 'inferredProxyServicesEnabled', false) this._setValue(defaults, 'memcachedCommandEnabled', false) + this._setValue(defaults, 'middleware', true) this._setValue(defaults, 'openAiLogsEnabled', false) this._setValue(defaults, 'openaiSpanCharLimit', 128) this._setValue(defaults, 'peerServiceMapping', {}) @@ -670,6 +671,7 @@ class Config { DD_TRACE_HEADER_TAGS, DD_TRACE_LEGACY_BAGGAGE_ENABLED, DD_TRACE_MEMCACHED_COMMAND_ENABLED, + DD_TRACE_MIDDLEWARE_ENABLED, DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP, DD_TRACE_PARTIAL_FLUSH_MIN_SPANS, DD_TRACE_PEER_SERVICE_MAPPING, @@ -801,6 +803,7 @@ class Config { this._setBoolean(env, 'logInjection', DD_LOGS_INJECTION) // Requires an accompanying DD_APM_OBFUSCATION_MEMCACHED_KEEP_COMMAND=true in the agent this._setBoolean(env, 'memcachedCommandEnabled', DD_TRACE_MEMCACHED_COMMAND_ENABLED) + this._setBoolean(env, 'middleware', DD_TRACE_MIDDLEWARE_ENABLED) this._setBoolean(env, 'openAiLogsEnabled', DD_OPENAI_LOGS_ENABLED) this._setValue(env, 'openaiSpanCharLimit', maybeInt(DD_OPENAI_SPAN_CHAR_LIMIT)) this._envUnprocessed.openaiSpanCharLimit = DD_OPENAI_SPAN_CHAR_LIMIT diff --git a/packages/dd-trace/src/plugins/util/web.js b/packages/dd-trace/src/plugins/util/web.js index 2d92c74ea91..7c5f48459a6 100644 --- a/packages/dd-trace/src/plugins/util/web.js +++ b/packages/dd-trace/src/plugins/util/web.js @@ -40,12 +40,12 @@ const web = { TYPE: WEB, // Ensure the configuration has the correct structure and defaults. - normalizeConfig (config) { + normalizeConfig (config, tracerConfig) { const headers = getHeadersToRecord(config) const validateStatus = getStatusValidator(config) const hooks = getHooks(config) const filter = urlFilter.getFilter(config) - const middleware = getMiddlewareSetting(config) + const middleware = getMiddlewareSetting(config, tracerConfig) const queryStringObfuscation = getQsObfuscator(config) return { @@ -570,11 +570,16 @@ function getHooks (config) { return { request } } -function getMiddlewareSetting (config) { - if (config && typeof config.middleware === 'boolean') { - return config.middleware - } else if (config && config.hasOwnProperty('middleware')) { - log.error('Expected `middleware` to be a boolean.') +function getMiddlewareSetting (config, tracerConfig) { + if (config) { + if (typeof config.middleware === 'boolean') { + return config.middleware + } else if (config.hasOwnProperty('middleware')) { + log.error('Expected `middleware` to be a boolean.') + } + } + if (tracerConfig && tracerConfig.middleware === false) { + return false } return true From 48d8784da57a39663960f6b0f3b25a0b606cc24e Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Tue, 7 Jan 2025 21:32:49 -0500 Subject: [PATCH 02/10] fix linter errors --- packages/datadog-plugin-express/test/index.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/datadog-plugin-express/test/index.spec.js b/packages/datadog-plugin-express/test/index.spec.js index e61ba3b0945..38778036a39 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1569,7 +1569,7 @@ describe('Plugin', () => { middlewareDisabledTest('plugin', [{ middleware: false }, { client: false }], {}) middlewareDisabledTest('tracer', [{}, { client: false }], { middleware: false }) - function middlewareDisabledTest(description, pluginConfig, tracerConfig) { + function middlewareDisabledTest (description, pluginConfig, tracerConfig) { describe(`with configuration for middleware disabled via ${description} config`, () => { before(() => { return agent.load(['express', 'http'], pluginConfig, tracerConfig) @@ -1581,6 +1581,7 @@ describe('Plugin', () => { beforeEach(() => { express = require(`../../../versions/express@${version}`).get() + // eslint-disable-next-line no-console console.log('loaded express', description) }) @@ -1773,7 +1774,6 @@ describe('Plugin', () => { }) }) } - }) }) }) From 53641565e3a589dae10acc470362586106b98b5f Mon Sep 17 00:00:00 2001 From: Bryan English Date: Wed, 15 Jan 2025 11:24:05 -0500 Subject: [PATCH 03/10] adding tracer config option for middleware --- packages/datadog-plugin-express/test/index.spec.js | 6 ++---- packages/dd-trace/src/config.js | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/datadog-plugin-express/test/index.spec.js b/packages/datadog-plugin-express/test/index.spec.js index 38778036a39..cf431dd717e 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1566,11 +1566,11 @@ describe('Plugin', () => { }) }) - middlewareDisabledTest('plugin', [{ middleware: false }, { client: false }], {}) + // middlewareDisabledTest('plugin', [{ middleware: false }, { client: false }], {}) middlewareDisabledTest('tracer', [{}, { client: false }], { middleware: false }) function middlewareDisabledTest (description, pluginConfig, tracerConfig) { - describe(`with configuration for middleware disabled via ${description} config`, () => { + describe.only(`with configuration for middleware disabled via ${description} config`, () => { before(() => { return agent.load(['express', 'http'], pluginConfig, tracerConfig) }) @@ -1581,8 +1581,6 @@ describe('Plugin', () => { beforeEach(() => { express = require(`../../../versions/express@${version}`).get() - // eslint-disable-next-line no-console - console.log('loaded express', description) }) it('should not activate a scope per middleware', done => { diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 6a167822e81..08ae47bb63a 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -985,6 +985,7 @@ class Config { this._setString(opts, 'llmobs.mlApp', options.llmobs?.mlApp) this._setBoolean(opts, 'logInjection', options.logInjection) this._setString(opts, 'lookup', options.lookup) + this._setBoolean(opts, 'middleware', options.middleware) this._setBoolean(opts, 'openAiLogsEnabled', options.openAiLogsEnabled) this._setValue(opts, 'peerServiceMapping', options.peerServiceMapping) this._setBoolean(opts, 'plugins', options.plugins) From b9d1a763d49139d2219fe64fd5002bc96d563165 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 15 Jan 2025 20:05:43 -0500 Subject: [PATCH 04/10] separate middleware tests into different files --- .../datadog-plugin-express/test/index.spec.js | 207 ---------------- .../middleware-disabled-plugin.spec.js | 5 + .../middleware-disabled-tests.js | 228 ++++++++++++++++++ .../middleware-disabled-tracer.spec.js | 5 + 4 files changed, 238 insertions(+), 207 deletions(-) create mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js create mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js create mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js diff --git a/packages/datadog-plugin-express/test/index.spec.js b/packages/datadog-plugin-express/test/index.spec.js index cf431dd717e..d3b514adf65 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1565,213 +1565,6 @@ describe('Plugin', () => { }) }) }) - - // middlewareDisabledTest('plugin', [{ middleware: false }, { client: false }], {}) - middlewareDisabledTest('tracer', [{}, { client: false }], { middleware: false }) - - function middlewareDisabledTest (description, pluginConfig, tracerConfig) { - describe.only(`with configuration for middleware disabled via ${description} config`, () => { - before(() => { - return agent.load(['express', 'http'], pluginConfig, tracerConfig) - }) - - after(() => { - return agent.close({ ritmReset: false }) - }) - - beforeEach(() => { - express = require(`../../../versions/express@${version}`).get() - }) - - it('should not activate a scope per middleware', done => { - const app = express() - - let span - - app.use((req, res, next) => { - span = tracer.scope().active() - next() - }) - - app.get('/user', (req, res) => { - res.status(200).send() - try { - expect(tracer.scope().active()).to.equal(span).and.to.not.be.null - done() - } catch (e) { - done(e) - } - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - axios.get(`http://localhost:${port}/user`) - .catch(done) - }) - }) - - it('should not do automatic instrumentation on middleware', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res, next) => { - res.status(200).send() - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(traces.length).to.equal(1) - }) - .then(done) - .catch(done) - - axios.get(`http://localhost:${port}/user`) - .catch(done) - }) - }) - - it('should handle error status codes', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res) => { - res.status(500).send() - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent.use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') - - done() - }) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - - it('should only handle errors for configured status codes', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res) => { - res.statusCode = 400 - throw new Error('boom') - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 0) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '400') - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 400 - }) - .catch(done) - }) - }) - - it('should handle middleware errors', done => { - const app = express() - const error = new Error('boom') - - app.use((req, res) => { throw error }) - // eslint-disable-next-line n/handle-callback-err - app.use((error, req, res, next) => res.status(500).send()) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - - it('should handle request errors', done => { - const app = express() - const error = new Error('boom') - - app.use(() => { throw error }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - }) - } }) }) }) diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js new file mode 100644 index 00000000000..33735967c27 --- /dev/null +++ b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js @@ -0,0 +1,5 @@ +'use strict' + +const test = require('./middleware-disabled-tests') + +test('plugin', [{ middleware: false }, { client: false }], {}) diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js new file mode 100644 index 00000000000..a9e44ea73f6 --- /dev/null +++ b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js @@ -0,0 +1,228 @@ +'use strict' + +const axios = require('axios') +const { ERROR_MESSAGE, ERROR_STACK, ERROR_TYPE } = require('../../../dd-trace/src/constants') +const agent = require('../../../dd-trace/test/plugins/agent') + +const sort = spans => spans.sort((a, b) => a.start.toString() >= b.start.toString() ? 1 : -1) + +module.exports = function middlewareDisabledTest (description, pluginConfig, tracerConfig) { + describe('express middleware spans disabled tests', () => { + let tracer + let express + let appListener + withVersions('express', 'express', version => { + beforeEach(() => { + tracer = require('../../../dd-trace') + }) + + afterEach(() => { + appListener && appListener.close() + appListener = null + }) + + describe(`with configuration for middleware disabled via ${description} config`, () => { + before(() => { + return agent.load(['express', 'http'], pluginConfig, tracerConfig) + }) + + after(() => { + return agent.close({ ritmReset: false }) + }) + + beforeEach(() => { + express = require(`../../../../versions/express@${version}`).get() + }) + + it('should not activate a scope per middleware', done => { + const app = express() + + let span + + app.use((req, res, next) => { + span = tracer.scope().active() + next() + }) + + app.get('/user', (req, res) => { + res.status(200).send() + try { + expect(tracer.scope().active()).to.equal(span).and.to.not.be.null + done() + } catch (e) { + done(e) + } + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + axios.get(`http://localhost:${port}/user`) + .catch(done) + }) + }) + + it('should not do automatic instrumentation on middleware', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res, next) => { + res.status(200).send() + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(traces.length).to.equal(1) + }) + .then(done) + .catch(done) + + axios.get(`http://localhost:${port}/user`) + .catch(done) + }) + }) + + it('should handle error status codes', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res) => { + res.status(500).send() + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent.use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(spans[0].meta).to.have.property('http.status_code', '500') + expect(spans[0].meta).to.have.property('component', 'express') + + done() + }) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + + it('should only handle errors for configured status codes', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res) => { + res.statusCode = 400 + throw new Error('boom') + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 0) + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(spans[0].meta).to.have.property('http.status_code', '400') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 400 + }) + .catch(done) + }) + }) + + it('should handle middleware errors', done => { + const app = express() + const error = new Error('boom') + + app.use((req, res) => { throw error }) + // eslint-disable-next-line n/handle-callback-err + app.use((error, req, res, next) => res.status(500).send()) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + + it('should handle request errors', done => { + const app = express() + const error = new Error('boom') + + app.use(() => { throw error }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('http.status_code', '500') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + }) + } + ) + }) +} diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js new file mode 100644 index 00000000000..fc3560d3098 --- /dev/null +++ b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js @@ -0,0 +1,5 @@ +'use strict' + +const test = require('./middleware-disabled-tests') + +test('tracer', [{}, { client: false }], { middleware: false }) From b32ca1ba2c36c45b5d404154fcf5b275204f731b Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 15 Jan 2025 20:31:07 -0500 Subject: [PATCH 05/10] fix linter errors --- .../middleware-disabled-test/middleware-disabled-tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js index a9e44ea73f6..f9a2e00a717 100644 --- a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js +++ b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js @@ -219,10 +219,10 @@ module.exports = function middlewareDisabledTest (description, pluginConfig, tra validateStatus: status => status === 500 }) .catch(done) - }) }) }) - } + }) + } ) }) } From f4ef66c81fabb56f33df7e64eccec5884df1c3dd Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 22 Jan 2025 20:01:05 -0500 Subject: [PATCH 06/10] convert unit test to integration test --- .../datadog-plugin-express/test/index.spec.js | 204 ++++++++++++++++ .../test/integration-test/client.spec.js | 16 ++ .../middleware-disabled-plugin.spec.js | 5 - .../middleware-disabled-tests.js | 228 ------------------ .../middleware-disabled-tracer.spec.js | 5 - 5 files changed, 220 insertions(+), 238 deletions(-) delete mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js delete mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js delete mode 100644 packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js diff --git a/packages/datadog-plugin-express/test/index.spec.js b/packages/datadog-plugin-express/test/index.spec.js index d3b514adf65..8899c34ecb3 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1565,6 +1565,210 @@ describe('Plugin', () => { }) }) }) + + describe('with configuration for middleware disabled', () => { + before(() => { + return agent.load(['express', 'http'], [{ + middleware: false + }, { client: false }]) + }) + + after(() => { + return agent.close({ ritmReset: false }) + }) + + beforeEach(() => { + express = require(`../../../versions/express@${version}`).get() + }) + + it('should not activate a scope per middleware', done => { + const app = express() + + let span + + app.use((req, res, next) => { + span = tracer.scope().active() + next() + }) + + app.get('/user', (req, res) => { + res.status(200).send() + try { + expect(tracer.scope().active()).to.equal(span).and.to.not.be.null + done() + } catch (e) { + done(e) + } + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + axios.get(`http://localhost:${port}/user`) + .catch(done) + }) + }) + + it('should not do automatic instrumentation on middleware', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res, next) => { + res.status(200).send() + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(traces.length).to.equal(1) + }) + .then(done) + .catch(done) + + axios.get(`http://localhost:${port}/user`) + .catch(done) + }) + }) + + it('should handle error status codes', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res) => { + res.status(500).send() + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent.use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(spans[0].meta).to.have.property('http.status_code', '500') + expect(spans[0].meta).to.have.property('component', 'express') + + done() + }) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + + it('should only handle errors for configured status codes', done => { + const app = express() + + app.use((req, res, next) => { + next() + }) + + app.get('/user', (req, res) => { + res.statusCode = 400 + throw new Error('boom') + }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 0) + expect(spans[0]).to.have.property('resource', 'GET /user') + expect(spans[0].meta).to.have.property('http.status_code', '400') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 400 + }) + .catch(done) + }) + }) + + it('should handle middleware errors', done => { + const app = express() + const error = new Error('boom') + + app.use((req, res) => { throw error }) + // eslint-disable-next-line n/handle-callback-err + app.use((error, req, res, next) => res.status(500).send()) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + + it('should handle request errors', done => { + const app = express() + const error = new Error('boom') + + app.use(() => { throw error }) + + appListener = app.listen(0, 'localhost', () => { + const port = appListener.address().port + + agent + .use(traces => { + const spans = sort(traces[0]) + + expect(spans[0]).to.have.property('error', 1) + expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) + expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) + expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) + expect(spans[0].meta).to.have.property('http.status_code', '500') + expect(spans[0].meta).to.have.property('component', 'express') + }) + .then(done) + .catch(done) + + axios + .get(`http://localhost:${port}/user`, { + validateStatus: status => status === 500 + }) + .catch(done) + }) + }) + }) }) }) }) diff --git a/packages/datadog-plugin-express/test/integration-test/client.spec.js b/packages/datadog-plugin-express/test/integration-test/client.spec.js index c13a4249892..858e2445dee 100644 --- a/packages/datadog-plugin-express/test/integration-test/client.spec.js +++ b/packages/datadog-plugin-express/test/integration-test/client.spec.js @@ -47,6 +47,22 @@ describe('esm', () => { assert.strictEqual(payload[0].length, numberOfSpans) assert.propertyVal(payload[0][0], 'name', 'express.request') assert.propertyVal(payload[0][1], 'name', 'express.middleware') + console.log(1, payload) + }) + }).timeout(50000) + + it('disables middleware spans when config.middleware is set to false through environment variable', async () => { + process.env.DD_TRACE_MIDDLEWARE_ENABLED = false + proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port) + const numberOfSpans = 1 + + return curlAndAssertMessage(agent, proc, ({ headers, payload }) => { + assert.propertyVal(headers, 'host', `127.0.0.1:${agent.port}`) + assert.isArray(payload) + assert.strictEqual(payload.length, 1) + assert.isArray(payload[0]) + assert.strictEqual(payload[0].length, numberOfSpans) + assert.propertyVal(payload[0][0], 'name', 'express.request') }) }).timeout(50000) }) diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js deleted file mode 100644 index 33735967c27..00000000000 --- a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-plugin.spec.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict' - -const test = require('./middleware-disabled-tests') - -test('plugin', [{ middleware: false }, { client: false }], {}) diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js deleted file mode 100644 index f9a2e00a717..00000000000 --- a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tests.js +++ /dev/null @@ -1,228 +0,0 @@ -'use strict' - -const axios = require('axios') -const { ERROR_MESSAGE, ERROR_STACK, ERROR_TYPE } = require('../../../dd-trace/src/constants') -const agent = require('../../../dd-trace/test/plugins/agent') - -const sort = spans => spans.sort((a, b) => a.start.toString() >= b.start.toString() ? 1 : -1) - -module.exports = function middlewareDisabledTest (description, pluginConfig, tracerConfig) { - describe('express middleware spans disabled tests', () => { - let tracer - let express - let appListener - withVersions('express', 'express', version => { - beforeEach(() => { - tracer = require('../../../dd-trace') - }) - - afterEach(() => { - appListener && appListener.close() - appListener = null - }) - - describe(`with configuration for middleware disabled via ${description} config`, () => { - before(() => { - return agent.load(['express', 'http'], pluginConfig, tracerConfig) - }) - - after(() => { - return agent.close({ ritmReset: false }) - }) - - beforeEach(() => { - express = require(`../../../../versions/express@${version}`).get() - }) - - it('should not activate a scope per middleware', done => { - const app = express() - - let span - - app.use((req, res, next) => { - span = tracer.scope().active() - next() - }) - - app.get('/user', (req, res) => { - res.status(200).send() - try { - expect(tracer.scope().active()).to.equal(span).and.to.not.be.null - done() - } catch (e) { - done(e) - } - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - axios.get(`http://localhost:${port}/user`) - .catch(done) - }) - }) - - it('should not do automatic instrumentation on middleware', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res, next) => { - res.status(200).send() - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(traces.length).to.equal(1) - }) - .then(done) - .catch(done) - - axios.get(`http://localhost:${port}/user`) - .catch(done) - }) - }) - - it('should handle error status codes', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res) => { - res.status(500).send() - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent.use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') - - done() - }) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - - it('should only handle errors for configured status codes', done => { - const app = express() - - app.use((req, res, next) => { - next() - }) - - app.get('/user', (req, res) => { - res.statusCode = 400 - throw new Error('boom') - }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 0) - expect(spans[0]).to.have.property('resource', 'GET /user') - expect(spans[0].meta).to.have.property('http.status_code', '400') - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 400 - }) - .catch(done) - }) - }) - - it('should handle middleware errors', done => { - const app = express() - const error = new Error('boom') - - app.use((req, res) => { throw error }) - // eslint-disable-next-line n/handle-callback-err - app.use((error, req, res, next) => res.status(500).send()) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - - it('should handle request errors', done => { - const app = express() - const error = new Error('boom') - - app.use(() => { throw error }) - - appListener = app.listen(0, 'localhost', () => { - const port = appListener.address().port - - agent - .use(traces => { - const spans = sort(traces[0]) - - expect(spans[0]).to.have.property('error', 1) - expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name) - expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) - expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) - expect(spans[0].meta).to.have.property('http.status_code', '500') - expect(spans[0].meta).to.have.property('component', 'express') - }) - .then(done) - .catch(done) - - axios - .get(`http://localhost:${port}/user`, { - validateStatus: status => status === 500 - }) - .catch(done) - }) - }) - }) - } - ) - }) -} diff --git a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js b/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js deleted file mode 100644 index fc3560d3098..00000000000 --- a/packages/datadog-plugin-express/test/middleware-disabled-test/middleware-disabled-tracer.spec.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict' - -const test = require('./middleware-disabled-tests') - -test('tracer', [{}, { client: false }], { middleware: false }) From 4fde2a1d9c308c20a713ebe6eadfb553469b428a Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 22 Jan 2025 20:18:34 -0500 Subject: [PATCH 07/10] fix integration test --- .../test/integration-test/client.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/datadog-plugin-express/test/integration-test/client.spec.js b/packages/datadog-plugin-express/test/integration-test/client.spec.js index 858e2445dee..d0511313b1b 100644 --- a/packages/datadog-plugin-express/test/integration-test/client.spec.js +++ b/packages/datadog-plugin-express/test/integration-test/client.spec.js @@ -36,6 +36,7 @@ describe('esm', () => { }) it('is instrumented', async () => { + process.env.DD_TRACE_MIDDLEWARE_ENABLED = true proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port) const numberOfSpans = semver.intersects(version, '<5.0.0') ? 4 : 3 @@ -46,8 +47,9 @@ describe('esm', () => { assert.isArray(payload[0]) assert.strictEqual(payload[0].length, numberOfSpans) assert.propertyVal(payload[0][0], 'name', 'express.request') - assert.propertyVal(payload[0][1], 'name', 'express.middleware') - console.log(1, payload) + for (let i = 1; i < numberOfSpans + 1; i++) { + assert.propertyVal(payload[0][i], 'name', 'express.middleware') + } }) }).timeout(50000) From af28561fecaf86b262662360d11d5d50d77ed807 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 22 Jan 2025 20:28:30 -0500 Subject: [PATCH 08/10] fix integration test --- .../test/integration-test/client.spec.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/datadog-plugin-express/test/integration-test/client.spec.js b/packages/datadog-plugin-express/test/integration-test/client.spec.js index d0511313b1b..68df57c255c 100644 --- a/packages/datadog-plugin-express/test/integration-test/client.spec.js +++ b/packages/datadog-plugin-express/test/integration-test/client.spec.js @@ -47,9 +47,7 @@ describe('esm', () => { assert.isArray(payload[0]) assert.strictEqual(payload[0].length, numberOfSpans) assert.propertyVal(payload[0][0], 'name', 'express.request') - for (let i = 1; i < numberOfSpans + 1; i++) { - assert.propertyVal(payload[0][i], 'name', 'express.middleware') - } + assert.propertyVal(payload[0][1], 'name', 'express.middleware') }) }).timeout(50000) From 65bf5cbef52fe6f69505dc2a60457b1f8d6304aa Mon Sep 17 00:00:00 2001 From: Ida Liu <119438987+ida613@users.noreply.github.com> Date: Thu, 23 Jan 2025 10:36:42 -0500 Subject: [PATCH 09/10] Update packages/datadog-plugin-express/test/integration-test/client.spec.js Co-authored-by: Bryan English --- .../datadog-plugin-express/test/integration-test/client.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/datadog-plugin-express/test/integration-test/client.spec.js b/packages/datadog-plugin-express/test/integration-test/client.spec.js index 68df57c255c..36f83499890 100644 --- a/packages/datadog-plugin-express/test/integration-test/client.spec.js +++ b/packages/datadog-plugin-express/test/integration-test/client.spec.js @@ -36,7 +36,7 @@ describe('esm', () => { }) it('is instrumented', async () => { - process.env.DD_TRACE_MIDDLEWARE_ENABLED = true + delete process.env.DD_TRACE_MIDDLEWARE_ENABLED proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port) const numberOfSpans = semver.intersects(version, '<5.0.0') ? 4 : 3 From a7eb98a3d251f9942b3bf15c95bcba47b47e7cff Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Fri, 24 Jan 2025 15:05:34 -0500 Subject: [PATCH 10/10] change middleware config value name --- .../test/integration-test/client.spec.js | 2 +- packages/dd-trace/src/config.js | 6 +++--- packages/dd-trace/src/plugins/util/web.js | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/datadog-plugin-express/test/integration-test/client.spec.js b/packages/datadog-plugin-express/test/integration-test/client.spec.js index 68df57c255c..15066ebbda1 100644 --- a/packages/datadog-plugin-express/test/integration-test/client.spec.js +++ b/packages/datadog-plugin-express/test/integration-test/client.spec.js @@ -51,7 +51,7 @@ describe('esm', () => { }) }).timeout(50000) - it('disables middleware spans when config.middleware is set to false through environment variable', async () => { + it('disables middleware spans when config.enableMiddlewareTracing is set to false through environment variable', async () => { process.env.DD_TRACE_MIDDLEWARE_ENABLED = false proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port) const numberOfSpans = 1 diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 08ae47bb63a..5af52e1e3e4 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -520,7 +520,7 @@ class Config { this._setValue(defaults, 'lookup', undefined) this._setValue(defaults, 'inferredProxyServicesEnabled', false) this._setValue(defaults, 'memcachedCommandEnabled', false) - this._setValue(defaults, 'middleware', true) + this._setValue(defaults, 'enableMiddlewareTracing', true) this._setValue(defaults, 'openAiLogsEnabled', false) this._setValue(defaults, 'openaiSpanCharLimit', 128) this._setValue(defaults, 'peerServiceMapping', {}) @@ -803,7 +803,7 @@ class Config { this._setBoolean(env, 'logInjection', DD_LOGS_INJECTION) // Requires an accompanying DD_APM_OBFUSCATION_MEMCACHED_KEEP_COMMAND=true in the agent this._setBoolean(env, 'memcachedCommandEnabled', DD_TRACE_MEMCACHED_COMMAND_ENABLED) - this._setBoolean(env, 'middleware', DD_TRACE_MIDDLEWARE_ENABLED) + this._setBoolean(env, 'enableMiddlewareTracing', DD_TRACE_MIDDLEWARE_ENABLED) this._setBoolean(env, 'openAiLogsEnabled', DD_OPENAI_LOGS_ENABLED) this._setValue(env, 'openaiSpanCharLimit', maybeInt(DD_OPENAI_SPAN_CHAR_LIMIT)) this._envUnprocessed.openaiSpanCharLimit = DD_OPENAI_SPAN_CHAR_LIMIT @@ -985,7 +985,7 @@ class Config { this._setString(opts, 'llmobs.mlApp', options.llmobs?.mlApp) this._setBoolean(opts, 'logInjection', options.logInjection) this._setString(opts, 'lookup', options.lookup) - this._setBoolean(opts, 'middleware', options.middleware) + this._setBoolean(opts, 'enableMiddlewareTracing', options.enableMiddlewareTracing) this._setBoolean(opts, 'openAiLogsEnabled', options.openAiLogsEnabled) this._setValue(opts, 'peerServiceMapping', options.peerServiceMapping) this._setBoolean(opts, 'plugins', options.plugins) diff --git a/packages/dd-trace/src/plugins/util/web.js b/packages/dd-trace/src/plugins/util/web.js index 7c5f48459a6..d11c6078dfc 100644 --- a/packages/dd-trace/src/plugins/util/web.js +++ b/packages/dd-trace/src/plugins/util/web.js @@ -45,7 +45,7 @@ const web = { const validateStatus = getStatusValidator(config) const hooks = getHooks(config) const filter = urlFilter.getFilter(config) - const middleware = getMiddlewareSetting(config, tracerConfig) + const enableMiddlewareTracing = getMiddlewareSetting(config, tracerConfig) const queryStringObfuscation = getQsObfuscator(config) return { @@ -54,7 +54,7 @@ const web = { validateStatus, hooks, filter, - middleware, + enableMiddlewareTracing, queryStringObfuscation } }, @@ -572,13 +572,13 @@ function getHooks (config) { function getMiddlewareSetting (config, tracerConfig) { if (config) { - if (typeof config.middleware === 'boolean') { - return config.middleware - } else if (config.hasOwnProperty('middleware')) { - log.error('Expected `middleware` to be a boolean.') + if (typeof config.enableMiddlewareTracing === 'boolean') { + return config.enableMiddlewareTracing + } else if (config.hasOwnProperty('enableMiddlewareTracing')) { + log.error('Expected `enableMiddlewareTracing` to be a boolean.') } } - if (tracerConfig && tracerConfig.middleware === false) { + if (tracerConfig && tracerConfig.enableMiddlewareTracing === false) { return false }