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

fix errors caused by instrumentation when the tracer is disabled #381

Merged
merged 1 commit into from
Nov 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions src/instrumenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ shimmer({ logger: () => {} })
class Instrumenter {
constructor (tracer) {
this._tracer = tracer
this._enabled = false
this._names = new Set()
this._plugins = new Map()
this._instrumented = new Map()
Expand Down Expand Up @@ -55,6 +56,8 @@ class Instrumenter {
}

reload () {
if (!this._enabled) return

const instrumentations = Array.from(this._plugins.keys())
.reduce((prev, current) => prev.concat(current), [])

Expand Down Expand Up @@ -128,6 +131,10 @@ class Instrumenter {
return moduleExports
}

enable () {
this._enabled = true
}

_set (plugin, meta) {
this._plugins.set(plugin, Object.assign({ config: {} }, meta))
}
Expand Down
1 change: 1 addition & 0 deletions src/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Tracer extends BaseTracer {
platform.configure(config)

this._tracer = new DatadogTracer(config)
this._instrumenter.enable()
this._instrumenter.patch(config)
}
} catch (e) {
Expand Down
39 changes: 38 additions & 1 deletion test/instrumenter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ describe('Instrumenter', () => {
let instrumenter
let integrations
let tracer
let tracerConfig
let shimmer

beforeEach(() => {
tracer = {
_tracer: 'tracer'
}

tracerConfig = { enabled: true }

integrations = {
http: {
name: 'http',
Expand Down Expand Up @@ -60,14 +63,18 @@ describe('Instrumenter', () => {
'./plugins/mysql-mock': integrations.mysql
})

instrumenter = new Instrumenter(tracer)
instrumenter = new Instrumenter(tracer, tracerConfig)
})

afterEach(() => {
delete require.cache[require.resolve('mysql-mock')]
})

describe('with integrations enabled', () => {
beforeEach(() => {
instrumenter.enable()
})

describe('use', () => {
it('should allow configuring a plugin by name', () => {
const config = { foo: 'bar' }
Expand Down Expand Up @@ -240,6 +247,10 @@ describe('Instrumenter', () => {
})

describe('with integrations disabled', () => {
beforeEach(() => {
instrumenter.enable()
})

describe('use', () => {
it('should still allow adding plugins manually by name', () => {
instrumenter.use('express-mock')
Expand All @@ -261,4 +272,30 @@ describe('Instrumenter', () => {
})
})
})

describe('with the instrumenter disabled', () => {
describe('use', () => {
it('should not patch modules when they are loaded when the tracer is disabled', () => {
tracerConfig.enabled = false

instrumenter.patch()

require('express-mock')

expect(integrations.express.patch).to.not.have.been.called
})
})

describe('patch', () => {
it('should not patch if the tracer is disabled', () => {
tracerConfig.enabled = false

instrumenter.use('express-mock')

require('express-mock')

expect(integrations.express.patch).to.not.have.been.called
})
})
})
})
2 changes: 2 additions & 0 deletions test/proxy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('TracerProxy', () => {
}

instrumenter = {
enable: sinon.spy(),
patch: sinon.spy(),
use: sinon.spy()
}
Expand Down Expand Up @@ -103,6 +104,7 @@ describe('TracerProxy', () => {
it('should set up automatic instrumentation', () => {
proxy.init()

expect(instrumenter.enable).to.have.been.called
expect(instrumenter.patch).to.have.been.called
})

Expand Down