feat: support node v17 (#2380)

There were a few changes (in mongodb instrumentation) and tests to
deal with `localhost` resolving to a IPv6 due to this change in node v17
nodejs/node#39987

Author: trentm

.ci/.jenkins_nightly_nodejs.yml

@@ -9,4 +9,4 @@
 # made, these will stop and there will be no value in testing v17 nightlies.
 #
 NODEJS_VERSION:
-  - "17"
+  - "18"

.ci/.jenkins_nodejs.yml

@@ -1,4 +1,5 @@
 NODEJS_VERSION:
+  - "17"
   - "16"
   - "16.0"
   - "14"

.ci/.jenkins_tav_nodejs.yml

@@ -1,4 +1,5 @@
 NODEJS_VERSION:
+  - "17"
   - "16"
   - "14"
   - "12"

.github/workflows/test.yml

@@ -120,6 +120,7 @@ jobs:
     strategy:
       matrix:
         node:
+          - '17'
           - '16'
           - '16.0'
           - '14'
@@ -138,4 +139,5 @@ jobs:
         node-version: ${{ matrix.node }}
     - run: docker ps  # show the services against which we'll be testing
     - run: npm install
+    - run: npm ls --all || true
     - run: npm test

CHANGELOG.asciidoc

@@ -37,6 +37,8 @@ Notes:
 [float]
 ===== Features
 
+* Add support for node v17.
+
 * When an error is captured, the APM agent will only immediately flush it to
   APM server if it is an "unhandled" error. Unhandled errors are typically those
   captured via the `uncaughtException` process event. Before this change, a

lib/instrumentation/modules/mongodb.js

@@ -3,8 +3,9 @@
 const semver = require('semver')
 const { getDBDestination } = require('../context')
 
-// Match expected `<hostname>:<port>`.
-const HOSTNAME_PORT_RE = /^([^:]+):(\d+)$/
+// Match expected `<hostname>:<port>`, e.g. "mongo:27017", "::1:27017",
+// "127.0.0.1:27017".
+const HOSTNAME_PORT_RE = /^(.+):(\d+)$/
 
 module.exports = (mongodb, agent, { version, enabled }) => {
   if (!enabled) return mongodb

package.json

@@ -43,7 +43,7 @@
     "url": "git://github.com/elastic/apm-agent-nodejs.git"
   },
   "engines": {
-    "node": "^8.6.0 || 10 || 12 || 14 || 15 || 16"
+    "node": "^8.6.0 || 10 || 12 || 14 || 15 || 16 || 17"
   },
   "keywords": [
     "opbeat",

test/instrumentation/modules/fastify/_async-await.js

@@ -30,7 +30,7 @@ test('transaction name', function (t) {
 
   fastify.listen(0, function (err, address) {
     t.error(err)
-    address = address || 'http://localhost:' + fastify.server.address().port
+    address = 'http://localhost:' + fastify.server.address().port
     http.get(`${address}/hello/world`, function (res) {
       const chunks = []
       res.on('data', chunks.push.bind(chunks))
@@ -81,7 +81,7 @@ if (semver.gte(fastifyVersion, '2.0.0-rc')) {
 
     fastify.listen(0, function (err, address) {
       t.error(err)
-      http.get(`${address}/hello/world`, function (res) {
+      http.get(`http://localhost:${fastify.server.address().port}/hello/world`, function (res) {
         const chunks = []
         res.on('data', chunks.push.bind(chunks))
         res.on('end', function () {

test/instrumentation/modules/hapi/shared.js

@@ -7,7 +7,8 @@ module.exports = (moduleName) => {
     captureExceptions: false,
     logLevel: 'fatal',
     metricsInterval: 0,
-    centralConfig: false
+    centralConfig: false,
+    cloudProvider: 'none'
   })
 
   var isHapiIncompat = require('../../../_is_hapi_incompat')
@@ -48,7 +49,7 @@ module.exports = (moduleName) => {
     agent.captureError = originalCaptureError
 
     var server = startServer(function (err, port) {
-      t.error(err)
+      t.error(err, 'no error from startServer')
       http.get('http://localhost:' + port + '/captureError?foo=bar')
     })
   })
@@ -523,7 +524,7 @@ module.exports = (moduleName) => {
   })
 
   function makeServer (opts) {
-    var server = new Hapi.Server()
+    var server = new Hapi.Server({ host: 'localhost' })
     if (semver.satisfies(pkg.version, '<17')) {
       server.connection(opts)
     }

test/instrumentation/modules/http2.test.js

@@ -25,7 +25,7 @@ isSecure.forEach(secure => {
   var method = secure ? 'createSecureServer' : 'createServer'
 
   test(`http2.${method} compatibility mode`, t => {
-    t.plan(15)
+    t.plan(16)
 
     // Note NODE_OPTIONS env because it sometimes has a setting relevant
     // for this test.
@@ -71,7 +71,7 @@ isSecure.forEach(secure => {
   })
 
   test(`http2.${method} stream respond`, t => {
-    t.plan(15)
+    t.plan(16)
 
     resetAgent((data) => {
       assert(t, data, secure, port)
@@ -114,7 +114,7 @@ isSecure.forEach(secure => {
   })
 
   test(`http2.${method} stream respondWithFD`, t => {
-    t.plan(16)
+    t.plan(17)
 
     resetAgent((data) => {
       assert(t, data, secure, port)
@@ -164,7 +164,7 @@ isSecure.forEach(secure => {
   })
 
   test(`http2.${method} stream respondWithFile`, t => {
-    t.plan(15)
+    t.plan(16)
 
     resetAgent((data) => {
       assert(t, data, secure, port)
@@ -278,7 +278,7 @@ isSecure.forEach(secure => {
   })
 
   test(`http2.request${secure ? ' secure' : ' '}`, t => {
-    t.plan(34)
+    t.plan(36)
 
     resetAgent(3, (data) => {
       t.strictEqual(data.transactions.length, 2)
@@ -478,17 +478,25 @@ function assertPath (t, trans, secure, port, path, httpVersion) {
       }
       break
   }
-  // Sometime between node (v8.6.0, v8.17.0] there was a fix such that
-  // socket.remoteAddress was set properly for https.
-  const expectedRemoteAddress = httpVersion === '1.1' && semver.lt(process.version, '8.17.0')
-    ? undefined : '::ffff:127.0.0.1'
+
+  // What is "expected" for transaction.context.request.socket.remote_address
+  // is a bit of a pain.
+  if (httpVersion === '1.1' && semver.lt(process.version, '8.17.0')) {
+    // Before node v8.17.0 `socket.remoteAddress` was not set for https.
+    t.pass(`skip checking on transaction.context.request.socket.remote_address on node ${process.version}`)
+  } else {
+    // With node v17 on Linux (or at least on the linux containers used for
+    // GitHub Action tests), the localhost socket.remoteAddress is "::1".
+    t.ok(trans.context.request.socket.remote_address === '::ffff:127.0.0.1' || trans.context.request.socket.remote_address === '::1',
+      'transaction.context.request.socket.remote_address is as expected: ' + trans.context.request.socket.remote_address)
+  }
+  delete trans.context.request.socket.remote_address
 
   t.deepEqual(trans.context.request, {
     http_version: httpVersion,
     method: 'GET',
     url: expectedUrl,
     socket: {
-      remote_address: expectedRemoteAddress,
       encrypted: secure
     },
     headers: expectedReqHeaders

test/script/run_tests.sh

@@ -69,7 +69,7 @@ fi
 # Skip for node v8 because it results in this warning:
 #   openssl config failed: error:25066067:DSO support routines:DLFCN_LOAD:could not load the shared library
 if [[ $major_node_version -gt 8 ]]; then
-  export NODE_OPTIONS="$NODE_OPTIONS --openssl-config=./test/openssl-config-for-testing.cnf"
+  export NODE_OPTIONS="$NODE_OPTIONS --openssl-config=$(pwd)/test/openssl-config-for-testing.cnf"
 fi
 
 if [[ "$CI" || "$1" == "none" ]]