From 9841cfc7defd371557517947b894560cadc7770f Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Fri, 17 Feb 2023 19:02:37 +0100 Subject: [PATCH 1/3] feat: make loglevel types stricter --- packages/logger/src/Logger.ts | 36 +++++++++++-------- packages/logger/src/types/Log.ts | 4 +-- packages/logger/tests/unit/Logger.test.ts | 2 +- .../formatter/PowertoolLogFormatter.test.ts | 2 +- packages/logger/tests/unit/helpers.test.ts | 2 +- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 2ce5df7059..e93083cd71 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -117,7 +117,7 @@ class Logger extends Utility implements ClassThatLogs { private customConfigService?: ConfigServiceInterface; - private static readonly defaultLogLevel: LogLevel = 'INFO'; + private static readonly defaultLogLevel: Uppercase = 'INFO'; // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; @@ -128,7 +128,7 @@ class Logger extends Utility implements ClassThatLogs { private logIndentation: number = LogJsonIndent.COMPACT; - private logLevel?: LogLevel; + private logLevel?: Uppercase; private readonly logLevelThresholds: LogLevelThresholds = { DEBUG: 8, @@ -554,12 +554,16 @@ class Logger extends Utility implements ClassThatLogs { /** * It returns the log level set for the Logger instance. + * + * Even though logLevel starts as undefined, it will always be set to a value + * during the Logger instance's initialization. So, we can safely use the non-null + * assertion operator here. * * @private * @returns {LogLevel} */ - private getLogLevel(): LogLevel { - return this.logLevel; + private getLogLevel(): Uppercase { + return this.logLevel!; } /** @@ -619,14 +623,14 @@ class Logger extends Utility implements ClassThatLogs { } /** - * It returns true if the provided log level is valid. + * It returns true and type guards the log level if a given log level is valid. * * @param {LogLevel} logLevel * @private * @returns {boolean} */ - private isValidLogLevel(logLevel?: LogLevel): boolean { - return typeof logLevel === 'string' && logLevel.toUpperCase() in this.logLevelThresholds; + private isValidLogLevel(logLevel?: LogLevel | string): logLevel is LogLevel { + return typeof logLevel === 'string' && logLevel in this.logLevelThresholds; } /** @@ -648,7 +652,8 @@ class Logger extends Utility implements ClassThatLogs { * It prints a given log with given log level. * * @param {LogLevel} logLevel - * @param {LogItem} log + * @param {LogItemMessage} input + * @param {LogItemExtraInput} extraInput * @private */ private processLogItem(logLevel: LogLevel, input: LogItemMessage, extraInput: LogItemExtraInput): void { @@ -744,19 +749,19 @@ class Logger extends Utility implements ClassThatLogs { */ private setLogLevel(logLevel?: LogLevel): void { if (this.isValidLogLevel(logLevel)) { - this.logLevel = (logLevel).toUpperCase(); + this.logLevel = logLevel?.toUpperCase() as Uppercase; return; } const customConfigValue = this.getCustomConfigService()?.getLogLevel(); if (this.isValidLogLevel(customConfigValue)) { - this.logLevel = (customConfigValue).toUpperCase(); + this.logLevel = customConfigValue?.toUpperCase() as Uppercase; return; } const envVarsValue = this.getEnvVarsService().getLogLevel(); if (this.isValidLogLevel(envVarsValue)) { - this.logLevel = (envVarsValue).toUpperCase(); + this.logLevel = envVarsValue?.toUpperCase() as Uppercase; return; } @@ -845,14 +850,15 @@ class Logger extends Utility implements ClassThatLogs { /** * It checks whether the current log item should/can be printed. * - * @param {string} serviceName - * @param {Environment} environment - * @param {LogAttributes} persistentLogAttributes + * @param {LogLevel} logLevel * @private * @returns {boolean} */ private shouldPrint(logLevel: LogLevel): boolean { - if (this.logLevelThresholds[logLevel] >= this.logLevelThresholds[this.getLogLevel()]) { + if ( + this.logLevelThresholds[logLevel.toUpperCase() as Uppercase] >= + this.logLevelThresholds[this.getLogLevel()] + ) { return true; } diff --git a/packages/logger/src/types/Log.ts b/packages/logger/src/types/Log.ts index 0b7cd5d30f..020b9cf200 100644 --- a/packages/logger/src/types/Log.ts +++ b/packages/logger/src/types/Log.ts @@ -3,10 +3,10 @@ type LogLevelInfo = 'INFO'; type LogLevelWarn = 'WARN'; type LogLevelError = 'ERROR'; -type LogLevel = LogLevelDebug | LogLevelInfo | LogLevelWarn | LogLevelError | string; +type LogLevel = LogLevelDebug | Lowercase | LogLevelInfo | Lowercase | LogLevelWarn | Lowercase | LogLevelError | Lowercase; type LogLevelThresholds = { - [key in LogLevel]: number; + [key in Uppercase]: number; }; type LogAttributeValue = unknown; diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 4b0775421d..5b3bcbaa0a 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -1419,7 +1419,7 @@ describe('Class: Logger', () => { }; const childLoggerWithSampleRateEnabled = parentLogger.createChild(optionsWithSampleRateEnabled); - const optionsWithErrorLogLevel = { + const optionsWithErrorLogLevel: ConstructorOptions = { logLevel: 'ERROR', }; const childLoggerWithErrorLogLevel = parentLogger.createChild(optionsWithErrorLogLevel); diff --git a/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts b/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts index 8b7f52a82e..9c02daeefa 100644 --- a/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts +++ b/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts @@ -23,7 +23,7 @@ describe('Class: PowertoolLogFormatter', () => { // Prepare const formatter = new PowertoolLogFormatter(); - const unformattedAttributes = { + const unformattedAttributes: UnformattedAttributes = { sampleRateValue: undefined, awsRegion: 'eu-west-1', environment: '', diff --git a/packages/logger/tests/unit/helpers.test.ts b/packages/logger/tests/unit/helpers.test.ts index 4f953620fc..6c0f839040 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -56,7 +56,7 @@ describe('Helper: createLogger function', () => { test('when no parameters are set, returns a Logger instance with the correct properties', () => { // Prepare - const loggerOptions = { + const loggerOptions: ConstructorOptions = { logLevel: 'WARN', serviceName: 'my-lambda-service', sampleRateValue: 1, From 5488c0af993d299fa3624be459077ad07155e1ca Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Sun, 26 Feb 2023 15:37:05 +0100 Subject: [PATCH 2/3] test: added unit test --- packages/logger/src/Logger.ts | 19 +++++++----- packages/logger/tests/unit/helpers.test.ts | 34 ++++++++++++++++++++-- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index e93083cd71..9cd96d0831 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -629,7 +629,7 @@ class Logger extends Utility implements ClassThatLogs { * @private * @returns {boolean} */ - private isValidLogLevel(logLevel?: LogLevel | string): logLevel is LogLevel { + private isValidLogLevel(logLevel?: LogLevel | string): logLevel is Uppercase { return typeof logLevel === 'string' && logLevel in this.logLevelThresholds; } @@ -748,20 +748,25 @@ class Logger extends Utility implements ClassThatLogs { * @returns {void} */ private setLogLevel(logLevel?: LogLevel): void { - if (this.isValidLogLevel(logLevel)) { - this.logLevel = logLevel?.toUpperCase() as Uppercase; + const constructorLogLevel = logLevel?.toUpperCase(); + if (this.isValidLogLevel(constructorLogLevel)) { + this.logLevel = constructorLogLevel; return; } - const customConfigValue = this.getCustomConfigService()?.getLogLevel(); + const customConfigValue = this.getCustomConfigService() + ?.getLogLevel() + ?.toUpperCase(); if (this.isValidLogLevel(customConfigValue)) { - this.logLevel = customConfigValue?.toUpperCase() as Uppercase; + this.logLevel = customConfigValue; return; } - const envVarsValue = this.getEnvVarsService().getLogLevel(); + const envVarsValue = this.getEnvVarsService() + ?.getLogLevel() + ?.toUpperCase(); if (this.isValidLogLevel(envVarsValue)) { - this.logLevel = envVarsValue?.toUpperCase() as Uppercase; + this.logLevel = envVarsValue; return; } diff --git a/packages/logger/tests/unit/helpers.test.ts b/packages/logger/tests/unit/helpers.test.ts index 6c0f839040..9946f918c4 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -199,10 +199,10 @@ describe('Helper: createLogger function', () => { })); }); - test('when a custom logLevel is passed, returns a Logger instance with the correct properties', () => { + test('when a custom uppercase logLevel is passed, returns a Logger instance with the correct properties', () => { // Prepare - const loggerOptions:ConstructorOptions = { + const loggerOptions: ConstructorOptions = { logLevel: 'ERROR', }; @@ -225,6 +225,36 @@ describe('Helper: createLogger function', () => { logLevel: 'ERROR', logFormatter: expect.any(PowertoolLogFormatter), })); + + }); + + test('when a custom lowercase logLevel is passed, returns a Logger instance with the correct properties', () => { + + // Prepare + const loggerOptions: ConstructorOptions = { + logLevel: 'warn', + }; + + // Act + const logger = createLogger(loggerOptions); + + // Assess + expect(logger).toBeInstanceOf(Logger); + expect(logger).toEqual(expect.objectContaining({ + logsSampled: false, + persistentLogAttributes: {}, + powertoolLogData: { + sampleRateValue: undefined, + awsRegion: 'eu-west-1', + environment: '', + serviceName: 'hello-world', + }, + envVarsService: expect.any(EnvironmentVariablesService), + customConfigService: undefined, + logLevel: 'WARN', + logFormatter: expect.any(PowertoolLogFormatter), + })); + }); test('when no log level is set, returns a Logger instance with INFO level', () => { From 2ed61c677a0c0121f60f890326256aaea3176193 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 27 Feb 2023 10:30:38 +0100 Subject: [PATCH 3/3] refactor: logLevel types --- packages/logger/src/Logger.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 9cd96d0831..663ce2bd37 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -651,12 +651,12 @@ class Logger extends Utility implements ClassThatLogs { /** * It prints a given log with given log level. * - * @param {LogLevel} logLevel + * @param {Uppercase} logLevel * @param {LogItemMessage} input * @param {LogItemExtraInput} extraInput * @private */ - private processLogItem(logLevel: LogLevel, input: LogItemMessage, extraInput: LogItemExtraInput): void { + private processLogItem(logLevel: Uppercase, input: LogItemMessage, extraInput: LogItemExtraInput): void { if (!this.shouldPrint(logLevel)) { return; } @@ -855,13 +855,13 @@ class Logger extends Utility implements ClassThatLogs { /** * It checks whether the current log item should/can be printed. * - * @param {LogLevel} logLevel + * @param {Uppercase} logLevel * @private * @returns {boolean} */ - private shouldPrint(logLevel: LogLevel): boolean { + private shouldPrint(logLevel: Uppercase): boolean { if ( - this.logLevelThresholds[logLevel.toUpperCase() as Uppercase] >= + this.logLevelThresholds[logLevel] >= this.logLevelThresholds[this.getLogLevel()] ) { return true;