From 03b01a3bf40c8902c29c772aee4d70af8020ae62 Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Mon, 25 Nov 2024 11:07:03 -0500 Subject: [PATCH] test: Migrated `test/integration/api` tests to `node:test` (#2787) --- ...custom-attribute-span-propogation.test.js} | 166 ++++----- test/integration/api/instrument-loaded.tap.js | 91 ----- .../integration/api/instrument-loaded.test.js | 100 ++++++ .../{instrument.tap.js => instrument.test.js} | 75 ++-- ...tice-error.tap.js => notice-error.test.js} | 22 +- .../api/set-error-group-callback.tap.js | 327 ------------------ .../api/set-error-group-callback.test.js | 303 ++++++++++++++++ .../api/{shutdown.tap.js => shutdown.test.js} | 90 +++-- 8 files changed, 585 insertions(+), 589 deletions(-) rename test/integration/api/{custom-attribute-span-propogation.tap.js => custom-attribute-span-propogation.test.js} (50%) delete mode 100644 test/integration/api/instrument-loaded.tap.js create mode 100644 test/integration/api/instrument-loaded.test.js rename test/integration/api/{instrument.tap.js => instrument.test.js} (73%) rename test/integration/api/{notice-error.tap.js => notice-error.test.js} (55%) delete mode 100644 test/integration/api/set-error-group-callback.tap.js create mode 100644 test/integration/api/set-error-group-callback.test.js rename test/integration/api/{shutdown.tap.js => shutdown.test.js} (50%) diff --git a/test/integration/api/custom-attribute-span-propogation.tap.js b/test/integration/api/custom-attribute-span-propogation.test.js similarity index 50% rename from test/integration/api/custom-attribute-span-propogation.tap.js rename to test/integration/api/custom-attribute-span-propogation.test.js index f0274c5e3e..d0ce371a3d 100644 --- a/test/integration/api/custom-attribute-span-propogation.tap.js +++ b/test/integration/api/custom-attribute-span-propogation.test.js @@ -5,75 +5,78 @@ 'use strict' -const tap = require('tap') - +const test = require('node:test') +const assert = require('node:assert') const API = require('../../../api') const AttributeFilter = require('../../../lib/config/attribute-filter') const helper = require('../../lib/agent_helper') const DESTINATIONS = AttributeFilter.DESTINATIONS -tap.test('#addAttribute', (t) => { - t.autoend() - - let agent = null - let api = null - - t.beforeEach(() => { - agent = helper.loadMockedAgent({ +test('#addAttribute', async (t) => { + t.beforeEach((ctx) => { + const agent = helper.loadMockedAgent({ distributed_tracing: { enabled: true } }) - api = new API(agent) + const api = new API(agent) + ctx.nr = { + agent, + api + } }) - t.afterEach(() => { - helper.unloadAgent(agent) + t.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) }) - t.test('should add attribute to current span', (t) => { + await t.test('should add attribute to current span', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { api.addCustomAttribute('key1', 'value1') const customSpanAttributes = getCustomSpanAttributes(agent) - t.equal(customSpanAttributes.key1, 'value1') + assert.equal(customSpanAttributes.key1, 'value1') - t.end() + end() }) }) }) - t.test('should overwrite for same key added via addCustomAttribute', (t) => { + await t.test('should overwrite for same key added via addCustomAttribute', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { api.addCustomAttribute('key1', 'value1') api.addCustomAttribute('key1', 'last-wins') const customSpanAttributes = getCustomSpanAttributes(agent) - t.equal(customSpanAttributes.key1, 'last-wins') + assert.equal(customSpanAttributes.key1, 'last-wins') - t.end() + end() }) }) }) - t.test('should not overwrite for same key added via addCustomSpanAttribute', (t) => { + await t.test('should not overwrite for same key added via addCustomSpanAttribute', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { api.addCustomSpanAttribute('key1', 'custom-span-wins') api.addCustomAttribute('key1', 'does-not-overwrite') const customSpanAttributes = getCustomSpanAttributes(agent) - t.equal(customSpanAttributes.key1, 'custom-span-wins') + assert.equal(customSpanAttributes.key1, 'custom-span-wins') - t.end() + end() }) }) }) - t.test('should not add attribute when over the limit', (t) => { + await t.test('should not add attribute when over the limit', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { // the cap is 64 @@ -86,34 +89,34 @@ tap.test('#addAttribute', (t) => { const customSpanAttributes = getCustomSpanAttributes(agent) const hasAttribute = Object.hasOwnProperty.bind(customSpanAttributes) - t.notOk(hasAttribute(unexpectedAttributeName)) + assert.ok(!hasAttribute(unexpectedAttributeName)) - t.end() + end() }) }) }) }) -tap.test('#addCustomSpanAttribute', (t) => { - t.autoend() - - let agent = null - let api = null - - t.beforeEach(() => { - agent = helper.loadMockedAgent({ +test('#addCustomSpanAttribute', async (t) => { + t.beforeEach((ctx) => { + const agent = helper.loadMockedAgent({ distributed_tracing: { enabled: true } }) - api = new API(agent) + const api = new API(agent) + ctx.nr = { + agent, + api + } }) - t.afterEach(() => { - helper.unloadAgent(agent) + t.afterEach((ctx) => { + helper.unloadAgent(ctx.nr.agent) }) - t.test('should not add attribute to transaction', (t) => { + await t.test('should not add attribute to transaction', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, (transaction) => { api.startSegment('segment1', true, () => { const unexpectedAttributeName = 'should-not-exist' @@ -122,14 +125,15 @@ tap.test('#addCustomSpanAttribute', (t) => { const customTransactionAttributes = getCustomTransactionAttributes(transaction) const hasAttribute = Object.hasOwnProperty.bind(customTransactionAttributes) - t.notOk(hasAttribute(unexpectedAttributeName)) + assert.ok(!hasAttribute(unexpectedAttributeName)) - t.end() + end() }) }) }) - t.test('should overwrite for same key added via addCustomAttribute', (t) => { + await t.test('should overwrite for same key added via addCustomAttribute', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { api.addCustomAttribute('key1', 'value1') @@ -137,14 +141,15 @@ tap.test('#addCustomSpanAttribute', (t) => { const customSpanAttributes = getCustomSpanAttributes(agent) - t.equal(customSpanAttributes.key1, 'custom-span-wins') + assert.equal(customSpanAttributes.key1, 'custom-span-wins') - t.end() + end() }) }) }) - t.test('should overwrite for same key added via addCustomSpanAttribute', (t) => { + await t.test('should overwrite for same key added via addCustomSpanAttribute', (t, end) => { + const { agent, api } = t.nr helper.runInTransaction(agent, () => { api.startSegment('segment1', true, () => { api.addCustomSpanAttribute('key1', 'value1') @@ -152,52 +157,59 @@ tap.test('#addCustomSpanAttribute', (t) => { const customSpanAttributes = getCustomSpanAttributes(agent) - t.equal(customSpanAttributes.key1, 'last-wins') + assert.equal(customSpanAttributes.key1, 'last-wins') - t.end() + end() }) }) }) - t.test('should replace newest added via addCustomAttribute when over the limit', (t) => { - helper.runInTransaction(agent, () => { - api.startSegment('segment1', true, () => { - // the cap is 64 - const lastAddedName = batchAddCustomAttributes(api, 32) - batchAddCustomSpanAttributes(api, 32) + await t.test( + 'should replace newest added via addCustomAttribute when over the limit', + (t, end) => { + const { agent, api } = t.nr + helper.runInTransaction(agent, () => { + api.startSegment('segment1', true, () => { + // the cap is 64 + const lastAddedName = batchAddCustomAttributes(api, 32) + batchAddCustomSpanAttributes(api, 32) - api.addCustomSpanAttribute('should-replace-add-custom', 'replaced') + api.addCustomSpanAttribute('should-replace-add-custom', 'replaced') - const customSpanAttributes = getCustomSpanAttributes(agent) - const hasAttribute = Object.hasOwnProperty.bind(customSpanAttributes) + const customSpanAttributes = getCustomSpanAttributes(agent) + const hasAttribute = Object.hasOwnProperty.bind(customSpanAttributes) - t.notOk(hasAttribute(lastAddedName), 'should drop last added via addCustomAttribute') + assert.ok(!hasAttribute(lastAddedName), 'should drop last added via addCustomAttribute') - t.equal(customSpanAttributes['should-replace-add-custom'], 'replaced') + assert.equal(customSpanAttributes['should-replace-add-custom'], 'replaced') - t.end() + end() + }) }) - }) - }) - - t.test('should not replace any added via addCustomSpanAttribute when over the limit', (t) => { - helper.runInTransaction(agent, () => { - api.startSegment('segment1', true, () => { - // the cap is 64 - batchAddCustomSpanAttributes(api, 64) - - const unexpectedAttributeName = 'should-not-replace-add-custom-span' - api.addCustomSpanAttribute(unexpectedAttributeName, 'does-not-exist') - - const customSpanAttributes = getCustomSpanAttributes(agent) - const hasAttribute = Object.hasOwnProperty.bind(customSpanAttributes) - - t.notOk(hasAttribute(unexpectedAttributeName)) - - t.end() + } + ) + + await t.test( + 'should not replace any added via addCustomSpanAttribute when over the limit', + (t, end) => { + const { agent, api } = t.nr + helper.runInTransaction(agent, () => { + api.startSegment('segment1', true, () => { + // the cap is 64 + batchAddCustomSpanAttributes(api, 64) + + const unexpectedAttributeName = 'should-not-replace-add-custom-span' + api.addCustomSpanAttribute(unexpectedAttributeName, 'does-not-exist') + + const customSpanAttributes = getCustomSpanAttributes(agent) + const hasAttribute = Object.hasOwnProperty.bind(customSpanAttributes) + + assert.ok(!hasAttribute(unexpectedAttributeName)) + end() + }) }) - }) - }) + } + ) }) function getCustomSpanAttributes(agent) { diff --git a/test/integration/api/instrument-loaded.tap.js b/test/integration/api/instrument-loaded.tap.js deleted file mode 100644 index 4d4cf4158e..0000000000 --- a/test/integration/api/instrument-loaded.tap.js +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' -const { test } = require('tap') -const express = require('express') -const API = require('../../../api') -const agentHelper = require('../../lib/agent_helper') - -test('ensures instrumentation with shim.require can run without an error', (t) => { - const agent = agentHelper.instrumentMockedAgent() - t.teardown(() => { - agentHelper.unloadAgent(agent) - }) - - const api = new API(agent) - - api.instrumentLoadedModule('express', express) - t.type(express, 'function') - t.end() -}) - -test('should return false when a function errors during instrumentation', (t) => { - t.plan(2) - - const agent = agentHelper.loadMockedAgent() - t.teardown(() => { - agentHelper.unloadAgent(agent) - }) - - const api = new API(agent) - - const moduleName = 'express' - api.instrument(moduleName, onRequire) - - function onRequire() { - t.ok('should hit the onRequire') - throw new Error('Oh No!') - } - - const result = api.instrumentLoadedModule(moduleName, {}) - - t.equal(result, false) -}) - -test('should return false when instrumentation handler returns false (did not instrument)', (t) => { - t.plan(2) - - const agent = agentHelper.loadMockedAgent() - t.teardown(() => { - agentHelper.unloadAgent(agent) - }) - - const api = new API(agent) - - const moduleName = 'express' - api.instrument(moduleName, onRequire) - - function onRequire() { - t.ok('should hit the onRequire') - return false - } - - const result = api.instrumentLoadedModule(moduleName, {}) - - t.equal(result, false) -}) - -test('should return true when instrumentation handler does not return anything', (t) => { - t.plan(2) - - const agent = agentHelper.loadMockedAgent() - t.teardown(() => { - agentHelper.unloadAgent(agent) - }) - - const api = new API(agent) - - const moduleName = 'express' - api.instrument(moduleName, onRequire) - - function onRequire() { - t.ok('should hit the onRequire') - } - - const result = api.instrumentLoadedModule(moduleName, {}) - - t.equal(result, true) -}) diff --git a/test/integration/api/instrument-loaded.test.js b/test/integration/api/instrument-loaded.test.js new file mode 100644 index 0000000000..66ffb4e0db --- /dev/null +++ b/test/integration/api/instrument-loaded.test.js @@ -0,0 +1,100 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const test = require('node:test') +const assert = require('node:assert') +const { tspl } = require('@matteo.collina/tspl') +const express = require('express') +const API = require('../../../api') +const agentHelper = require('../../lib/agent_helper') + +test('ensures instrumentation with shim.require can run without an error', (t) => { + const agent = agentHelper.instrumentMockedAgent() + t.after(() => { + agentHelper.unloadAgent(agent) + }) + + const api = new API(agent) + + api.instrumentLoadedModule('express', express) + assert.ok(typeof express === 'function') +}) + +// Rest of these tests are in parent because this does not bootstrap instrumentation +// but instead loads agent and every test manually instruments a package +test('manual instrumenting', async (t) => { + t.beforeEach((ctx) => { + const agent = agentHelper.loadMockedAgent() + const api = new API(agent) + ctx.nr = { + agent, + api + } + }) + + t.afterEach((ctx) => { + agentHelper.unloadAgent(ctx.nr.agent) + }) + + await t.test('should return false when a function errors during instrumentation', async (t) => { + const { api } = t.nr + const plan = tspl(t, { plan: 2 }) + + const moduleName = 'express' + api.instrument(moduleName, onRequire) + + function onRequire() { + plan.ok(1, 'should hit the onRequire') + throw new Error('Oh No!') + } + + const result = api.instrumentLoadedModule(moduleName, {}) + + plan.equal(result, false) + await plan.completed + }) + + await t.test( + 'should return false when instrumentation handler returns false (did not instrument)', + async (t) => { + const { api } = t.nr + const plan = tspl(t, { plan: 2 }) + + const moduleName = 'express' + api.instrument(moduleName, onRequire) + + function onRequire() { + plan.ok(1, 'should hit the onRequire') + return false + } + + const result = api.instrumentLoadedModule(moduleName, {}) + + plan.equal(result, false) + await plan.completed + } + ) + + await t.test( + 'should return true when instrumentation handler does not return anything', + async (t) => { + const { api } = t.nr + const plan = tspl(t, { plan: 2 }) + + const moduleName = 'express' + api.instrument(moduleName, onRequire) + + function onRequire() { + plan.ok(1, 'should hit the onRequire') + } + + const result = api.instrumentLoadedModule(moduleName, {}) + + plan.equal(result, true) + await plan.completed + } + ) +}) diff --git a/test/integration/api/instrument.tap.js b/test/integration/api/instrument.test.js similarity index 73% rename from test/integration/api/instrument.tap.js rename to test/integration/api/instrument.test.js index 7ec345ddf2..083a44ff38 100644 --- a/test/integration/api/instrument.tap.js +++ b/test/integration/api/instrument.test.js @@ -4,7 +4,8 @@ */ 'use strict' -const { test } = require('tap') +const test = require('node:test') +const assert = require('node:assert') const API = require('../../../api') const agentHelper = require('../../lib/agent_helper') const symbols = require('../../../lib/symbols') @@ -12,23 +13,24 @@ const sinon = require('sinon') const moduleName = 'TestMod' const modulePath = './node_modules/TestMod' -test('instrument', (t) => { - t.autoend() - let agent - let api - - t.beforeEach(() => { - agent = agentHelper.instrumentMockedAgent() - api = new API(agent) +test('instrument', async (t) => { + t.beforeEach((ctx) => { + const agent = agentHelper.instrumentMockedAgent() + const api = new API(agent) + ctx.nr = { + agent, + api + } }) - t.afterEach(() => { + t.afterEach((ctx) => { const mod = require.resolve(modulePath) delete require.cache[mod] - agentHelper.unloadAgent(agent) + agentHelper.unloadAgent(ctx.nr.agent) }) - t.test('should allow registering of multiple onRequire hooks', (t) => { + await t.test('should allow registering of multiple onRequire hooks', (t, end) => { + const { api } = t.nr api.instrument(moduleName, onRequire) api.instrument(moduleName, onRequire2) let firstShim @@ -37,41 +39,42 @@ test('instrument', (t) => { function onRequire(shim, TestMod) { firstShim = shim shim.wrap(TestMod.prototype, 'foo', function wrapFoo(shim, orig) { - t.notOk(shim.isWrapped(orig)) + assert.ok(!shim.isWrapped(orig)) return function wrappedFoo(...args) { - t.ok(secondShim.isWrapped(TestMod.prototype.foo)) + assert.ok(secondShim.isWrapped(TestMod.prototype.foo)) args[0] = `${args[0]} in onRequire` return orig.apply(this, args) } }) - t.ok(shim.isWrapped(TestMod.prototype.foo)) + assert.ok(shim.isWrapped(TestMod.prototype.foo)) } function onRequire2(shim, TestMod) { secondShim = shim shim.wrap(TestMod.prototype, 'foo', function wrapFoo(shim, orig) { - t.ok(firstShim.isWrapped(orig)) - t.notOk(shim.isWrapped(TestMod.prototype.foo)) + assert.ok(firstShim.isWrapped(orig)) + assert.ok(!shim.isWrapped(TestMod.prototype.foo)) return function wrappedFoo2(...args) { - t.ok(shim.isWrapped(TestMod.prototype.foo)) + assert.ok(shim.isWrapped(TestMod.prototype.foo)) args[0] = `${args[0]} in onRequire2` return orig.apply(this, args) } }) - t.ok(shim.isWrapped(TestMod.prototype.foo)) + assert.ok(shim.isWrapped(TestMod.prototype.foo)) } const TestMod = require(modulePath) const testMod = new TestMod() const ret = testMod.foo('this is orig arg') - t.equal(ret, 'value of this is orig arg in onRequire2 in onRequire') - t.end() + assert.equal(ret, 'value of this is orig arg in onRequire2 in onRequire') + end() }) - t.test( + await t.test( 'should allow checking for isWrapped relevant to the wrapping you are about to do', - (t) => { + (t, end) => { + const { api } = t.nr api.instrument(moduleName, onRequire) api.instrument(moduleName, onRequire2) @@ -104,12 +107,13 @@ test('instrument', (t) => { const testMod = new TestMod() const ret = testMod.foo('this is orig arg') require(modulePath) - t.equal(ret, 'value of this is orig arg in onRequire2 in onRequire') - t.end() + assert.equal(ret, 'value of this is orig arg in onRequire2 in onRequire') + end() } ) - t.test('shim.unwrap should not break instrumentation registered after it', (t) => { + await t.test('shim.unwrap should not break instrumentation registered after it', (t, end) => { + const { api } = t.nr api.instrument(moduleName, onRequire) function onRequire(shim, TestMod) { @@ -125,15 +129,16 @@ test('instrument', (t) => { const testMod = new TestMod() const ret = testMod.foo('Hello world') - t.equal(ret, 'value of Hello world') + assert.equal(ret, 'value of Hello world') const shim = TestMod[symbols.shim] - t.notOk(shim.isWrapped(TestMod.prototype.foo), 'should unwrap as expected') - t.end() + assert.ok(!shim.isWrapped(TestMod.prototype.foo), 'should unwrap as expected') + end() }) - t.test( + await t.test( 'shim.unwrap should not log warning if you try to unwrap and it has been wrapped more than once', - (t) => { + (t, end) => { + const { api } = t.nr api.instrument(moduleName, onRequire) api.instrument(moduleName, onRequire2) let shim1 @@ -166,15 +171,15 @@ test('instrument', (t) => { const testMod = new TestMod() testMod.foo('this is orig arg') testMod.foo('this is call 2') - t.ok(shim2.isWrapped(TestMod.prototype.foo), 'should unwrap as expected') + assert.ok(shim2.isWrapped(TestMod.prototype.foo), 'should unwrap as expected') ;[loggerSpy1, loggerSpy2].forEach((spy) => { - t.equal( + assert.equal( spy.args[0][0], 'Attempting to unwrap %s, which its unwrapped version is also wrapped. This is unsupported, unwrap will not occur.' ) - t.equal(spy.args[0][1], 'foo') + assert.equal(spy.args[0][1], 'foo') }) - t.end() + end() } ) }) diff --git a/test/integration/api/notice-error.tap.js b/test/integration/api/notice-error.test.js similarity index 55% rename from test/integration/api/notice-error.tap.js rename to test/integration/api/notice-error.test.js index 0acf67909a..3dddc5cc6b 100644 --- a/test/integration/api/notice-error.tap.js +++ b/test/integration/api/notice-error.test.js @@ -4,14 +4,17 @@ */ 'use strict' - -const test = require('tap').test +const test = require('node:test') +const { tspl } = require('@matteo.collina/tspl') const helper = require('../../lib/agent_helper') -test('http errors are noticed correctly', function testError(t) { - const agent = helper.loadTestAgent(t) +test('http errors are noticed correctly', async function testError(t) { + const agent = helper.instrumentMockedAgent() + t.after(() => { + helper.unloadAgent(agent) + }) - t.plan(3) + const plan = tspl(t, { plan: 3 }) const http = require('http') const server = http.createServer(handler) @@ -26,6 +29,8 @@ test('http errors are noticed correctly', function testError(t) { close ) + await plan.completed + function handler(req, res) { agent.errors.add(agent.getTransaction(), new Error('notice me!')) req.resume() @@ -38,10 +43,9 @@ test('http errors are noticed correctly', function testError(t) { } function check() { - t.equal(agent.errors.traceAggregator.errors.length, 1, 'should be 1 error') + plan.equal(agent.errors.traceAggregator.errors.length, 1, 'should be 1 error') const error = agent.errors.traceAggregator.errors[0] - t.equal(error[1], 'WebTransaction/NormalizedUri/*', 'should have correct transaction') - t.equal(error[2], 'notice me!', 'should have right name') - t.end() + plan.equal(error[1], 'WebTransaction/NormalizedUri/*', 'should have correct transaction') + plan.equal(error[2], 'notice me!', 'should have right name') } }) diff --git a/test/integration/api/set-error-group-callback.tap.js b/test/integration/api/set-error-group-callback.tap.js deleted file mode 100644 index 7414868a3b..0000000000 --- a/test/integration/api/set-error-group-callback.tap.js +++ /dev/null @@ -1,327 +0,0 @@ -/* - * Copyright 2023 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const tap = require('tap') -const helper = require('../../lib/agent_helper') -const API = require('../../../api') - -tap.test('api.setErrorGroupCallback()', (t) => { - t.autoend() - - let agent - let api - let http - let express - let app - let server - - t.beforeEach(() => { - agent = helper.loadTestAgent(t) - api = new API(agent) - http = require('http') - express = require('express') - app = express() - }) - - t.afterEach(() => { - server.close() - helper.unloadAgent(agent) - }) - - t.test('should not add the Error Group when callback is not a function', (t) => { - api.setErrorGroupCallback('this-is-a-string') - - app.get('/test', (req, res) => { - return res.status(500).json({ message: 'Boom' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.notOk( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'should not add `error.group.name` attribute to trace' - ) - t.notOk( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'should not add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should not add the Error Group when callback throws', (t) => { - api.setErrorGroupCallback(function callback() { - throw new Error('whoops') - }) - - app.get('/test', (req, res) => { - return res.status(500).json({ message: 'Boom' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.notOk( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'should not add `error.group.name` attribute to trace' - ) - t.notOk( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'should not add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should not add the Error Group when callback returns empty string', (t) => { - api.setErrorGroupCallback(function callback() { - return '' - }) - - app.get('/test', (req, res) => { - return res.status(500).json({ message: 'Boom' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.notOk( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'should not add `error.group.name` attribute to trace' - ) - t.notOk( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'should not add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should not add the Error Group when callback returns not string', (t) => { - api.setErrorGroupCallback(function callback() { - return { 'error.group.name': 'test-group' } - }) - - app.get('/test', (req, res) => { - return res.status(500).json({ message: 'Boom' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.notOk( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'should not add `error.group.name` attribute to trace' - ) - t.notOk( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'should not add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should pass the correct arguments to the callback (transaction)', (t) => { - api.setErrorGroupCallback(function callback(metadata) { - t.equal(metadata['request.uri'], '/test', 'should give the request.uri attribute') - t.equal(metadata['http.statusCode'], '500', 'should give the http.statusCode attribute') - t.equal(metadata['http.method'], 'GET', 'should give the http.method attribute') - - return 'test-group' - }) - - app.get('/test', (req, res) => { - return res.status(500).json({ message: 'Boom' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.equal( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to trace' - ) - t.equal( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should pass the correct arguments to the callback (error)', (t) => { - const expectedError = new Error('boom') - api.setErrorGroupCallback(function callback(metadata) { - t.equal(metadata.error, expectedError, 'should give the error attribute') - - return 'test-group' - }) - - app.get('/test', () => { - throw expectedError - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 500, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.equal( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to trace' - ) - t.equal( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should pass the correct arguments to the callback (custom attributes)', (t) => { - api.setErrorGroupCallback(function callback(metadata) { - t.same(metadata.customAttributes, { foo: 'bar' }) - - return 'test-group' - }) - - app.get('/test', (req, res) => { - api.addCustomAttribute('foo', 'bar') - api.noticeError(new Error('boom')) - return res.status(200).json({ message: 'OK' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 200, 'request should return a 500') - t.end() - }) - - agent.on('transactionFinished', function () { - t.equal( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to trace' - ) - t.equal( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should pass the correct arguments to the callback (noticeError + is expected)', (t) => { - const expectedError = new Error('boom') - api.setErrorGroupCallback(function callback(metadata) { - t.equal(metadata.error, expectedError, 'should give the error') - t.equal(metadata['error.expected'], true, 'should give the error.expected') - t.equal(metadata['request.uri'], '/test', 'should give the request.uri') - t.equal(metadata['http.statusCode'], '200', 'should give the http.statusCode') - t.equal(metadata['http.method'], 'GET', 'should give the http.method') - - return 'test-group' - }) - - app.get('/test', (req, res) => { - api.noticeError(expectedError, true) - return res.status(200).json({ message: 'OK' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 200, 'request should return a 200') - t.end() - }) - - agent.on('transactionFinished', function () { - t.equal( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to trace' - ) - t.equal( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'test-group', - 'should add `error.group.name` attribute to event' - ) - }) - }) - - t.test('should overwrite previous callbacks if called more than once', (t) => { - const expectedError = new Error('boom') - api.setErrorGroupCallback(function callback() { - return 'group #1' - }) - - app.get('/test', (req, res) => { - api.setErrorGroupCallback(function secondCallback() { - return 'group #2' - }) - api.noticeError(expectedError, true) - return res.status(200).json({ message: 'OK' }) - }) - - server = app.listen(0) - const url = `http://localhost:${server.address().port}/test` - - http.get(url, function (res) { - t.equal(res.statusCode, 200, 'request should return a 200') - t.end() - }) - - agent.on('transactionFinished', function () { - t.equal( - agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], - 'group #2', - 'should add `error.group.name` attribute to trace' - ) - t.equal( - agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], - 'group #2', - 'should add `error.group.name` attribute to event' - ) - }) - }) -}) diff --git a/test/integration/api/set-error-group-callback.test.js b/test/integration/api/set-error-group-callback.test.js new file mode 100644 index 0000000000..9835b29fd2 --- /dev/null +++ b/test/integration/api/set-error-group-callback.test.js @@ -0,0 +1,303 @@ +/* + * Copyright 2023 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' +const test = require('node:test') +const assert = require('node:assert') +const helper = require('../../lib/agent_helper') +const API = require('../../../api') + +test('api.setErrorGroupCallback()', async (t) => { + t.beforeEach((ctx) => { + const agent = helper.loadMockedAgent() + const api = new API(agent) + const http = require('http') + const express = require('express') + const app = express() + app.get('/test', (req, res) => { + res.status(500).json({ message: 'Boom' }) + }) + + const expectedError = new Error('Boom') + app.get('/test-throw', () => { + throw expectedError + }) + + app.get('/test-attrs', (req, res) => { + api.addCustomAttribute('foo', 'bar') + api.noticeError(expectedError) + res.status(200).json({ message: 'OK' }) + }) + + app.get('/test-notice-error', (req, res) => { + api.noticeError(expectedError, true) + return res.status(200).json({ message: 'OK' }) + }) + + const server = app.listen(0) + const baseUrl = `http://localhost:${server.address().port}` + ctx.nr = { + agent, + api, + baseUrl, + expectedError, + http, + server + } + }) + + t.afterEach((ctx) => { + const { agent, server } = ctx.nr + server.close() + helper.unloadAgent(agent) + }) + + await t.test('should not add the Error Group when callback is not a function', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + api.setErrorGroupCallback('this-is-a-string') + + const url = `${baseUrl}/test` + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.ok( + !agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'should not add `error.group.name` attribute to trace' + ) + assert.ok( + !agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'should not add `error.group.name` attribute to event' + ) + }) + }) + + await t.test('should not add the Error Group when callback throws', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test` + api.setErrorGroupCallback(function callback() { + throw new Error('whoops') + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.ok( + !agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'should not add `error.group.name` attribute to trace' + ) + assert.ok( + !agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'should not add `error.group.name` attribute to event' + ) + }) + }) + + await t.test('should not add the Error Group when callback returns empty string', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test` + api.setErrorGroupCallback(function callback() { + return '' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.ok( + !agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'should not add `error.group.name` attribute to trace' + ) + assert.ok( + !agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'should not add `error.group.name` attribute to event' + ) + }) + }) + + await t.test('should not add the Error Group when callback returns not string', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test` + api.setErrorGroupCallback(function callback() { + return { 'error.group.name': 'test-group' } + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.ok( + !agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'should not add `error.group.name` attribute to trace' + ) + assert.ok( + !agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'should not add `error.group.name` attribute to event' + ) + }) + }) + + await t.test('should pass the correct arguments to the callback (transaction)', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test` + api.setErrorGroupCallback(function callback(metadata) { + assert.equal(metadata['request.uri'], '/test', 'should give the request.uri attribute') + assert.equal(metadata['http.statusCode'], '500', 'should give the http.statusCode attribute') + assert.equal(metadata['http.method'], 'GET', 'should give the http.method attribute') + + return 'test-group' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.equal( + agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to trace' + ) + assert.equal( + agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to event' + ) + }) + }) + + await t.test('should pass the correct arguments to the callback (error)', (t, end) => { + const { agent, api, http, baseUrl, expectedError } = t.nr + const url = `${baseUrl}/test-throw` + api.setErrorGroupCallback(function callback(metadata) { + assert.equal(metadata.error, expectedError, 'should give the error attribute') + + return 'test-group' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 500, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.equal( + agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to trace' + ) + assert.equal( + agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to event' + ) + }) + }) + + await t.test( + 'should pass the correct arguments to the callback (custom attributes)', + (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test-attrs` + api.setErrorGroupCallback(function callback(metadata) { + assert.deepEqual(metadata.customAttributes, { foo: 'bar' }) + + return 'test-group' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 200, 'request should return a 500') + end() + }) + + agent.on('transactionFinished', function () { + assert.equal( + agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to trace' + ) + assert.equal( + agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to event' + ) + }) + } + ) + + await t.test( + 'should pass the correct arguments to the callback (noticeError + is expected)', + (t, end) => { + const { agent, api, http, baseUrl, expectedError } = t.nr + const url = `${baseUrl}/test-notice-error` + api.setErrorGroupCallback(function callback(metadata) { + assert.equal(metadata.error, expectedError, 'should give the error') + assert.equal(metadata['error.expected'], true, 'should give the error.expected') + assert.equal(metadata['request.uri'], '/test', 'should give the request.uri') + assert.equal(metadata['http.statusCode'], '200', 'should give the http.statusCode') + assert.equal(metadata['http.method'], 'GET', 'should give the http.method') + + return 'test-group' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 200, 'request should return a 200') + end() + }) + + agent.on('transactionFinished', function () { + assert.equal( + agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to trace' + ) + assert.equal( + agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'test-group', + 'should add `error.group.name` attribute to event' + ) + }) + } + ) + + await t.test('should overwrite previous callbacks if called more than once', (t, end) => { + const { agent, api, http, baseUrl } = t.nr + const url = `${baseUrl}/test-notice-error` + api.setErrorGroupCallback(function callback() { + return 'group #1' + }) + api.setErrorGroupCallback(function secondCallback() { + return 'group #2' + }) + + http.get(url, function (res) { + assert.equal(res.statusCode, 200, 'request should return a 200') + end() + }) + + agent.on('transactionFinished', function () { + assert.equal( + agent.errors.traceAggregator.errors[0][4].agentAttributes['error.group.name'], + 'group #2', + 'should add `error.group.name` attribute to trace' + ) + assert.equal( + agent.errors.eventAggregator.getEvents()[0][2]['error.group.name'], + 'group #2', + 'should add `error.group.name` attribute to event' + ) + }) + }) +}) diff --git a/test/integration/api/shutdown.tap.js b/test/integration/api/shutdown.test.js similarity index 50% rename from test/integration/api/shutdown.tap.js rename to test/integration/api/shutdown.test.js index 86f1e34286..976253e54a 100644 --- a/test/integration/api/shutdown.tap.js +++ b/test/integration/api/shutdown.test.js @@ -5,41 +5,44 @@ 'use strict' -const tap = require('tap') +const test = require('node:test') +const assert = require('node:assert') const nock = require('nock') - const helper = require('../../lib/agent_helper') const API = require('../../../api') - // This key is hardcoded in the agent helper const EXPECTED_LICENSE_KEY = 'license key here' const TEST_DOMAIN = 'test-collector.newrelic.com' const TEST_COLLECTOR_URL = `https://${TEST_DOMAIN}` // TODO: should work after the agent has restarted -tap.test('#shutdown', (t) => { - t.autoend() - - let agent = null - let api = null - - t.beforeEach(() => { - nock.disableNetConnect() - - agent = helper.loadMockedAgent({ - license_key: EXPECTED_LICENSE_KEY, - host: TEST_DOMAIN, - utilization: { - detect_aws: false - } - }) +/** + * Tests fix for a bug where the agent stayed in a 'connected' state and + * never hit a 'started' state after a restart. This prevented ever triggering + * a final harvest, the shutdown callback would never be invoked and the process + * would not be held open. + * + * When broken, the callback is not invoked. The test ends prematurely and the metric_data + * and shutdown endpoints are left pending in nock. + * x test unfinished + * x Failed to hit all expected endpoints. + */ +test('#shutdown should force harvest and callback after agent restart', (t, end) => { + nock.disableNetConnect() + + const agent = helper.loadMockedAgent({ + license_key: EXPECTED_LICENSE_KEY, + host: TEST_DOMAIN, + utilization: { + detect_aws: false + } + }) - agent.config.no_immediate_harvest = true + agent.config.no_immediate_harvest = true - api = new API(agent) - }) + const api = new API(agent) - t.afterEach((t) => { + t.after(() => { helper.unloadAgent(agent) if (!nock.isDone()) { @@ -48,38 +51,25 @@ tap.test('#shutdown', (t) => { /* eslint-enable no-console */ nock.cleanAll() - t.fail('Failed to hit all expected endpoints.') + assert.fail('Failed to hit all expected endpoints.') } nock.enableNetConnect() }) - /** - * Tests fix for a bug where the agent stayed in a 'connected' state and - * never hit a 'started' state after a restart. This prevented ever triggering - * a final harvest, the shutdown callback would never be invoked and the process - * would not be held open. - * - * When broken, the callback is not invoked. The test ends prematurely and the metric_data - * and shutdown endpoints are left pending in nock. - * x test unfinished - * x Failed to hit all expected endpoints. - */ - t.test('should force harvest and callback after agent restart', (t) => { - setupConnectionEndpoints('run-id-1') - agent.start((error) => { - t.error(error) - - setupConnectionEndpoints('run-id-2') - agent.collector.restart(() => { - const endpoints = setupShutdownEndpoints('run-id-2') - api.shutdown({ collectPendingData: true }, (error) => { - t.error(error) - - t.ok(endpoints.metric_data.isDone()) - t.ok(endpoints.shutdown.isDone()) - t.end() - }) + setupConnectionEndpoints('run-id-1') + agent.start((error) => { + assert.ok(!error) + + setupConnectionEndpoints('run-id-2') + agent.collector.restart(() => { + const endpoints = setupShutdownEndpoints('run-id-2') + api.shutdown({ collectPendingData: true }, (error) => { + assert.ok(!error) + + assert.ok(endpoints.metric_data.isDone()) + assert.ok(endpoints.shutdown.isDone()) + end() }) }) })