From cb21d2c95e5bb0de7e16535ecd4b2f5a77dc6fb7 Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Thu, 25 Apr 2024 09:54:50 -0400 Subject: [PATCH] feat: Added API `ignoreApdex` to ignore calculating apdex for the active transaction (#2166) --- api.js | 20 +++++++++ lib/transaction/index.js | 6 +++ test/unit/api/api-ignore-apdex.test.js | 58 ++++++++++++++++++++++++++ test/unit/api/stub.test.js | 7 +++- test/unit/transaction.test.js | 8 ++++ test/versioned/express/render.tap.js | 38 ++++++++++++++++- 6 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 test/unit/api/api-ignore-apdex.test.js diff --git a/api.js b/api.js index 338cf46d40..ab69e5e616 100644 --- a/api.js +++ b/api.js @@ -1882,4 +1882,24 @@ API.prototype.setLlmTokenCountCallback = function setLlmTokenCountCallback(callb this.agent.llm.tokenCountCallback = callback } +/** + * Ignores the current transaction when calculating your {@link https://docs.newrelic.com/docs/apm/new-relic-apm/apdex/apdex-measuring-user-satisfaction/|Apdex score}. + * This is useful when you have either very short or very long transactions (such as file downloads) that can skew your Apdex score. + */ +API.prototype.ignoreApdex = function ignoreApdex() { + const metric = this.agent.metrics.getOrCreateMetric(NAMES.SUPPORTABILITY.API + '/ignoreApdex') + metric.incrementCallCount() + + const transaction = this.agent.tracer.getTransaction() + + if (!transaction) { + logger.warn( + 'Apdex will not be ignored. ignoreApdex must be called within the scope of a transaction.' + ) + return + } + + transaction.ignoreApdex = true +} + module.exports = API diff --git a/lib/transaction/index.js b/lib/transaction/index.js index d874bb5ac6..1d8c4e08f4 100644 --- a/lib/transaction/index.js +++ b/lib/transaction/index.js @@ -157,6 +157,7 @@ function Transaction(agent) { this.sampled = null this.traceContext = new TraceContext(this) this.logs = new Logs(agent) + this.ignoreApdex = false agent.emit('transactionStarted', this) this.probe('Transaction created', { id: this.id }) @@ -720,6 +721,11 @@ Transaction.prototype.measure = function measure(name, scope, duration, exclusiv * derive apdex from timing in milliseconds */ Transaction.prototype._setApdex = function _setApdex(name, duration, keyApdexInMillis) { + if (this.ignoreApdex) { + logger.warn('Ignoring the collection of apdex stats for %s as ignoreApdex is true', this.name) + return + } + const apdexStats = this.metrics.getOrCreateApdexMetric(name, null, keyApdexInMillis) // if we have an error-like status code, and all the errors are diff --git a/test/unit/api/api-ignore-apdex.test.js b/test/unit/api/api-ignore-apdex.test.js new file mode 100644 index 0000000000..5f7f555971 --- /dev/null +++ b/test/unit/api/api-ignore-apdex.test.js @@ -0,0 +1,58 @@ +/* + * Copyright 2023 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const tap = require('tap') +const sinon = require('sinon') +const proxyquire = require('proxyquire') +const loggerMock = require('../mocks/logger')() + +const helper = require('../../lib/agent_helper') +const API = proxyquire('../../../api', { + './lib/logger': { + child: sinon.stub().callsFake(() => loggerMock) + } +}) + +tap.test('Agent API = ignore apdex', (t) => { + let agent = null + let api + + t.beforeEach(() => { + loggerMock.warn.reset() + agent = helper.loadMockedAgent({ + attributes: { + enabled: true + } + }) + api = new API(agent) + }) + + t.afterEach(() => { + helper.unloadAgent(agent) + }) + + t.test('should set ignoreApdex on active transaction', (t) => { + helper.runInTransaction(agent, (tx) => { + api.ignoreApdex() + t.equal(tx.ignoreApdex, true) + t.equal(loggerMock.warn.callCount, 0) + t.end() + }) + }) + + t.test('should log warning if not in active transaction', (t) => { + api.ignoreApdex() + t.equal(loggerMock.warn.callCount, 1) + t.equal( + loggerMock.warn.args[0][0], + 'Apdex will not be ignored. ignoreApdex must be called within the scope of a transaction.' + ) + t.end() + }) + + t.end() +}) diff --git a/test/unit/api/stub.test.js b/test/unit/api/stub.test.js index 980b24a307..68401c64a1 100644 --- a/test/unit/api/stub.test.js +++ b/test/unit/api/stub.test.js @@ -8,7 +8,7 @@ const tap = require('tap') const API = require('../../../stub_api') -const EXPECTED_API_COUNT = 35 +const EXPECTED_API_COUNT = 36 tap.test('Agent API - Stubbed Agent API', (t) => { t.autoend() @@ -355,4 +355,9 @@ tap.test('Agent API - Stubbed Agent API', (t) => { t.type(api.recordLlmFeedbackEvent, 'function') t.end() }) + + t.test('exports ignoreApdex', (t) => { + t.type(api.ignoreApdex, 'function') + t.end() + }) }) diff --git a/test/unit/transaction.test.js b/test/unit/transaction.test.js index 5ec18ac72f..0c623d8041 100644 --- a/test/unit/transaction.test.js +++ b/test/unit/transaction.test.js @@ -204,6 +204,14 @@ tap.test('Transaction unit tests', (t) => { t.equal(another.apdexT, 0.1, 'should not require a key transaction apdexT') t.end() }) + + t.test('should ignore calculating apdex when ignoreApdex is true', (t) => { + txn.ignoreApdex = true + txn._setApdex('Apdex/TestController/key', 1200, 667) + const metric = txn.metrics.getMetric('Apdex/TestController/key') + t.notOk(metric) + t.end() + }) }) tap.test('Transaction naming tests', (t) => { diff --git a/test/versioned/express/render.tap.js b/test/versioned/express/render.tap.js index c8f82b168e..d50abe0b15 100644 --- a/test/versioned/express/render.tap.js +++ b/test/versioned/express/render.tap.js @@ -89,7 +89,7 @@ function runTests(conf) { }) test('agent instrumentation of Express', function (t) { - t.plan(6) + t.plan(7) let agent = null let app = null @@ -137,7 +137,7 @@ function runTests(conf) { t.ok(stats, 'found unscoped stats for request path') t.equal(stats.callCount, 1, '/test was only requested once') - stats = agent.metrics.getOrCreateApdexMetric('Apdex/Expressjs/GET//test') + stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') t.ok(stats, 'found apdex stats for request path') t.equal(stats.satisfying, 1, 'got satisfactory response time') t.equal(stats.tolerating, 0, 'got no tolerable requests') @@ -162,6 +162,40 @@ function runTests(conf) { }) }) + t.test('ignore apdex when ignoreApdex is true on transaction', { timeout: 1000 }, function (t) { + // set apdexT so apdex stats will be recorded + agent.config.apdex_t = 1 + + app.get(TEST_PATH, function (req, res) { + const tx = agent.getTransaction() + tx.ignoreApdex = true + res.send({ yep: true }) + }) + + server.listen(0, TEST_HOST, function () { + const port = server.address().port + helper.makeGetRequest(TEST_URL + port + TEST_PATH, function () { + let stats + + stats = agent.metrics.getMetric('WebTransaction/Expressjs/GET//test') + t.ok(stats, 'found unscoped stats for request path') + t.equal(stats.callCount, 1, '/test was only requested once') + + stats = agent.metrics.getMetric('Apdex/Expressjs/GET//test') + t.notOk(stats, 'should not have apdex metrics') + + stats = agent.metrics.getMetric('WebTransaction') + t.ok(stats, 'found roll-up statistics for web requests') + t.equal(stats.callCount, 1, 'only one web request was made') + + stats = agent.metrics.getMetric('HttpDispatcher') + t.ok(stats, 'found HTTP dispatcher statistics') + t.equal(stats.callCount, 1, 'only one HTTP-dispatched request was made') + t.end() + }) + }) + }) + t.test('using EJS templates', { timeout: 1000 }, function (t) { app.set('views', __dirname + '/views') app.set('view engine', 'ejs')