Make exception logging in `RequestLoggingMiddleware` optional

[cremor]

Is your feature request related to a problem? Please describe. If my application uses UseSerilogRequestLogging all unhandled exceptions are logged twice. This causes logs to be more long/complicated than they need to be.

Describe the solution you'd like RequestLoggingOptions should have an option to not log the exception.

Describe alternatives you've considered Maybe exception logging should even be disabled by default since I couldn't find any case where the exception isn't logged by something else.

Additional context There are multiple other classes which already log the exception, depending on your middleware configuration:

• If you use UseDeveloperExceptionPage (or are running in Development hosting environment which enables this by default) then the exception is logged by Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.
• If you use UseExceptionHandler then the exception is logged by Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.
• If you use neither then the exception is logged by Microsoft.AspNetCore.Server.Kestrel.

[sungam3r]

It looks like you can disable particular log sources by Serilog's level overrides.

[cremor]

I don't want to disable the log. I still want to see the log line HTTP GET /api/TodoItems responded 500 in 8.9821 ms. I just don't want to see the exception details with that log line.

[nblumhardt]

I think this is doable, but we'd have to be careful about how we handle any exceptions captured by IDiagnosticContext (collector, in the middleware code), since those won't propagate and would still need to be logged by the middleware regardless of the optional "include exceptions" setting.

This pushes the setting name/semantics towards IncludeEscapingExceptions or something of that kind - meaning, turning the option off would not necessarily suppress all exception information in the completion events.

🤔

[cremor]

Where/how is this IDiagnosticContext used? Or rather, who calls its SetException method?

[pcmcoomans]

Have there any updates regarding this issue?

[nblumhardt]

@cremor users can call IDiagnosticContext.SetException() from anywhere in an executing web request to set the Exception included in the final log event. I think we'd still need to record these, because they'll never reach any other exception handler.

Given the possible additional complications around logging cancellation exceptions, how about an enum for the option, which gives us some flexibility for future extensions?

The final values for the IncludeEscapingExceptions setting then might be along the lines of:

• None - don't log any exceptions ever
• ExplicitOnly - only log exceptions received from IDiagnosticScope
• IgnoreCanceled - log everything except cancellation exceptions (Task and Operation?)
• All - the default, current behavior

Initially we could consider implementing only the first and last, or some selection of those.

[cremor]

I think this is a good idea. Some details:

• About None and ExplicitOnly: That would still log the line "HTTP GET /api/test responded 500 in 55.9105 ms", right? So only the exception part is suppressed?
• About "Task and Operation?": TaskCanceledException is derived from OperationCanceledException.
• About IgnoreCanceled: DeveloperExceptionPageMiddlewareImpl and ExceptionHandlerMiddlewareImpl implement a bit more logic than just checking the exception type to detect cancelled requests. Maybe RequestLoggingMiddleware should do the same.
• I'd set ExplicitOnly as the new default. If you don't want that, maybe at least set IgnoreCanceled? See #372 for my reasons.