Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Added API ignoreApdex to ignore calculating apdex for the active transaction #2166

Merged
merged 1 commit into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1548,7 +1548,7 @@
* @param {string} params.traceId Identifier for the feedback event.
* Obtained from {@link getTraceMetadata}.
* @param {string} params.category A tag for the event.
* @param {string} params.rating A indicator of how useful the message was.

Check warning on line 1551 in api.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

The type 'getTraceMetadata' is undefined
* @param {string} [params.message] The message that triggered the event.
* @param {object} [params.metadata] Additional key-value pairs to associate
* with the recorded event.
Expand Down Expand Up @@ -1882,4 +1882,24 @@
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
Loading