From 3adaab99e1e471b207f0b99a5404ae9efc160bde Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 30 Mar 2023 17:16:31 -0700 Subject: [PATCH 1/5] feat: only send 'activation_method' to APM server >=8.7.1 This works around an issue in APM server 8.7.0 which had problems receiving metadata with activation_method. Closes: #3230 --- CHANGELOG.asciidoc | 22 ++++++++++++++++--- lib/instrumentation/azure-functions.js | 2 +- lib/lambda.js | 2 +- package-lock.json | 14 ++++++------ package.json | 4 ++-- test/_capturing_transport.js | 4 ++++ test/_mock_http_client.js | 3 +++ test/_mock_http_client_states.js | 3 +++ .../activation-method.test.js | 2 +- test/agent.test.js | 12 +++++----- test/config.test.js | 4 ++++ test/outcome.test.js | 3 ++- test/test.js | 13 ++++++++++- 13 files changed, 64 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 5a4f4e7b48..1e12b6fe35 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,8 +32,7 @@ Notes: === Node.js Agent version 3.x -[[release-notes-3.43.0]] -==== 3.43.0 2023/03/02 +==== Unreleased [float] ===== Breaking changes @@ -41,6 +40,23 @@ Notes: [float] ===== Features +[float] +===== Bug fixes + +* Ensure `metadata.service.agent.activation_method` is only sent for APM + server version 8.7.1 or later. APM server 8.7.0 included a bug where + receiving `activation_method` is harmful. ({issues}3230[#3230]) + +[float] +===== Chores + + +[[release-notes-3.43.0]] +==== 3.43.0 2023/03/02 + +[float] +===== Features + * Support mongodb v5. ({issues}3138[#3138]) * Propagate trace-context in message attributes for SQS (SendMessage and @@ -55,7 +71,7 @@ Notes: * Make `Agent.flush()` return a `Promise` if no callback is passed as param. This means that flush is now `await`able: `await apm.flush()`. - ({issues}2857(#2857)) + ({issues}2857[#2857]) [float] ===== Bug fixes diff --git a/lib/instrumentation/azure-functions.js b/lib/instrumentation/azure-functions.js index 61cd681de5..39cd53ef99 100644 --- a/lib/instrumentation/azure-functions.js +++ b/lib/instrumentation/azure-functions.js @@ -252,7 +252,7 @@ function getAzureFunctionsExtraMetadata () { // Passing this service.framework.name to Client#setExtraMetadata() // ensures that it "wins" over a framework name from // `agent.setFramework()`, because in the client `_extraMetadata` - // wins over `_conf.metadata`. + // wins over `_conf.frameworkName`. name: 'Azure Functions', version: process.env.FUNCTIONS_EXTENSION_VERSION }, diff --git a/lib/lambda.js b/lib/lambda.js index eb399edc4b..698dcca5b2 100644 --- a/lib/lambda.js +++ b/lib/lambda.js @@ -58,7 +58,7 @@ function getMetadata (agent, cloudAccountId) { // Passing this service.framework.name to Client#setExtraMetadata() // ensures that it "wins" over a framework name from // `agent.setFramework()`, because in the client `_extraMetadata` - // wins over `_conf.metadata`. + // wins over `_conf.frameworkName`. name: 'AWS Lambda' }, runtime: { diff --git a/package-lock.json b/package-lock.json index 69f4e01001..8beef661ba 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,7 @@ "cookie": "^0.5.0", "core-util-is": "^1.0.2", "debug": "^4.1.1", - "elastic-apm-http-client": "11.2.0", + "elastic-apm-http-client": "11.3.0", "end-of-stream": "^1.4.4", "error-callsites": "^2.0.4", "error-stack-parser": "^2.0.6", @@ -6180,9 +6180,9 @@ "dev": true }, "node_modules/elastic-apm-http-client": { - "version": "11.2.0", - "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.2.0.tgz", - "integrity": "sha512-XHXK+gQmd34eRN/ffrml7AN4h1VwujB79WEO2C/J59ufvEk+mT1OGBhl6pntHPUWn4Um52C5m84O6jIXzaQwfw==", + "version": "11.3.0", + "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.0.tgz", + "integrity": "sha512-oDs0JvS3pg/nTMarj8UknWv4VVdxc0OPSg2G7IJD/dxnAUb/SgxWAC8oaiaqWe9X/PeOVwBrlTcs+reIpeauaw==", "dependencies": { "agentkeepalive": "^4.2.1", "breadth-filter": "^2.0.0", @@ -20239,9 +20239,9 @@ "dev": true }, "elastic-apm-http-client": { - "version": "11.2.0", - "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.2.0.tgz", - "integrity": "sha512-XHXK+gQmd34eRN/ffrml7AN4h1VwujB79WEO2C/J59ufvEk+mT1OGBhl6pntHPUWn4Um52C5m84O6jIXzaQwfw==", + "version": "11.3.0", + "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.0.tgz", + "integrity": "sha512-oDs0JvS3pg/nTMarj8UknWv4VVdxc0OPSg2G7IJD/dxnAUb/SgxWAC8oaiaqWe9X/PeOVwBrlTcs+reIpeauaw==", "requires": { "agentkeepalive": "^4.2.1", "breadth-filter": "^2.0.0", diff --git a/package.json b/package.json index a172e74ecb..62cf4060df 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "lint:yaml-files": "./dev-utils/lint-yaml-files.sh # requires node >=10", "coverage": "COVERAGE=true ./test/script/run_tests.sh", "test": "./test/script/run_tests.sh", - "test:deps": "dependency-check index.js start.js start-next.js 'lib/**/*.js' 'test/**/*.js' '!test/activation-method/fixtures' '!test/instrumentation/modules/next/a-nextjs-app' --no-dev -i async_hooks -i perf_hooks -i node:http -i @azure/functions-core", + "test:deps": "dependency-check index.js start.js start-next.js 'lib/**/*.js' 'test/**/*.js' '!test/activation-method/fixtures' '!test/instrumentation/azure-functions/fixtures' '!test/instrumentation/modules/next/a-nextjs-app' --no-dev -i async_hooks -i perf_hooks -i node:http -i @azure/functions-core", "test:tav": "tav --quiet && (cd test/instrumentation/modules/next/a-nextjs-app && tav --quiet)", "test:docs": "./test/script/docker/run_docs.sh", "test:types": "tsc --project test/types/tsconfig.json && tsc --project test/types/transpile/tsconfig.json && node test/types/transpile/index.js && tsc --project test/types/transpile-default/tsconfig.json && node test/types/transpile-default/index.js # requires node >=12.20", @@ -95,7 +95,7 @@ "cookie": "^0.5.0", "core-util-is": "^1.0.2", "debug": "^4.1.1", - "elastic-apm-http-client": "11.2.0", + "elastic-apm-http-client": "11.3.0", "end-of-stream": "^1.4.4", "error-callsites": "^2.0.4", "error-stack-parser": "^2.0.6", diff --git a/test/_capturing_transport.js b/test/_capturing_transport.js index 1625a23eef..a731152cf2 100644 --- a/test/_capturing_transport.js +++ b/test/_capturing_transport.js @@ -98,6 +98,10 @@ class CapturingTransport { return true } + supportsActivationMethodField () { + return true + } + // Inherited from Writable, called in agent.js. destroy () {} } diff --git a/test/_mock_http_client.js b/test/_mock_http_client.js index 4ad07d3c22..74ccea3a1d 100644 --- a/test/_mock_http_client.js +++ b/test/_mock_http_client.js @@ -69,6 +69,9 @@ module.exports = function (expected, done) { }, supportsKeepingUnsampledTransaction () { return true + }, + supportsActivationMethodField () { + return true } } diff --git a/test/_mock_http_client_states.js b/test/_mock_http_client_states.js index df781b7e6e..952ccef894 100644 --- a/test/_mock_http_client_states.js +++ b/test/_mock_http_client_states.js @@ -50,6 +50,9 @@ module.exports = function (expectations = [], done) { }, supportsKeepingUnsampledTransaction () { return true + }, + supportsActivationMethodField () { + return true } } } diff --git a/test/activation-method/activation-method.test.js b/test/activation-method/activation-method.test.js index 9175020280..675cbf627a 100644 --- a/test/activation-method/activation-method.test.js +++ b/test/activation-method/activation-method.test.js @@ -107,7 +107,7 @@ tape.test('metadata.system.agent.activation_method fixtures', function (suite) { const envStr = c.env ? Object.keys(c.env).map(k => `${k}="${c.env[k]}"`).join(' ') : '' suite.test(`${envStr} node ${(c.nodeOpts || []).join(' ')} ${c.script}`, t => { - const server = new MockAPMServer() + const server = new MockAPMServer({ apmServerVersion: '8.7.1' }) const args = c.nodeOpts || [] args.push(c.script) server.start(function (serverUrl) { diff --git a/test/agent.test.js b/test/agent.test.js index 148f5760f0..95f3d6560a 100644 --- a/test/agent.test.js +++ b/test/agent.test.js @@ -34,7 +34,10 @@ const agentOpts = { metricsInterval: '0s', cloudProvider: 'none', spanStackTraceMinDuration: 0, // Always have span stacktraces. - logLevel: 'warn' + logLevel: 'warn', + // Ensure the APM client's `GET /` requests do not get in the way of test + // asserts. Also ensure it is new enough to include 'activation_method'. + apmServerVersion: '8.7.1' } const agentOptsNoopTransport = Object.assign( {}, @@ -574,12 +577,7 @@ test('filters', function (t) { filterAgentOpts = Object.assign( {}, agentOpts, - { - serverUrl, - // Ensure the APM client's `GET /` requests do not get in the way of - // the test asserts below. - apmServerVersion: '8.0.0' - } + { serverUrl } ) t.end() }) diff --git a/test/config.test.js b/test/config.test.js index cc46049bdc..3e47bbecc2 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -1129,6 +1129,10 @@ test('custom transport', function (t) { supportsKeepingUnsampledTransaction () { return true } + + supportsActivationMethodField () { + return true + } } const myTransport = new MyTransport() diff --git a/test/outcome.test.js b/test/outcome.test.js index 9e448ea032..b3a84fb768 100644 --- a/test/outcome.test.js +++ b/test/outcome.test.js @@ -20,7 +20,8 @@ const noOpClient = { sendError () {}, sendMetricSet () {}, flush () {}, - supportsKeepingUnsampledTransaction () { return true } + supportsKeepingUnsampledTransaction () { return true }, + supportsActivationMethodField () { return true } } agent._transport = noOpClient diff --git a/test/test.js b/test/test.js index 2eece3f5ac..bd24fa0926 100644 --- a/test/test.js +++ b/test/test.js @@ -28,6 +28,9 @@ function getDirectoriesWithTests (path = './test', results = [], exclude = []) { const result = fs.readdirSync(path) for (const file of result) { const pathToTest = `${path}/${file}` + console.log('XXX pathToTest: ', pathToTest) + // XXX pathToTest: ./test/instrumentation/modules/next/a-nextjs-app/node_modules/tslib/tslib.js + // XXX just use glob?? if ( file.indexOf('.test.js') !== -1 && // is a test file results.indexOf(path) === -1 && // has not been found yet @@ -179,7 +182,15 @@ if (opts.help) { // './test/start/env', './test/start/file' which are special // cases var directories = getDirectoriesWithTests( - './test', [], ['./test/start/env', './test/start/file'] + './test', + [], + [ + './test/start/env', + './test/start/file', + './test/activation-method/fixtures', + './test/instrumentation/modules/next/a-nextjs-app', + './test/instrumentation/azure-functions/fixtures' + ] ) mapSeries(directories, readdir, function (err, directoryFiles) { From fbc2eb15613bb9f0a6d0d087aabd85834d33905c Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 30 Mar 2023 17:19:36 -0700 Subject: [PATCH 2/5] tweaks, drop some dev/debug code --- CHANGELOG.asciidoc | 7 ++++--- test/test.js | 7 +------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1e12b6fe35..f457330464 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -40,13 +40,14 @@ Notes: [float] ===== Features -[float] -===== Bug fixes - * Ensure `metadata.service.agent.activation_method` is only sent for APM server version 8.7.1 or later. APM server 8.7.0 included a bug where receiving `activation_method` is harmful. ({issues}3230[#3230]) +[float] +===== Bug fixes + + [float] ===== Chores diff --git a/test/test.js b/test/test.js index bd24fa0926..c97c31574b 100644 --- a/test/test.js +++ b/test/test.js @@ -28,9 +28,6 @@ function getDirectoriesWithTests (path = './test', results = [], exclude = []) { const result = fs.readdirSync(path) for (const file of result) { const pathToTest = `${path}/${file}` - console.log('XXX pathToTest: ', pathToTest) - // XXX pathToTest: ./test/instrumentation/modules/next/a-nextjs-app/node_modules/tslib/tslib.js - // XXX just use glob?? if ( file.indexOf('.test.js') !== -1 && // is a test file results.indexOf(path) === -1 && // has not been found yet @@ -187,9 +184,7 @@ var directories = getDirectoriesWithTests( [ './test/start/env', './test/start/file', - './test/activation-method/fixtures', - './test/instrumentation/modules/next/a-nextjs-app', - './test/instrumentation/azure-functions/fixtures' + './test/activation-method/fixtures' ] ) From a0e7d5f45c20539d0322f6d8e446e40e85512b8e Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 30 Mar 2023 17:21:26 -0700 Subject: [PATCH 3/5] put in correct changelog section --- CHANGELOG.asciidoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f457330464..538a55872e 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -40,13 +40,13 @@ Notes: [float] ===== Features +[float] +===== Bug fixes + * Ensure `metadata.service.agent.activation_method` is only sent for APM server version 8.7.1 or later. APM server 8.7.0 included a bug where receiving `activation_method` is harmful. ({issues}3230[#3230]) -[float] -===== Bug fixes - [float] ===== Chores From b0c8b17ed1b0f76fb08fa808edd6c573e9ca79e2 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Mon, 3 Apr 2023 14:00:07 -0700 Subject: [PATCH 4/5] bump to http-client 11.3.1 to get new tweaked logic to optimistically assume non-8.7.0 if don't know yet --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8beef661ba..169f06c458 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,7 @@ "cookie": "^0.5.0", "core-util-is": "^1.0.2", "debug": "^4.1.1", - "elastic-apm-http-client": "11.3.0", + "elastic-apm-http-client": "11.3.1", "end-of-stream": "^1.4.4", "error-callsites": "^2.0.4", "error-stack-parser": "^2.0.6", @@ -6180,9 +6180,9 @@ "dev": true }, "node_modules/elastic-apm-http-client": { - "version": "11.3.0", - "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.0.tgz", - "integrity": "sha512-oDs0JvS3pg/nTMarj8UknWv4VVdxc0OPSg2G7IJD/dxnAUb/SgxWAC8oaiaqWe9X/PeOVwBrlTcs+reIpeauaw==", + "version": "11.3.1", + "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.1.tgz", + "integrity": "sha512-AicMDPJYmO4tGTRcZQZFS7nhyGAQ6f4hUTKQVxiM/6mHo6F895JsJoPVwOE2yCathKDnD9LVS9zWdT2wvG8gxA==", "dependencies": { "agentkeepalive": "^4.2.1", "breadth-filter": "^2.0.0", @@ -20239,9 +20239,9 @@ "dev": true }, "elastic-apm-http-client": { - "version": "11.3.0", - "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.0.tgz", - "integrity": "sha512-oDs0JvS3pg/nTMarj8UknWv4VVdxc0OPSg2G7IJD/dxnAUb/SgxWAC8oaiaqWe9X/PeOVwBrlTcs+reIpeauaw==", + "version": "11.3.1", + "resolved": "https://registry.npmjs.org/elastic-apm-http-client/-/elastic-apm-http-client-11.3.1.tgz", + "integrity": "sha512-AicMDPJYmO4tGTRcZQZFS7nhyGAQ6f4hUTKQVxiM/6mHo6F895JsJoPVwOE2yCathKDnD9LVS9zWdT2wvG8gxA==", "requires": { "agentkeepalive": "^4.2.1", "breadth-filter": "^2.0.0", diff --git a/package.json b/package.json index 62cf4060df..7f072ffd33 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,7 @@ "cookie": "^0.5.0", "core-util-is": "^1.0.2", "debug": "^4.1.1", - "elastic-apm-http-client": "11.3.0", + "elastic-apm-http-client": "11.3.1", "end-of-stream": "^1.4.4", "error-callsites": "^2.0.4", "error-stack-parser": "^2.0.6", From df6064e8da2cb041a1c536461d7fc5fbfe4e0ce4 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Mon, 3 Apr 2023 14:13:30 -0700 Subject: [PATCH 5/5] fix merge of CHANGELOG --- CHANGELOG.asciidoc | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d33a70fb43..1d2a344d5c 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,7 @@ Notes: [[release-notes-3.x]] === Node.js Agent version 3.x + ==== Unreleased [float] @@ -45,26 +46,10 @@ Notes: [float] ===== Bug fixes -[float] -===== Chores - - -==== Unreleased - -[float] -===== Breaking changes - -[float] -===== Features - -[float] -===== Bug fixes - * Ensure `metadata.service.agent.activation_method` is only sent for APM server version 8.7.1 or later. APM server 8.7.0 included a bug where receiving `activation_method` is harmful. ({issues}3230[#3230]) - [float] ===== Chores