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

chore: warn for missing units in config and for errorMessagesMaxLength usage #3553

Merged
merged 14 commits into from
Aug 4, 2023
Merged
2 changes: 1 addition & 1 deletion CHANGELOG4.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
29 changes: 28 additions & 1 deletion docs/upgrade-to-v4.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,34 @@ 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-warnings]]
==== Log warnings

This section documents some new log output warnings from the APM agent, and how to avoid them.

[[v4-warning-duration-units]]
=====
trentm marked this conversation as resolved.
Show resolved Hide resolved


[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]]
=====
trentm marked this conversation as resolved.
Show resolved Hide resolved

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`).
22 changes: 19 additions & 3 deletions lib/config/normalizers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 value "%s" for "%s" config option. Use one of b|kb|mb|gb',
trentm marked this conversation as resolved.
Show resolved Hide resolved
input,
key,
);
return Number(input);
}

const suffix = matches[2].toLowerCase();
let value = Number(matches[1]);
Expand Down Expand Up @@ -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"',
);
}
}
}
}

Expand Down
17 changes: 16 additions & 1 deletion test/config/normalizers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ test('#normalizeBytes()', function (t) {
'numberBytes',
'badWithDefault',
'badWithoutDefault',
'errorMessageMaxLength',
];
const defaults = { badWithDefault: '25kb' };
const opts = {
Expand All @@ -149,6 +150,7 @@ test('#normalizeBytes()', function (t) {
badWithDefault: 'not-bytes',
badWithoutDefault: 'not-bytes',
anotherProperty: 25,
errorMessageMaxLength: '100kb',
};

normalizeBytes(opts, fields, defaults, logger);
Expand All @@ -162,9 +164,22 @@ 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 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();
});

Expand Down