From 5af51d319dee68d7a7ba832721580d7a6e655249 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 27 Feb 2023 10:56:51 +0100 Subject: [PATCH] feat(logger): make loglevel types stricter (#1313) * feat: make loglevel types stricter * test: added unit test * refactor: logLevel types --- packages/logger/src/Logger.ts | 53 +++++++++++-------- 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 | 35 ++++++++++-- 5 files changed, 68 insertions(+), 28 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 2ce5df705..663ce2bd3 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 Uppercase { + return typeof logLevel === 'string' && logLevel in this.logLevelThresholds; } /** @@ -647,11 +651,12 @@ class Logger extends Utility implements ClassThatLogs { /** * It prints a given log with given log level. * - * @param {LogLevel} logLevel - * @param {LogItem} log + * @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; } @@ -743,20 +748,25 @@ class Logger extends Utility implements ClassThatLogs { * @returns {void} */ private setLogLevel(logLevel?: LogLevel): void { - if (this.isValidLogLevel(logLevel)) { - this.logLevel = (logLevel).toUpperCase(); + 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(); + this.logLevel = customConfigValue; return; } - const envVarsValue = this.getEnvVarsService().getLogLevel(); + const envVarsValue = this.getEnvVarsService() + ?.getLogLevel() + ?.toUpperCase(); if (this.isValidLogLevel(envVarsValue)) { - this.logLevel = (envVarsValue).toUpperCase(); + this.logLevel = envVarsValue; return; } @@ -845,14 +855,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 {Uppercase} logLevel * @private * @returns {boolean} */ - private shouldPrint(logLevel: LogLevel): boolean { - if (this.logLevelThresholds[logLevel] >= this.logLevelThresholds[this.getLogLevel()]) { + private shouldPrint(logLevel: Uppercase): boolean { + if ( + this.logLevelThresholds[logLevel] >= + this.logLevelThresholds[this.getLogLevel()] + ) { return true; } diff --git a/packages/logger/src/types/Log.ts b/packages/logger/src/types/Log.ts index 0b7cd5d30..020b9cf20 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 5c2b3fd95..eedb348c8 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -1486,7 +1486,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 4d8d4100f..1aff6d111 100644 --- a/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts +++ b/packages/logger/tests/unit/formatter/PowertoolLogFormatter.test.ts @@ -22,7 +22,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 7235dec4a..31842614f 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -55,7 +55,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, @@ -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', }; @@ -227,6 +227,35 @@ describe('Helper: createLogger function', () => { })); }); + + 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', () => {