feat(logger): align sampling debug logs feature implementation with the 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

Author: shdq

Diff for: packages/logger/src/Logger.ts

@@ -155,8 +155,6 @@ class Logger extends Utility implements ClassThatLogs {
     SILENT: 28,
   };
 
-  private logsSampled = false;
-
   private persistentLogAttributes?: LogAttributes = {};
 
   private powertoolLogData: PowertoolLogData = <PowertoolLogData>{};
@@ -235,6 +233,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(
@@ -308,15 +307,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.
@@ -456,15 +446,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);
   }
 
   /**
@@ -515,19 +504,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.
    *
@@ -555,36 +531,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.
    *
@@ -750,20 +721,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.
    *
@@ -777,6 +734,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.
    *
@@ -817,13 +791,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)
-    );
   }
 
   /**
@@ -905,6 +878,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.
@@ -943,20 +947,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.
@@ -981,10 +971,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();
 
@@ -1014,7 +1003,6 @@ class Logger extends Utility implements ClassThatLogs {
           environment ||
           this.getCustomConfigService()?.getCurrentEnvironment() ||
           this.getEnvVarsService().getCurrentEnvironment(),
-        sampleRateValue: this.getSampleRateValue(),
         serviceName:
           serviceName ||
           this.getCustomConfigService()?.getServiceName() ||

Diff for: packages/logger/src/config/EnvironmentVariablesService.ts

@@ -100,7 +100,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);

Diff for: packages/logger/src/types/Logger.ts

@@ -47,7 +47,7 @@ type LambdaFunctionContext = {
 type PowertoolLogData = LogAttributes & {
   environment?: Environment;
   serviceName: string;
-  sampleRateValue?: number;
+  sampleRateValue: number;
   lambdaFunctionContext: LambdaFunctionContext;
   xRayTraceId?: string;
   awsRegion: string;

Diff for: packages/logger/tests/e2e/sampleRate.decorator.test.ts

@@ -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`);