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

feat(logger): make loglevel types stricter #1313

Merged
merged 4 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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>;
am29d marked this conversation as resolved.
Show resolved Hide resolved

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 = {
dreamorosi marked this conversation as resolved.
Show resolved Hide resolved
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 = {
am29d marked this conversation as resolved.
Show resolved Hide resolved
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