Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

env var to disable all middleware spans #5044

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
+34 −9
Draft

env var to disable all middleware spans #5044

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7d3a417
env var to disable all middleware spans
bengl Dec 19, 2024
48d8784
fix linter errors
ida613 Jan 8, 2025
5364156
adding tracer config option for middleware
bengl Jan 15, 2025
b9d1a76
separate middleware tests into different files
ida613 Jan 16, 2025
b32ca1b
fix linter errors
ida613 Jan 16, 2025
f4ef66c
convert unit test to integration test
ida613 Jan 23, 2025
4fde2a1
fix integration test
ida613 Jan 23, 2025
af28561
fix integration test
ida613 Jan 23, 2025
65bf5cb
Update packages/datadog-plugin-express/test/integration-test/client.s…
ida613 Jan 23, 2025
a7eb98a
change middleware config value name
ida613 Jan 24, 2025
ada7ede
Merge branch 'bengl/disable-all-middleware-option' of github.com:Data…
ida613 Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions packages/datadog-plugin-express/test/integration-test/client.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('esm', () => {
})

it('is instrumented', async () => {
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

Expand All @@ -49,5 +50,20 @@ describe('esm', () => {
assert.propertyVal(payload[0][1], 'name', 'express.middleware')
})
}).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)
})
})
2 changes: 1 addition & 1 deletion packages/datadog-plugin-web/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions packages/dd-trace/src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name seems too simple and easy to confuse the purpose. How about enableMiddlewareTracing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this name, yes.

this._setValue(defaults, 'openAiLogsEnabled', false)
this._setValue(defaults, 'openaiSpanCharLimit', 128)
this._setValue(defaults, 'peerServiceMapping', {})
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -982,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)
Expand Down
19 changes: 12 additions & 7 deletions packages/dd-trace/src/plugins/util/web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading