Skip to content
This repository was 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 :-)

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants