Skip to content

Commit

Permalink
feat(logger): make loglevel types stricter (#1313)
Browse files Browse the repository at this point in the history
* feat: make loglevel types stricter

* test: added unit test

* refactor: logLevel types
  • Loading branch information
dreamorosi authored Feb 27, 2023
1 parent 919853e commit 5af51d3
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 28 deletions.
53 changes: 32 additions & 21 deletions packages/logger/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class Logger extends Utility implements ClassThatLogs {

private customConfigService?: ConfigServiceInterface;

private static readonly defaultLogLevel: LogLevel = 'INFO';
private static readonly defaultLogLevel: Uppercase<LogLevel> = 'INFO';

// envVarsService is always initialized in the constructor in setOptions()
private envVarsService!: EnvironmentVariablesService;
Expand All @@ -128,7 +128,7 @@ class Logger extends Utility implements ClassThatLogs {

private logIndentation: number = LogJsonIndent.COMPACT;

private logLevel?: LogLevel;
private logLevel?: Uppercase<LogLevel>;

private readonly logLevelThresholds: LogLevelThresholds = {
DEBUG: 8,
Expand Down Expand Up @@ -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 <LogLevel> this.logLevel;
private getLogLevel(): Uppercase<LogLevel> {
return this.logLevel!;
}

/**
Expand Down Expand Up @@ -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<LogLevel> {
return typeof logLevel === 'string' && logLevel in this.logLevelThresholds;
}

/**
Expand All @@ -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>} logLevel
* @param {LogItemMessage} input
* @param {LogItemExtraInput} extraInput
* @private
*/
private processLogItem(logLevel: LogLevel, input: LogItemMessage, extraInput: LogItemExtraInput): void {
private processLogItem(logLevel: Uppercase<LogLevel>, input: LogItemMessage, extraInput: LogItemExtraInput): void {
if (!this.shouldPrint(logLevel)) {
return;
}
Expand Down Expand Up @@ -743,20 +748,25 @@ class Logger extends Utility implements ClassThatLogs {
* @returns {void}
*/
private setLogLevel(logLevel?: LogLevel): void {
if (this.isValidLogLevel(logLevel)) {
this.logLevel = (<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 = (<LogLevel>customConfigValue).toUpperCase();
this.logLevel = customConfigValue;

return;
}
const envVarsValue = this.getEnvVarsService().getLogLevel();
const envVarsValue = this.getEnvVarsService()
?.getLogLevel()
?.toUpperCase();
if (this.isValidLogLevel(envVarsValue)) {
this.logLevel = (<LogLevel>envVarsValue).toUpperCase();
this.logLevel = envVarsValue;

return;
}
Expand Down Expand Up @@ -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>} logLevel
* @private
* @returns {boolean}
*/
private shouldPrint(logLevel: LogLevel): boolean {
if (this.logLevelThresholds[logLevel] >= this.logLevelThresholds[this.getLogLevel()]) {
private shouldPrint(logLevel: Uppercase<LogLevel>): boolean {
if (
this.logLevelThresholds[logLevel] >=
this.logLevelThresholds[this.getLogLevel()]
) {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/logger/src/types/Log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ type LogLevelInfo = 'INFO';
type LogLevelWarn = 'WARN';
type LogLevelError = 'ERROR';

type LogLevel = LogLevelDebug | LogLevelInfo | LogLevelWarn | LogLevelError | string;
type LogLevel = LogLevelDebug | Lowercase<LogLevelDebug> | LogLevelInfo | Lowercase<LogLevelInfo> | LogLevelWarn | Lowercase<LogLevelWarn> | LogLevelError | Lowercase<LogLevelError>;

type LogLevelThresholds = {
[key in LogLevel]: number;
[key in Uppercase<LogLevel>]: number;
};

type LogAttributeValue = unknown;
Expand Down
2 changes: 1 addition & 1 deletion packages/logger/tests/unit/Logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1486,7 +1486,7 @@ describe('Class: Logger', () => {
};
const childLoggerWithSampleRateEnabled = parentLogger.createChild(optionsWithSampleRateEnabled);

const optionsWithErrorLogLevel = {
const optionsWithErrorLogLevel: ConstructorOptions = {
logLevel: 'ERROR',
};
const childLoggerWithErrorLogLevel = parentLogger.createChild(optionsWithErrorLogLevel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('Class: PowertoolLogFormatter', () => {

// Prepare
const formatter = new PowertoolLogFormatter();
const unformattedAttributes = {
const unformattedAttributes: UnformattedAttributes = {
sampleRateValue: undefined,
awsRegion: 'eu-west-1',
environment: '',
Expand Down
35 changes: 32 additions & 3 deletions packages/logger/tests/unit/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
};

Expand All @@ -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', () => {

Expand Down

0 comments on commit 5af51d3

Please sign in to comment.