From 6aabe19017a937e584a7e0394ed4763c18cd3a4a Mon Sep 17 00:00:00 2001 From: Bryan English Date: Thu, 19 Dec 2024 07:05:22 -0500 Subject: [PATCH] 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 55a608f4adf..f07000d10e2 100644 --- a/packages/datadog-plugin-express/test/index.spec.js +++ b/packages/datadog-plugin-express/test/index.spec.js @@ -1502,209 +1502,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 0703c1550cc..af82c3adccc 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -514,6 +514,7 @@ class Config { this._setValue(defaults, 'logInjection', false) this._setValue(defaults, 'lookup', undefined) this._setValue(defaults, 'memcachedCommandEnabled', false) + this._setValue(defaults, 'middleware', true) this._setValue(defaults, 'openAiLogsEnabled', false) this._setValue(defaults, 'openaiSpanCharLimit', 128) this._setValue(defaults, 'peerServiceMapping', {}) @@ -654,6 +655,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, @@ -773,6 +775,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 832044b29f8..a7605f176ff 100644 --- a/packages/dd-trace/src/plugins/util/web.js +++ b/packages/dd-trace/src/plugins/util/web.js @@ -39,12 +39,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 { @@ -542,11 +542,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