Skip to content

Commit

Permalink
feat(logger): align sampling debug logs feature implementation with t…
Browse files Browse the repository at this point in the history
…he other runtimes (#1744)

* test(logger): remove logsSampled field, add default sampleRateValue

* test(logger): add tests for sampling debug logs feature

* feat(logger): change implementation to make sampling decision at per-function level

* refactor(logger): remove redundant createLogger method

* refactor(logger): remove getSampleRateValue method

* test(logger): improve tests

* refactor(logger): return createLogger() back with the detailed comment of the method importance

* test(logger): add constructor/custom config/env var priority tests for sampling rate feature, improve description

* refactor(logger): address review comments

* feat(logger): add refreshSampleRateCalculation method and tests

* test(logger): adjust end-to-end tests
  • Loading branch information
shdq authored and dreamorosi committed Feb 5, 2024
1 parent 164ee6e commit fd4f301
Show file tree
Hide file tree
Showing 7 changed files with 552 additions and 175 deletions.
168 changes: 78 additions & 90 deletions packages/logger/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@ class Logger extends Utility implements ClassThatLogs {
SILENT: 28,
};

private logsSampled = false;

private persistentLogAttributes?: LogAttributes = {};

private powertoolLogData: PowertoolLogData = <PowertoolLogData>{};
Expand Down Expand Up @@ -236,6 +234,7 @@ class Logger extends Utility implements ClassThatLogs {
logLevel: this.getLevelName(),
customConfigService: this.getCustomConfigService(),
logFormatter: this.getLogFormatter(),
sampleRateValue: this.powertoolLogData.sampleRateValue,
};
const parentsPowertoolsLogData = this.getPowertoolLogData();
const childLogger = this.createLogger(
Expand Down Expand Up @@ -309,15 +308,6 @@ class Logger extends Utility implements ClassThatLogs {
return this.logEvent;
}

/**
* It returns a boolean value, if true all the logs will be printed.
*
* @returns {boolean}
*/
public getLogsSampled(): boolean {
return this.logsSampled;
}

/**
* It returns the persistent log attributes, which are the attributes
* that will be logged in all log items.
Expand Down Expand Up @@ -457,15 +447,14 @@ class Logger extends Utility implements ClassThatLogs {
}

/**
* If the sample rate feature is enabled, the calculation that determines whether the logs
* will actually be printed or not for this invocation is done when the Logger class is
* initialized.
* This method will repeat that calculation (with possible different outcome).
* This method allows recalculating the initial sampling decision for changing
* the log level to DEBUG based on a sample rate value used during initialization,
* potentially yielding a different outcome.
*
* @returns {void}
*/
public refreshSampleRateCalculation(): void {
this.setLogsSampled();
this.setInitialSampleRate(this.powertoolLogData.sampleRateValue);
}

/**
Expand Down Expand Up @@ -520,19 +509,6 @@ class Logger extends Utility implements ClassThatLogs {
this.persistentLogAttributes = attributes;
}

/**
* It sets the user-provided sample rate value.
*
* @param {number} [sampleRateValue]
* @returns {void}
*/
public setSampleRateValue(sampleRateValue?: number): void {
this.powertoolLogData.sampleRateValue =
sampleRateValue ||
this.getCustomConfigService()?.getSampleRateValue() ||
this.getEnvVarsService().getSampleRateValue();
}

/**
* It checks whether the current Lambda invocation event should be printed in the logs or not.
*
Expand Down Expand Up @@ -560,36 +536,31 @@ class Logger extends Utility implements ClassThatLogs {
}

/**
* Creates a new Logger instance.
* Factory method for instantiating logger instances. Used by `createChild` method.
* Important for customization and subclassing. It allows subclasses, like `MyOwnLogger`,
* to override its behavior while keeping the main business logic in `createChild` intact.
*
* @param {ConstructorOptions} [options]
* @returns {Logger}
* @example
* ```typescript
* // MyOwnLogger subclass
* class MyOwnLogger extends Logger {
* protected createLogger(options?: ConstructorOptions): MyOwnLogger {
* return new MyOwnLogger(options);
* }
* // No need to re-implement business logic from `createChild` and keep track on changes
* public createChild(options?: ConstructorOptions): MyOwnLogger {
* return super.createChild(options) as MyOwnLogger;
* }
* }
* ```
*
* @param {ConstructorOptions} [options] Logger configuration options.
* @returns {Logger} A new logger instance.
*/
protected createLogger(options?: ConstructorOptions): Logger {
return new Logger(options);
}

/**
* Decides whether the current log item should be printed or not.
*
* The decision is based on the log level and the sample rate value.
* A log item will be printed if:
* 1. The log level is greater than or equal to the Logger's log level.
* 2. The log level is less than the Logger's log level, but the
* current sampling value is set to `true`.
*
* @param {number} logLevel
* @returns {boolean}
* @protected
*/
protected shouldPrint(logLevel: number): boolean {
if (logLevel >= this.logLevel) {
return true;
}

return this.getLogsSampled();
}

/**
* It stores information that is printed in all log items.
*
Expand Down Expand Up @@ -779,20 +750,6 @@ class Logger extends Utility implements ClassThatLogs {
};
}

/**
* It returns the numeric sample rate value.
*
* @private
* @returns {number}
*/
private getSampleRateValue(): number {
if (!this.powertoolLogData.sampleRateValue) {
this.setSampleRateValue();
}

return this.powertoolLogData.sampleRateValue as number;
}

/**
* It returns true and type guards the log level if a given log level is valid.
*
Expand All @@ -806,6 +763,23 @@ class Logger extends Utility implements ClassThatLogs {
return typeof logLevel === 'string' && logLevel in this.logLevelThresholds;
}

/**
* It returns true and type guards the sample rate value if a given value is valid.
*
* @param sampleRateValue
* @private
* @returns {boolean}
*/
private isValidSampleRate(
sampleRateValue?: number
): sampleRateValue is number {
return (
typeof sampleRateValue === 'number' &&
0 <= sampleRateValue &&
sampleRateValue <= 1
);
}

/**
* It prints a given log with given log level.
*
Expand Down Expand Up @@ -846,13 +820,12 @@ class Logger extends Utility implements ClassThatLogs {
input: LogItemMessage,
extraInput: LogItemExtraInput
): void {
if (!this.shouldPrint(logLevel)) {
return;
if (logLevel >= this.logLevel) {
this.printLog(
logLevel,
this.createAndPopulateLogItem(logLevel, input, extraInput)
);
}
this.printLog(
logLevel,
this.createAndPopulateLogItem(logLevel, input, extraInput)
);
}

/**
Expand Down Expand Up @@ -938,6 +911,37 @@ class Logger extends Utility implements ClassThatLogs {
}
}

/**
* It sets sample rate value with the following prioprity:
* 1. Constructor value
* 2. Custom config service value
* 3. Environment variable value
* 4. Default value (zero)
*
* @private
* @param {number} [sampleRateValue]
* @returns {void}
*/
private setInitialSampleRate(sampleRateValue?: number): void {
this.powertoolLogData.sampleRateValue = 0;
const constructorValue = sampleRateValue;
const customConfigValue =
this.getCustomConfigService()?.getSampleRateValue();
const envVarsValue = this.getEnvVarsService().getSampleRateValue();
for (const value of [constructorValue, customConfigValue, envVarsValue]) {
if (this.isValidSampleRate(value)) {
this.powertoolLogData.sampleRateValue = value;

if (value && randomInt(0, 100) / 100 <= value) {
this.setLogLevel('DEBUG');
this.debug('Setting log level to DEBUG due to sampling rate');
}

return;
}
}
}

/**
* If the log event feature is enabled via env variable, it sets a property that tracks whether
* the event passed to the Lambda function handler should be logged or not.
Expand Down Expand Up @@ -976,20 +980,6 @@ class Logger extends Utility implements ClassThatLogs {
}
}

/**
* If the sample rate feature is enabled, it sets a property that tracks whether this Lambda function invocation
* will print logs or not.
*
* @private
* @returns {void}
*/
private setLogsSampled(): void {
const sampleRateValue = this.getSampleRateValue();
this.logsSampled =
sampleRateValue !== undefined &&
(sampleRateValue === 1 || randomInt(0, 100) / 100 <= sampleRateValue);
}

/**
* It configures the Logger instance settings that will affect the Logger's behaviour
* and the content of all logs.
Expand All @@ -1014,10 +1004,9 @@ class Logger extends Utility implements ClassThatLogs {
this.setConsole();
this.setCustomConfigService(customConfigService);
this.setInitialLogLevel(logLevel);
this.setSampleRateValue(sampleRateValue);
this.setLogsSampled();
this.setLogFormatter(logFormatter);
this.setPowertoolLogData(serviceName, environment);
this.setInitialSampleRate(sampleRateValue);
this.setLogEvent();
this.setLogIndentation();

Expand Down Expand Up @@ -1047,7 +1036,6 @@ class Logger extends Utility implements ClassThatLogs {
environment ||
this.getCustomConfigService()?.getCurrentEnvironment() ||
this.getEnvVarsService().getCurrentEnvironment(),
sampleRateValue: this.getSampleRateValue(),
serviceName:
serviceName ||
this.getCustomConfigService()?.getServiceName() ||
Expand Down
2 changes: 1 addition & 1 deletion packages/logger/src/config/EnvironmentVariablesService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class EnvironmentVariablesService
/**
* It returns the value of the POWERTOOLS_LOGGER_SAMPLE_RATE environment variable.
*
* @returns {string|undefined}
* @returns {number|undefined}
*/
public getSampleRateValue(): number | undefined {
const value = this.get(this.sampleRateValueVariable);
Expand Down
2 changes: 1 addition & 1 deletion packages/logger/src/types/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type LambdaFunctionContext = {
type PowertoolLogData = LogAttributes & {
environment?: Environment;
serviceName: string;
sampleRateValue?: number;
sampleRateValue: number;
lambdaFunctionContext: LambdaFunctionContext;
xRayTraceId?: string;
awsRegion: string;
Expand Down
7 changes: 6 additions & 1 deletion packages/logger/tests/e2e/sampleRate.decorator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,12 @@ describe(`Logger E2E tests, sample rate and injectLambdaContext()`, () => {

if (logMessages.length === 1 && logMessages[0].includes('ERROR')) {
countNotSampled++;
} else if (logMessages.length === 4) {
} else if (
logMessages.length === 5 &&
logMessages[0].includes(
'Setting log level to DEBUG due to sampling rate'
)
) {
countSampled++;
} else {
console.error(`Log group ${logGroupName} contains missing log`);
Expand Down
Loading

0 comments on commit fd4f301

Please sign in to comment.