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

Update to Serilog.Sinks.PeriodicBatching v4.0 and break dependency on deprecated inheritance API #579

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

nblumhardt
Copy link
Contributor

What issue does this PR address?

Inheriting from PeriodicBatchingSink has been deprecated for a few versions, and with v4 of that sink is no longer supported.

Does this PR introduce a breaking change?

Yes, it's no longer possible to cast ElasticsearchSink to PeriodicBatchingSink. The forced major version upgrades for Serilog.Sinks.PeriodicBatching and Serilog also constitute a major-version-bump-worthy change.

Practically, no - ElasticsearchSink still supports ILogEventSink and IDisposable as it did before.

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) N/A

Other information:

The newer PeriodicBatchingSink implementation supports IAsyncDisposable, and in doing so avoids a rare but theoretically-possible shutdown hang. I've added a net6.0 target and implemented forwarding of IAsyncDisposable.DisposeAsync() to the internal wrapped sink to make this accessible. Happy to back this out if it conflicts with other goals/intentions here.

See also serilog/serilog-sinks-periodicbatching#69

… deprecated `PeriodicBatchingSink` inheritance API
@nblumhardt
Copy link
Contributor Author

Test failure seems to be an intermittent HTTP problem unrelated to the changes.

@mivano mivano merged commit 4b1c592 into serilog-contrib:dev Mar 4, 2024
4 of 5 checks passed
@mivano
Copy link
Contributor

mivano commented Mar 4, 2024

Thanks for the PR! This project is so dormant that I missed this change.

@nblumhardt
Copy link
Contributor Author

Thanks! 👍

@nblumhardt
Copy link
Contributor Author

Is there a CI package feed I can point people to who encounter this one while we wait for a release build? Cheers! 👋

@mivano
Copy link
Contributor

mivano commented Mar 12, 2024

Sorry, I rolled out a v10.0.0 version (https://www.nuget.org/packages/Serilog.Sinks.Elasticsearch/10.0.0)

@nblumhardt
Copy link
Contributor Author

Thanks a bunch :-)

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.

2 participants