From f476e9c2c2a648670718e253c0a8cd564483ab78 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 10:28:59 +0200 Subject: [PATCH 01/13] chore: add logs for missing size units --- lib/config/normalizers.js | 16 +++++++++++++--- test/config/normalizers.test.js | 19 ++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index 0f2314c549..f02627430f 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -86,11 +86,21 @@ function normalizeInfinity(opts, fields, defaults, logger) { * Translates a string byte size, e.g. '10kb', into an integer number of bytes. * * @param {String} input + * @param {string} key - the config option key + * @param {import('../logging.js').Logger} logger * @returns {Number|undefined} */ -function bytes(input) { +function bytes(input, key, logger) { const matches = input.match(/^(\d+)(b|kb|mb|gb)$/i); - if (!matches) return Number(input); + if (!matches) { + console.log('no match!!!!', input); + logger.warn( + 'units missing in value "%s" for "%s" config option. Use one of b|kb|mb|gb', + input, + key, + ); + return Number(input); + } const suffix = matches[2].toLowerCase(); let value = Number(matches[1]); @@ -125,7 +135,7 @@ function bytes(input) { */ function normalizeBytes(opts, fields, defaults, logger) { for (const key of fields) { - if (key in opts) opts[key] = bytes(String(opts[key])); + if (key in opts) opts[key] = bytes(String(opts[key]), key, logger); } } diff --git a/test/config/normalizers.test.js b/test/config/normalizers.test.js index 9f1e631bae..ece4d3a651 100644 --- a/test/config/normalizers.test.js +++ b/test/config/normalizers.test.js @@ -164,7 +164,24 @@ test('#normalizeBytes()', function (t) { anotherProperty: 25, }); - t.ok(logger.calls.length === 0, 'we got no warnings for bad byte options'); + const warnString = 'units missing in value'; + const warnings = logger.calls; + t.ok(warnings.length === 3, 'we got warnings for missing units'); + t.ok( + warnings[0].message.indexOf(warnString) !== -1, + 'warns about missing unit', + ); + t.deepEqual(warnings[0].interpolation, ['12345678', 'numberBytes']); + t.ok( + warnings[1].message.indexOf(warnString) !== -1, + 'warns about missing unit', + ); + t.deepEqual(warnings[1].interpolation, ['not-bytes', 'badWithDefault']); + t.ok( + warnings[2].message.indexOf(warnString) !== -1, + 'warns about missing unit', + ); + t.deepEqual(warnings[2].interpolation, ['not-bytes', 'badWithoutDefault']); t.end(); }); From eaaceeb00c3f8b27796dd713fcb175689ada3ae2 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 11:02:44 +0200 Subject: [PATCH 02/13] docs: update changelog4 and migration guide --- CHANGELOG4.asciidoc | 2 +- docs/upgrade-to-v4.asciidoc | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGELOG4.asciidoc b/CHANGELOG4.asciidoc index c6206e88b3..581dbc11ce 100644 --- a/CHANGELOG4.asciidoc +++ b/CHANGELOG4.asciidoc @@ -36,7 +36,7 @@ [float] ===== Chores -* Add a warning message when a duration config option is provided +* Add a warning message when a duration or size config option is provided without units. ({issues}2121[#2121]) [[release-notes-3.x]] diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index 55b6afc0d0..a6c41d9069 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -59,7 +59,28 @@ via: [source,js] ---- const {stringify} = require('querystring'); -console.log( stringify(span.ids, ' ', '=')) ); +console.log(stringify(span.ids, ' ', '=')); ---- For log correlation with _structured_ logs, see <>. + +[[v4-warnings]] +==== Log warnings + +There are some + +[[v4-warning-duration-units]] +===== + +Configuration options that define a duration like `apiRequestTime` or +`exitSpanMinDuration` expect to have their units specified in the value +(e.g. `10s`, `100ms`). If the unit is not in the value tha agent will +warn about it and fallback to its default unit which is specified in +the default value. + +[[v4-warning-size-units]] +===== + +`apiRequestSize` and `errorMessageMaxLength` config options expect to +have the size specified in the value (e.g. `10kb`, `1gb`). If the unit +is not in the value tha agent will warn about it and fallback to `bytes`. From c14fd1fd2bf7fbccbc64cb0df0ce29009963486a Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 11:07:42 +0200 Subject: [PATCH 03/13] chore: remove console.log --- lib/config/normalizers.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index f02627430f..309a816648 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -93,7 +93,6 @@ function normalizeInfinity(opts, fields, defaults, logger) { function bytes(input, key, logger) { const matches = input.match(/^(\d+)(b|kb|mb|gb)$/i); if (!matches) { - console.log('no match!!!!', input); logger.warn( 'units missing in value "%s" for "%s" config option. Use one of b|kb|mb|gb', input, From 7d1750f0710d1afa1eedf69d7852bff6aac8d62a Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 11:17:46 +0200 Subject: [PATCH 04/13] docs: update docs --- docs/upgrade-to-v4.asciidoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index a6c41d9069..575162078e 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -81,6 +81,6 @@ the default value. [[v4-warning-size-units]] ===== -`apiRequestSize` and `errorMessageMaxLength` config options expect to -have the size specified in the value (e.g. `10kb`, `1gb`). If the unit -is not in the value tha agent will warn about it and fallback to `bytes`. +Byte size config options like `apiRequestSize` expect to have the size +units specified in the value (e.g. `10kb`, `1gb`). If the unit is not +in the value tha agent will warn about it and fallback to `bytes`. From f2e6458fdd5e163699d0aef768dc144153cc587c Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 12:22:53 +0200 Subject: [PATCH 05/13] chore: add warning for erroMessageMaxLenght deprecation --- docs/upgrade-to-v4.asciidoc | 6 ++++++ lib/config/normalizers.js | 9 ++++++++- test/config/normalizers.test.js | 24 +++++++++++------------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index 575162078e..87ae119ee8 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -84,3 +84,9 @@ the default value. Byte size config options like `apiRequestSize` expect to have the size units specified in the value (e.g. `10kb`, `1gb`). If the unit is not in the value tha agent will warn about it and fallback to `bytes`. + +[[v4-warning-error-message-max-length]] +===== + +Agent will warn if the deprecated config option `errorMessageMaxLength` +is set diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index 309a816648..c0e7f2c930 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -134,7 +134,14 @@ function bytes(input, key, logger) { */ function normalizeBytes(opts, fields, defaults, logger) { for (const key of fields) { - if (key in opts) opts[key] = bytes(String(opts[key]), key, logger); + if (key in opts) { + opts[key] = bytes(String(opts[key]), key, logger); + if (key === 'errorMessageMaxLength') { + logger.warn( + 'config option "errorMessageMaxLength" is deprecated. Use "longFieldMaxLength"', + ); + } + } } } diff --git a/test/config/normalizers.test.js b/test/config/normalizers.test.js index ece4d3a651..23b7580b48 100644 --- a/test/config/normalizers.test.js +++ b/test/config/normalizers.test.js @@ -138,6 +138,7 @@ test('#normalizeBytes()', function (t) { 'numberBytes', 'badWithDefault', 'badWithoutDefault', + 'errorMessageMaxLength', ]; const defaults = { badWithDefault: '25kb' }; const opts = { @@ -149,6 +150,7 @@ test('#normalizeBytes()', function (t) { badWithDefault: 'not-bytes', badWithoutDefault: 'not-bytes', anotherProperty: 25, + errorMessageMaxLength: '100kb', }; normalizeBytes(opts, fields, defaults, logger); @@ -162,26 +164,22 @@ test('#normalizeBytes()', function (t) { badWithDefault: NaN, badWithoutDefault: NaN, anotherProperty: 25, + errorMessageMaxLength: 102400, }); - const warnString = 'units missing in value'; + const isMissingUnitsWarn = (s) => s.indexOf('units missing in value') !== -1; const warnings = logger.calls; - t.ok(warnings.length === 3, 'we got warnings for missing units'); - t.ok( - warnings[0].message.indexOf(warnString) !== -1, - 'warns about missing unit', - ); + t.ok(warnings.length === 4, 'we got warnings'); + t.ok(isMissingUnitsWarn(warnings[0].message), 'warns about missing unit'); t.deepEqual(warnings[0].interpolation, ['12345678', 'numberBytes']); - t.ok( - warnings[1].message.indexOf(warnString) !== -1, - 'warns about missing unit', - ); + t.ok(isMissingUnitsWarn(warnings[1].message), 'warns about missing unit'); t.deepEqual(warnings[1].interpolation, ['not-bytes', 'badWithDefault']); + t.ok(isMissingUnitsWarn(warnings[2].message), 'warns about missing unit'); + t.deepEqual(warnings[2].interpolation, ['not-bytes', 'badWithoutDefault']); t.ok( - warnings[2].message.indexOf(warnString) !== -1, - 'warns about missing unit', + warnings[3].message.indexOf('"errorMessageMaxLength" is deprecated') !== -1, + 'warns about errorMessageMaxLength deprecation', ); - t.deepEqual(warnings[2].interpolation, ['not-bytes', 'badWithoutDefault']); t.end(); }); From 9f41d9c0742399a2658c978709e14bed6fa4e657 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 19:11:08 +0200 Subject: [PATCH 06/13] Update docs/upgrade-to-v4.asciidoc Co-authored-by: Trent Mick --- docs/upgrade-to-v4.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index 87ae119ee8..a232ad7eb9 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -67,7 +67,7 @@ For log correlation with _structured_ logs, see <>. [[v4-warnings]] ==== Log warnings -There are some +This section documents some new log output warnings from the APM agent, and how to avoid them. [[v4-warning-duration-units]] ===== From 72208675a9c1f2f05137b7d33cc93f70d39a39de Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 19:11:33 +0200 Subject: [PATCH 07/13] Update docs/upgrade-to-v4.asciidoc Co-authored-by: Trent Mick --- docs/upgrade-to-v4.asciidoc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index a232ad7eb9..f4ba3cc55e 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -72,11 +72,17 @@ This section documents some new log output warnings from the APM agent, and how [[v4-warning-duration-units]] ===== -Configuration options that define a duration like `apiRequestTime` or -`exitSpanMinDuration` expect to have their units specified in the value -(e.g. `10s`, `100ms`). If the unit is not in the value tha agent will -warn about it and fallback to its default unit which is specified in -the default value. + +[source,json] +---- +{"log.level":"warn","@timestamp":"2023-08-04T16:54:03.116Z","log":{"logger":"elastic-apm-node"},"ecs":{"version":"1.6.0"},"message":"units missing in duration value \"5\" for \"metricsInterval\" config option: using default units \"s\""} +---- + +Configuration options that define a duration, like `metricsInterval` or +`exitSpanMinDuration`, expect to have their units specified in the value +(e.g. `"10s"`, `"100ms"`). While current duration options have a default +unit, to avoid ambiguity the APM agent will now warn if the units are not +provided. [[v4-warning-size-units]] ===== From a92971ea4b63fa012191c3ac3158ed2b38035420 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 19:11:54 +0200 Subject: [PATCH 08/13] Update docs/upgrade-to-v4.asciidoc Co-authored-by: Trent Mick --- docs/upgrade-to-v4.asciidoc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index f4ba3cc55e..31e713f3b2 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -90,9 +90,3 @@ provided. Byte size config options like `apiRequestSize` expect to have the size units specified in the value (e.g. `10kb`, `1gb`). If the unit is not in the value tha agent will warn about it and fallback to `bytes`. - -[[v4-warning-error-message-max-length]] -===== - -Agent will warn if the deprecated config option `errorMessageMaxLength` -is set From 8de985553230026ab2600997be3daf8688a74ed5 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 19:12:20 +0200 Subject: [PATCH 09/13] Update docs/upgrade-to-v4.asciidoc Co-authored-by: Trent Mick --- docs/upgrade-to-v4.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index 31e713f3b2..b37490b1e3 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -88,5 +88,5 @@ provided. ===== Byte size config options like `apiRequestSize` expect to have the size -units specified in the value (e.g. `10kb`, `1gb`). If the unit is not -in the value tha agent will warn about it and fallback to `bytes`. +units specified in the value (e.g. `"10kb"`, `"1gb"`). If the unit is not +in the value, the agent will warn about it and fallback to bytes (`b`). From c558bec7afb466a65f24d0e4ebfd7a56c03f133d Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 4 Aug 2023 11:01:51 -0700 Subject: [PATCH 10/13] Update docs/upgrade-to-v4.asciidoc --- docs/upgrade-to-v4.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index b37490b1e3..f561ec688a 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -70,7 +70,7 @@ For log correlation with _structured_ logs, see <>. This section documents some new log output warnings from the APM agent, and how to avoid them. [[v4-warning-duration-units]] -===== +===== "units missing in duration value" [source,json] From 0dc77d82bfc635f0ae92054c39502adb595fa693 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 4 Aug 2023 11:01:56 -0700 Subject: [PATCH 11/13] Update docs/upgrade-to-v4.asciidoc --- docs/upgrade-to-v4.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index f561ec688a..14df6c630b 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -85,7 +85,7 @@ unit, to avoid ambiguity the APM agent will now warn if the units are not provided. [[v4-warning-size-units]] -===== +===== "units missing in size value" Byte size config options like `apiRequestSize` expect to have the size units specified in the value (e.g. `"10kb"`, `"1gb"`). If the unit is not From f99c840ca748ddcda0886dde250532f580b791c4 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 4 Aug 2023 11:02:02 -0700 Subject: [PATCH 12/13] Update lib/config/normalizers.js --- lib/config/normalizers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index c0e7f2c930..f9e64575cd 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -94,7 +94,7 @@ function bytes(input, key, logger) { const matches = input.match(/^(\d+)(b|kb|mb|gb)$/i); if (!matches) { logger.warn( - 'units missing in value "%s" for "%s" config option. Use one of b|kb|mb|gb', + 'units missing in size value "%s" for "%s" config option. Use one of b|kb|mb|gb', input, key, ); From b65305c2f80ff980e81cb57fd9b4e9866e96ff80 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 4 Aug 2023 11:37:03 -0700 Subject: [PATCH 13/13] fix test failure I introduced with log warn msg change --- docs/upgrade-to-v4.asciidoc | 4 ++-- test/config/normalizers.test.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index 14df6c630b..cc4baf94c7 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -78,14 +78,14 @@ This section documents some new log output warnings from the APM agent, and how {"log.level":"warn","@timestamp":"2023-08-04T16:54:03.116Z","log":{"logger":"elastic-apm-node"},"ecs":{"version":"1.6.0"},"message":"units missing in duration value \"5\" for \"metricsInterval\" config option: using default units \"s\""} ---- -Configuration options that define a duration, like `metricsInterval` or +Configuration options that define a duration, like `metricsInterval` or `exitSpanMinDuration`, expect to have their units specified in the value (e.g. `"10s"`, `"100ms"`). While current duration options have a default unit, to avoid ambiguity the APM agent will now warn if the units are not provided. [[v4-warning-size-units]] -===== "units missing in size value" +===== "units missing in size value" Byte size config options like `apiRequestSize` expect to have the size units specified in the value (e.g. `"10kb"`, `"1gb"`). If the unit is not diff --git a/test/config/normalizers.test.js b/test/config/normalizers.test.js index 23b7580b48..8e05b27715 100644 --- a/test/config/normalizers.test.js +++ b/test/config/normalizers.test.js @@ -167,7 +167,8 @@ test('#normalizeBytes()', function (t) { errorMessageMaxLength: 102400, }); - const isMissingUnitsWarn = (s) => s.indexOf('units missing in value') !== -1; + const isMissingUnitsWarn = (s) => + s.indexOf('units missing in size value') !== -1; const warnings = logger.calls; t.ok(warnings.length === 4, 'we got warnings'); t.ok(isMissingUnitsWarn(warnings[0].message), 'warns about missing unit');