feat(logging): add Enabled property to LogBuffering configuration#1125
Open
feat(logging): add Enabled property to LogBuffering configuration#1125
Conversation
- Add `Enabled` property to `LogBufferingOptions` to explicitly control log buffering activation - Update `PowertoolsLoggerConfiguration` to expose `LogBuffering` as a configurable property - Refactor log buffering initialization to check `Enabled` flag before activating buffer - Update documentation to reflect new configuration pattern using property assignment instead of constructor - Simplify buffering setup by allowing incremental property configuration - Update all test cases to use new `LogBuffering.Enabled` pattern - Add `Enabled` parameter to buffering configuration table in documentation - Improve clarity of log buffering setup examples in ASP.NET and core logging guides
boring-cyborg
bot
added
area/logging
pull-request-size
bot
added
the
size/L
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1125 +/- ##
===========================================
+ Coverage 79.23% 79.26% +0.02%
===========================================
Files 299 299
Lines 12441 12455 +14
Branches 1490 1490
===========================================
+ Hits 9858 9872 +14
Misses 2128 2128
Partials 455 455 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pull-request-size
bot
added
size/XL
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the log buffering configuration API to make it more intuitive by adding an Enabled property to LogBufferingOptions and initializing it by default in PowertoolsLoggerConfiguration. This addresses issue #1042 where users found the configuration pattern confusing because log buffering was only activated when the LogBuffering property was not null, requiring users to instantiate an empty options object even when using defaults.
Changes:
- Added
Enabledboolean property toLogBufferingOptionsto explicitly control buffering activation - Initialized
LogBufferingproperty with a default instance inPowertoolsLoggerConfigurationto enable property assignment pattern - Updated buffering activation logic to check
LogBuffering?.Enabled == trueinstead of null check - Refactored all tests and documentation to use the new pattern:
config.LogBuffering.Enabled = true
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| LogBufferingOptions.cs | Added Enabled property with documentation and examples |
| PowertoolsLoggerConfiguration.cs | Initialized LogBuffering property with default instance and updated documentation |
| PowertoolsLoggingBuilderExtensions.cs | Updated buffering registration check to use Enabled flag |
| PowertoolsLoggerBuilder.cs | Updated WithLogBuffering to set Enabled = true instead of creating new instance |
| Internal/PowertoolsLogger.cs | Added Enabled check to log level filtering logic |
| Internal/LoggingAspect.cs | Updated buffering flag initialization to check Enabled property |
| PowertoolsLoggerBuilderTests.cs | Added comprehensive tests for enabled/disabled/null buffering scenarios |
| Various test files | Updated all existing tests to use new property assignment pattern |
| BasePersistenceStoreTests.cs | Added tests for idempotency persistence (appears unrelated to logging changes) |
| Documentation files | Updated all examples to demonstrate new configuration pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...aries/tests/AWS.Lambda.Powertools.Idempotency.Tests/Persistence/BasePersistenceStoreTests.cs
Show resolved
Hide resolved
libraries/src/AWS.Lambda.Powertools.Logging/PowertoolsLoggerConfiguration.cs
Outdated
Show resolved
Hide resolved
…nfiguration.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Henrique Graca <999396+hjgraca@users.noreply.github.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: #1042
Summary
Changes
Enabledproperty toLogBufferingOptionsto explicitly control log buffering activationPowertoolsLoggerConfigurationto exposeLogBufferingas a configurable propertyEnabledflag before activating bufferLogBuffering.EnabledpatternEnabledparameter to buffering configuration table in documentationUser experience
Checklist
Please leave checklist items unchecked if they do not apply to your change.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
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.
closes #1042