Skip to content

Commit

Permalink
feat: Added API ignoreApdex to ignore calculating apdex for the act…
Browse files Browse the repository at this point in the history
…ive transaction (#2166)
  • Loading branch information
bizob2828 authored Apr 25, 2024
1 parent 260f16b commit cb21d2c
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 3 deletions.
20 changes: 20 additions & 0 deletions api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions lib/transaction/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down Expand Up @@ -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
Expand Down
58 changes: 58 additions & 0 deletions test/unit/api/api-ignore-apdex.test.js
Original file line number Diff line number Diff line change
@@ -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()
})
7 changes: 6 additions & 1 deletion test/unit/api/stub.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
})
})
8 changes: 8 additions & 0 deletions test/unit/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
38 changes: 36 additions & 2 deletions test/versioned/express/render.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand Down

0 comments on commit cb21d2c

Please sign in to comment.