From 321244c357bc5dd9b4aeefc308cda5e80b8012b0 Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Wed, 24 Jul 2024 13:24:55 -0400 Subject: [PATCH] feat!: Removed legacy context manager (#2404) --- .github/workflows/versioned-coverage.yml | 38 - documentation/feature-flags.md | 6 - lib/context-manager/create-context-manager.js | 11 - lib/context-manager/legacy-context-manager.js | 66 -- lib/feature_flags.js | 3 +- lib/instrumentation/core/async-hooks.js | 132 ---- lib/instrumentation/core/globals.js | 5 - lib/instrumentation/core/timers.js | 73 +- package.json | 2 - .../async-hooks-new-promise-unresolved.tap.js | 15 - .../core/native-promises/async-hooks.js | 623 ---------------- .../core/native-promises/async-hooks.tap.js | 12 - .../core/native-promises/native-promises.js | 636 ---------------- .../native-promises/native-promises.tap.js | 632 +++++++++++++++- test/integration/core/promises.js | 682 ------------------ test/integration/core/promises.tap.js | 28 - test/integration/core/timers.tap.js | 23 +- .../instrumentation/promises/segments.js | 316 -------- .../promises/transaction-state.js | 2 +- .../context-manager/context-manager-tests.js | 4 +- .../create-context-manager.test.js | 11 - .../legacy-context-manager.test.js | 21 - .../instrumentation/core/promises.test.js | 10 +- test/versioned/bluebird/methods.js | 2 +- .../bluebird/transaction-state.tap.js | 4 +- .../when}/legacy-promise-segments.js | 4 +- test/versioned/when/when.tap.js | 5 +- 27 files changed, 653 insertions(+), 2713 deletions(-) delete mode 100644 .github/workflows/versioned-coverage.yml delete mode 100644 lib/context-manager/legacy-context-manager.js delete mode 100644 lib/instrumentation/core/async-hooks.js delete mode 100644 test/integration/core/native-promises/async-hooks-new-promise-unresolved.tap.js delete mode 100644 test/integration/core/native-promises/async-hooks.js delete mode 100644 test/integration/core/native-promises/async-hooks.tap.js delete mode 100644 test/integration/core/native-promises/native-promises.js delete mode 100644 test/integration/core/promises.js delete mode 100644 test/integration/core/promises.tap.js delete mode 100644 test/integration/instrumentation/promises/segments.js rename test/{integration/instrumentation => lib}/promises/transaction-state.js (99%) delete mode 100644 test/unit/context-manager/legacy-context-manager.test.js rename test/{integration/instrumentation/promises => versioned/when}/legacy-promise-segments.js (99%) diff --git a/.github/workflows/versioned-coverage.yml b/.github/workflows/versioned-coverage.yml deleted file mode 100644 index b72c3d0bfc..0000000000 --- a/.github/workflows/versioned-coverage.yml +++ /dev/null @@ -1,38 +0,0 @@ -# This workflow is intended to be used to run versioned tests for different scenarios(i.e.- legacy context manager, etc) - -name: Nightly Versioned Scenario Runs - -on: - workflow_dispatch: - schedule: - - cron: '0 9 * * 1-5' - -env: - # Enable versioned runner quiet mode to make CI output easier to read: - OUTPUT_MODE: quiet - -jobs: - legacy-context: - runs-on: ubuntu-latest - - strategy: - fail-fast: false - matrix: - node-version: [18.x, 20.x, 22.x] - - steps: - - uses: actions/checkout@v4 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v4 - with: - node-version: ${{ matrix.node-version }} - - name: Install Dependencies - run: npm install - - name: Run Docker Services - run: npm run services - - name: Run Legacy Context Versioned Tests - run: TEST_CHILD_TIMEOUT=600000 npm run versioned:legacy-context - env: - VERSIONED_MODE: --major - JOBS: 4 # 2 per CPU seems to be the sweet spot in GHA (July 2022) - SKIP_C8: true diff --git a/documentation/feature-flags.md b/documentation/feature-flags.md index 22b8a8d438..1a45b72f3d 100644 --- a/documentation/feature-flags.md +++ b/documentation/feature-flags.md @@ -33,12 +33,6 @@ Any prerelease flags can be enabled or disabled in your agent config by adding a * Description: Now that `new_promise_tracking` is the default async context tracking behavior in the agent, `unresolved_promise_cleanup` is enabled by default. Disabling it can help with performance of agent when an application creates many promises. * **WARNING**: If you set `unresolved_promise_cleanup` to `false`, failure to resolve all promises in your application will result in memory leaks even if those promises are garbage collected. -#### legacy_context_manager -* Enabled by default: `false` -* Configuration: `{ feature_flag: { legacy_context_manager: true|false }}` -* Environment Variable: `NEW_RELIC_FEATURE_FLAG_LEGACY_CONTEXT_MANAGER` -* Description: The legacy context manager was replaced by AsyncLocalContextManager for async context propagation. If your application is not recording certain spans or creating orphaned data, you may want to enable this older context manager. Enabling this feature flag may increase the agent's use of memory and CPU. - #### kakfajs_instrumentation * Enabled by default: `false` * Configuration: `{ feature_flag: { kafkajs_instrumentation: true|false }}` diff --git a/lib/context-manager/create-context-manager.js b/lib/context-manager/create-context-manager.js index 8c7d6cf710..336d79c1af 100644 --- a/lib/context-manager/create-context-manager.js +++ b/lib/context-manager/create-context-manager.js @@ -16,10 +16,6 @@ const logger = require('../logger') * the current configuration. */ function createContextManager(config) { - if (config.feature_flag.legacy_context_manager) { - return createLegacyContextManager(config) - } - return createAsyncLocalContextManager(config) } @@ -30,11 +26,4 @@ function createAsyncLocalContextManager(config) { return new AsyncLocalContextManager(config) } -function createLegacyContextManager(config) { - logger.info('Using LegacyContextManager') - - const LegacyContextManager = require('./legacy-context-manager') - return new LegacyContextManager(config) -} - module.exports = createContextManager diff --git a/lib/context-manager/legacy-context-manager.js b/lib/context-manager/legacy-context-manager.js deleted file mode 100644 index 7a42561039..0000000000 --- a/lib/context-manager/legacy-context-manager.js +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2021 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -/** - * Class for managing state in the agent. - * Keeps track of a single context instance. - * - * Given current usage with every instrumented function, the functions in this - * class should do as little work as possible to avoid unnecessary overhead. - * - * @class - */ -class LegacyContextManager { - /** - * @param {object} config New Relic config instance - */ - constructor(config) { - this._config = config - this._context = null - } - - /** - * Get the currently active context. - * - * @returns {object} The current active context. - */ - getContext() { - return this._context - } - - /** - * Set a new active context. Not bound to function execution. - * - * @param {object} newContext The context to set as active. - */ - setContext(newContext) { - this._context = newContext - } - - /** - * Run a function with the passed in context as the active context. - * Restores the previously active context upon completion. - * - * @param {object} context The context to set as active during callback execution. - * @param {Function} callback The function to execute in context. - * @param {Function} [cbThis] Optional `this` to apply to the callback. - * @param {Array<*>} [args] Optional arguments object or args array to invoke the callback with. - * @returns {*} Returns the value returned by the callback function. - */ - runInContext(context, callback, cbThis, args) { - const oldContext = this.getContext() - this.setContext(context) - - try { - return callback.apply(cbThis, args) - } finally { - this.setContext(oldContext) - } - } -} - -module.exports = LegacyContextManager diff --git a/lib/feature_flags.js b/lib/feature_flags.js index 590e34329d..974a4a4a7a 100644 --- a/lib/feature_flags.js +++ b/lib/feature_flags.js @@ -15,7 +15,6 @@ exports.prerelease = { reverse_naming_rules: false, undici_async_tracking: true, unresolved_promise_cleanup: true, - legacy_context_manager: false, kafkajs_instrumentation: false } @@ -43,4 +42,4 @@ exports.released = [ ] // flags that are no longer used for unreleased features -exports.unreleased = ['unreleased'] +exports.unreleased = ['unreleased', 'legacy_context_manager'] diff --git a/lib/instrumentation/core/async-hooks.js b/lib/instrumentation/core/async-hooks.js deleted file mode 100644 index 0b5bc9fb37..0000000000 --- a/lib/instrumentation/core/async-hooks.js +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const logger = require('../../logger').child({ component: 'async_hooks' }) -const asyncHooks = require('async_hooks') - -module.exports = initialize - -function initialize(agent, shim) { - if (!agent.config.feature_flag.legacy_context_manager) { - logger.debug( - 'New AsyncLocalStorage context enabled. Not enabling manual async_hooks or promise instrumentation' - ) - - return - } - - // this map is reused to track the segment that was active when - // the before callback is called to be replaced in the after callback - const segmentMap = new Map() - module.exports.segmentMap = segmentMap - - const hookHandlers = getHookHandlers(segmentMap, agent, shim) - maybeRegisterDestroyHook(segmentMap, agent, hookHandlers) - - const hook = asyncHooks.createHook(hookHandlers) - hook.enable() - - agent.on('unload', function disableHook() { - hook.disable() - }) - - return true -} - -/** - * Registers the async hooks events - * - * Note: The init only fires when the type is PROMISE. - * - * @param {Map} segmentMap map of async ids and segments - * @param {Agent} agent New Relic APM agent - * @param {Shim} shim instance of shim - * @returns {object} event handlers for async hooks - */ -function getHookHandlers(segmentMap, agent, shim) { - return { - init: function initHook(id, type, triggerId) { - if (type !== 'PROMISE') { - return - } - - const parentSegment = segmentMap.get(triggerId) - - if (parentSegment && !parentSegment.transaction.isActive()) { - // Stop propagating if the transaction was ended. - return - } - - if (!parentSegment && !agent.getTransaction()) { - return - } - - const activeSegment = shim.getActiveSegment() || parentSegment - - segmentMap.set(id, activeSegment) - }, - - before: function beforeHook(id) { - const hookSegment = segmentMap.get(id) - - if (!hookSegment) { - return - } - - segmentMap.set(id, shim.getActiveSegment()) - shim.setActiveSegment(hookSegment) - }, - after: function afterHook(id) { - const hookSegment = segmentMap.get(id) - - // hookSegment is the segment that was active before the promise - // executed. If the promise is executing before a segment has been - // restored, hookSegment will be null and should be restored. Thus - // undefined is the only invalid value here. - if (hookSegment === undefined) { - return - } - - segmentMap.set(id, shim.getActiveSegment()) - shim.setActiveSegment(hookSegment) - }, - promiseResolve: function promiseResolveHandler(id) { - const hookSegment = segmentMap.get(id) - segmentMap.delete(id) - - if (hookSegment === undefined) { - return - } - - // Because the ID will no-longer be in memory until dispose to propagate the null - // we need to set it active here or else we may continue to propagate the wrong tree. - // May be some risk of setting this at the wrong time - if (hookSegment === null) { - shim.setActiveSegment(hookSegment) - } - } - } -} - -/** - * Adds the destroy async hook event that will lean up any unresolved promises that have been destroyed. - * This defaults to true but does have a significant performance impact - * when customers have a lot of promises. - * See: https://github.com/newrelic/node-newrelic/issues/760 - * - * @param {Map} segmentMap map of async ids and segments - * @param {Agent} agent New Relic APM agent - * @param {object} hooks async-hook events - */ -function maybeRegisterDestroyHook(segmentMap, agent, hooks) { - if (agent.config.feature_flag.unresolved_promise_cleanup) { - logger.info('Adding destroy hook to clean up unresolved promises.') - hooks.destroy = function destroyHandler(id) { - segmentMap.delete(id) - } - } -} diff --git a/lib/instrumentation/core/globals.js b/lib/instrumentation/core/globals.js index da9174ca13..2cd4a2dfba 100644 --- a/lib/instrumentation/core/globals.js +++ b/lib/instrumentation/core/globals.js @@ -5,7 +5,6 @@ 'use strict' -const asyncHooks = require('./async-hooks') const symbols = require('../../symbols') module.exports = initialize @@ -63,8 +62,4 @@ function initialize(agent, nodule, name, shim) { return original.apply(this, arguments) } } - - // This will initialize the most optimal native-promise instrumentation that - // we have available. - asyncHooks(agent, shim) } diff --git a/lib/instrumentation/core/timers.js b/lib/instrumentation/core/timers.js index bd4f336179..1de2aa0d03 100644 --- a/lib/instrumentation/core/timers.js +++ b/lib/instrumentation/core/timers.js @@ -11,50 +11,10 @@ const Timers = require('timers') module.exports = initialize -function initialize(agent, timers, _moduleName, shim) { - const isLegacyContext = agent.config.feature_flag.legacy_context_manager - - if (isLegacyContext) { - instrumentProcessMethods(shim, process) - instrumentSetImmediate(shim, [timers, global]) - } - +function initialize(_agent, timers, _moduleName, shim) { instrumentTimerMethods(shim, [timers, global]) } -/** - * Sets up instrumentation for setImmediate on both timers and global. - * - * We do not want to create segments for setImmediate calls, - * as the object allocation may incur too much overhead in some situations - * - * @param {Shim} shim instance of shim - * @param {Array} pkgs array with references to timers and global - */ -function instrumentSetImmediate(shim, pkgs) { - pkgs.forEach((nodule) => { - if (shim.isWrapped(nodule.setImmediate)) { - return - } - - shim.wrap(nodule, 'setImmediate', function wrapSetImmediate(shim, fn) { - return function wrappedSetImmediate() { - const segment = shim.getActiveSegment() - if (!segment) { - return fn.apply(this, arguments) - } - - const args = shim.argsToArray.apply(shim, arguments, segment) - shim.bindSegment(args, shim.FIRST) - - return fn.apply(this, args) - } - }) - - copySymbols(shim, nodule, 'setImmediate') - }) -} - /** * Sets up instrumentation for setTimeout, setInterval and clearTimeout * on timers and global. @@ -107,37 +67,6 @@ function recordAsynchronizers(shim, _fn, name) { return new RecorderSpec({ name: 'timers.' + name, callback: shim.FIRST }) } -/** - * Instruments core process methods: nextTick, _nextDomainTick, _tickDomainCallback - * Note: This does not get registered when the context manager is async local - * - * @param {Shim} shim instance of shim - * @param {process} process global process object - */ -function instrumentProcessMethods(shim, process) { - const processMethods = ['nextTick', '_nextDomainTick', '_tickDomainCallback'] - - shim.wrap(process, processMethods, function wrapProcess(shim, fn) { - return function wrappedProcess() { - const segment = shim.getActiveSegment() - if (!segment) { - return fn.apply(this, arguments) - } - - // Manual copy because helper methods add significant overhead in some usages - const len = arguments.length - const args = new Array(len) - for (let i = 0; i < len; ++i) { - args[i] = arguments[i] - } - - shim.bindSegment(args, shim.FIRST, segment) - - return fn.apply(this, args) - } - }) -} - /** * Copies the symbols from original setTimeout and setInterval onto the wrapped functions * diff --git a/package.json b/package.json index 5f8c7e2ca0..838664f753 100644 --- a/package.json +++ b/package.json @@ -187,8 +187,6 @@ "versioned:external": "npm run checkout-external-versioned && SKIP_C8=true EXTERNAL_MODE=only time ./bin/run-versioned-tests.sh", "versioned:major": "VERSIONED_MODE=--major npm run versioned", "versioned": "npm run checkout-external-versioned && npm run prepare-test && time ./bin/run-versioned-tests.sh", - "versioned:legacy-context": "NEW_RELIC_FEATURE_FLAG_LEGACY_CONTEXT_MANAGER=1 npm run versioned", - "versioned:legacy-context:major": "NEW_RELIC_FEATURE_FLAG_LEGACY_CONTEXT_MANAGER=1 npm run versioned:major", "versioned:security": "NEW_RELIC_SECURITY_AGENT_ENABLED=true npm run versioned", "versioned:security:major": "NEW_RELIC_SECURITY_AGENT_ENABLED=true npm run versioned:major", "prepare": "husky install" diff --git a/test/integration/core/native-promises/async-hooks-new-promise-unresolved.tap.js b/test/integration/core/native-promises/async-hooks-new-promise-unresolved.tap.js deleted file mode 100644 index 4b149c8d13..0000000000 --- a/test/integration/core/native-promises/async-hooks-new-promise-unresolved.tap.js +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const exec = require('child_process').execSync -exec( - 'NEW_RELIC_FEATURE_FLAG_LEGACY_CONTEXT_MANAGER=1 NEW_RELIC_FEATURE_FLAG_UNRESOLVED_PROMISE_CLEANUP=false node --expose-gc ./async-hooks.js', - { - stdio: 'inherit', - cwd: __dirname - } -) diff --git a/test/integration/core/native-promises/async-hooks.js b/test/integration/core/native-promises/async-hooks.js deleted file mode 100644 index 949ac76ffc..0000000000 --- a/test/integration/core/native-promises/async-hooks.js +++ /dev/null @@ -1,623 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const test = require('tap').test -const helper = require('../../../lib/agent_helper') -const asyncHooks = require('async_hooks') - -test('await', function (t) { - const { agent } = setupAgent(t) - - helper.runInTransaction(agent, async function (txn) { - let transaction = agent.getTransaction() - t.equal(transaction && transaction.id, txn.id, 'should start in a transaction') - - const segmentMap = require('../../../../lib/instrumentation/core/async-hooks').segmentMap - - const promise = new Promise((resolve) => { - // don't immediately resolve so logic can kick in. - setImmediate(resolve) - }) - - // There may be extra promises in play - const promiseId = [...segmentMap.keys()].pop() - - await promise - - t.notOk(segmentMap.has(promiseId), 'should have removed segment for promise after resolve') - - transaction = agent.getTransaction() - t.equal( - transaction && transaction.id, - txn.id, - 'should resume in the same transaction after await' - ) - - txn.end() - - // Let the loop iterate to clear the microtask queue - setImmediate(() => { - t.equal(segmentMap.size, 0, 'should clear segments after all promises resolved') - t.end() - }) - }) -}) - -test("the agent's async hook", function (t) { - class TestResource extends asyncHooks.AsyncResource { - constructor(id) { - super('PROMISE', id) - } - - doStuff(callback) { - process.nextTick(() => { - if (this.runInAsyncScope) { - this.runInAsyncScope(callback) - } else { - this.emitBefore() - callback() - this.emitAfter() - } - }) - } - } - - t.autoend() - t.test('does not crash on multiple resolve calls', function (t) { - const { agent } = setupAgent(t) - helper.runInTransaction(agent, function () { - t.doesNotThrow(function () { - new Promise(function (resolve) { - resolve() - resolve() - }).then(t.end) - }) - }) - }) - - t.test('does not restore a segment for a resource created outside a transaction', function (t) { - const { agent, contextManager } = setupAgent(t) - - const testResource = new TestResource(1) - helper.runInTransaction(agent, function () { - const root = contextManager.getContext() - const segmentMap = require('../../../../lib/instrumentation/core/async-hooks').segmentMap - - t.equal(segmentMap.size, 0, 'no segments should be tracked') - testResource.doStuff(function () { - t.ok(contextManager.getContext(), 'should be in a transaction') - t.equal( - contextManager.getContext().name, - root.name, - 'loses transaction state for resources created outside of a transaction' - ) - t.end() - }) - }) - }) - - t.test('restores context in inactive transactions', function (t) { - const { agent, contextManager } = setupAgent(t) - - helper.runInTransaction(agent, function (txn) { - const testResource = new TestResource(1) - const root = contextManager.getContext() - txn.end() - testResource.doStuff(function () { - t.equal( - contextManager.getContext(), - root, - 'the hooks restore a segment when its transaction has been ended' - ) - t.end() - }) - }) - }) - - /** - * Represents same test as 'parent promises persist perspective to problematic progeny' - * from async_hooks.js. - * - * This specific use case is not currently supported with the implementation that clears - * segment references on promise resolve. - */ - t.test( - 'parent promises that are already resolved DO NOT persist to continuations ' + - 'scheduled after a timer async hop.', - function (t) { - const { agent } = setupAgent(t) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const p = Promise.resolve() - - tasks.push(() => { - p.then(() => { - const tx = agent.getTransaction() - t.not( - tx ? tx.id : null, - txn.id, - 'If this failed, this use case now works! Time to switch to "t.equal"' - ) - t.end() - }) - }) - }) - } - ) - - /** - * Variation of 'parent promises persist perspective to problematic progeny' from async_hooks.js. - * - * For unresolved parent promises, persistance should stil work as expected. - */ - t.test('unresolved parent promises persist perspective to problematic progeny', function (t) { - const { agent } = setupAgent(t) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - let parentResolve = null - const p = new Promise((resolve) => { - parentResolve = resolve - }) - - tasks.push(() => { - p.then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - - t.end() - }) - - // resolve parent after continuation scheduled - parentResolve() - }) - }) - }) - - /** - * Represents same test as 'maintains transaction context' from async_hooks.js. - * - * Combination of a timer that does not propagate state and the new resolve - * mechanism that clears (and sets hook as active) causes this to fail. - */ - t.test('DOES NOT maintain transaction context', function (t) { - const { agent } = setupAgent(t) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - const segment = txn.trace.root - agent.tracer.bindFunction(one, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - } - - function executor(resolve) { - tasks.push(() => { - next().then(() => { - const tx = agent.getTransaction() - t.not( - tx ? tx.id : null, - txn.id, - 'If this failed, this use case now works! Time to switch to "t.equal"' - ) - resolve() - }) - }) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) - - t.test('maintains transaction context for unresolved promises', function (t) { - const { agent } = setupAgent(t) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function executor(resolve) { - setImmediate(() => { - next().then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - resolve() - }) - }) - } - - function next() { - return new Promise((resolve) => { - const val = wrapperTwo() - setImmediate(() => { - resolve(val) - }) - }) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return new Promise((resolve) => { - const val = wrapperThree() - setImmediate(() => { - resolve(val) - }) - }) - } - - function three() {} - }) - }) - - t.test('stops propagation on transaction end', function (t) { - const { agent, contextManager } = setupAgent(t) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - const segment = txn.trace.root - agent.tracer.bindFunction(one, segment)() - - function one() { - return new Promise((done) => { - const currentSegment = contextManager.getContext() - t.ok(currentSegment, 'should have propagated a segment') - txn.end() - - done() - }).then(() => { - const currentSegment = contextManager.getContext() - t.notOk(currentSegment, 'should not have a propagated segment') - t.end() - }) - } - }) - }) - - t.test('loses transaction context', function (t) { - const { agent } = setupAgent(t) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - - function executor(resolve) { - tasks.push(() => { - next().then(() => { - const tx = agent.getTransaction() - // We know tx will be null here because no promise was returned - // If this test fails, that's actually a good thing, - // so throw a party/update Koa. - t.equal(tx, null) - resolve() - }) - }) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - // No promise is returned to reinstate transaction context - } - }) - }) - - t.test('handles multientry callbacks correctly', function (t) { - const { agent, contextManager } = setupAgent(t) - - const segmentMap = require('../../../../lib/instrumentation/core/async-hooks').segmentMap - helper.runInTransaction(agent, function () { - const root = contextManager.getContext() - - const aSeg = agent.tracer.createSegment('A') - contextManager.setContext(aSeg) - const resA = new TestResource(1) - - const bSeg = agent.tracer.createSegment('B') - contextManager.setContext(bSeg) - const resB = new TestResource(2) - - contextManager.setContext(root) - - t.equal(segmentMap.size, 2, 'all resources should create an entry on init') - - resA.doStuff(() => { - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - - resB.doStuff(() => { - t.equal( - contextManager.getContext().name, - bSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - - t.end() - }) - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a callback was called' - ) - }) - t.equal( - contextManager.getContext().name, - root.name, - 'root should be restored after we are finished' - ) - resA.doStuff(() => { - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - }) - }) - }) - - t.test( - 'cleans up unresolved promises on destroy', - { skip: process.env.NEW_RELIC_FEATURE_FLAG_UNRESOLVED_PROMISE_CLEANUP === 'false' }, - (t) => { - const { agent } = setupAgent(t) - const segmentMap = require('../../../../lib/instrumentation/core/async-hooks').segmentMap - - helper.runInTransaction(agent, () => { - /* eslint-disable no-unused-vars */ - let promise = unresolvedPromiseFunc() - - t.equal(segmentMap.size, 1, 'segment map should have 1 element') - - promise = null - - global.gc && global.gc() - - setImmediate(() => { - t.equal(segmentMap.size, 0, 'segment map should clean up unresolved promises on destroy') - t.end() - }) - }) - - function unresolvedPromiseFunc() { - return new Promise(() => {}) - } - } - ) - - t.test( - 'does not clean up unresolved promises on destroy when `unresolved_promise_cleanup` is set to false', - { skip: process.env.NEW_RELIC_FEATURE_FLAG_UNRESOLVED_PROMISE_CLEANUP !== 'false' }, - (t) => { - const { agent } = setupAgent(t) - const segmentMap = require('../../../../lib/instrumentation/core/async-hooks').segmentMap - - helper.runInTransaction(agent, () => { - /* eslint-disable no-unused-vars */ - let promise = unresolvedPromiseFunc() - - t.equal(segmentMap.size, 1, 'segment map should have 1 element') - - promise = null - - global.gc && global.gc() - - setImmediate(() => { - t.equal( - segmentMap.size, - 1, - 'segment map should not clean up unresolved promise on destroy' - ) - t.end() - }) - }) - - function unresolvedPromiseFunc() { - return new Promise(() => {}) - } - } - ) -}) - -function checkCallMetrics(t, testMetrics) { - // Tap also creates promises, so these counts don't quite match the tests. - const TAP_COUNT = 1 - - t.equal(testMetrics.initCalled - TAP_COUNT, 2, 'two promises were created') - t.equal(testMetrics.beforeCalled, 1, 'before hook called for all async promises') - t.equal( - testMetrics.beforeCalled, - testMetrics.afterCalled, - 'before should be called as many times as after' - ) - - if (global.gc) { - global.gc() - return setTimeout(function () { - t.equal( - testMetrics.initCalled - TAP_COUNT, - testMetrics.destroyCalled, - 'all promises created were destroyed' - ) - t.end() - }, 10) - } - t.end() -} - -test('promise hooks', function (t) { - t.autoend() - const testMetrics = { - initCalled: 0, - beforeCalled: 0, - afterCalled: 0, - destroyCalled: 0 - } - - const promiseIds = {} - const hook = asyncHooks.createHook({ - init: function initHook(id, type) { - if (type === 'PROMISE') { - promiseIds[id] = true - testMetrics.initCalled++ - } - }, - before: function beforeHook(id) { - if (promiseIds[id]) { - testMetrics.beforeCalled++ - } - }, - after: function afterHook(id) { - if (promiseIds[id]) { - testMetrics.afterCalled++ - } - }, - destroy: function destHook(id) { - if (promiseIds[id]) { - testMetrics.destroyCalled++ - } - } - }) - hook.enable() - - t.test('are only called once during the lifetime of a promise', function (t) { - new Promise(function (resolve) { - setTimeout(resolve, 10) - }).then(function () { - setImmediate(checkCallMetrics, t, testMetrics) - }) - }) -}) - -function setupAgent(t) { - const agent = helper.instrumentMockedAgent({ - feature_flag: { - await_support: true - } - }) - - const contextManager = helper.getContextManager() - - t.teardown(function () { - helper.unloadAgent(agent) - }) - - return { - agent, - contextManager - } -} diff --git a/test/integration/core/native-promises/async-hooks.tap.js b/test/integration/core/native-promises/async-hooks.tap.js deleted file mode 100644 index 05414f3e76..0000000000 --- a/test/integration/core/native-promises/async-hooks.tap.js +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const exec = require('child_process').execSync -exec('NEW_RELIC_FEATURE_FLAG_LEGACY_CONTEXT_MANAGER=1 node --expose-gc ./async-hooks.js', { - stdio: 'inherit', - cwd: __dirname -}) diff --git a/test/integration/core/native-promises/native-promises.js b/test/integration/core/native-promises/native-promises.js deleted file mode 100644 index dc2d55d609..0000000000 --- a/test/integration/core/native-promises/native-promises.js +++ /dev/null @@ -1,636 +0,0 @@ -/* - * Copyright 2021 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { test } = require('tap') - -const helper = require('../../../lib/agent_helper') -const asyncHooks = require('async_hooks') - -test('AsyncLocalStorage based tracking', (t) => { - t.autoend() - - const config = {} - - createPromiseTests(t, config) - - // Negative assertion case mirroring test for async-hooks. - // This is a new limitation due to AsyncLocalStorage propagation only on init. - // The timer-hop without context prior to .then() continuation causes the state loss. - t.test('DOES NOT maintain transaction context over contextless timer', (t) => { - const { agent } = setupAgent(t, config) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const segment = txn.trace.root - agent.tracer.bindFunction(one, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - } - - function executor(resolve) { - tasks.push(() => { - next().then(() => { - const tx = agent.getTransaction() - t.notOk( - tx, - 'If fails, we have fixed a limitation and should check equal transaction IDs' - ) - resolve() - }) - }) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) - - // Negative assertion case mirroring test for async-hooks. - // This is a new limitation due to AsyncLocalStorage propagation only on init. - // The timer-hop without context prior to .then() continuation causes the state loss. - t.test('parent promises DO NOT persist perspective to problematic progeny', (t) => { - const { agent } = setupAgent(t, config) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const p = Promise.resolve() - - tasks.push(() => { - p.then(() => { - const tx = agent.getTransaction() - - t.notOk(tx, 'If fails, we have fixed a limitation and should check equal transaction IDs') - t.end() - }) - }) - }) - }) - - // Negative assertion case mirroring test for async-hooks. - // This is a new limitation due to AsyncLocalStorage propagation only on init. - // The timer-hop without context prior to .then() continuation causes the state loss. - t.test('unresolved parent promises DO NOT persist to problematic progeny', (t) => { - const { agent } = setupAgent(t, config) - const tasks = [] - const intervalId = setInterval(() => { - while (tasks.length) { - tasks.pop()() - } - }, 10) - - t.teardown(() => { - clearInterval(intervalId) - }) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - let parentResolve = null - const p = new Promise((resolve) => { - parentResolve = resolve - }) - - tasks.push(() => { - p.then(() => { - const tx = agent.getTransaction() - t.notOk(tx, 'If fails, we have fixed a limitation and should check equal transaction IDs') - - t.end() - }) - - // resolve parent after continuation scheduled - parentResolve() - }) - }) - }) -}) - -function createPromiseTests(t, config) { - t.test('maintains context across await', function (t) { - const { agent } = setupAgent(t, config) - helper.runInTransaction(agent, async function (txn) { - let transaction = agent.getTransaction() - t.equal(transaction && transaction.id, txn.id, 'should start in a transaction') - - await Promise.resolve("i'll be back") - - transaction = agent.getTransaction() - t.equal( - transaction && transaction.id, - txn.id, - 'should resume in the same transaction after await' - ) - - txn.end() - t.end() - }) - }) - - t.test('maintains context across multiple awaits', async (t) => { - const { agent } = setupAgent(t, config) - await helper.runInTransaction(agent, async function (createdTransaction) { - let transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id, 'should start in a transaction') - - await firstFunction() - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - - await secondFunction() - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - - createdTransaction.end() - - async function firstFunction() { - await childFunction() - - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - } - - async function childFunction() { - await new Promise((resolve) => { - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - - setTimeout(resolve, 1) - }) - } - - async function secondFunction() { - await new Promise((resolve) => { - setImmediate(resolve) - }) - } - }) - }) - - t.test('maintains context across promise chain', (t) => { - const { agent } = setupAgent(t, config) - helper.runInTransaction(agent, function (createdTransaction) { - let transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id, 'should start in a transaction') - firstFunction() - .then(() => { - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - return secondFunction() - }) - .then(() => { - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - createdTransaction.end() - t.end() - }) - - function firstFunction() { - return childFunction() - } - - function childFunction() { - return new Promise((resolve) => { - transaction = agent.getTransaction() - t.equal(transaction && transaction.id, createdTransaction.id) - - setTimeout(resolve, 1) - }) - } - - function secondFunction() { - return new Promise((resolve) => { - setImmediate(resolve) - }) - } - }) - }) - - t.test('does not crash on multiple resolve calls', function (t) { - const { agent } = setupAgent(t, config) - helper.runInTransaction(agent, function () { - t.doesNotThrow(function () { - new Promise(function (res) { - res() - res() - }).then(t.end) - }) - }) - }) - - t.test('restores context in inactive transactions', function (t) { - const { agent, contextManager } = setupAgent(t, config) - - helper.runInTransaction(agent, function (txn) { - const res = new TestResource(1) - const root = contextManager.getContext() - txn.end() - res.doStuff(function () { - t.equal( - contextManager.getContext(), - root, - 'should restore a segment when its transaction has been ended' - ) - t.end() - }) - }) - }) - - t.test('handles multi-entry callbacks correctly', function (t) { - const { agent, contextManager } = setupAgent(t, config) - - helper.runInTransaction(agent, function () { - const root = contextManager.getContext() - - const aSeg = agent.tracer.createSegment('A') - contextManager.setContext(aSeg) - - const resA = new TestResource(1) - - const bSeg = agent.tracer.createSegment('B') - contextManager.setContext(bSeg) - const resB = new TestResource(2) - - contextManager.setContext(root) - - resA.doStuff(() => { - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - - resB.doStuff(() => { - t.equal( - contextManager.getContext().name, - bSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - - t.end() - }) - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a callback was called' - ) - }) - t.equal( - contextManager.getContext().name, - root.name, - 'root should be restored after we are finished' - ) - resA.doStuff(() => { - t.equal( - contextManager.getContext().name, - aSeg.name, - 'runInAsyncScope should restore the segment active when a resource was made' - ) - }) - }) - }) - - t.test('maintains transaction context over setImmediate in-context', (t) => { - const { agent } = setupAgent(t, config) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function executor(resolve) { - setImmediate(() => { - next().then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - resolve() - }) - }) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) - - t.test('maintains transaction context over process.nextTick in-context', (t) => { - const { agent } = setupAgent(t, config) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function executor(resolve) { - process.nextTick(() => { - next().then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - resolve() - }) - }) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) - - t.test('maintains transaction context over setTimeout in-context', (t) => { - const { agent } = setupAgent(t, config) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function executor(resolve) { - setTimeout(() => { - next().then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - resolve() - }) - }, 1) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) - - t.test('maintains transaction context over setInterval in-context', (t) => { - const { agent } = setupAgent(t, config) - - helper.runInTransaction(agent, function (txn) { - t.ok(txn, 'transaction should not be null') - - const segment = txn.trace.root - agent.tracer.bindFunction(function one() { - return new Promise(executor).then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - t.end() - }) - }, segment)() - - const wrapperTwo = agent.tracer.bindFunction(function () { - return two() - }, segment) - const wrapperThree = agent.tracer.bindFunction(function () { - return three() - }, segment) - - function executor(resolve) { - const ref = setInterval(() => { - clearInterval(ref) - - next().then(() => { - const tx = agent.getTransaction() - t.equal(tx ? tx.id : null, txn.id) - resolve() - }) - }, 1) - } - - function next() { - return Promise.resolve(wrapperTwo()) - } - - function two() { - return nextTwo() - } - - function nextTwo() { - return Promise.resolve(wrapperThree()) - } - - function three() {} - }) - }) -} - -function checkCallMetrics(t, testMetrics) { - // Tap also creates promises, so these counts don't quite match the tests. - const TAP_COUNT = 1 - - t.equal(testMetrics.initCalled - TAP_COUNT, 2, 'two promises were created') - t.equal(testMetrics.beforeCalled, 1, 'before hook called for all async promises') - t.equal( - testMetrics.beforeCalled, - testMetrics.afterCalled, - 'before should be called as many times as after' - ) - - if (global.gc) { - global.gc() - return setTimeout(function () { - t.equal( - testMetrics.initCalled - TAP_COUNT, - testMetrics.destroyCalled, - 'all promises created were destroyed' - ) - t.end() - }, 10) - } - t.end() -} - -test('promise hooks', function (t) { - t.autoend() - const testMetrics = { - initCalled: 0, - beforeCalled: 0, - afterCalled: 0, - destroyCalled: 0 - } - - const promiseIds = {} - const hook = asyncHooks.createHook({ - init: function initHook(id, type) { - if (type === 'PROMISE') { - promiseIds[id] = true - testMetrics.initCalled++ - } - }, - before: function beforeHook(id) { - if (promiseIds[id]) { - testMetrics.beforeCalled++ - } - }, - after: function afterHook(id) { - if (promiseIds[id]) { - testMetrics.afterCalled++ - } - }, - destroy: function destHook(id) { - if (promiseIds[id]) { - testMetrics.destroyCalled++ - } - } - }) - hook.enable() - - t.test('are only called once during the lifetime of a promise', function (t) { - new Promise(function (res) { - setTimeout(res, 10) - }).then(function () { - setImmediate(checkCallMetrics, t, testMetrics) - }) - }) -}) - -function setupAgent(t, config) { - const agent = helper.instrumentMockedAgent(config) - t.teardown(function () { - helper.unloadAgent(agent) - }) - - const contextManager = helper.getContextManager() - - return { - agent, - contextManager - } -} - -class TestResource extends asyncHooks.AsyncResource { - constructor(id) { - super('PROMISE', id) - } - - doStuff(callback) { - process.nextTick(() => { - if (this.runInAsyncScope) { - this.runInAsyncScope(callback) - } else { - this.emitBefore() - callback() - this.emitAfter() - } - }) - } -} diff --git a/test/integration/core/native-promises/native-promises.tap.js b/test/integration/core/native-promises/native-promises.tap.js index 3297f1fafb..dc2d55d609 100644 --- a/test/integration/core/native-promises/native-promises.tap.js +++ b/test/integration/core/native-promises/native-promises.tap.js @@ -5,8 +5,632 @@ 'use strict' -const exec = require('child_process').execSync -exec('node --expose-gc ./native-promises.js', { - stdio: 'inherit', - cwd: __dirname +const { test } = require('tap') + +const helper = require('../../../lib/agent_helper') +const asyncHooks = require('async_hooks') + +test('AsyncLocalStorage based tracking', (t) => { + t.autoend() + + const config = {} + + createPromiseTests(t, config) + + // Negative assertion case mirroring test for async-hooks. + // This is a new limitation due to AsyncLocalStorage propagation only on init. + // The timer-hop without context prior to .then() continuation causes the state loss. + t.test('DOES NOT maintain transaction context over contextless timer', (t) => { + const { agent } = setupAgent(t, config) + const tasks = [] + const intervalId = setInterval(() => { + while (tasks.length) { + tasks.pop()() + } + }, 10) + + t.teardown(() => { + clearInterval(intervalId) + }) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const segment = txn.trace.root + agent.tracer.bindFunction(one, segment)() + + const wrapperTwo = agent.tracer.bindFunction(function () { + return two() + }, segment) + const wrapperThree = agent.tracer.bindFunction(function () { + return three() + }, segment) + + function one() { + return new Promise(executor).then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + t.end() + }) + } + + function executor(resolve) { + tasks.push(() => { + next().then(() => { + const tx = agent.getTransaction() + t.notOk( + tx, + 'If fails, we have fixed a limitation and should check equal transaction IDs' + ) + resolve() + }) + }) + } + + function next() { + return Promise.resolve(wrapperTwo()) + } + + function two() { + return nextTwo() + } + + function nextTwo() { + return Promise.resolve(wrapperThree()) + } + + function three() {} + }) + }) + + // Negative assertion case mirroring test for async-hooks. + // This is a new limitation due to AsyncLocalStorage propagation only on init. + // The timer-hop without context prior to .then() continuation causes the state loss. + t.test('parent promises DO NOT persist perspective to problematic progeny', (t) => { + const { agent } = setupAgent(t, config) + const tasks = [] + const intervalId = setInterval(() => { + while (tasks.length) { + tasks.pop()() + } + }, 10) + + t.teardown(() => { + clearInterval(intervalId) + }) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const p = Promise.resolve() + + tasks.push(() => { + p.then(() => { + const tx = agent.getTransaction() + + t.notOk(tx, 'If fails, we have fixed a limitation and should check equal transaction IDs') + t.end() + }) + }) + }) + }) + + // Negative assertion case mirroring test for async-hooks. + // This is a new limitation due to AsyncLocalStorage propagation only on init. + // The timer-hop without context prior to .then() continuation causes the state loss. + t.test('unresolved parent promises DO NOT persist to problematic progeny', (t) => { + const { agent } = setupAgent(t, config) + const tasks = [] + const intervalId = setInterval(() => { + while (tasks.length) { + tasks.pop()() + } + }, 10) + + t.teardown(() => { + clearInterval(intervalId) + }) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + let parentResolve = null + const p = new Promise((resolve) => { + parentResolve = resolve + }) + + tasks.push(() => { + p.then(() => { + const tx = agent.getTransaction() + t.notOk(tx, 'If fails, we have fixed a limitation and should check equal transaction IDs') + + t.end() + }) + + // resolve parent after continuation scheduled + parentResolve() + }) + }) + }) }) + +function createPromiseTests(t, config) { + t.test('maintains context across await', function (t) { + const { agent } = setupAgent(t, config) + helper.runInTransaction(agent, async function (txn) { + let transaction = agent.getTransaction() + t.equal(transaction && transaction.id, txn.id, 'should start in a transaction') + + await Promise.resolve("i'll be back") + + transaction = agent.getTransaction() + t.equal( + transaction && transaction.id, + txn.id, + 'should resume in the same transaction after await' + ) + + txn.end() + t.end() + }) + }) + + t.test('maintains context across multiple awaits', async (t) => { + const { agent } = setupAgent(t, config) + await helper.runInTransaction(agent, async function (createdTransaction) { + let transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id, 'should start in a transaction') + + await firstFunction() + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + + await secondFunction() + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + + createdTransaction.end() + + async function firstFunction() { + await childFunction() + + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + } + + async function childFunction() { + await new Promise((resolve) => { + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + + setTimeout(resolve, 1) + }) + } + + async function secondFunction() { + await new Promise((resolve) => { + setImmediate(resolve) + }) + } + }) + }) + + t.test('maintains context across promise chain', (t) => { + const { agent } = setupAgent(t, config) + helper.runInTransaction(agent, function (createdTransaction) { + let transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id, 'should start in a transaction') + firstFunction() + .then(() => { + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + return secondFunction() + }) + .then(() => { + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + createdTransaction.end() + t.end() + }) + + function firstFunction() { + return childFunction() + } + + function childFunction() { + return new Promise((resolve) => { + transaction = agent.getTransaction() + t.equal(transaction && transaction.id, createdTransaction.id) + + setTimeout(resolve, 1) + }) + } + + function secondFunction() { + return new Promise((resolve) => { + setImmediate(resolve) + }) + } + }) + }) + + t.test('does not crash on multiple resolve calls', function (t) { + const { agent } = setupAgent(t, config) + helper.runInTransaction(agent, function () { + t.doesNotThrow(function () { + new Promise(function (res) { + res() + res() + }).then(t.end) + }) + }) + }) + + t.test('restores context in inactive transactions', function (t) { + const { agent, contextManager } = setupAgent(t, config) + + helper.runInTransaction(agent, function (txn) { + const res = new TestResource(1) + const root = contextManager.getContext() + txn.end() + res.doStuff(function () { + t.equal( + contextManager.getContext(), + root, + 'should restore a segment when its transaction has been ended' + ) + t.end() + }) + }) + }) + + t.test('handles multi-entry callbacks correctly', function (t) { + const { agent, contextManager } = setupAgent(t, config) + + helper.runInTransaction(agent, function () { + const root = contextManager.getContext() + + const aSeg = agent.tracer.createSegment('A') + contextManager.setContext(aSeg) + + const resA = new TestResource(1) + + const bSeg = agent.tracer.createSegment('B') + contextManager.setContext(bSeg) + const resB = new TestResource(2) + + contextManager.setContext(root) + + resA.doStuff(() => { + t.equal( + contextManager.getContext().name, + aSeg.name, + 'runInAsyncScope should restore the segment active when a resource was made' + ) + + resB.doStuff(() => { + t.equal( + contextManager.getContext().name, + bSeg.name, + 'runInAsyncScope should restore the segment active when a resource was made' + ) + + t.end() + }) + t.equal( + contextManager.getContext().name, + aSeg.name, + 'runInAsyncScope should restore the segment active when a callback was called' + ) + }) + t.equal( + contextManager.getContext().name, + root.name, + 'root should be restored after we are finished' + ) + resA.doStuff(() => { + t.equal( + contextManager.getContext().name, + aSeg.name, + 'runInAsyncScope should restore the segment active when a resource was made' + ) + }) + }) + }) + + t.test('maintains transaction context over setImmediate in-context', (t) => { + const { agent } = setupAgent(t, config) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const segment = txn.trace.root + agent.tracer.bindFunction(function one() { + return new Promise(executor).then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + t.end() + }) + }, segment)() + + const wrapperTwo = agent.tracer.bindFunction(function () { + return two() + }, segment) + const wrapperThree = agent.tracer.bindFunction(function () { + return three() + }, segment) + + function executor(resolve) { + setImmediate(() => { + next().then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + resolve() + }) + }) + } + + function next() { + return Promise.resolve(wrapperTwo()) + } + + function two() { + return nextTwo() + } + + function nextTwo() { + return Promise.resolve(wrapperThree()) + } + + function three() {} + }) + }) + + t.test('maintains transaction context over process.nextTick in-context', (t) => { + const { agent } = setupAgent(t, config) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const segment = txn.trace.root + agent.tracer.bindFunction(function one() { + return new Promise(executor).then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + t.end() + }) + }, segment)() + + const wrapperTwo = agent.tracer.bindFunction(function () { + return two() + }, segment) + const wrapperThree = agent.tracer.bindFunction(function () { + return three() + }, segment) + + function executor(resolve) { + process.nextTick(() => { + next().then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + resolve() + }) + }) + } + + function next() { + return Promise.resolve(wrapperTwo()) + } + + function two() { + return nextTwo() + } + + function nextTwo() { + return Promise.resolve(wrapperThree()) + } + + function three() {} + }) + }) + + t.test('maintains transaction context over setTimeout in-context', (t) => { + const { agent } = setupAgent(t, config) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const segment = txn.trace.root + agent.tracer.bindFunction(function one() { + return new Promise(executor).then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + t.end() + }) + }, segment)() + + const wrapperTwo = agent.tracer.bindFunction(function () { + return two() + }, segment) + const wrapperThree = agent.tracer.bindFunction(function () { + return three() + }, segment) + + function executor(resolve) { + setTimeout(() => { + next().then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + resolve() + }) + }, 1) + } + + function next() { + return Promise.resolve(wrapperTwo()) + } + + function two() { + return nextTwo() + } + + function nextTwo() { + return Promise.resolve(wrapperThree()) + } + + function three() {} + }) + }) + + t.test('maintains transaction context over setInterval in-context', (t) => { + const { agent } = setupAgent(t, config) + + helper.runInTransaction(agent, function (txn) { + t.ok(txn, 'transaction should not be null') + + const segment = txn.trace.root + agent.tracer.bindFunction(function one() { + return new Promise(executor).then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + t.end() + }) + }, segment)() + + const wrapperTwo = agent.tracer.bindFunction(function () { + return two() + }, segment) + const wrapperThree = agent.tracer.bindFunction(function () { + return three() + }, segment) + + function executor(resolve) { + const ref = setInterval(() => { + clearInterval(ref) + + next().then(() => { + const tx = agent.getTransaction() + t.equal(tx ? tx.id : null, txn.id) + resolve() + }) + }, 1) + } + + function next() { + return Promise.resolve(wrapperTwo()) + } + + function two() { + return nextTwo() + } + + function nextTwo() { + return Promise.resolve(wrapperThree()) + } + + function three() {} + }) + }) +} + +function checkCallMetrics(t, testMetrics) { + // Tap also creates promises, so these counts don't quite match the tests. + const TAP_COUNT = 1 + + t.equal(testMetrics.initCalled - TAP_COUNT, 2, 'two promises were created') + t.equal(testMetrics.beforeCalled, 1, 'before hook called for all async promises') + t.equal( + testMetrics.beforeCalled, + testMetrics.afterCalled, + 'before should be called as many times as after' + ) + + if (global.gc) { + global.gc() + return setTimeout(function () { + t.equal( + testMetrics.initCalled - TAP_COUNT, + testMetrics.destroyCalled, + 'all promises created were destroyed' + ) + t.end() + }, 10) + } + t.end() +} + +test('promise hooks', function (t) { + t.autoend() + const testMetrics = { + initCalled: 0, + beforeCalled: 0, + afterCalled: 0, + destroyCalled: 0 + } + + const promiseIds = {} + const hook = asyncHooks.createHook({ + init: function initHook(id, type) { + if (type === 'PROMISE') { + promiseIds[id] = true + testMetrics.initCalled++ + } + }, + before: function beforeHook(id) { + if (promiseIds[id]) { + testMetrics.beforeCalled++ + } + }, + after: function afterHook(id) { + if (promiseIds[id]) { + testMetrics.afterCalled++ + } + }, + destroy: function destHook(id) { + if (promiseIds[id]) { + testMetrics.destroyCalled++ + } + } + }) + hook.enable() + + t.test('are only called once during the lifetime of a promise', function (t) { + new Promise(function (res) { + setTimeout(res, 10) + }).then(function () { + setImmediate(checkCallMetrics, t, testMetrics) + }) + }) +}) + +function setupAgent(t, config) { + const agent = helper.instrumentMockedAgent(config) + t.teardown(function () { + helper.unloadAgent(agent) + }) + + const contextManager = helper.getContextManager() + + return { + agent, + contextManager + } +} + +class TestResource extends asyncHooks.AsyncResource { + constructor(id) { + super('PROMISE', id) + } + + doStuff(callback) { + process.nextTick(() => { + if (this.runInAsyncScope) { + this.runInAsyncScope(callback) + } else { + this.emitBefore() + callback() + this.emitAfter() + } + }) + } +} diff --git a/test/integration/core/promises.js b/test/integration/core/promises.js deleted file mode 100644 index c5f145ef78..0000000000 --- a/test/integration/core/promises.js +++ /dev/null @@ -1,682 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const genericTestDir = '../../integration/instrumentation/promises/' -const helper = require('../../lib/agent_helper') -const util = require('util') -const testPromiseSegments = require(genericTestDir + 'segments') -const testTransactionState = require(genericTestDir + 'transaction-state') - -module.exports = function runTests(t, flags) { - t.test('transaction state', function (t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - t.autoend() - testTransactionState(t, agent, Promise) - }) - - // XXX Promise segments in native instrumentation are currently less than ideal - // XXX in structure. Transaction state is correctly maintained, and all segments - // XXX are created, but the heirarchy is not correct. - t.test('segments', function (t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - t.autoend() - testPromiseSegments(t, agent, Promise) - }) - - t.test('then', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(executor).then(done, fail) - - function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - accept(15) - reject(10) - }, 0) - } - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('multi then', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - accept(15) - reject(10) - }, 0) - }) - .then(next, fail) - .then(function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }, fail) - - function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - return val - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('multi then async', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - accept(15) - reject(10) - }, 0) - }) - .then(next, fail) - .then(function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }, fail) - - function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - return new Promise(function wait(accept) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - accept(val) - }, 0) - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('then reject', function testThenReject(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(executor).then(fail, done) - - function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - } - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('multi then reject', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - }) - .then(fail, next) - .then(fail, done) - - function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - throw val - } - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('multi then async reject', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - }) - .then(fail, next) - .then(fail, function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - - function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - return new Promise(function wait(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(val) - }, 0) - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('catch', function testCatch(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - }).catch(function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - }) - }) - - t.test('multi catch', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - }) - .catch(function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - throw val - }) - .catch(function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - }) - }) - - t.test('multi catch async', function testThen(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function executor(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(10) - accept(15) - }, 0) - }) - .catch(function next(val) { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - return new Promise(function wait(accept, reject) { - segment = agent.tracer.getSegment() - setTimeout(function resolve() { - reject(val) - }, 0) - }) - }) - .catch(function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 10, 'should resolve with the correct value') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - }) - }) - - t.test('Promise.resolve', function testResolve(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function resolve() { - Promise.resolve(15) - .then(function (val) { - segment = agent.tracer.getSegment() - return val - }) - .then(done, fail) - }, 0) - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.equal(val, 15, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('Promise.reject', function testReject(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function reject() { - Promise.reject(10) - .then(null, function (error) { - segment = agent.tracer.getSegment() - throw error - }) - .then(fail, function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal( - id(agent.getTransaction()), - id(transaction), - 'transaction should be preserved' - ) - t.equal(val, 10, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - }, 0) - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('Promise.all', function testAll(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function resolve() { - const a = Promise.resolve(15) - const b = Promise.resolve(25) - Promise.all([a, b]) - .then(function (val) { - segment = agent.tracer.getSegment() - return val - }) - .then(done, fail) - }, 0) - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.same(val, [15, 25], 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('Promise.all reject', function testAllReject(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function reject() { - const a = Promise.resolve(15) - const b = Promise.reject(10) - Promise.all([a, b]) - .then(null, function (err) { - segment = agent.tracer.getSegment() - throw err - }) - .then(fail, done) - }, 0) - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.same(val, 10, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('Promise.race', function testRace(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function () { - const a = Promise.resolve(15) - const b = new Promise(function (resolve) { - setTimeout(resolve, 100) - }) - Promise.race([a, b]) - .then(function (val) { - segment = agent.tracer.getSegment() - return val - }) - .then(done, fail) - }, 0) - - function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal(id(agent.getTransaction()), id(transaction), 'transaction should be preserved') - t.same(val, 15, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('Promise.race reject', function testRaceReject(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment - - helper.runInTransaction(agent, function inTransaction(transaction) { - setTimeout(function reject() { - const a = new Promise(function (resolve) { - setTimeout(resolve, 100) - }) - const b = Promise.reject(10) - Promise.race([a, b]) - .then(null, function (err) { - segment = agent.tracer.getSegment() - throw err - }) - .then(fail, function done(val) { - t.equal(this, void 0, 'context should be undefined') - process.nextTick(function finish() { - t.equal( - id(agent.getTransaction()), - id(transaction), - 'transaction should be preserved' - ) - t.same(val, 10, 'value should be preserved') - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - }) - }, 0) - - function fail() { - t.fail('should not be called') - t.end() - } - }) - }) - - t.test('should throw when called without executor', function testNoExecutor(t) { - const OriginalPromise = Promise - let unwrappedError - let wrappedError - let wrapped - let unwrapped - helper.loadTestAgent(t, { feature_flag: flags }) - - try { - unwrapped = new OriginalPromise(null) - } catch (err) { - unwrappedError = err - } - - try { - wrapped = new Promise(null) - } catch (err) { - wrappedError = err - } - - t.equal(wrapped, void 0, 'should not be set') - t.equal(unwrapped, void 0, 'should not be set') - t.ok(unwrappedError instanceof Error, 'should error') - t.ok(wrappedError instanceof Error, 'should error') - t.equal(wrappedError.message, unwrappedError.message, 'should have same message') - - t.end() - }) - - t.test('should work if something wraps promises first', function testWrapSecond(t) { - const OriginalPromise = Promise - - util.inherits(WrappedPromise, Promise) - global.Promise = WrappedPromise - - function WrappedPromise(executor) { - const promise = new OriginalPromise(executor) - promise.__proto__ = WrappedPromise.prototype - return promise - } - - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - t.teardown(function () { - global.Promise = OriginalPromise - }) - - helper.runInTransaction(agent, function () { - const p = new Promise(function noop() {}) - - t.ok(p instanceof Promise, 'instanceof should work on nr wrapped Promise') - t.ok(p instanceof WrappedPromise, 'instanceof should work on wrapped Promise') - t.ok(p instanceof OriginalPromise, 'instanceof should work on unwrapped Promise') - - t.end() - }) - }) - - t.test('should work if something wraps promises after', function testWrapFirst(t) { - const OriginalPromise = Promise - - helper.loadTestAgent(t, { feature_flag: flags }) - util.inherits(WrappedPromise, Promise) - global.Promise = WrappedPromise - - t.teardown(function () { - global.Promise = OriginalPromise - }) - - /* eslint-disable-next-line sonarjs/no-identical-functions -- Disabled due to wrapping behavior and scoping issue */ - function WrappedPromise(executor) { - const promise = new OriginalPromise(executor) - promise.__proto__ = WrappedPromise.prototype - return promise - } - - const p = new Promise(function noop() {}) - - t.ok(p instanceof Promise, 'instanceof should work on nr wrapped Promise') - t.ok(p instanceof WrappedPromise, 'instanceof should work on wrapped Promise') - t.ok(p instanceof OriginalPromise, 'instanceof should work on unwrapped Promise') - - t.end() - }) - - t.test('throw in executor', function testCatch(t) { - const agent = helper.loadTestAgent(t, { feature_flag: flags }) - let segment = null - const exception = {} - - helper.runInTransaction(agent, function inTransaction(transaction) { - new Promise(function () { - segment = agent.tracer.getSegment() - throw exception - }).then( - function () { - t.fail('should have rejected promise') - t.end() - }, - function (val) { - t.equal(this, undefined, 'context should be undefined') - - process.nextTick(function () { - const keptTx = agent.tracer.getTransaction() - t.equal(keptTx && keptTx.id, transaction.id, 'transaction should be preserved') - t.equal(val, exception, 'should pass through error') - - // Using `.ok` intead of `.equal` to avoid giant test message that is - // not useful in this case. - t.ok(agent.tracer.getSegment() === segment, 'segment should be preserved') - - t.end() - }) - } - ) - }) - }) -} - -function id(tx) { - return tx && tx.id -} diff --git a/test/integration/core/promises.tap.js b/test/integration/core/promises.tap.js deleted file mode 100644 index 91992a4e7b..0000000000 --- a/test/integration/core/promises.tap.js +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { test } = require('tap') - -const runTests = require('./promises') - -test('Promises (await_support: false)', (t) => { - t.autoend() - - runTests(t, { - await_support: false, - legacy_context_manager: true - }) -}) - -test('Promises (await_support: true)', (t) => { - t.autoend() - - runTests(t, { - await_support: true, - legacy_context_manager: true - }) -}) diff --git a/test/integration/core/timers.tap.js b/test/integration/core/timers.tap.js index 26859d7902..85c63e088c 100644 --- a/test/integration/core/timers.tap.js +++ b/test/integration/core/timers.tap.js @@ -93,15 +93,22 @@ tap.test('setImmediate', function testSetImmediate(t) { }) t.test('should not propagate segments for ended transaction', (t) => { - const { agent, contextManager } = setupAgent(t) + const { agent } = setupAgent(t) t.notOk(agent.getTransaction(), 'should not start in a transaction') helper.runInTransaction(agent, (transaction) => { transaction.end() - setImmediate(() => { - t.notOk(contextManager.getContext(), 'should not have segment for ended transaction') - t.end() + helper.runInSegment(agent, 'test-segment', () => { + const segment = agent.tracer.getSegment() + t.not(segment.name, 'test-segment') + t.equal(segment.children.length, 0, 'should not propagate segments when transaction ends') + setImmediate(() => { + const segment = agent.tracer.getSegment() + t.not(segment.name, 'test-segment') + t.equal(segment.children.length, 0, 'should not propagate segments when transaction ends') + t.end() + }) }) }) }) @@ -281,13 +288,7 @@ tap.test('clearTimeout should not ignore parent segment when internal', (t) => { }) function setupAgent(t) { - const config = { - feature_flag: { - legacy_context_manager: true - } - } - - const agent = helper.instrumentMockedAgent(config) + const agent = helper.instrumentMockedAgent() const contextManager = helper.getContextManager() t.teardown(function tearDown() { diff --git a/test/integration/instrumentation/promises/segments.js b/test/integration/instrumentation/promises/segments.js deleted file mode 100644 index d12b3d80ea..0000000000 --- a/test/integration/instrumentation/promises/segments.js +++ /dev/null @@ -1,316 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const helper = require('../../../lib/agent_helper') -// load the assertSegments assertion -require('../../../lib/metrics_helper') - -module.exports = runTests - -function runTests(t, agent, Promise) { - segmentsEnabledTests(t, agent, Promise, doSomeWork) - segmentsDisabledTests(t, agent, Promise, doSomeWork) - - // simulates a function that returns a promise and has a segment created for itself - function doSomeWork(segmentName, shouldReject) { - const tracer = agent.tracer - const segment = tracer.createSegment(segmentName) - return tracer.bindFunction(actualWork, segment)() - function actualWork() { - segment.touch() - return new Promise(function startSomeWork(resolve, reject) { - if (shouldReject) { - process.nextTick(function () { - reject('some reason') - }) - } else { - process.nextTick(function () { - resolve(123) - }) - } - }) - } - } -} - -function segmentsEnabledTests(t, agent, Promise, doSomeWork) { - const tracer = agent.tracer - - t.test('segments: child segment is created inside then handler', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 2) - - t.assertSegments(tx.trace.root, ['doSomeWork', 'someChildSegment']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doSomeWork').then(function () { - const childSegment = tracer.createSegment('someChildSegment') - // touch the segment, so that it is not truncated - childSegment.touch() - tracer.bindFunction(function () {}, childSegment) - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('segments: then handler that returns a new promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 3) - t.assertSegments(tx.trace.root, ['doWork1', 'doWork2', 'secondThen']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return doSomeWork('doWork2') - }) - .then(function secondThen() { - const s = tracer.createSegment('secondThen') - s.start() - s.end() - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('segments: then handler that returns a value', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doWork1']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return 'some value' - }) - .then(function secondThen() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('segments: catch handler with error from original promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doWork1']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1', true) - .then(function firstThen() { - return 'some value' - }) - .catch(function catchHandler() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('segments: catch handler with error from subsequent promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 3) - t.assertSegments(tx.trace.root, ['doWork1', 'doWork2', 'catchHandler']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return doSomeWork('doWork2', true) - }) - .then(function secondThen() { - const s = tracer.createSegment('secondThen') - s.start() - s.end() - }) - .catch(function catchHandler() { - const s = tracer.createSegment('catchHandler') - s.start() - s.end() - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('segments: when promise is created beforehand', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doSomeWork'], true) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - let resolve - const p = new Promise(function startSomeWork(r) { - resolve = r - }) - - const segment = tracer.createSegment('doSomeWork') - resolve = tracer.bindFunction(resolve, segment) - - p.then(function myThen() { - segment.touch() - process.nextTick(transaction.end.bind(transaction)) - }) - - // Simulate call that resolves the promise, but its segment is created - // after the promise is created - resolve() - }) - }) -} - -function segmentsDisabledTests(t, agent, Promise, doSomeWork) { - const tracer = agent.tracer - - t.test('no segments: child segment is created inside then handler', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 2) - - t.assertSegments(tx.trace.root, ['doSomeWork', 'someChildSegment']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doSomeWork').then(function () { - const childSegment = tracer.createSegment('someChildSegment') - // touch the segment, so that it is not truncated - childSegment.touch() - tracer.bindFunction(function () {}, childSegment) - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('no segments: then handler that returns a new promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doWork1']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return new Promise(function secondChain(res) { - res() - }) - }) - .then(function secondThen() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('no segments: then handler that returns a value', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doWork1']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return 'some value' - }) - .then(function secondThen() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('no segments: catch handler with error from original promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doWork1']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1', true) - .then(function firstThen() { - return 'some value' - }) - .catch(function catchHandler() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('no segments: catch handler with error from subsequent promise', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 2) - - t.assertSegments(tx.trace.root, ['doWork1', 'doWork2']) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - doSomeWork('doWork1') - .then(function firstThen() { - return doSomeWork('doWork2', true) - }) - .then(function secondThen() {}) - .catch(function catchHandler() { - process.nextTick(transaction.end.bind(transaction)) - }) - }) - }) - - t.test('no segments: when promise is created beforehand', function (t) { - agent.once('transactionFinished', function (tx) { - t.equal(tx.trace.root.children.length, 1) - - t.assertSegments(tx.trace.root, ['doSomeWork'], true) - - t.end() - }) - - helper.runInTransaction(agent, function transactionWrapper(transaction) { - let resolve - const p = new Promise(function startSomeWork(r) { - resolve = r - }) - - const segment = tracer.createSegment('doSomeWork') - resolve = tracer.bindFunction(resolve, segment) - - p.then(function myThen() { - segment.touch() - process.nextTick(transaction.end.bind(transaction)) - }) - - // Simulate call that resolves the promise, but its segment is created - // after the promise is created. - resolve() - }) - }) -} diff --git a/test/integration/instrumentation/promises/transaction-state.js b/test/lib/promises/transaction-state.js similarity index 99% rename from test/integration/instrumentation/promises/transaction-state.js rename to test/lib/promises/transaction-state.js index fdf6164417..901a0cd48f 100644 --- a/test/integration/instrumentation/promises/transaction-state.js +++ b/test/lib/promises/transaction-state.js @@ -5,7 +5,7 @@ 'use strict' -const helper = require('../../../lib/agent_helper') +const helper = require('../agent_helper') const COUNT = 2 diff --git a/test/unit/context-manager/context-manager-tests.js b/test/unit/context-manager/context-manager-tests.js index faff610eb2..58098277a6 100644 --- a/test/unit/context-manager/context-manager-tests.js +++ b/test/unit/context-manager/context-manager-tests.js @@ -9,7 +9,7 @@ * Add a standard set of Legacy Context Manager test cases for testing * either the standard or diagnostic versions. */ -function runLegacyTests(t, createContextManager) { +function runContextMangerTests(t, createContextManager) { t.test('Should default to null context', (t) => { const contextManager = createContextManager() @@ -170,4 +170,4 @@ function runLegacyTests(t, createContextManager) { }) } -module.exports = runLegacyTests +module.exports = runContextMangerTests diff --git a/test/unit/context-manager/create-context-manager.test.js b/test/unit/context-manager/create-context-manager.test.js index 2d548cbb67..88998e1a04 100644 --- a/test/unit/context-manager/create-context-manager.test.js +++ b/test/unit/context-manager/create-context-manager.test.js @@ -8,7 +8,6 @@ const { test } = require('tap') const createImplementation = require('../../../lib/context-manager/create-context-manager') -const LegacyContextManager = require('../../../lib/context-manager/legacy-context-manager') const AsyncLocalContextManager = require('../../../lib/context-manager/async-local-context-manager') test('Should return AsyncLocalContextManager by default', (t) => { @@ -20,13 +19,3 @@ test('Should return AsyncLocalContextManager by default', (t) => { t.ok(contextManager instanceof AsyncLocalContextManager) t.end() }) - -test('Should return LegacyContextManager when enabled', (t) => { - const contextManager = createImplementation({ - logging: {}, - feature_flag: { legacy_context_manager: true } - }) - - t.ok(contextManager instanceof LegacyContextManager) - t.end() -}) diff --git a/test/unit/context-manager/legacy-context-manager.test.js b/test/unit/context-manager/legacy-context-manager.test.js deleted file mode 100644 index 9a31dbea19..0000000000 --- a/test/unit/context-manager/legacy-context-manager.test.js +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2021 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { test } = require('tap') - -const runContextManagerTests = require('./context-manager-tests') -const LegacyContextManager = require('../../../lib/context-manager/legacy-context-manager') - -test('Legacy Context Manager', (t) => { - t.autoend() - - runContextManagerTests(t, createLegacyContextManager) -}) - -function createLegacyContextManager() { - return new LegacyContextManager({}) -} diff --git a/test/unit/instrumentation/core/promises.test.js b/test/unit/instrumentation/core/promises.test.js index 721363dd3b..4e81501a2f 100644 --- a/test/unit/instrumentation/core/promises.test.js +++ b/test/unit/instrumentation/core/promises.test.js @@ -12,7 +12,7 @@ const helper = require('../../../lib/agent_helper') /** * Note: These test had more meaning when we had legacy promise tracking. - * We now rely on async hooks to do to promise async propagation. But unlike legacy + * We now rely on AsyncLocalStorage context maanger to do to promise async propagation. But unlike legacy * promise instrumentation this will only propagate the same base promise segment. * * The tests still exist to prove some more complex promise chains will not lose context @@ -23,13 +23,7 @@ test('Promise trace', (t) => { let agent = null t.beforeEach(() => { - agent = helper.instrumentMockedAgent({ - feature_flag: { - promise_segments: true, - await_support: false, - legacy_context_manager: true - } - }) + agent = helper.instrumentMockedAgent() }) t.afterEach(() => { diff --git a/test/versioned/bluebird/methods.js b/test/versioned/bluebird/methods.js index fd41e7bd8a..25aead2343 100644 --- a/test/versioned/bluebird/methods.js +++ b/test/versioned/bluebird/methods.js @@ -6,7 +6,7 @@ 'use strict' const helper = require('../../lib/agent_helper') -const testTransactionState = require('../../integration/instrumentation/promises/transaction-state') +const testTransactionState = require('../../lib/promises/transaction-state') const util = require('util') const symbols = require('../../../lib/symbols') diff --git a/test/versioned/bluebird/transaction-state.tap.js b/test/versioned/bluebird/transaction-state.tap.js index 8db73d4355..1e2c10d337 100644 --- a/test/versioned/bluebird/transaction-state.tap.js +++ b/test/versioned/bluebird/transaction-state.tap.js @@ -5,11 +5,9 @@ 'use strict' -const testsDir = '../../integration/instrumentation/promises' - const helper = require('../../lib/agent_helper') const tap = require('tap') -const testTransactionState = require(testsDir + '/transaction-state') +const testTransactionState = require('../../lib/promises/transaction-state') tap.test('bluebird', function (t) { t.autoend() diff --git a/test/integration/instrumentation/promises/legacy-promise-segments.js b/test/versioned/when/legacy-promise-segments.js similarity index 99% rename from test/integration/instrumentation/promises/legacy-promise-segments.js rename to test/versioned/when/legacy-promise-segments.js index c80285a0c2..3edc160bc6 100644 --- a/test/integration/instrumentation/promises/legacy-promise-segments.js +++ b/test/versioned/when/legacy-promise-segments.js @@ -5,8 +5,8 @@ 'use strict' -const helper = require('../../../lib/agent_helper') -require('../../../lib/metrics_helper') +const helper = require('../../lib/agent_helper') +require('../../lib/metrics_helper') module.exports = runTests diff --git a/test/versioned/when/when.tap.js b/test/versioned/when/when.tap.js index a243731ac8..0d2bfb2c1a 100644 --- a/test/versioned/when/when.tap.js +++ b/test/versioned/when/when.tap.js @@ -6,9 +6,8 @@ 'use strict' const helper = require('../../lib/agent_helper') -const TEST_DIR = '../../integration/instrumentation/promises/' -const testPromiseSegments = require(`${TEST_DIR}/legacy-promise-segments`) -const testTransactionState = require(`${TEST_DIR}/transaction-state`) +const testPromiseSegments = require(`./legacy-promise-segments`) +const testTransactionState = require(`../../lib/promises/transaction-state`) // grab process emit before tap / async-hooks-domain can mess with it const originalEmit = process.emit