From 4959fee8b94eb20769adae5c81eea0bac1594c23 Mon Sep 17 00:00:00 2001 From: rochdev Date: Wed, 18 Jul 2018 14:44:22 -0400 Subject: [PATCH 1/2] fix scope manager being instantiated multiple times --- src/scope/scope_manager.js | 4 ++- test/plugins/agent.js | 1 - test/scope/scope_manager.spec.js | 4 +-- test/setup.js | 47 +++++++++++++++++++++++++++++++- test/tracer.spec.js | 4 --- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/scope/scope_manager.js b/src/scope/scope_manager.js index 8412b05ff20..ee406b26719 100644 --- a/src/scope/scope_manager.js +++ b/src/scope/scope_manager.js @@ -5,7 +5,7 @@ const Scope = require('./scope') const Context = require('./context') const ContextExecution = require('./context_execution') -const singleton = null +let singleton = null /** * The Datadog Scope Manager. This is used for context propagation. @@ -18,6 +18,8 @@ class ScopeManager { return singleton } + singleton = this + const id = -1 const execution = new ContextExecution() diff --git a/test/plugins/agent.js b/test/plugins/agent.js index 95084fa014b..29d9736bad8 100644 --- a/test/plugins/agent.js +++ b/test/plugins/agent.js @@ -49,7 +49,6 @@ module.exports = { server.on('close', () => { tracer._instrumenter.unpatch() - tracer.scopeManager()._disable() tracer = null }) diff --git a/test/scope/scope_manager.spec.js b/test/scope/scope_manager.spec.js index 4c262bfb2e9..f3803ae8ef6 100644 --- a/test/scope/scope_manager.spec.js +++ b/test/scope/scope_manager.spec.js @@ -31,8 +31,8 @@ describe('ScopeManager', () => { scopeManager = new ScopeManager() }) - afterEach(() => { - scopeManager._disable() + it('should be a singleton', () => { + expect(new ScopeManager()).to.equal(scopeManager) }) it('should enable its hooks', () => { diff --git a/test/setup.js b/test/setup.js index 688f50c0a12..1e321a9bf50 100644 --- a/test/setup.js +++ b/test/setup.js @@ -14,6 +14,9 @@ const elasticsearch = require('elasticsearch') const amqplib = require('amqplib/callback_api') const platform = require('../src/platform') const node = require('../src/platform/node') +const ScopeManager = require('../src/scope/scope_manager') + +const scopeManager = new ScopeManager() const retryOptions = { retries: 60, @@ -33,6 +36,10 @@ global.wrapIt = wrapIt platform.use(node) +after(() => { + scopeManager._disable() +}) + waitForServices() .then(run) .catch(err => { @@ -185,6 +192,44 @@ function waitForRabbitMQ () { }) } +function withoutScope (fn) { + return function () { + let active + + while ((active = scopeManager.active())) { + active.close() + } + + return fn.apply(this, arguments) + } +} + function wrapIt () { - // placeholder + const it = global.it + + global.it = function (title, fn) { + if (!fn) { + return it.apply(this, arguments) + } + + if (fn.length > 0) { + return it.call(this, title, function (done) { + arguments[0] = withoutScope(done) + + return fn.apply(this, arguments) + }) + } else { + return it.call(this, title, function () { + const result = fn.apply(this, arguments) + + if (result && result.then) { + return result + .then(withoutScope(res => res)) + .catch(withoutScope(err => Promise.reject(err))) + } + + return result + }) + } + } } diff --git a/test/tracer.spec.js b/test/tracer.spec.js index 7ef969bc5f1..00e82482b44 100644 --- a/test/tracer.spec.js +++ b/test/tracer.spec.js @@ -29,10 +29,6 @@ describe('Tracer', () => { tracer = new Tracer(config) }) - afterEach(() => { - tracer.scopeManager()._disable() - }) - describe('trace', () => { it('should run the callback with the new span', done => { tracer.trace('name', current => { From d6d99e498f00900def5c5daec79a3d197c2858db Mon Sep 17 00:00:00 2001 From: rochdev Date: Wed, 18 Jul 2018 14:44:37 -0400 Subject: [PATCH 2/2] fix missing active scope in mysql/mysql2 query callback --- src/plugins/mysql.js | 11 ++++++++--- src/plugins/mysql2.js | 11 ++++++++--- test/plugins/mysql.spec.js | 3 +-- test/plugins/mysql2.spec.js | 3 +-- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/plugins/mysql.js b/src/plugins/mysql.js index 43495aeaf12..d3b462b3cc4 100644 --- a/src/plugins/mysql.js +++ b/src/plugins/mysql.js @@ -6,8 +6,9 @@ function createWrapQuery (tracer, config) { return function wrapQuery (query) { return function queryWithTrace (sql, values, cb) { const parentScope = tracer.scopeManager().active() + const parent = parentScope && parentScope.span() const span = tracer.startSpan('mysql.query', { - childOf: parentScope && parentScope.span(), + childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, 'service.name': config.service || 'mysql', @@ -28,7 +29,7 @@ function createWrapQuery (tracer, config) { span.setTag('resource.name', sequence.sql) if (sequence._callback) { - sequence._callback = wrapCallback(tracer, span, sequence._callback) + sequence._callback = wrapCallback(tracer, span, parent, sequence._callback) } else { sequence.on('end', () => { span.finish() @@ -40,7 +41,7 @@ function createWrapQuery (tracer, config) { } } -function wrapCallback (tracer, span, done) { +function wrapCallback (tracer, span, parent, done) { return (err, res) => { if (err) { span.addTags({ @@ -52,6 +53,10 @@ function wrapCallback (tracer, span, done) { span.finish() + if (parent) { + tracer.scopeManager().activate(parent) + } + done(err, res) } } diff --git a/src/plugins/mysql2.js b/src/plugins/mysql2.js index 927bd80e4fa..26d8ff6b086 100644 --- a/src/plugins/mysql2.js +++ b/src/plugins/mysql2.js @@ -6,8 +6,9 @@ function createWrapQuery (tracer, config) { return function wrapQuery (query) { return function queryWithTrace (sql, values, cb) { const parentScope = tracer.scopeManager().active() + const parent = parentScope && parentScope.span() const span = tracer.startSpan('mysql.query', { - childOf: parentScope && parentScope.span(), + childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, 'service.name': config.service || 'mysql', @@ -28,7 +29,7 @@ function createWrapQuery (tracer, config) { span.setTag('resource.name', sequence.sql) if (sequence.onResult) { - sequence.onResult = wrapCallback(span, sequence.onResult) + sequence.onResult = wrapCallback(tracer, span, parent, sequence.onResult) } else { sequence.on('end', () => { span.finish() @@ -40,7 +41,7 @@ function createWrapQuery (tracer, config) { } } -function wrapCallback (span, done) { +function wrapCallback (tracer, span, parent, done) { return (err, res) => { if (err) { span.addTags({ @@ -52,6 +53,10 @@ function wrapCallback (span, done) { span.finish() + if (parent) { + tracer.scopeManager().activate(parent) + } + done(err, res) } } diff --git a/test/plugins/mysql.spec.js b/test/plugins/mysql.spec.js index 173ec3c99a8..8f4c13a0d35 100644 --- a/test/plugins/mysql.spec.js +++ b/test/plugins/mysql.spec.js @@ -48,8 +48,7 @@ describe('Plugin', () => { connection.query('SELECT 1 + 1 AS solution', () => { const active = tracer.scopeManager().active() - scope.close() - expect(active).to.equal(scope) + expect(active.span()).to.equal(scope.span()) done() }) }) diff --git a/test/plugins/mysql2.spec.js b/test/plugins/mysql2.spec.js index 4a4ca3c9d98..d46921bca17 100644 --- a/test/plugins/mysql2.spec.js +++ b/test/plugins/mysql2.spec.js @@ -48,8 +48,7 @@ describe('Plugin', () => { connection.query('SELECT 1 + 1 AS solution', () => { const active = tracer.scopeManager().active() - scope.close() - expect(active).to.equal(scope) + expect(active.span()).to.equal(scope.span()) done() }) })