Skip to content

Commit 1beea6b

Browse files
committed
fixes
1 parent 481b730 commit 1beea6b

File tree

10 files changed

+168
-95
lines changed

10 files changed

+168
-95
lines changed

lib/subscribers/resolve-package-version.js

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ const defaultLogger = require('#agentlib/logger.js').child({
1010
component: 'resolve-module-version'
1111
})
1212

13+
const dcFuncFrame = /^\s*at Channel\.publish/
14+
const modPathReg = /at .+ \((.+):\d+:\d+\)/
15+
1316
module.exports = resolveModuleVersion
1417

1518
/**
@@ -18,21 +21,36 @@ module.exports = resolveModuleVersion
1821
* been bundled with Node.js (e.g. `undici`), a package manifest will not be
1922
* available. In this case, the string "unknown" will be returned.
2023
*
24+
* This version resolver assumes that it will be invoked through our
25+
* diagnostics channel subscriber instrumentations. That is, it expects the
26+
* call tree to be similar to:
27+
*
28+
* 1. some-module.function()
29+
* 2. diagnostics_channel.publish()
30+
* 3. subscriber.handler()
31+
*
2132
* @param {string} moduleSpecifier What would be passed to `resolve()`.
2233
* @param {object} [deps] Optional dependencies.
2334
* @param {object} [deps.logger] Agent logger instance.
24-
* @param {Function} [deps.req] Node.js require function.
2535
*
2636
* @returns {string} The version string from the package manifest or "unknown".
2737
*/
28-
function resolveModuleVersion(moduleSpecifier, {
29-
logger = defaultLogger,
30-
req = require
31-
} = {}) {
38+
function resolveModuleVersion(moduleSpecifier, { logger = defaultLogger } = {}) {
3239
let pkgPath
33-
try {
34-
pkgPath = req.resolve(moduleSpecifier)
35-
} catch {
40+
// We'd prefer to use `require.resolve(moduleSpecifier)` here, but it gets
41+
// a bit confused when there are non-standard module directories in play.
42+
// Once we are able to refactor our "on require" metric recording to
43+
// utilize `module.registerHooks`, we should be able to eliminate this
44+
// slow algorithm.
45+
const err = Error()
46+
const stack = err.stack.split('\n')
47+
do {
48+
stack.shift()
49+
} while (dcFuncFrame.test(stack[0]) === false && stack.length > 0)
50+
const matches = modPathReg.exec(stack[1])
51+
pkgPath = matches?.[1]
52+
53+
if (!pkgPath) {
3654
logger.warn(
3755
{ moduleSpecifier },
3856
'Could not resolve module path. Possibly a built-in or Node.js bundled module.'
@@ -47,7 +65,7 @@ function resolveModuleVersion(moduleSpecifier, {
4765
do {
4866
try {
4967
pkgPath = path.join(base, 'package.json')
50-
pkg = req(pkgPath)
68+
pkg = require(pkgPath)
5169
} catch {
5270
base = path.resolve(path.join(base, '..'))
5371
if (base === cwd) {

test/lib/custom-assertions/assert-pkg-tracking-metrics.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,21 @@ module.exports = function assertPackageMetrics(
5050
metrics.push([{ name: `${prefix}${suffix}` }])
5151
}
5252

53-
assertMetrics(agent.metrics, metrics, false, false, { assert })
53+
try {
54+
assertMetrics(agent.metrics, metrics, false, false, { assert })
55+
} catch {
56+
const expected = metrics.flat().map((m) => m.name)
57+
const foundMetrics = Object.keys(agent.metrics._metrics.unscoped).filter(
58+
(k) => k.toLowerCase().startsWith('supportability')
59+
)
60+
const msg = '\nExpected supportability metrics:\n' +
61+
JSON.stringify(expected, null, 2) +
62+
'\nBut only present supportability metrics:\n' +
63+
JSON.stringify(foundMetrics, null, 2)
64+
if (typeof assert.fail === 'function') {
65+
assert.fail(msg)
66+
} else {
67+
throw Error(msg)
68+
}
69+
}
5470
}

test/unit/lib/subscribers/dc-base.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const loggerMock = require('../../mocks/logger')
1212
const helper = require('#testlib/agent_helper.js')
1313
const Subscriber = require('#agentlib/subscribers/dc-base.js')
1414

15-
const PROCESS_MAJOR = process.version.slice(1).split('.', 1)
15+
const PROCESS_MAJOR = require('../../../../package.json').version.split('.')[0]
1616

1717
test.beforeEach((ctx) => {
1818
const agent = helper.loadMockedAgent()
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright 2026 New Relic Corporation. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
'use strict'
7+
8+
const dc = require('node:diagnostics_channel')
9+
const chan = dc.channel('baz.test')
10+
11+
module.exports = class Foo {
12+
baz() {
13+
chan.publish({ hello: 'world' })
14+
}
15+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright 2026 New Relic Corporation. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
'use strict'
7+
8+
const dc = require('node:diagnostics_channel')
9+
const chan = dc.channel('foo.test')
10+
11+
module.exports = class Foo {
12+
foo() {
13+
chan.publish({ hello: 'world' })
14+
}
15+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright 2026 New Relic Corporation. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
'use strict'
7+
8+
const dc = require('node:diagnostics_channel')
9+
const chan = dc.channel('bar.test')
10+
11+
module.exports = class Bar {
12+
bar() {
13+
chan.publish({ hello: 'world' })
14+
}
15+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"version": "1.0.0"
3+
}

test/unit/lib/subscribers/resolve-package-version.test.js

Lines changed: 63 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,14 @@
66
'use strict'
77

88
const test = require('node:test')
9+
const dc = require('node:diagnostics_channel')
910
const resolvePackageVersion = require('#agentlib/subscribers/resolve-package-version.js')
1011

12+
const nrPkg = require('../../../../package.json')
13+
const Foo = require('./fixtures/foo/index.js')
14+
const Bar = require('./fixtures/foo/lib/bar.js')
15+
const Baz = require('./fixtures/baz/index.js')
16+
1117
test.beforeEach((ctx) => {
1218
ctx.nr = {
1319
logs: {
@@ -36,96 +42,72 @@ test('logs if module cannot be found', (t) => {
3642
])
3743
})
3844

39-
test('returns unknown if manifest does not have version field', (t) => {
40-
t.plan(4)
41-
const req = (specifier) => {
42-
t.assert.equal(specifier, 'test/package.json')
43-
return {}
44-
}
45-
req.resolve = (specifier) => {
46-
t.assert.equal(specifier, 'foo')
47-
return 'test/foo'
48-
}
45+
test('iterates up the tree to the package.json', (t) => {
46+
t.plan(2)
4947

50-
const result = resolvePackageVersion('foo', { ...t.nr, req })
51-
t.assert.equal(result, 'unknown')
52-
t.assert.deepStrictEqual(t.nr.logs.trace, [
53-
[
54-
{ moduleSpecifier: 'foo', version: 'unknown' },
55-
'Resolved package version.'
56-
]
57-
])
58-
})
48+
t.after(() => {
49+
dc.unsubscribe('bar.test', handler)
50+
})
5951

60-
test('iterates up the tree to the package.json', (t) => {
61-
t.plan(3)
62-
const req = (specifier) => {
63-
if (specifier === 'test/lib/package.json') {
64-
throw Error('not found')
65-
}
66-
if (specifier.endsWith('test/package.json')) {
67-
return { version: '1.0.0' }
68-
}
69-
}
70-
req.resolve = (specifier) => {
71-
t.assert.equal(specifier, 'foo')
72-
return 'test/lib/foo'
73-
}
52+
dc.subscribe('bar.test', handler)
53+
const bar = new Bar()
54+
bar.bar()
7455

75-
const result = resolvePackageVersion('foo', { ...t.nr, req })
76-
t.assert.equal(result, '1.0.0')
77-
t.assert.deepStrictEqual(t.nr.logs.trace, [
78-
[
79-
{ moduleSpecifier: 'foo', version: '1.0.0' },
80-
'Resolved package version.'
81-
]
82-
])
56+
function handler() {
57+
const result = resolvePackageVersion('foo', t.nr)
58+
t.assert.equal(result, '1.0.0')
59+
t.assert.deepStrictEqual(t.nr.logs.trace, [
60+
[
61+
{ moduleSpecifier: 'foo', version: '1.0.0' },
62+
'Resolved package version.'
63+
]
64+
])
65+
}
8366
})
8467

8568
test('stops looking after reaching app root', (t) => {
86-
t.plan(3)
87-
const req = (specifier) => {
88-
if (specifier === 'test/lib/package.json') {
89-
throw Error('not found')
90-
}
91-
if (specifier.endsWith('test/package.json')) {
92-
throw Error('try again')
93-
}
94-
throw Error('app root, no manifest')
95-
}
96-
req.resolve = (specifier) => {
97-
t.assert.equal(specifier, 'foo')
98-
return 'test/lib/foo'
99-
}
69+
t.plan(2)
10070

101-
const result = resolvePackageVersion('foo', { ...t.nr, req })
102-
t.assert.equal(result, 'unknown')
103-
t.assert.deepStrictEqual(t.nr.logs.trace, [
104-
[
105-
{ moduleSpecifier: 'foo', version: 'unknown' },
106-
'Resolved package version.'
107-
]
108-
])
71+
t.after(() => {
72+
dc.unsubscribe('baz.test', handler)
73+
})
74+
75+
dc.subscribe('baz.test', handler)
76+
const baz = new Baz()
77+
baz.baz()
78+
79+
function handler() {
80+
const result = resolvePackageVersion('baz', t.nr)
81+
t.assert.equal(result, nrPkg.version)
82+
t.assert.deepStrictEqual(t.nr.logs.trace, [
83+
[
84+
{ moduleSpecifier: 'baz', version: nrPkg.version },
85+
'Resolved package version.'
86+
]
87+
])
88+
}
10989
})
11090

11191
test('returns version', (t) => {
112-
t.plan(5)
113-
const req = (specifier) => {
114-
t.assert.equal(specifier, 'test/package.json')
115-
return { version: '1.0.0' }
116-
}
117-
req.resolve = (specifier) => {
118-
t.assert.equal(specifier, 'foo')
119-
return 'test/foo'
120-
}
92+
t.plan(2)
12193

122-
const result = resolvePackageVersion('foo', { ...t.nr, req })
123-
t.assert.equal(result, '1.0.0')
124-
t.assert.deepStrictEqual(t.nr.logs.trace, [
125-
[
126-
{ moduleSpecifier: 'foo', version: '1.0.0' },
127-
'Resolved package version.'
128-
]
129-
])
130-
t.assert.equal(t.nr.logs.warn.length, 0)
94+
t.after(() => {
95+
dc.unsubscribe('foo.test', handler)
96+
})
97+
98+
dc.subscribe('foo.test', handler)
99+
const foo = new Foo()
100+
foo.foo()
101+
102+
// eslint-disable-next-line sonarjs/no-identical-functions
103+
function handler() {
104+
const result = resolvePackageVersion('foo', t.nr)
105+
t.assert.equal(result, '1.0.0')
106+
t.assert.deepStrictEqual(t.nr.logs.trace, [
107+
[
108+
{ moduleSpecifier: 'foo', version: '1.0.0' },
109+
'Resolved package version.'
110+
]
111+
])
112+
}
131113
})

test/unit/lib/subscribers/undici.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ test('undici instrumentation', async function (t) {
7272
segment.start()
7373
agent.tracer.setSegment({ segment, transaction: tx })
7474
channels.create.publish({ request: { origin: HOST, path: '/foo' } })
75-
assert.ok(loggerMock.trace.callCount, 1)
75+
assert.equal(loggerMock.trace.callCount, 1)
7676
assert.deepEqual(loggerMock.trace.args[0], [
7777
'Not capturing data for outbound request (%s) because parent segment opaque (%s)',
7878
'/foo',

test/versioned/elastic/elasticsearch.test.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,17 @@ test('Elasticsearch instrumentation', async (t) => {
406406
}
407407
expected['Datastore/instance/ElasticSearch/' + HOST_ID] = 5
408408
checkMetrics(unscoped, expected)
409-
const pkgName = '@elastic/elasticsearch'
410-
assertPackageMetrics({ agent, pkg: pkgName, version: pkgVersion, subscriberType: true })
409+
410+
let version = pkgVersion
411+
let pkgName
412+
const major = Number(pkgVersion.split('.', 1))
413+
if (major >= 8) {
414+
pkgName = '@elastic/transport'
415+
version = helper.readPackageVersion(__dirname, pkgName)
416+
} else {
417+
pkgName = '@elastic/elasticsearch'
418+
}
419+
assertPackageMetrics({ agent, pkg: pkgName, version, subscriberType: true })
411420
})
412421
})
413422

0 commit comments

Comments
 (0)