Maintenance: Mark private members as readonly and fix code quality issues in logger package

[dreamorosi]

Summary

The packages/logger package contains 7 code quality issues identified by SonarQube that affect core logging functionality and test reliability:

1. Readonly Member Issues (5 MAJOR issues): Class members that are never reassigned after initialization but are not marked as readonly

   • packages/logger/src/Logger.ts (4 members: lines 118, 150, 170, 177)
   • packages/logger/src/formatter/LogFormatter.ts (1 member: line 193)
   • packages/logger/src/formatter/PowertoolsLogFormatter.ts (1 member: line 27)

2. Test Code Issues (2 issues): Code quality problems in test files

   • packages/logger/tests/unit/sampling.test.ts (1 MAJOR issue: line 6)
   • packages/logger/tests/e2e/basicFeatures.middy.test.ts (1 MAJOR issue: line 272)

3. Code Structure Issues (1 MINOR issue): Redundant code that can be simplified

   • packages/logger/src/Logger.ts (1 issue: line 1198)

These issues affect the core Logger class, formatting utilities, and test reliability.

Why is this needed?

1. Code Quality: Improves code clarity by explicitly marking immutable members as readonly
2. SonarQube Compliance: Resolves 7 code quality issues (typescript:S2933, typescript:S3626, typescript:S4030)
3. Type Safety: Prevents accidental reassignment of critical logging configuration members
4. Developer Intent: Makes the immutable nature of these members explicit to future maintainers
5. Test Reliability: Ensures test code follows best practices and doesn't have unused collections
6. Best Practices: Follows TypeScript best practices for class member declarations

Solution

Important

The following changes are included as reference to help you understand the refactoring. Before implementing, please make sure to check the codebase and ensure that they make sense and they are exhaustive.

1. Fix Logger.ts readonly members (4 issues)

Current code:

class Logger {
  // ...
  powertoolsLogData: PowertoolsLogData;
  // ...
  #isInitialized: boolean;
  // ...
  #keys: LogKeys;
  // ...
}

Improved code:

class Logger {
  // ...
  readonly powertoolsLogData: PowertoolsLogData;
  // ...
  readonly #isInitialized: boolean;
  // ...
  readonly #keys: LogKeys;
  // ...
}

2. Fix LogFormatter.ts readonly member (1 issue)

Current code:

class LogFormatter {
  // ...
  #getDateFormatter: () => Intl.DateTimeFormat;
  // ...
}

Improved code:

class LogFormatter {
  // ...
  readonly #getDateFormatter: () => Intl.DateTimeFormat;
  // ...
}

3. Fix PowertoolsLogFormatter.ts readonly member (1 issue)

Current code:

class PowertoolsLogFormatter extends LogFormatter {
  // ...
  #logRecordOrder: string[];
  // ...
}

Improved code:

class PowertoolsLogFormatter extends LogFormatter {
  // ...
  readonly #logRecordOrder: string[];
  // ...
}

4. Fix sampling.test.ts readonly member (1 issue)

Current code:

class TestClass {
  // ...
  #sampleRateValue: number;
  // ...
}

Improved code:

class TestClass {
  // ...
  readonly #sampleRateValue: number;
  // ...
}

5. Fix redundant jump in Logger.ts (1 issue)

Current code (line 1198):

// rest of function
if (this.isValidLogLevel(envVarsValue)) {
  this.logLevel = LogLevelThreshold[envVarsValue];
  this.#initialLogLevel = this.logLevel;

  return;
}
// rest of function

Improved code:

// rest of function
if (this.isValidLogLevel(envVarsValue)) {
  this.logLevel = LogLevelThreshold[envVarsValue];
  this.#initialLogLevel = this.logLevel;
}
// rest of function continues naturally

6. Fix unused collection in basicFeatures.middy.test.ts (1 issue)

Current code (line 272):

// Collection that is created but never used
const traceIds: string[] = [];
for (const message of logMessages) {
  const log = TestInvocationLogs.parseFunctionLog(message);
  expect(log).toHaveProperty('xray_trace_id');
  expect(log.xray_trace_id).toMatch(XRAY_TRACE_ID_REGEX);
  traceIds.push(log.xray_trace_id as string);
}
// Collection is not used anywhere

Improved code:

for (const message of logMessages) {
  const log = TestInvocationLogs.parseFunctionLog(message);
  expect(log).toHaveProperty('xray_trace_id');
  expect(log.xray_trace_id).toMatch(XRAY_TRACE_ID_REGEX);
}

These changes:

• Make the immutable nature of class members explicit
• Prevent accidental reassignment in future code changes
• Follow TypeScript best practices for class design
• Improve code readability and maintainability
• Ensure test code follows best practices

Implementation Details

1. Files to modify:

   • packages/logger/src/Logger.ts (lines 150, 170, 177, 1198)
   • packages/logger/src/formatter/LogFormatter.ts (line 193)
   • packages/logger/src/formatter/PowertoolsLogFormatter.ts (line 27)
   • packages/logger/tests/unit/sampling.test.ts (line 6)
   • packages/logger/tests/e2e/basicFeatures.middy.test.ts (line 272)

2. Testing:

   • Ensure all existing tests continue to pass
   • Run type checking to verify no TypeScript errors are introduced
   • Verify that the readonly modifier doesn't break any existing functionality
   • Confirm test improvements don't affect test behavior

3. Validation:

   • Run npm run test:unit -w packages/logger to ensure no regressions
   • Run npm run lint -w packages/logger to verify code style compliance
   • Run npm run build -ws to verify TypeScript compilation
   • Run SonarQube analysis to confirm issues are resolved

Additional Context

This issue was identified as part of a SonarQube code quality review. The Logger package is a foundational utility used throughout the Powertools ecosystem for structured logging, making code quality improvements here particularly valuable for reliability and maintainability.

These changes are primarily declarative improvements that should not change any functionality or behavior - they simply make the immutable nature of class members explicit and clean up minor code quality issues.

SonarQube Issue Keys:

• AZItlgxgcEUs2753LD6- - Logger.ts powertoolsLogData
• AZItlgxgcEUs2753LD7A - Logger.ts #isInitialized
• AZItlgxgcEUs2753LD7B - Logger.ts #keys
• AZItlgxgcEUs2753LD7C - LogFormatter.ts #getDateFormatter
• AZItlgyDcEUs2753LD7D - PowertoolsLogFormatter.ts #logRecordOrder
• AZItlgwecEUs2753LD68 - sampling.test.ts #sampleRateValue
• AYlpRza_OvI06ztqLT_4 - Logger.ts redundant jump
• AYlpRzXGOvI06ztqLT-Z - basicFeatures.middy.test.ts unused collection

Acknowledgment

• This request meets Powertools for AWS Lambda (TypeScript) Tenets
• Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[powertools-for-aws-oss-automation]

Note

For those interested in contributing, please leave a comment below so that we can assign the issue to you and make sure we don't duplicate efforts. If you have any further questions, please don't hesitate to ask below or on our Discord server.