chore: warn for missing size units in config and for `errorMessagesMa…

…xLength` usage (#3553)

Closes: #2121
Closes: #2314
Co-authored-by: Trent Mick <[email protected]>

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`.

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 <<log-correlation-ids>>.
 
-[[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`).
 

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"',
+        );
+      }
+    }
   }
 }
 

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();
 });