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

There are some
david-luna marked this conversation as resolved.
Show resolved Hide resolved

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

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.
david-luna marked this conversation as resolved.
Show resolved Hide resolved

[[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 tha agent will warn about it and fallback to `bytes`.
david-luna marked this conversation as resolved.
Show resolved Hide resolved

[[v4-warning-error-message-max-length]]
=====

Agent will warn if the deprecated config option `errorMessageMaxLength`
is set
david-luna marked this conversation as resolved.
Show resolved Hide resolved
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