From 8fa63351ed2e6b7a7095cdd8a1064fd863fb9404 Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 16 Nov 2018 13:51:00 -0500 Subject: [PATCH] fix errors caused by plugins instrumenting even when the tracer is disabled --- src/instrumenter.js | 7 +++++++ src/proxy.js | 1 + test/instrumenter.spec.js | 39 ++++++++++++++++++++++++++++++++++++++- test/proxy.spec.js | 2 ++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/instrumenter.js b/src/instrumenter.js index 35a5ba4fac1..ae1eb1264ce 100644 --- a/src/instrumenter.js +++ b/src/instrumenter.js @@ -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() @@ -55,6 +56,8 @@ class Instrumenter { } reload () { + if (!this._enabled) return + const instrumentations = Array.from(this._plugins.keys()) .reduce((prev, current) => prev.concat(current), []) @@ -128,6 +131,10 @@ class Instrumenter { return moduleExports } + enable () { + this._enabled = true + } + _set (plugin, meta) { this._plugins.set(plugin, Object.assign({ config: {} }, meta)) } diff --git a/src/proxy.js b/src/proxy.js index cfcead0b662..69a474c553d 100644 --- a/src/proxy.js +++ b/src/proxy.js @@ -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) { diff --git a/test/instrumenter.spec.js b/test/instrumenter.spec.js index 4cd0f6ab6a8..9e61b83e97a 100644 --- a/test/instrumenter.spec.js +++ b/test/instrumenter.spec.js @@ -7,6 +7,7 @@ describe('Instrumenter', () => { let instrumenter let integrations let tracer + let tracerConfig let shimmer beforeEach(() => { @@ -14,6 +15,8 @@ describe('Instrumenter', () => { _tracer: 'tracer' } + tracerConfig = { enabled: true } + integrations = { http: { name: 'http', @@ -60,7 +63,7 @@ describe('Instrumenter', () => { './plugins/mysql-mock': integrations.mysql }) - instrumenter = new Instrumenter(tracer) + instrumenter = new Instrumenter(tracer, tracerConfig) }) afterEach(() => { @@ -68,6 +71,10 @@ describe('Instrumenter', () => { }) describe('with integrations enabled', () => { + beforeEach(() => { + instrumenter.enable() + }) + describe('use', () => { it('should allow configuring a plugin by name', () => { const config = { foo: 'bar' } @@ -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') @@ -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 + }) + }) + }) }) diff --git a/test/proxy.spec.js b/test/proxy.spec.js index b00c178c62d..214c59067ff 100644 --- a/test/proxy.spec.js +++ b/test/proxy.spec.js @@ -35,6 +35,7 @@ describe('TracerProxy', () => { } instrumenter = { + enable: sinon.spy(), patch: sinon.spy(), use: sinon.spy() } @@ -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 })