Always flush sink on Fatal events

[BernhardMarconato]

Implements #247

Always flush the FileSink when a LogLevelEvent.Fatal event is emitted. This is a breaking change in behavior.

A common use case is having a flush interval set, but still wanting to force flush Fatal events to the file before the process terminates. Previously, these log events would have been lost.

Add the option to provide a minimum log level using flushAtMinimumLevel for which the sink should flush to disk immediately.
To retain backwards compatibility, the default value is LevelAlias.Off.

[nblumhardt]

Hi @BernhardMarconato!

Thinking about the scenario you describe, it sounds like that one in particular would be solved by calling FlushToDisk() in FileSink.Dispose(): when a fatal error is logged, generally it's within some kind of unwinding which can also close/flush the logging pipeline. Flushing to disk in there should have the same effect, without requiring any additional API. What do you think?

[BernhardMarconato]

Thanks for taking a look!

You’re right that the filestream should also be flushed during disposal. However, in most cases, flushing the TextWriter (as is currently done) seems to be sufficient and flushes to the file as well. That’s what I’m currently doing by calling Serilog.Log.CloseAndFlush();. But due to platform limitations (WinUI), I can’t guarantee that the app will actually terminate upon receiving an unhandled exception. If the logger is disposed eagerly, it might stop logging too soon.

I’m not sure how you feel about breaking changes, but I think it could be justified to always flush for Fatal events, even without an additional API for configuring this.

Alternatively, I’d be happy if there was a way to access the IFlushableFileSink instance directly so I could flush manually when needed. As far as I know, this isn’t possible at the moment due to the nested sink structure (but please correct me if I’m wrong).

[nblumhardt]

Thanks for the analysis and suggestions, much appreciated.

Having fatal errors fully flushed to disk, even when buffering is disabled, seems like it would be acceptable and a decent improvement, given that there's no plan to expose flushing via another mechanism. E.g. thinking it would be:

if (logEvent.Level == LogEventLevel.Fatal)
    FlushToDisk();
else if (!_buffered)
    _output.Flush();

It does open up the possibility of causing a performance drain if a large number of Fatal errors are logged in quick succession, but at that point the wheels are likely to be falling off anyway. Interested in opinions on this if anyone following along thinks otherwise.

It'd be worth bumping the major version and documenting this in the WriteTo.File() XDOC.

[BernhardMarconato]

Thanks @nblumhardt, I applied your suggestions! Let me know if anything else is required.