Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support node v17 #2380

Merged
merged 10 commits into from
Oct 21, 2021
2 changes: 1 addition & 1 deletion .ci/.jenkins_nightly_nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
# made, these will stop and there will be no value in testing v17 nightlies.
#
NODEJS_VERSION:
- "17"
- "18"
1 change: 1 addition & 0 deletions .ci/.jenkins_nodejs.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
NODEJS_VERSION:
- "17"
- "16"
- "16.0"
- "14"
Expand Down
1 change: 1 addition & 0 deletions .ci/.jenkins_tav_nodejs.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
NODEJS_VERSION:
- "17"
- "16"
- "14"
- "12"
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ jobs:
strategy:
matrix:
node:
- '17'
- '16'
- '16.0'
- '14'
Expand All @@ -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
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions lib/instrumentation/modules/mongodb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions test/instrumentation/modules/fastify/_async-await.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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 () {
Expand Down
7 changes: 4 additions & 3 deletions test/instrumentation/modules/hapi/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
})
})
Expand Down Expand Up @@ -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)
}
Expand Down
28 changes: 18 additions & 10 deletions test/instrumentation/modules/http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/script/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]]
Expand Down