-
Notifications
You must be signed in to change notification settings - Fork 97
feat(logging): Log buffering support for Logj42 and Logback #2103
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
Conversation
…or option to Logging aspect.
…ause we configured two appenders as proxies.
…to avoid circular dependency when flushing buffer on error.
Co-authored-by: Stefano Vozza <[email protected]>
Co-authored-by: Stefano Vozza <[email protected]>
|
Hi @svozza, did you by chance also review the implementation or just the docs? |
Yes, it's a big diff so I only really focused on the implementation's Java classes. The only thing I noticed was that |
Thanks @svozza and @dreamorosi for the feedback. It is a good point and I thought about this as well. Some of the code flow is shared but there are notable differences in the typing and behavior of the log events / Appenders in these frameworks. The main candidate for abstraction would be the append() method. However, you can see that log4j2 requires to create a immutable copy of the log event first while logback doesn't. Also the log level comparison for the buffering decision is different due to the Another argument I fought in my mind was whether the Historically, this is also the reason why the JSON structured logging was implemented independently for both log4j2 and logback. The tradeoff between abstraction to remove code redundancy and flexibility in embedding the logic in the framework-specific plugin system is not big enough here IMO. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the super quick turnaround on this 🚀
Thanks folks for your feedback. Let's bring this feature live in the next release 🚀 💚 |
Summary
Docs preview: https://dealn7fl31ram.cloudfront.net/core/logging/#buffering-logs
This PR introduces a log buffering feature to Powertools for AWS Lambda (Java). Log buffering is the mechanism of buffering lower level logs and only flush them (to STDOUT on Lambda) in case of error or manually if needed. It is a good way to reduce the noise caused by e.g. DEBUG logs and can be seen as a more elaborate way of "log sampling".
While the other Powertools runtimes (Python, TypeScript, .NET) follow a programmatic approach to configuring log buffering, this PR uses the concept of "Appenders" in the corresponding logging backend (log4j2, logback). This means that log buffering is configured using the configuration files of the respective logging framework which is compatible with the way how structured JSON logging is already implemented. This also means that log buffering can be used independent of the
@Logging
annotation (except for automatic flushing on exceptions) and it is also compatible with any other Appender. The idea is to wrap an arbitrary existing Appender (e.g. a STDOUT JSON structured appender) with theBufferingAppender
which will enable log buffering with zero code changes. Here is an example for log4j2:Note: This XML based configuration supersedes the original design proposal of
@BufferingConfig
made in the feature request issue (see below).Changes
Issue number: #2095
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.