From be4e6e51ae80436f3fa75b11fa62e292f4b8ee71 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Aug 2023 21:49:50 +0200 Subject: [PATCH] chore: warn for missing size units in config and for `errorMessagesMaxLength` usage (#3553) Closes: #2121 Closes: #2314 Co-authored-by: Trent Mick --- CHANGELOG4.asciidoc | 2 +- docs/upgrade-to-v4.asciidoc | 44 +++++++++++++++++++++++++++------ lib/config/normalizers.js | 22 ++++++++++++++--- test/config/normalizers.test.js | 18 +++++++++++++- 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/CHANGELOG4.asciidoc b/CHANGELOG4.asciidoc index 74bb0f5a433..7d7044f16bf 100644 --- a/CHANGELOG4.asciidoc +++ b/CHANGELOG4.asciidoc @@ -39,7 +39,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]) * Change default value of `useElasticTraceparentHeader` config option to `false`. diff --git a/docs/upgrade-to-v4.asciidoc b/docs/upgrade-to-v4.asciidoc index b9590f12931..c88f8decdb3 100644 --- a/docs/upgrade-to-v4.asciidoc +++ b/docs/upgrade-to-v4.asciidoc @@ -50,6 +50,15 @@ start options. [[v4-api-changes]] ==== API changes +[[v4-api-start-transaction]] +===== `agent.startTransaction()` + +The `agent.startTransaction()` method has been changed to return a do-nothing +no-op Transaction, if the agent is not yet started. The return type has changed to +no longer include `| null`. The intent of these changes is to allow the user to use +`.startTransaction()` without having to worry if the agent is yet started, nor to +have to handle a possible `null` return value. + [[v4-api-to-string]] ===== `span.toString()`, `transaction.toString()` @@ -71,17 +80,36 @@ 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-api-start-transaction]] -===== `agent.startTransaction()` -The `agent.startTransaction()` method has been changed to return a do-nothing -no-op Transaction, if the agent is not yet started. The return type has changed to -no longer include `| null`. The intent of these changes is to allow the user to use -`.startTransaction()` without having to worry if the agent is yet started, nor to -have to handle a possible `null` return value. +[[v4-warnings]] +==== Log warnings + +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] +---- +{"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]] +===== "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 +in the value, the agent will warn about it and fallback to bytes (`b`). diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index 0f2314c549b..f9e64575cdb 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -86,11 +86,20 @@ 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) { + logger.warn( + 'units missing in size 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 +134,14 @@ 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); + 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 9f1e631baeb..8e05b277150 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,9 +164,23 @@ test('#normalizeBytes()', function (t) { badWithDefault: NaN, badWithoutDefault: NaN, anotherProperty: 25, + errorMessageMaxLength: 102400, }); - t.ok(logger.calls.length === 0, 'we got no warnings for bad byte options'); + 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'); + t.deepEqual(warnings[0].interpolation, ['12345678', 'numberBytes']); + 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[3].message.indexOf('"errorMessageMaxLength" is deprecated') !== -1, + 'warns about errorMessageMaxLength deprecation', + ); t.end(); });