chore(deps): Updated @newrelic/security-agent to v2.2.0 #2842
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2842 +/- ##
==========================================
- Coverage 97.26% 97.21% -0.05%
==========================================
Files 294 294
Lines 46405 46405
==========================================
- Hits 45135 45113 -22
- Misses 1270 1292 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -36,13 +36,12 @@ test('ignoring an Express route', async function (t) { | |||
|
|
|||
| const metrics = agent.metrics._metrics.unscoped | |||
| // loading k2 adds instrumentation metrics for things it loads | |||
| const expectedMetrics = helper.isSecurityAgentEnabled(agent) ? 11 : 3 | |||
| const expectedMetrics = helper.isSecurityAgentEnabled(agent) ? Object.keys(metrics).length : 3 | |||
There was a problem hiding this comment.
What is the actual expected number? How many metrics does the security agent add on top of the baseline 3 that are expected without the security agent?
There was a problem hiding this comment.
This number changes whenever there is code refactoring or change in invocation of instrumentation module of security agent. In past, I have updated these numbers as well.
#2239
There was a problem hiding this comment.
Although 11 works fine but in one run it was expecting 12
There was a problem hiding this comment.
If Security agent starts instrumenting native module like crypto or Random then the metric count changes
There was a problem hiding this comment.
but just doing Object.keys(metrics) isn't going to catch when things aren't working. what was added that makes this test fail?
There was a problem hiding this comment.
Updated metric count to 13 now.
There was a problem hiding this comment.
Here is the result when metric count is set to 11 and 13 respectively. Please check difference between both runs.
https://github.com/newrelic/node-newrelic/actions/runs/12395203935/job/34600383248?pr=2842
https://github.com/newrelic/node-newrelic/actions/runs/12395625367/job/34601802350?pr=2842
There was a problem hiding this comment.
i don't see the update to the test. your addition to express router only exists in 5.x+ so you'll have to check the express version before deciding how many metrics exist with the security agent enabled
There was a problem hiding this comment.
diff --git a/test/versioned/express/ignoring.test.js b/test/versioned/express/ignoring.test.js
index a1a94a9ae..827a3c858 100644
--- a/test/versioned/express/ignoring.test.js
+++ b/test/versioned/express/ignoring.test.js
@@ -18,7 +18,7 @@ test.beforeEach(async (ctx) => {
test.afterEach(teardown)
test('ignoring an Express route', async function (t) {
- const { agent, app, port } = t.nr
+ const { agent, app, port, isExpress5 } = t.nr
const plan = tsplan(t, { plan: 7 })
const api = new API(agent)
@@ -36,7 +36,7 @@ test('ignoring an Express route', async function (t) {
const metrics = agent.metrics._metrics.unscoped
// loading k2 adds instrumentation metrics for things it loads
- const expectedMetrics = helper.isSecurityAgentEnabled(agent) ? 11 : 3
+ const expectedMetrics = helper.isSecurityAgentEnabled(agent) ? isExpress5 ? 13 : 11 : 3
Description
v2.2.0 (2024-12-18)
Features
Bug fixes
How to Test
npm run versioned:security