serilog / serilog-extensions-hosting

Serilog logging for Microsoft.Extensions.Hosting
Apache License 2.0
140 stars 33 forks source link

Avoid disposable where possible in `Reload()` or log messages in `SelfLogFailureListener.OnLoggingFailed` #94

Open erichiller opened 6 days ago

erichiller commented 6 days ago

This commit changing line 53 in BackgroundWorkerSink.cs made it so that these errors were visible to me for the first time.

2024-11-12T03:39:34.0829845Z Serilog.Sinks.Async.BackgroundWorkerSink: the sink has been disposed (Final, 1 events)

This occurs because I am logging to something being called by configure(new LoggerConfiguration()).CreateLogger(); in ReloadableLogger, which happens right after _logger.Dispose(); in Reload().

As you mentioned in this comment there may be some issues with locks, etc in changing the order of the Dispose and configure() calls.

One idea I had was rather than disposing regardless of changes, Reload() (or the code it calls) could check if the underlying loggers are equivalent during configuration (perhaps by implementing IEquatable<T>), and if they are move the old logger to the new logger rather than re-create, and only Dispose() of the old logger if they are different?

If not possible or too difficult of the above, perhaps render the failed log event(s) in SelfLogFailureListener.OnLoggingFailed() rather than only listing the quantity.

nblumhardt commented 5 days ago

Hi @erichiller, thanks for the note!

If not possible or too difficult of the above, perhaps render the failed log event(s) in SelfLogFailureListener.OnLoggingFailed() rather than only listing the quantity.

This is what WriteTo.Fallible() enables - you can pass your own implementation of ILoggingFailureListener and write out the included events or try buffering them to pick up and write through another sink later.

Reload() shouldn't be causing events to reach a disposed sink, unless the thing writing the events is part of the logging infrastructure itself. This could be something to do with your configuration - if you're able to post the code we might be able to spot it.

Hope this helps, Nick