Skip to content

feat(logger): add circular buffer #3593

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

Merged
merged 7 commits into from
Feb 13, 2025

Conversation

VatsalGoel3
Copy link
Contributor

@VatsalGoel3 VatsalGoel3 commented Feb 12, 2025

Summary

This PR adds a new log buffering module for the logger package.

Changes

  • Implemented a log buffering module with the following components:
    • SizedItem: Encapsulates a log entry and calculates its byte size using JSON serialization.
    • SizedSet: Extends Set to track the cumulative size of log entries and provides a FIFO removal method via shift().
    • CircularMap: Maps request identifiers to SizedSet buffers, enforces a maximum buffer size (in bytes), and evicts the oldest entries when necessary. Includes an optional onBufferOverflow callback.
  • Added unit tests (packages/logger/tests/unit/logBuffer.test.ts) to validate the log buffering module.

Issue number: closes #3589


By submitting this pull request, I confirm that I have read and followed the Powertools for AWS Lambda (TypeScript) contributing guidelines, and I confirm that you can use, modify, copy, and redistribute this contribution under the terms of your choice.

@VatsalGoel3 VatsalGoel3 requested a review from a team February 12, 2025 09:41
@VatsalGoel3 VatsalGoel3 requested a review from a team as a code owner February 12, 2025 09:41
@boring-cyborg boring-cyborg bot added logger This item relates to the Logger Utility tests PRs that add or change tests labels Feb 12, 2025
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Feb 12, 2025
@dreamorosi
Copy link
Contributor

Hi @VatsalGoel3, thank you for the PR.

Could you please restore the PR template instead of rewriting it from scratch?

Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @VatsalGoel3, great PR so far!

Left a few notes that I'd like us to address before moving forward, let me know if you have any questions.

@dreamorosi dreamorosi changed the title Feature/log buffering feat(logger): add circular buffer Feb 12, 2025
@github-actions github-actions bot added the feature PRs that introduce new features or minor changes label Feb 12, 2025
@VatsalGoel3
Copy link
Contributor Author

VatsalGoel3 commented Feb 13, 2025

@dreamorosi

I've incorporated the requested changes and made a few refinements:

Refactored the test for buffer overflow eviction

Removed JSON.stringify() in SizedItem

  1. Now enforcing string values at runtime using isString()

  2. Switched to to reflect the intended usage better.
    Let me know if we should revisit this for potential flexibility down the road!

    Updated test cases

    1. Added type adjustments in SizedSet and CircularMap to align with enforcing string values.

    2. Reworked the eviction test case to ensure correct behavior.

    Updated the code for Quality Gate failed.

@dreamorosi dreamorosi self-requested a review February 13, 2025 09:29
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR and for addressing my comments!

I'm going to be creating several more issues that open for contributions in the next day or so, if you're interested keep an eye out.

@dreamorosi dreamorosi merged commit 618cdee into aws-powertools:main Feb 13, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature PRs that introduce new features or minor changes logger This item relates to the Logger Utility size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: add circular buffer to hold logs
3 participants