serilog / serilog-sinks-periodicbatching

Infrastructure for Serilog sinks that process events in batches.
Apache License 2.0
70 stars 29 forks source link

Are there any plans to support Blazor WASM projects? #79

Open uhfath opened 1 month ago

uhfath commented 1 month ago

In it's current state (4.1.0) there is an exception inside Dispose(bool disposing) method: https://github.com/serilog/serilog-sinks-periodicbatching/blob/d7b0406169671d55dc34cee97eedc72c137fcff0/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs#L331-L355

This line: https://github.com/serilog/serilog-sinks-periodicbatching/blob/d7b0406169671d55dc34cee97eedc72c137fcff0/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs#L339

Causes this:

PeriodicBatchingSink (Serilog.Sinks.Seq.Batched.BatchedSeqSink): caught exception during disposal
System.PlatformNotSupportedException: Cannot wait on monitors on this runtime.

This exception is output in Serilog's self log (when enabled). In our project we wanted to use serilog-sinks-seq or serilog-sinks-http sinks to send logs to Seq server directly from browser, but they both depend on this project and both fail on this.

Just wanted to know if there are any plans to somehow workaround WASM limitations?

nblumhardt commented 1 month ago

Hello! In your Blazor component, are you able to use await DisposeAsync() instead of Dispose() on your Logger, service collection, or host (whatever is responsible for shutting down Serilog)?

If you're using Log.CloseAndFlush(), does await Log.CloseAndFlushAsync() resolve it?

Thanks!

uhfath commented 1 month ago

@nblumhardt actually we are using MEL and Serilog is configured like this:

public static IServiceCollection AddConfigurableSerilog(this IServiceCollection services, IConfiguration configuration) =>
    services
        .AddSerilog(new LoggerConfiguration()
            .ReadFrom.Configuration(configuration)
            .CreateLogger(),
        true)
    ;

So we do not use Serilog's shared logger or create it's ILogger instances manually. We use MEL's ILogger<T>.

And there are some third party components which are not always async disposable so we have no control here either.

nblumhardt commented 1 month ago

Where are you tearing down the service collection, though? It should be possible to async-dispose it, I believe this will use the non-async dispose for components that don't support IAsyncDisposable.

Logging to Seq (and most other network targets) is asynchronous for performance reasons; if you want to flush the background buffer on disposal then a network call is required, and in the browser these generally need to be async.

Definitely open to (and keen to) improve the default experience here, but I suspect there may not be many options available 🤔

uhfath commented 1 month ago

It's not just the components that use logging but other services too. And not all of them are IAsyncDisposable. Do you think if we could simply async/void in this case? With proper try/catch for logging exceptions, of course.

nblumhardt commented 1 month ago

Using DisposeAsync() on your service collection (or await app.RunAsync() in your Program.cs, IIRC) should still call Dispose() on services that don't support IAsyncDisposable. If you try it and it doesn't work, letting me know what breaks should make it easier to pinpoint the problem/a solution. HTH!

uhfath commented 1 month ago

We are using app.RunAsync() and more then that we dispose WebAssemblyHost manually through await host.DisposeAsync() but that doesn't make any difference since exceptions are thrown not at the end of the app lifecycle but on almost after every event logged. Sorry for not making it clearer in the first post.

nblumhardt commented 1 month ago

Ah, that's interesting. Are you somehow creating multiple instances of PeriodicBatchingSink? The sink shouldn't be being disposed after every logged event, this normally only happens when the root Logger is shut down. Could some other piece of code be somehow prematurely disposing the logger? (Accidental "transient" registration of the logger in an IoC container, etc. etc.?)

Providing the full call stack for the associated exception would narrow this down.

uhfath commented 1 month ago

Good catch. After your comment I did some digging and found there were some pieces of code that created an ILoggerFactory, an ILogger<T> from it, output some logs and then disposed the former. After removing this code there are no exceptions. So the question is how to properly create and dispose these? Interfaces do not implement IAsyncDisposable. As for the call stack, it doesn't really help that much:

PeriodicBatchingSink (Serilog.Sinks.Seq.Batched.BatchedSeqSink): caught exception during disposal
System.PlatformNotSupportedException: Cannot wait on monitors on this runtime.
   at System.Threading.Monitor.ObjWait(Int32 millisecondsTimeout, Object obj)
   at System.Threading.Monitor.Wait(Object obj, Int32 millisecondsTimeout)
   at System.Threading.ManualResetEventSlim.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.SpinThenBlockingWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.InternalWaitCore(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.InternalWait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()