feat: Provided ability to define newrelic config as newrelic.mjs (n…

…ewrelic#2836)

lib/config/index.js

@@ -45,7 +45,9 @@ let logger = null // Lazy-loaded in `initialize`.
 let _configInstance = null
 
 const getConfigFileNames = () =>
-  [process.env.NEW_RELIC_CONFIG_FILENAME, 'newrelic.js', 'newrelic.cjs'].filter(Boolean)
+  [process.env.NEW_RELIC_CONFIG_FILENAME, 'newrelic.js', 'newrelic.cjs', 'newrelic.mjs'].filter(
+    Boolean
+  )
 
 const getConfigFileLocations = () =>
   [

package.json

@@ -161,7 +161,7 @@
     "bench": "node ./bin/run-bench.js",
     "docker-env": "./bin/docker-env-vars.sh",
     "docs": "rm -rf ./out && jsdoc -c ./jsdoc-conf.jsonc --private -r .",
-    "integration": "npm run prepare-test && npm run sub-install && BORP_CONF_FILE=.borp.int.yaml time c8 -o ./coverage/integration borp --timeout 600000 --reporter ./test/lib/test-reporter.mjs",
+    "integration": "npm run sub-install && BORP_CONF_FILE=.borp.int.yaml time c8 -o ./coverage/integration borp --timeout 600000 --reporter ./test/lib/test-reporter.mjs",
     "integration:esm": "NODE_OPTIONS='--loader=./esm-loader.mjs' BORP_CONF_FILE=.borp.int-esm.yaml time c8 -o ./coverage/integration-esm borp --reporter ./test/lib/test-reporter.mjs",
     "prepare-test": "npm run docker-env",
     "lint": "eslint ./*.{js,mjs} lib test bin",

test/integration/config/config-esm.test.js

@@ -7,19 +7,37 @@
 
 const test = require('node:test')
 const assert = require('node:assert')
+const semver = require('semver')
 
 const match = require('../../lib/custom-assertions/match')
 const util = require('util')
 const exec = util.promisify(require('child_process').exec)
 
+// allowing require of esm made this test change
+// see: https://github.com/nodejs/node/pull/55085/
+// depending on node version this will either verify
+// it cannot require ESM configuration or can
 test('should gracefully handle ESM imports', async (t) => {
-  await t.test('when newrelic.js is misnamed', async () => {
-    const { stderr } = await exec('node index.mjs', { cwd: `${__dirname}/esm-bad` })
-    match(stderr, 'ERR_REQUIRE_ESM', 'should mention ERR_REQUIRE_ESM in error message')
+  await t.test('when requiring newrelic.js in ESM app', async () => {
+    const { stdout, stderr } = await exec('node index.mjs', { cwd: `${__dirname}/esm-js` })
+    if (semver.gte(process.version, '22.12.0')) {
+      match(stdout, 'Hello esm-test')
+    } else {
+      match(stderr, 'ERR_REQUIRE_ESM', 'should mention ERR_REQUIRE_ESM in error message')
+    }
   })
 
-  await t.test('when newrelic.cjs is properly named', async () => {
-    const { stdout, stderr } = await exec('node index.mjs', { cwd: `${__dirname}/esm-good` })
+  await t.test('when requiring newrelic.mjs in ESM app', async () => {
+    const { stdout, stderr } = await exec('node index.mjs', { cwd: `${__dirname}/esm-mjs` })
+    if (semver.gte(process.version, '22.12.0')) {
+      match(stdout, 'Hello esm-test')
+    } else {
+      match(stderr, 'ERR_REQUIRE_ESM', 'should mention ERR_REQUIRE_ESM in error message')
+    }
+  })
+
+  await t.test('when requiring newrelic.cjs in ESM app', async () => {
+    const { stdout, stderr } = await exec('node index.mjs', { cwd: `${__dirname}/esm-cjs` })
     assert.deepStrictEqual(stdout, 'Hello good-esm\n', 'should greet in stdout')
     assert.deepStrictEqual(stderr, '', 'all should be quiet in stderr')
   })

test/integration/config/esm-cjs/package.json

@@ -0,0 +1,7 @@
+{
+  "type": "module",
+  "files": [
+    "../../../../index.js"
+  ]
+}
+

test/integration/config/esm-js/index.mjs

@@ -0,0 +1,16 @@
+/*
+ * Copyright 2022 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import newrelic from '../../../../index.js'
+
+export default function greeter(name) {
+  return `Hello ${name}`
+}
+
+if (newrelic.agent) {
+  /* eslint-disable no-console */
+  console.log(greeter(newrelic.agent.config.app_name))
+  /* eslint-enable no-console */
+}

test/integration/config/esm-bad/newrelic.js → test/integration/config/esm-js/newrelic.js

@@ -5,7 +5,8 @@
 
 'use strict'
 
-exports.config = {
-  app_name: ['bad-esm'],
+export const config = {
+  app_name: ['esm-test'],
   license_key: 'nonsensical-balderdash'
 }
+

test/integration/config/esm-mjs/.eslintrc

@@ -0,0 +1,3 @@
+{
+  "ignorePatterns": ["newrelic.mjs"]
+}

test/integration/config/esm-mjs/index.mjs

@@ -0,0 +1,16 @@
+/*
+ * Copyright 2022 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import newrelic from '../../../../index.js'
+
+export default function greeter(name) {
+  return `Hello ${name}`
+}
+
+if (newrelic.agent) {
+  /* eslint-disable no-console */
+  console.log(greeter(newrelic.agent.config.app_name))
+  /* eslint-enable no-console */
+}

test/integration/config/esm-mjs/newrelic.mjs

@@ -0,0 +1,12 @@
+/*
+ * Copyright 2022 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+export const config = {
+  app_name: ['esm-test'],
+  license_key: 'nonsensical-balderdash'
+}
+

test/integration/config/esm-mjs/package.json

@@ -0,0 +1,6 @@
+{
+  "type": "module",
+  "files": [
+    "../../../../index.js"
+  ]
+}

test/unit/instrumentation/http/http.test.js

@@ -123,7 +123,7 @@ test('built-in http module instrumentation', async (t) => {
   })
 
   await t.test('when running a request', async (t) => {
-    t.beforeEach((ctx) => {
+    t.beforeEach(async (ctx) => {
       ctx.nr = {}
       const agent = helper.instrumentMockedAgent()
 
@@ -156,7 +156,7 @@ test('built-in http module instrumentation', async (t) => {
         makeRequest(
           http,
           {
-            port: 8321,
+            port: ctx.nr.externalPort,
             host: 'localhost',
             path: '/status',
             method: 'GET'
@@ -180,34 +180,41 @@ test('built-in http module instrumentation', async (t) => {
       ctx.nr.external = external
       ctx.nr.server = server
 
-      return new Promise((resolve) => {
-        external.listen(8321, 'localhost', function () {
-          server.listen(8123, 'localhost', function () {
+      const ports = await new Promise((resolve) => {
+        external.listen(0, 'localhost', function () {
+          const { port: externalPort } = this.address()
+          server.listen(0, 'localhost', function () {
             // The transaction doesn't get created until after the instrumented
             // server handler fires.
             assert.ok(!agent.getTransaction())
-            resolve()
+            const { port: serverPort } = this.address()
+            resolve({ externalPort, serverPort })
           })
         })
       })
+      ctx.nr.externalPort = ports.externalPort
+      ctx.nr.serverPort = ports.serverPort
     })
 
-    t.afterEach((ctx) => {
+    t.afterEach(async (ctx) => {
       const { agent, external, server } = ctx.nr
-      external.close()
-      server.close()
+      await new Promise((resolve) => {
+        external.close(() => {
+          server.close(resolve)
+        })
+      })
       helper.unloadAgent(agent)
     })
 
     await t.test(
       'when allow_all_headers is false, only collect allowed agent-specified headers',
       (t, end) => {
-        const { agent, http } = t.nr
+        const { agent, http, serverPort } = t.nr
         agent.config.allow_all_headers = false
         makeRequest(
           http,
           {
-            port: 8123,
+            port: serverPort,
             host: 'localhost',
             path: '/path',
             method: 'GET',
@@ -234,15 +241,15 @@ test('built-in http module instrumentation', async (t) => {
     await t.test(
       'when allow_all_headers is true, collect all headers not filtered by `exclude` rules',
       (t, end) => {
-        const { agent, http } = t.nr
+        const { agent, http, serverPort } = t.nr
         agent.config.allow_all_headers = true
         agent.config.attributes.exclude = ['request.headers.x*']
         // have to emit attributes getting updated so all filters get updated
         agent.config.emit('attributes.exclude')
         makeRequest(
           http,
           {
-            port: 8123,
+            port: serverPort,
             host: 'localhost',
             path: '/path',
             method: 'GET',
@@ -277,7 +284,7 @@ test('built-in http module instrumentation', async (t) => {
     await t.test(
       'when url_obfuscation regex pattern is set, obfuscate segment url attributes',
       (t, end) => {
-        const { agent, http } = t.nr
+        const { agent, http, serverPort } = t.nr
         agent.config.url_obfuscation = {
           enabled: true,
           regex: {
@@ -288,7 +295,7 @@ test('built-in http module instrumentation', async (t) => {
         makeRequest(
           http,
           {
-            port: 8123,
+            port: serverPort,
             host: 'localhost',
             path: '/foo4/bar4',
             method: 'GET'
@@ -309,11 +316,11 @@ test('built-in http module instrumentation', async (t) => {
     )
 
     await t.test('request.uri should not contain request params', (t, end) => {
-      const { http } = t.nr
+      const { http, serverPort } = t.nr
       makeRequest(
         http,
         {
-          port: 8123,
+          port: serverPort,
           host: 'localhost',
           path: '/foo5/bar5?region=here&auth=secretString',
           method: 'GET'
@@ -333,14 +340,14 @@ test('built-in http module instrumentation', async (t) => {
     })
 
     await t.test('successful request', (t, end) => {
-      const { agent, http } = t.nr
+      const { agent, http, externalPort, serverPort } = t.nr
       const refererUrl = 'https://www.google.com/search/cats?scrubbed=false'
       const userAgent = 'Palm680/RC1'
 
       makeRequest(
         http,
         {
-          port: 8123,
+          port: serverPort,
           host: 'localhost',
           path: '/path',
           method: 'GET',
@@ -360,7 +367,7 @@ test('built-in http module instrumentation', async (t) => {
         const callStats = agent.metrics.getOrCreateMetric('WebTransaction/NormalizedUri/*')
         const dispatcherStats = agent.metrics.getOrCreateMetric('HttpDispatcher')
         const reqStats = transaction.metrics.getOrCreateMetric(
-          'External/localhost:8321/http',
+          `External/localhost:${externalPort}/http`,
           'WebTransaction/NormalizedUri/*'
         )
 
@@ -392,7 +399,7 @@ test('built-in http module instrumentation', async (t) => {
           1,
           'associates outbound HTTP requests with the inbound transaction'
         )
-        assert.equal(transaction.port, 8123, "set transaction.port to the server's port")
+        assert.equal(transaction.port, serverPort, "set transaction.port to the server's port")
         assert.equal(transaction2.id, transaction.id, 'only create one transaction for the request')
 
         end()