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.

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

Add support for disabling plugins by environment variable #733

Merged
merged 24 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0a49708
add check for integrations disabled config option on tracer init plug…
ericmustin Oct 29, 2019
67620cf
[in prog] add specs to test that enable skipped disabled integrations
ericmustin Oct 30, 2019
ed4b363
merge master to resolve conflicts
ericmustin Nov 2, 2019
989506d
fix tests and linting, improve naming convention
ericmustin Nov 2, 2019
7ec6e45
add api docs markdown and typescript definition formatting
ericmustin Nov 2, 2019
8e087c6
improve debug log language
ericmustin Nov 2, 2019
c66ab4c
add ternary operator in configuration to handle converting string env…
ericmustin Nov 2, 2019
1a1ddd2
merge master
ericmustin Nov 17, 2019
64209ef
[wip] refactor to use plugins object in private _set method, add disa…
ericmustin Nov 17, 2019
badbcae
account for edge case where .use adds a plugin before disabledPlugins…
ericmustin Nov 19, 2019
0cde86a
handle plugins option config via multiple formats
ericmustin Nov 19, 2019
a218ba4
add specs and lint cleanup
ericmustin Nov 19, 2019
831adbd
fix specs
ericmustin Nov 19, 2019
1388b9c
fix .enable edgecase where .use is called beforehand by matching w/co…
ericmustin Nov 19, 2019
d6049d3
use correct env var and update api docs
ericmustin Nov 19, 2019
e1dde71
remember to reset disabled plugins on .disable
ericmustin Nov 19, 2019
8aab172
adjust plugins config to only take boolean or array of disabled plugi…
ericmustin Nov 24, 2019
19df1a9
typo fix in instrumenter
ericmustin Nov 24, 2019
31feed7
refactor configuration to have disabledPlugins as seperate env var, u…
ericmustin Dec 1, 2019
a8dc2d0
refactor to only use env var and update specs and docs accoordingly
ericmustin Dec 5, 2019
9556769
typo fix
ericmustin Dec 5, 2019
1a2bf7c
remove added whitespace
ericmustin Dec 5, 2019
ee1dcb4
make tests more explicit for env var, cleanup language clarity
ericmustin Dec 5, 2019
b524953
delete env var instead of setting to undefined
ericmustin Dec 6, 2019
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
1 change: 1 addition & 0 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ Options can be configured as a parameter to the [init()](./interfaces/tracer.htm
| reportHostname | `DD_TRACE_REPORT_HOSTNAME` | `false` | Whether to report the system's hostname for each trace. When disabled, the hostname of the agent will be used instead. |
| experimental | - | `{}` | Experimental features can be enabled all at once using boolean `true` or individually using key/value pairs. Please contact us to learn more about the available experimental features. |
| plugins | - | `true` | Whether or not to enable automatic instrumentation of external libraries using the built-in plugins. |
| - | `DD_TRACE_DISABLED_PLUGINS` | - | A comma-separated string of integration names automatically disabled when tracer is initialized. Environment variable only e.g. `DD_TRACE_DISABLED_PLUGINS=express,dns`. |
| clientToken | `DD_CLIENT_TOKEN` | - | Client token for browser tracing. Can be generated in the UI at `Integrations -> APIs`. |
| logLevel | `DD_TRACE_LOG_LEVEL` | `debug` | A string for the minimum log level for the tracer to use when debug logging is enabled, e.g. `'error'`, `'debug'`. |

Expand Down
15 changes: 13 additions & 2 deletions packages/dd-trace/src/instrumenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ shimmer({ logger: () => {} })

const plugins = platform.plugins

const disabldPlugins = platform.env('DD_TRACE_DISABLED_PLUGINS')

const collectDisabledPlugins = () => {
return new Set(disabldPlugins && disabldPlugins.split(',').map(plugin => plugin.trim()))
}

class Instrumenter {
constructor (tracer) {
this._tracer = tracer
Expand All @@ -16,6 +22,7 @@ class Instrumenter {
this._names = new Set()
this._plugins = new Map()
this._instrumented = new Map()
this._disabledPlugins = collectDisabledPlugins()
}

use (name, config) {
Expand Down Expand Up @@ -149,8 +156,12 @@ class Instrumenter {
}

_set (plugin, meta) {
this._plugins.set(plugin, meta)
this.load(plugin, meta)
if (this._disabledPlugins.has(meta.name)) {
log.debug(`Plugin "${meta.name}" was disabled via configuration option.`)
} else {
this._plugins.set(plugin, meta)
this.load(plugin, meta)
}
}
}

Expand Down
69 changes: 69 additions & 0 deletions packages/dd-trace/test/instrumenter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,73 @@ describe('Instrumenter', () => {
})
})
})

describe('with plugins disabled via DD_TRACE_DISABLED_PLUGINS environment variable', () => {
beforeEach(() => {
process.env.DD_TRACE_DISABLED_PLUGINS = 'http,mysql-mock'

Instrumenter = proxyquire('../src/instrumenter', {
'shimmer': shimmer,
'./platform': {
plugins: {
'http': integrations.http,
'express-mock': integrations.express,
'mysql-mock': integrations.mysql,
'other': integrations.other
}
},
'../../datadog-plugin-http/src': integrations.http,
'../../datadog-plugin-express-mock/src': integrations.express,
'../../datadog-plugin-mysql-mock/src': integrations.mysql,
'../../datadog-plugin-other/src': integrations.other
})

instrumenter = new Instrumenter(tracer)
})

afterEach(() => {
delete process.env.DD_TRACE_DISABLED_PLUGINS
})

describe('enable', () => {
it('should not patch plugins disabled from environnment variable configuration option', () => {
instrumenter.enable()

require('http')
require('mysql-mock')

expect(integrations.http.patch).to.not.have.been.called
expect(integrations.mysql[0].patch).to.not.have.been.called
expect(integrations.mysql[1].patch).to.not.have.been.called
})

it('should patch plugins not disabled by environnment variable configuration option', () => {
const configDefault = {}
instrumenter.enable()

const express = require('express-mock')

expect(integrations.express.patch).to.have.been.calledWith(express, 'tracer', configDefault)
expect(process.env.DD_TRACE_DISABLED_PLUGINS.indexOf('express-mock')).to.equal(-1)
})

it('should not patch plugins called by .use that have been disabled by environment variable', () => {
const configDefault = {}

instrumenter.use('http', configDefault)
instrumenter.use('mysql-mock', configDefault)
instrumenter.use('express-mock', configDefault)
instrumenter.enable()

const express = require('express-mock')
require('http')
require('mysql-mock')

expect(integrations.express.patch).to.have.been.calledWith(express, 'tracer', configDefault)
expect(integrations.http.patch).to.not.have.been.called
expect(integrations.mysql[0].patch).to.not.have.been.called
expect(integrations.mysql[1].patch).to.not.have.been.called
})
})
})
})