From d226cfc5a949f425fe54f7296760b697fd8f196b Mon Sep 17 00:00:00 2001 From: David Luna Date: Thu, 3 Aug 2023 16:51:47 +0200 Subject: [PATCH 1/2] chore: move internal types to its corresponding module --- lib/config/normalizers.js | 92 +++++++++++++++++---------------------- lib/logging.js | 68 ++++++++++++++++++++--------- 2 files changed, 88 insertions(+), 72 deletions(-) diff --git a/lib/config/normalizers.js b/lib/config/normalizers.js index 3a6ee9e522..7faaafb30c 100644 --- a/lib/config/normalizers.js +++ b/lib/config/normalizers.js @@ -19,18 +19,6 @@ const { CONTEXT_MANAGER_ASYNCLOCALSTORAGE, } = require('./schema'); -// TODO: move this typedef to logger.js -/** - * @typedef {Object} Logger - * @property {function(Object | String, any, any, any): undefined} log - * @property {function(Object | String, any, any, any): undefined} info - * @property {function(Object | String, any, any, any): undefined} warn - * @property {function(Object | String, any, any, any): undefined} error - * @property {function(Object | String, any, any, any): undefined} fatal - * @property {function(Object | String, any, any, any): undefined} debug - * @property {function(Object | String, any, any, any): undefined} trace - */ - /** * Normalizes the key/value pairs properties of the config options object * KeyValuePairs config vars are either an object or a comma-separated string @@ -38,10 +26,10 @@ const { * {'foo': 'bar', 'eggs': 'spam'} => [['foo', 'bar'], ['eggs', 'spam']] * foo=bar, eggs=spam => [['foo', 'bar'], ['eggs', 'spam']] * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as key/value pair - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeKeyValuePairs(opts, fields, defaults, logger) { for (const key of fields) { @@ -69,10 +57,10 @@ function normalizeKeyValuePairs(opts, fields, defaults, logger) { /** * Normalizes the number properties of the config options object * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as number - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeNumbers(opts, fields, defaults, logger) { for (const key of fields) { @@ -83,10 +71,10 @@ function normalizeNumbers(opts, fields, defaults, logger) { /** * Normalizes the number properties of the config options object * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as number - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeInfinity(opts, fields, defaults, logger) { for (const key of fields) { @@ -97,8 +85,8 @@ function normalizeInfinity(opts, fields, defaults, logger) { /** * Translates a string byte size, e.g. '10kb', into an integer number of bytes. * - * @param {string} input - * @returns {number|undefined} + * @param {String} input + * @returns {Number|undefined} */ function bytes(input) { const matches = input.match(/^(\d+)(b|kb|mb|gb)$/i); @@ -130,10 +118,10 @@ function bytes(input) { /** * Normalizes the byte properties of the config options object * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as bytes - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeBytes(opts, fields, defaults, logger) { for (const key of fields) { @@ -152,15 +140,15 @@ function normalizeBytes(opts, fields, defaults, logger) { * secondsFromDuration('-1ms', 's', ['ms', 's', 'm'], true) // => -0.001 * secondsFromDuration(500, 'ms', ['us', 'ms', 's', 'm'], false) // => 0.5 * - * @param {string|number} duration - Typically a string of the form ``, + * @param {String|Number} duration - Typically a string of the form ``, * for example `30s`, `-1ms`, `2m`. The `defaultUnit` is used if a unit is * not part of the string, or if duration is a number. If given as a string, * decimal ('1.5s') and exponential-notation ('1e-3s') values are not allowed. - * @param {string} defaultUnit - * @param {Array} allowedUnits - An array of the allowed unit strings. This + * @param {String} defaultUnit + * @param {Array} allowedUnits - An array of the allowed unit strings. This * array may include any number of `us`, `ms`, `s`, and `m`. * @param {Boolean} allowNegative - Whether a negative number is allowed. - * @returns {number|null} + * @returns {Number|null} */ function secondsFromDuration( duration, @@ -223,10 +211,10 @@ function secondsFromDuration( /** * Normalizes the duration properties of the config options object * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {Array} fields the list of fields to normalize as duration (with name, defaultUnit, allowedUnits, allowNegative) - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeDurationOptions(opts, fields, defaults, logger) { for (const optSpec of fields) { @@ -274,10 +262,10 @@ function normalizeDurationOptions(opts, fields, defaults, logger) { * comma-separated string (whitespace is trimmed): * 'foo, bar' => ['foo', 'bar'] * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as arrays - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeArrays(opts, fields, defaults, logger) { for (const key of fields) { @@ -290,10 +278,10 @@ function normalizeArrays(opts, fields, defaults, logger) { /** * Parses "true"|"false" to boolean if not a boolean already and returns it. Returns undefined otherwise * - * @param {Logger} logger - * @param {string} key + * @param {import('../logging.js').Logger} logger + * @param {String} key * @param {any} value - * @returns {boolean|undefined} + * @returns {Boolean|undefined} */ function strictBool(logger, key, value) { if (typeof value === 'boolean') { @@ -316,10 +304,10 @@ function strictBool(logger, key, value) { * Boolean config vars are either already a boolean, or a string * representation of the boolean value: `true` or `false` * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as boolean - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeBools(opts, fields, defaults, logger) { for (const key of fields) { @@ -330,10 +318,10 @@ function normalizeBools(opts, fields, defaults, logger) { /** * Checks validity of the URL properties of the config options object. * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as boolean - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeUrls(opts, fields, defaults, logger) { for (const key of fields) { @@ -361,10 +349,10 @@ function normalizeUrls(opts, fields, defaults, logger) { * ['foo', /url/pathname$/] => ['foo'] (strings are placed into a specific config option) * => [/url/pathname$/] (RegExps are placed into a specific config option) * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as boolean - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeIgnoreOptions(opts, fields, defaults, logger) { // Params are meant to be used in upcoming changes @@ -417,7 +405,7 @@ function normalizeIgnoreOptions(opts, fields, defaults, logger) { * Normalizes the wildcard matchers of sanitizeFieldNames and thansforms the into RegExps * * TODO: we are doing the same to some ignoreOptions - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize */ function normalizeSanitizeFieldNames(opts) { if (opts.sanitizeFieldNames) { @@ -458,10 +446,10 @@ function normalizeElasticsearchCaptureBodyUrls(opts) { /** * Makes sure the cloudProvider options is valid othherwise it set the default value. * - * @param {Record} opts the configuration options to normalize + * @param {Record} opts the configuration options to normalize * @param {String[]} fields the list of fields to normalize as duration - * @param {Record} defaults the configuration defaults - * @param {Logger} logger + * @param {Record} defaults the configuration defaults + * @param {import('../logging.js').Logger} logger */ function normalizeCloudProvider(opts, fields, defaults, logger) { if ('cloudProvider' in opts) { diff --git a/lib/logging.js b/lib/logging.js index 58951563e3..ed40102c05 100644 --- a/lib/logging.js +++ b/lib/logging.js @@ -6,6 +6,17 @@ 'use strict'; +/** + * @typedef {Object} Logger + * @property {function(Record | string, ...any): undefined} log + * @property {function(Record | string, ...any): undefined} info + * @property {function(Record | string, ...any): undefined} warn + * @property {function(Record | string, ...any): undefined} error + * @property {function(Record | string, ...any): undefined} fatal + * @property {function(Record | string, ...any): undefined} debug + * @property {function(Record | string, ...any): undefined} trace + */ + // Internal logging for the Elastic APM Node.js Agent. // // Promised interface: @@ -81,25 +92,31 @@ class SafePinoDestWrapper { } } -// Create a pino logger for the agent. -// -// By default `createLogger()` will return a pino logger that logs to stdout -// in ecs-logging format, set to the "info" level. -// -// @param {String} levelName - Optional, default "info". It is meant to be one -// of the log levels specified in the top of file comment. For backward -// compatibility it falls back to "trace". -// @param {Object} customLogger - Optional. A custom logger object to which -// log messages will be passed. It must provide -// trace/debug/info/warn/error/fatal methods that take a string argument. -// -// Internally the agent uses structured logging using the pino API -// (https://getpino.io/#/docs/api?id=logger). However, with a custom logger, -// log record fields other than the *message* are dropped, to avoid issues -// with incompatible logger APIs. -// -// As a special case, if the provided logger is a *pino logger instance*, -// then it will be used directly. +/** + * Creates a pino logger for the agent. + * + * By default `createLogger()` will return a pino logger that logs to stdout + * in ecs-logging format, set to the "info" level. + * + * @param {String} levelName - Optional, default "info". It is meant to be one + * of the log levels specified in the top of file comment. For backward + * compatibility it falls back to "trace". + * @param {Object} customLogger - Optional. A custom logger object to which + * log messages will be passed. It must provide + * trace/debug/info/warn/error/fatal methods that take a string argument. + * + * Internally the agent uses structured logging using the pino API + * (https://getpino.io/#/docs/api?id=logger). However, with a custom logger, + * log record fields other than the *message* are dropped, to avoid issues + * with incompatible logger APIs. + * + * As a special case, if the provided logger is a *pino logger instance*, + * then it will be used directly. + * + * @param {string} [levelName=info] log level we want for the created logger + * @param {Logger} [customLogger] custom logger object provided by the user + * @returns {Logger} + */ function createLogger(levelName, customLogger) { let dest; const serializers = { @@ -163,11 +180,22 @@ function createLogger(levelName, customLogger) { return logger; } +/** + * Returns true if the logger is not ours + * + * @param {Logger} logger + * @returns {boolean} + */ function isLoggerCustom(logger) { return !logger[LOGGER_IS_OURS_SYM]; } -// Adjust the level on the given logger. +/** + * Adjust the level on the given logger. + * + * @param {Logger} logger + * @param {string} levelName + */ function setLogLevel(logger, levelName) { const pinoLevel = PINO_LEVEL_FROM_LEVEL_NAME[levelName]; if (!pinoLevel) { From 502d9c54ab3f98fe496758daaa988359b9b43194 Mon Sep 17 00:00:00 2001 From: David Luna Date: Thu, 3 Aug 2023 18:09:21 +0200 Subject: [PATCH 2/2] chore: re-order methods by log level --- lib/logging.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/logging.js b/lib/logging.js index ed40102c05..bbecf85865 100644 --- a/lib/logging.js +++ b/lib/logging.js @@ -8,11 +8,10 @@ /** * @typedef {Object} Logger - * @property {function(Record | string, ...any): undefined} log - * @property {function(Record | string, ...any): undefined} info - * @property {function(Record | string, ...any): undefined} warn - * @property {function(Record | string, ...any): undefined} error * @property {function(Record | string, ...any): undefined} fatal + * @property {function(Record | string, ...any): undefined} error + * @property {function(Record | string, ...any): undefined} warn + * @property {function(Record | string, ...any): undefined} info * @property {function(Record | string, ...any): undefined} debug * @property {function(Record | string, ...any): undefined} trace */