Repository navigation
fix(logger): return a copy of the Lambda context to formatters - #5864
Tharaka-Chathuranga wants to merge 1 commit into
Conversation
…ontext I noticed getLambdaContext() was returning the actual context object the logger holds onto, not a copy. Since that object gets passed straight to formatAttributes(), a custom formatter that edits it (like redacting the request id) ends up changing it for every log line after that, and for any child logger created later too. Returning a shallow copy fixes it. The context only has strings and a boolean, so there's nothing nested that could still leak. Added a test with a formatter that redacts the request id - it fails on main and passes with this change. Refs aws-powertools#5555
|
Not all issues are linked correctly. Please link each issue to the PR either manually or using a closing keyword in the format If mentioning more than one issue, separate them with commas: i.e. |
|
Hi @Tharaka-Chathuranga, thanks for the pull request, and for the clear write-up. This follows the smaller fix I suggested in #5555, but that suggestion hasn't been agreed on. For the Lambda context specifically, the issue author proposed a design based on Immutable, which would hand out the stored value without copying it. Merging a copy-on-read change now could mean undoing it later. Until the design is settled I'd rather hold off on implementation, so I'm going to close this PR. Once there's a decision on #5555, you're welcome to pick this up again. The regression test with the redacting formatter would be useful whichever approach we go with. |
Summary
LogAttributesStore.getLambdaContext()returned the stored Lambda context object itself. Because that object is passed toformatAttributes()on every log call, a customLogFormatterthat changed it (for example, to redact the request id) silently changed the context for every later log line in the invocation, and for any child logger created afterwards. This PR makes the getter return a copy instead.Changes
getLambdaContext()now returns a shallow copy of the stored context. All fields are strings or booleans, so a shallow copy fully separates it from the stored one.awsRequestId: the second log line still shows the real id. This test fails onmain, where it logs"redacted".This covers the Logger part of the smaller fix suggested in #5555. The
getAllMetrics()part is being handled separately by @vishwakt, so this PR doesn't close the issue.Issue number: #5555
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.