From 4fa0a75db2987c1e8344af8eb11e136e90735beb Mon Sep 17 00:00:00 2001 From: Roch Devost Date: Tue, 10 Sep 2019 17:03:17 -0400 Subject: [PATCH] add async scope tracking as an official feature enabled by default (#664) --- docs/test.ts | 4 ++-- index.d.ts | 8 ++++++-- package.json | 2 +- packages/.eslintrc.json | 1 - packages/dd-trace/src/config.js | 4 ++-- packages/dd-trace/src/scope/async_hooks.js | 12 +++++++++--- packages/dd-trace/test/.eslintrc.json | 1 - packages/dd-trace/test/config.spec.js | 8 ++++---- .../dd-trace/test/platform/node/index.spec.js | 1 + .../dd-trace/test/scope/async_hooks.spec.js | 19 ++++++++++++++++++- packages/dd-trace/test/setup/core.js | 6 +----- 11 files changed, 44 insertions(+), 22 deletions(-) diff --git a/docs/test.ts b/docs/test.ts index 801d6ecef38..b32bd3e3b6b 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -39,9 +39,9 @@ tracer.init({ analytics: true, env: 'test', runtimeMetrics: true, + trackAsyncScope: false, experimental: { - b3: true, - thenables: true + b3: true }, hostname: 'agent', logger: { diff --git a/index.d.ts b/index.d.ts index fc57819eeb1..1aa0a163c27 100644 --- a/index.d.ts +++ b/index.d.ts @@ -230,6 +230,12 @@ export declare interface TracerOptions { */ runtimeMetrics?: boolean + /** + * Whether to track the scope of async functions. This is needed for async/await to work with non-native promises (thenables). Only disable this if you are sure only native promises are used with async/await. + * @default true + */ + trackAsyncScope?: boolean + /** * Experimental features can be enabled all at once by using true or individually using key / value pairs. * @default {} @@ -241,8 +247,6 @@ export declare interface TracerOptions { * @default false */ exporter?: 'log' | 'browser' | 'agent' - - thenables?: boolean }; /** diff --git a/package.json b/package.json index 25d0d0a81ec..54fdf624368 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,7 @@ "get-port": "^3.2.0", "graphql": "0.13.2", "mocha": "^5.2.0", - "nock": "^9.6.1", + "nock": "^11.3.3", "node-abi": "^2.8.0", "nyc": "^14.1.1", "prebuildify": "^2.11.0", diff --git a/packages/.eslintrc.json b/packages/.eslintrc.json index afebed27d8a..d50e6adbf49 100644 --- a/packages/.eslintrc.json +++ b/packages/.eslintrc.json @@ -9,7 +9,6 @@ "expect": true, "sinon": true, "proxyquire": true, - "nock": true, "wrapIt": true, "withVersions": true }, diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 859070a9b03..73fe779e0c9 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -53,10 +53,10 @@ class Config { port: String(coalesce(dogstatsd.port, platform.env('DD_DOGSTATSD_PORT'), 8125)) } this.runtimeMetrics = String(runtimeMetrics) === 'true' + this.trackAsyncScope = options.trackAsyncScope !== false this.experimental = { b3: !(!options.experimental || !options.experimental.b3), - exporter: options.experimental && options.experimental.exporter, - thenables: !(!options.experimental || !options.experimental.thenables) + exporter: options.experimental && options.experimental.exporter } this.reportHostname = String(reportHostname) === 'true' this.scope = process.env.DD_CONTEXT_PROPAGATION === 'false' ? scopes.NOOP : scope diff --git a/packages/dd-trace/src/scope/async_hooks.js b/packages/dd-trace/src/scope/async_hooks.js index 749ca6ac192..9dadf40551e 100644 --- a/packages/dd-trace/src/scope/async_hooks.js +++ b/packages/dd-trace/src/scope/async_hooks.js @@ -18,7 +18,7 @@ class Scope extends Base { singleton = this - this._thenables = config.experimental.thenables + this._trackAsyncScope = config.trackAsyncScope this._current = null this._spans = Object.create(null) this._types = Object.create(null) @@ -62,12 +62,17 @@ class Scope extends Base { } _exit (span) { - this._thenables && this._await(span) + this._trackAsyncScope && this._await(span) this._current = span this._stack[this._depth] = null this._depth-- } + _exitNative () { + this._current = null + this._promises[0] = false + } + _await (span) { if (!this._promises[this._depth]) return @@ -106,12 +111,13 @@ class Scope extends Base { platform.metrics().increment('async.resources') platform.metrics().increment('async.resources.by.type', `resource_type:${type}`) - if (this._thenables && type === 'PROMISE') { + if (this._trackAsyncScope && type === 'PROMISE') { this._initPromise() } } _before (asyncId) { + this._depth === 0 && this._exitNative() this._enter(this._spans[asyncId]) } diff --git a/packages/dd-trace/test/.eslintrc.json b/packages/dd-trace/test/.eslintrc.json index 2fe338462b3..43a96a43ea4 100644 --- a/packages/dd-trace/test/.eslintrc.json +++ b/packages/dd-trace/test/.eslintrc.json @@ -9,7 +9,6 @@ "expect": true, "sinon": true, "proxyquire": true, - "nock": true, "wrapIt": true, "withVersions": true }, diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 9838936a51d..e7d28d81c4d 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -27,6 +27,7 @@ describe('Config', () => { expect(config).to.have.property('flushInterval', 2000) expect(config).to.have.property('sampleRate', 1) expect(config).to.have.property('runtimeMetrics', false) + expect(config).to.have.property('trackAsyncScope', true) expect(config).to.have.deep.property('tags', {}) expect(config).to.have.property('plugins', true) expect(config).to.have.property('env', undefined) @@ -35,7 +36,6 @@ describe('Config', () => { expect(config).to.have.property('apiKey', undefined) expect(config).to.have.property('appKey', undefined) expect(config).to.have.nested.property('experimental.b3', false) - expect(config).to.have.nested.property('experimental.thenables', false) }) it('should initialize from the default service', () => { @@ -116,14 +116,14 @@ describe('Config', () => { tags, flushInterval: 5000, runtimeMetrics: true, + trackAsyncScope: false, reportHostname: true, plugins: false, scope: 'noop', apiKey: '123', appKey: '456', experimental: { - b3: true, - thenables: true + b3: true } }) @@ -141,6 +141,7 @@ describe('Config', () => { expect(config.tags).to.have.property('foo', 'bar') expect(config).to.have.property('flushInterval', 5000) expect(config).to.have.property('runtimeMetrics', true) + expect(config).to.have.property('trackAsyncScope', false) expect(config).to.have.property('reportHostname', true) expect(config).to.have.property('plugins', false) expect(config).to.have.property('scope', 'noop') @@ -150,7 +151,6 @@ describe('Config', () => { 'foo': 'bar' }) expect(config).to.have.nested.property('experimental.b3', true) - expect(config).to.have.nested.property('experimental.thenables', true) }) it('should initialize from the options with url taking precedence', () => { diff --git a/packages/dd-trace/test/platform/node/index.spec.js b/packages/dd-trace/test/platform/node/index.spec.js index 0d397fd5155..6e67ed6e32f 100644 --- a/packages/dd-trace/test/platform/node/index.spec.js +++ b/packages/dd-trace/test/platform/node/index.spec.js @@ -1,5 +1,6 @@ 'use strict' +const nock = require('nock') const semver = require('semver') wrapIt() diff --git a/packages/dd-trace/test/scope/async_hooks.spec.js b/packages/dd-trace/test/scope/async_hooks.spec.js index 695fa98188c..f5ebca9cc5b 100644 --- a/packages/dd-trace/test/scope/async_hooks.spec.js +++ b/packages/dd-trace/test/scope/async_hooks.spec.js @@ -91,7 +91,7 @@ describe('Scope (async_hooks)', () => { beforeEach(() => { thenable = { - then: () => {} + then: onFulfill => onFulfill() } test = async () => { @@ -155,6 +155,23 @@ describe('Scope (async_hooks)', () => { scope.activate(span, () => test()) }) }) + + it('should use the active span when using nested awaits', async () => { + return scope.activate(span, async () => { + await thenable + + thenable.then = (onFulfill, onReject) => { + try { + expect() + onFulfill() + } catch (e) { + onReject(e) + } + } + + await thenable + }) + }) }) testScope(() => scope) diff --git a/packages/dd-trace/test/setup/core.js b/packages/dd-trace/test/setup/core.js index 9d5b411db43..11509fdebcf 100644 --- a/packages/dd-trace/test/setup/core.js +++ b/packages/dd-trace/test/setup/core.js @@ -4,7 +4,6 @@ const sinon = require('sinon') const chai = require('chai') const sinonChai = require('sinon-chai') const proxyquire = require('../proxyquire') -const nock = require('nock') const semver = require('semver') const platform = require('../../src/platform') const node = require('../../src/platform/node') @@ -13,9 +12,7 @@ const agent = require('../plugins/agent') const externals = require('../plugins/externals.json') const asyncHooksScope = new AsyncHooksScope({ - experimental: { - thenables: true - } + trackAsyncScope: true }) chai.use(sinonChai) @@ -23,7 +20,6 @@ chai.use(sinonChai) global.sinon = sinon global.expect = chai.expect global.proxyquire = proxyquire -global.nock = nock global.wrapIt = wrapIt global.withVersions = withVersions