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

Do not throw PlatformNotSupportedException #52

Merged
merged 2 commits into from
Jun 9, 2024

Conversation

0xced
Copy link
Member

@0xced 0xced commented Mar 29, 2024

The PlatformNotSupportedException is problematic when the EventLog sink is configured through Serilog.Settings.Configuration, where it's impossible to conditionally enable on Windows and disable on other platforms.

Real scenario: the production app will run on Windows and the appsettings configuration looks like this (using TOML instead of JSON):

[Serilog.WriteTo.EventLogSink]
Name = 'EventLog'
Args = { source = 'MyApplication', manageEventSource = true, restrictedToMinimumLevel = 'Warning' }

When developing on Linux or macOS, it's impossible to remove the EventLog configuration so it's better for this sink to be ignored since EventLog doesn't exist on these platforms anyway.

Note: this is exactly how the Notepad sink works and it's a good solution.

Copy link
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

LGTM

The `PlatformNotSupportedException` is problematic when the EventLog sink is configured through [Serilog.Settings.Configuration][1], where it's impossible to conditionally enable on Windows and disable on other platforms.

Real scenario: the production app will run on Windows and the appsettings configuration looks like this (using TOML instead of JSON):

```toml
[Serilog.WriteTo.EventLogSink]
Name = 'EventLog'
Args = { source = 'MyApplication', manageEventSource = true, restrictedToMinimumLevel = 'Warning' }
```

When developing on Linux or macOS, it's [impossible to remove][2] the EventLog configuration so it's better for this sink to be ignored since EventLog doesn't exist on these platforms anyway.

Note: this is exactly how the [Notepad sink][3] works and it's a good solution.

[1]: https://github.com/serilog/serilog-settings-configuration
[2]: dotnet/runtime#36543
[3]: https://github.com/serilog-contrib/serilog-sinks-notepad
@0xced 0xced force-pushed the RemovePlatformNotSupportedException branch from 357dd3b to 3af0c6e Compare April 11, 2024 12:10
@nblumhardt nblumhardt merged commit 3b41474 into serilog:dev Jun 9, 2024
1 check passed
@nblumhardt nblumhardt mentioned this pull request Jun 9, 2024
@0xced 0xced deleted the RemovePlatformNotSupportedException branch June 9, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants