Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Added logLevelSwitch option #139

Merged
merged 2 commits into from
Jan 18, 2018
Merged

Conversation

piaste
Copy link
Contributor

@piaste piaste commented Dec 20, 2017

What issue does this PR address?
Allows the sink to be passed a LoggingLevelSwitch object as an alternative to a plain minimum log level. See #138.

Does this PR introduce a breaking change?
No. The switch is the last argument in the public method in which it appears.

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

I don't think unit tests would be appropriate here since all the code is in Serilog itself, the PR simply passes in an extra argument.

I've tested it manually and the switches operate correctly as expected (open two ES sinks, pass different switches, set different minimum levels, log only goes to one).

@@ -89,7 +94,8 @@ public static class LoggerConfigurationElasticsearchExtensions
string bufferBaseFilename = null,
long? bufferFileSizeLimitBytes = null,
long bufferLogShippingInterval = 5000,
string connectionGlobalHeaders = null)
string connectionGlobalHeaders = null,
LoggingLevelSwitch loggingLevelSwitch = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other public APIs exposed by Serilog, we call this levelSwitch - would be good to be consistent (if this change is accepted).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will align.

@mivano
Copy link
Contributor

mivano commented Jan 18, 2018

Thanks for the PR, nice addition.

@mivano mivano merged commit a7f8b81 into serilog-contrib:dev Jan 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants