Skip to content
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

fix(logger): clear_state regression on absent standard keys #1088

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Apr 1, 2022

Issue #, if available: #1084 #1087

Description of changes:

When using clear_state=True, standard logging keys like level, location, timestamp are no longer being preserved. This is a regression introduced in 1.25.3, when fixing custom formatters for clear_state behaviour. Our tests weren't robust enough for clear_state - this PR adds additional tests to cover gaps we had.

Issue was that when clear_state=True was used, the default keys (level, location, timestamp) weren't being re-added as part of a new clear_state method for LambdaPowertoolsFormatter implemented in 1.25.3, hence the regression.

{
            "level": "%(levelname)s",
            "location": "%(funcName)s:%(lineno)d",
            "timestamp": "%(asctime)s",
}

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement 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.

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 1, 2022
@boring-cyborg boring-cyborg bot added the tests label Apr 1, 2022
@github-actions github-actions bot added the bug Something isn't working label Apr 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2022

Codecov Report

Merging #1088 (80cb6ec) into develop (09f98f8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1088   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5376     5377    +1     
  Branches       613      613           
========================================
+ Hits          5374     5375    +1     
  Partials         2        2           
Impacted Files Coverage Δ
aws_lambda_powertools/logging/formatter.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09f98f8...80cb6ec. Read the comment docs.

@heitorlessa heitorlessa changed the title fix(logger): clear_state regression with standard keys absent fix(logger): clear_state regression on absent standard keys Apr 1, 2022
@heitorlessa heitorlessa merged commit a88f535 into aws-powertools:develop Apr 1, 2022
@heitorlessa heitorlessa deleted the fix/logger-clear-state-regression branch April 1, 2022 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants