From 945d6b3714142eec793d751e53b6d42df541f9f8 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 19 Oct 2021 15:54:20 -0700 Subject: [PATCH 1/9] feat: support node v17 --- .ci/.jenkins_nodejs.yml | 1 + .ci/.jenkins_tav_nodejs.yml | 1 + .github/workflows/test.yml | 1 + package.json | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.ci/.jenkins_nodejs.yml b/.ci/.jenkins_nodejs.yml index bb31294f62..81d426ca47 100644 --- a/.ci/.jenkins_nodejs.yml +++ b/.ci/.jenkins_nodejs.yml @@ -1,4 +1,5 @@ NODEJS_VERSION: + - "17" - "16" - "16.0" - "14" diff --git a/.ci/.jenkins_tav_nodejs.yml b/.ci/.jenkins_tav_nodejs.yml index bc9d0de737..d25fd6970c 100644 --- a/.ci/.jenkins_tav_nodejs.yml +++ b/.ci/.jenkins_tav_nodejs.yml @@ -1,4 +1,5 @@ NODEJS_VERSION: + - "17" - "16" - "14" - "12" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0d778991a9..a0627f8000 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -120,6 +120,7 @@ jobs: strategy: matrix: node: + - '17' - '16' - '16.0' - '14' diff --git a/package.json b/package.json index 7af039544c..a93c01579b 100644 --- a/package.json +++ b/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", From c9c503c63c9cb7a88c430c99e05e49e210ffec0f Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 19 Oct 2021 16:31:57 -0700 Subject: [PATCH 2/9] use abs path for '--openssl-config=...' used in testing Otherwise the "test/start/..." tests that change the cwd and cannot find the relative path to our openssl config file: running (cwd: ./test/start/env): node --unhandled-rejections=strict test.test.js OpenSSL configuration error: C007EDBE107F0000:error:80000002:system library:BIO_new_file:No such file or directory:../deps/openssl/openssl/crypto/bio/bss_file.c:67:calling fopen(./test/openssl-config-for-testing.cnf, rb) C007EDBE107F0000:error:10000080:BIO routines:BIO_new_file:no such file:../deps/openssl/openssl/crypto/bio/bss_file.c:75: C007EDBE107F0000:error:07000072:configuration file routines:def_load:no such file:../deps/openssl/openssl/crypto/conf/conf_def.c:179: /home/runner/work/apm-agent-nodejs/apm-agent-nodejs/test/test.js:132 if (err) throw err This does not fail with node v16 and earlier. I don't know why. Perhaps it is the change to OpenSSL v3 in node v17. --- test/script/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/script/run_tests.sh b/test/script/run_tests.sh index b4a1364323..5d3017dbdd 100755 --- a/test/script/run_tests.sh +++ b/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" ]] From 2e5ece5dbc666955912ec991cf65bfaba3f3616b Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 19 Oct 2021 17:05:54 -0700 Subject: [PATCH 3/9] cope with ::1 for socket.remoteAddress in GH action linux container with node v17 --- test/instrumentation/modules/http2.test.js | 28 ++++++++++++++-------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/test/instrumentation/modules/http2.test.js b/test/instrumentation/modules/http2.test.js index c68c5fd4ee..40f11d2143 100644 --- a/test/instrumentation/modules/http2.test.js +++ b/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 From 58366f4e5ca12a908adc5cb15ea969c9bfb5d55a Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 19 Oct 2021 17:26:19 -0700 Subject: [PATCH 4/9] support ipv6 in event.address from mongodb start event --- lib/instrumentation/modules/mongodb.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/instrumentation/modules/mongodb.js b/lib/instrumentation/modules/mongodb.js index 45b51612aa..17f734d66d 100644 --- a/lib/instrumentation/modules/mongodb.js +++ b/lib/instrumentation/modules/mongodb.js @@ -3,8 +3,9 @@ const semver = require('semver') const { getDBDestination } = require('../context') -// Match expected `:`. -const HOSTNAME_PORT_RE = /^([^:]+):(\d+)$/ +// Match expected `:`, 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 From fa2cef4109ca153dc4ed385c9b3ec8fbd6a80695 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 20 Oct 2021 10:46:02 -0700 Subject: [PATCH 5/9] avoid using IPv6 address with http.get to workaround #2382 --- test/instrumentation/modules/fastify/_async-await.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/instrumentation/modules/fastify/_async-await.js b/test/instrumentation/modules/fastify/_async-await.js index 900ecc95b1..9abbd65339 100644 --- a/test/instrumentation/modules/fastify/_async-await.js +++ b/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 () { From f466ab4c02a6c88ef1ed693637cc6957dec1e85f Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 20 Oct 2021 14:30:22 -0700 Subject: [PATCH 6/9] hapi test: use localhost rather than '0.0.0.0' default to match client request address This is a workaround for https://github.com/nodejs/node/issues/40537 --- test/instrumentation/modules/hapi/shared.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/instrumentation/modules/hapi/shared.js b/test/instrumentation/modules/hapi/shared.js index 1e21f38719..36ac6fbba8 100644 --- a/test/instrumentation/modules/hapi/shared.js +++ b/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) } From 8bf5edb80e205cdd0b56117e96856dd1a5752c9d Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 20 Oct 2021 14:51:24 -0700 Subject: [PATCH 7/9] dump the versions of modules we've installed --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a0627f8000..641ba90fda 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -139,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 From 73e9852447f22756f5d09c2128572b3cccd96d68 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 20 Oct 2021 15:59:51 -0700 Subject: [PATCH 8/9] they are starting to have v18 nightlies now --- .ci/.jenkins_nightly_nodejs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/.jenkins_nightly_nodejs.yml b/.ci/.jenkins_nightly_nodejs.yml index ab5bedc9aa..634f7cef1a 100644 --- a/.ci/.jenkins_nightly_nodejs.yml +++ b/.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" From db85abb6c2da9842a63480d1dcf7b32b63e53954 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 21 Oct 2021 09:29:29 -0700 Subject: [PATCH 9/9] changelog entry --- CHANGELOG.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ed2e1b4cc5..b2270ee66f 100644 --- a/CHANGELOG.asciidoc +++ b/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