Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make exception logging in RequestLoggingMiddleware optional #341

Open
cremor opened this issue Jul 17, 2023 · 5 comments
Open

Make exception logging in RequestLoggingMiddleware optional #341

cremor opened this issue Jul 17, 2023 · 5 comments

Comments

@cremor
Copy link

cremor commented Jul 17, 2023

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
Copy link
Contributor

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

@cremor
Copy link
Author

cremor commented Jul 18, 2023

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
Copy link
Member

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
Copy link
Author

cremor commented Aug 4, 2023

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

@pcmcoomans
Copy link

Have there any updates regarding this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants