Skip to content

Commit

Permalink
refactor: Removed Supportability/Features/ESM/UnsupportedLoader as …
Browse files Browse the repository at this point in the history
…it is no longer applicable in Node.js 18+ (#2393)
  • Loading branch information
bizob2828 authored Jul 23, 2024
1 parent 303aa99 commit d22c368
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 48 deletions.
11 changes: 1 addition & 10 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const featureFlags = require('./lib/feature_flags').prerelease
const psemver = require('./lib/util/process-version')
let logger = require('./lib/logger') // Gets re-loaded after initialization.
const NAMES = require('./lib/metrics/names')
const isESMSupported = psemver.satisfies('>=16.2.0')

const pkgJSON = require('./package.json')
logger.info(
Expand Down Expand Up @@ -246,15 +245,7 @@ function recordLoaderMetric(agent) {
(arg === '--loader' || arg === '--experimental-loader') &&
process.execArgv[index + 1] === 'newrelic/esm-loader.mjs'
) {
if (isESMSupported) {
agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.LOADER).incrementCallCount()
} else {
agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.UNSUPPORTED_LOADER)
logger.warn(
'New Relic for Node.js ESM loader requires a version of Node >= v16.12.0; your version is %s. Instrumentation will not be registered.',
process.version
)
}
agent.metrics.getOrCreateMetric(NAMES.FEATURES.ESM.LOADER).incrementCallCount()
}
})

Expand Down
3 changes: 1 addition & 2 deletions lib/metrics/names.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ const INFINITE_TRACING = {

const FEATURES = {
ESM: {
LOADER: `${SUPPORTABILITY.FEATURES}/ESM/Loader`,
UNSUPPORTED_LOADER: `${SUPPORTABILITY.FEATURES}/ESM/UnsupportedLoader`
LOADER: `${SUPPORTABILITY.FEATURES}/ESM/Loader`
},
CJS: {
PRELOAD: `${SUPPORTABILITY.FEATURES}/CJS/Preload`,
Expand Down
43 changes: 7 additions & 36 deletions test/unit/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,28 +132,6 @@ test('loader metrics', (t) => {
t.end()
})

t.test('should load preload unsupported metric if node version is <16.2.0', (t) => {
const processVersionStub = {
satisfies: sandbox.stub()
}
processVersionStub.satisfies.onCall(0).returns(false)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(true)
process.execArgv = ['--loader', 'newrelic/esm-loader.mjs']
const agent = proxyquire('../../index', {
'./lib/util/process-version': processVersionStub,
'./lib/agent': MockAgent,
'./lib/shimmer': shimmerMock,
'./api': ApiMock
})

const metricCall = agent.agent.metrics.getOrCreateMetric

t.equal(metricCall.args.length, 2)
t.equal(metricCall.args[0][0], 'Supportability/Features/ESM/UnsupportedLoader')
t.end()
})

t.test('should load require metric when agent is required', (t) => {
const agent = proxyquire('../../index', {
'./lib/agent': MockAgent,
Expand Down Expand Up @@ -222,8 +200,7 @@ test('index tests', (t) => {
sandbox.stub(console, 'error')
k2Stub = { start: sandbox.stub() }
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
mockConfig.applications.returns(['my-app-name'])
MockAgent.prototype.start.yields(null)
shimmerMock = createShimmerMock(sandbox)
Expand Down Expand Up @@ -287,7 +264,7 @@ test('index tests', (t) => {
})

t.test('should throw error if using an unsupported version of Node.js', (t) => {
processVersionStub.satisfies.onCall(1).returns(false)
processVersionStub.satisfies.onCall(0).returns(false)
loadIndex()
t.equal(loggerMock.error.callCount, 1, 'should log an error')
t.match(loggerMock.error.args[0][0], /New Relic for Node.js requires a version of Node/)
Expand All @@ -297,7 +274,6 @@ test('index tests', (t) => {
t.test('should log warning if using an odd version of node', (t) => {
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(true)
configMock.getOrCreateInstance.returns(null)
loadIndex()
t.equal(loggerMock.warn.callCount, 1, 'should log an error')
Expand All @@ -308,8 +284,7 @@ test('index tests', (t) => {
t.test('should use stub api if no config detected', (t) => {
configMock.getOrCreateInstance.returns(null)
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
const api = loadIndex()
t.equal(loggerMock.info.callCount, 2, 'should log info logs')
t.equal(loggerMock.info.args[1][0], 'No configuration detected. Not starting.')
Expand All @@ -320,8 +295,7 @@ test('index tests', (t) => {
t.test('should use stub api if agent_enabled is false', (t) => {
configMock.getOrCreateInstance.returns({ agent_enabled: false })
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
const api = loadIndex()
t.equal(loggerMock.info.callCount, 2, 'should log info logs')
t.equal(loggerMock.info.args[1][0], 'Module disabled in configuration. Not starting.')
Expand All @@ -332,8 +306,7 @@ test('index tests', (t) => {
t.test('should log warning when logging diagnostics is enabled', (t) => {
mockConfig.logging.diagnostics = true
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
loadIndex()
t.equal(
loggerMock.warn.args[0][0],
Expand All @@ -344,8 +317,7 @@ test('index tests', (t) => {

t.test('should throw error is app name is not set in config', (t) => {
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
mockConfig.applications.returns([])
loadIndex()
t.equal(loggerMock.error.callCount, 1, 'should log an error')
Expand All @@ -355,8 +327,7 @@ test('index tests', (t) => {

t.test('should log error if agent startup failed', (t) => {
processVersionStub.satisfies.onCall(0).returns(true)
processVersionStub.satisfies.onCall(1).returns(true)
processVersionStub.satisfies.onCall(2).returns(false)
processVersionStub.satisfies.onCall(1).returns(false)
mockConfig.applications.returns(['my-app-name'])
const err = new Error('agent start failed')
MockAgent.prototype.start.yields(err)
Expand Down

0 comments on commit d22c368

Please sign in to comment.