Skip to content

Maintenance: Fix code quality issues in tracer package #4257

@dreamorosi

Description

@dreamorosi

Summary

The packages/tracer package contains 5 code quality issues identified by SonarQube that affect tracing functionality and test reliability:

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

    • packages/tracer/tests/unit/Tracer.test.ts (1 member: line 978)
  2. Code Structure Issues (3 MINOR issues): Code quality improvements for better maintainability

    • packages/tracer/tests/unit/Tracer.test.ts (1 union type issue: line 695)
    • packages/tracer/src/Tracer.ts (1 redundant jump issue: line 825)
    • packages/tracer/src/Tracer.ts (1 type annotation issue: line 871)

These issues affect the core Tracer class, configuration utilities, and test reliability across the Tracer package.

Why is this needed?

  1. Code Quality: Improves code clarity by explicitly marking immutable members as readonly
  2. SonarQube Compliance: Resolves 5 code quality issues (typescript:S2933, typescript:S4323, typescript:S3626, typescript:S6565)
  3. Type Safety: Prevents accidental reassignment of critical configuration members
  4. Code Clarity: Removes redundant code and improves type annotations
  5. Developer Intent: Makes the immutable nature of these members explicit to future maintainers
  6. Best Practices: Follows TypeScript best practices for class member declarations and type definitions

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 Tracer.test.ts readonly member (1 issue)

Current code:

class Lambda implements LambdaInterface {
  private memberVariable: string;
  // ...
}

Improved code:

class Lambda implements LambdaInterface {
  // ...
  readonly memberVariable: string;
  // ...
}

2. Remove union type alias in Tracer.test.ts (1 issue)

Current code:

// Inline union type that should be removed
const newSubsegment: Segment | Subsegment | undefined = new Subsegment(
  '## foo.bar'
);

Improved code:

// Remove unnecessary union type alias for all occurrences in file
const newSubsegment = new Subsegment(
  '## foo.bar'
);

3. Fix redundant jump in Tracer.ts (1 issue)

Current code:

// Function with unnecessary return statement
if (this.#envConfig.captureHTTPsRequests.toLowerCase() === 'false') {
  this.captureHTTPsRequests = false;
  return;
}
// rest of function

Improved code:

// Remove redundant return statement
if (this.#envConfig.captureHTTPsRequests.toLowerCase() === 'false') {
  this.captureHTTPsRequests = false;
}
// rest of function continues naturally

4. Fix type annotation in Tracer.ts (1 issue)

Current code:

// Method that should use 'this' type instead of explicit class type
private setOptions(options: TracerOptions): Tracer {
  // method implementation
  return this;
}

Improved code:

// Use 'this' type for method return type to improve type safety
private setOptions(options: TracerOptions): this {
  // method implementation
  return this;
}

These changes:

  • Make the immutable nature of class members explicit
  • Prevent accidental reassignment in future code changes
  • Improve type safety and inference
  • Remove unnecessary code for better readability
  • Follow TypeScript best practices for class design and type definitions

Implementation Details

  1. Files to modify:

    • packages/tracer/tests/unit/Tracer.test.ts (lines 695, 978)
    • packages/tracer/src/Tracer.ts (lines 825, 871)
  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 type changes don't affect method chaining or return types
  3. Validation:

    • Run npm run test:unit -w packages/tracer to ensure no regressions
    • Run npm run lint -w packages/tracer 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 Tracer package is essential for distributed tracing and observability in AWS Lambda functions, making code quality improvements here particularly valuable for debugging and monitoring capabilities.

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 improve type safety.

Note: This issue focuses on straightforward code quality improvements. More complex refactoring tasks (such as reducing function nesting complexity in tests) are being addressed separately by maintainers.

SonarQube Issue Keys:

Acknowledgment

Future readers

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

Metadata

Metadata

Assignees

Labels

completedThis item is complete and has been merged/shippedgood-first-issueSomething that is suitable for those who want to start contributingtracerThis item relates to the Tracer Utility

Type

No type

Projects

Status

Shipped

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions