Skip to content

fix(logger): buffer logs emitted during init #2277

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
Mar 27, 2024
Merged

fix(logger): buffer logs emitted during init #2277

merged 7 commits into from
Mar 27, 2024

Conversation

dreamorosi
Copy link
Contributor

Description of your changes

This PR updates the initialization logic of the Logger utility so that any log emitted during this phase is buffered until the Logger has been fully initialized.

As part of the utility initialization we need to perform a number of actions to configure the content of the logs, their format, and the general behavior of the utility.

Some of these setup actions can emit warnings or debug logs if certain conditions are met. One of these is when the customer initializes the Logger with a log level that is more verbose than the one set in the Lambda ALC.

For those setup steps that require logging it was important that things like the log formatter were already configured, otherwise the operation would fail as reported in #2269.

Rather than simply swapping the order of the steps to fix the issue in the linked issue, this PR removes this type of dependency by introducing a rudimentary log buffer that will store any log emitted during the init phase. These logs are buffered until the initialization is complete and then are emitted.

In order to support this feature I had to do some minor refactor to ensure that the logs are emitted using the correct timestamp - aka the one at which they were emitted and not the one at which the buffer is flushed.

While there I also took the opportunity to simplify and document the logic of the method that initializes the log item.

Finally, I also took the liberty to refactor a test fixture that was extremely verbose and hard to understand as well as add new test cases to ensure that the logs are buffered and emitted correctly.

Related issues, RFCs

Issue number: #2269

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Mar 25, 2024
@dreamorosi dreamorosi requested a review from a team March 25, 2024 19:08
@dreamorosi dreamorosi requested a review from a team as a code owner March 25, 2024 19:08
@boring-cyborg boring-cyborg bot added logger This item relates to the Logger Utility tests PRs that add or change tests labels Mar 25, 2024
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Mar 25, 2024
@github-actions github-actions bot added the bug Something isn't working label Mar 25, 2024
@dreamorosi
Copy link
Contributor Author

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@sthulb sthulb merged commit 90d3b84 into main Mar 27, 2024
10 checks passed
@sthulb sthulb deleted the fix/logger_levels branch March 27, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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.

Bug: Exception instead of warning when log level doesn't match AWS log level
2 participants