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

Reduce log clutter by getting rid of duplicate Exception.ToString()s #1632

Open
greymistcube opened this issue Dec 1, 2021 · 8 comments
Open
Labels
good first issue Good for newcomers stale The issue is stale

Comments

@greymistcube
Copy link
Contributor

Default output format via Serilog is to have {exception} at the end of log messages. This is to automatically output a message from a thrown Exepction when using Error(). However, currently, many, if not most, of Serilog's Error() in the code base explicitly include Exception.Message in message parameter. The result is unnecessary cluttering of the log output with duplicated Exception.Message.

@greymistcube greymistcube added the good first issue Good for newcomers label Dec 7, 2021
@dahlia
Copy link
Contributor

dahlia commented Dec 14, 2021

Does Serilog's default behavior maintain stack traces as well? If so, we'd better not to pass exceptions by hand.

@greymistcube
Copy link
Contributor Author

As far as I know, it does. In any case,

Log.Error(e, "Some exception happened");

Automatically outputs the exception via the default setting of outputTemplate in appsettings.json as seen in the format

[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj}{NewLine}{Exception}

where exception's e.Message is printed.

What

Log.Error(e, "Some exception happened {E}", e);

does is that not only does it duplicate the printing of e.Message for {E} and {Exception} on console, but also duplicates the property in json with E property and Exception property.

@greymistcube
Copy link
Contributor Author

I haven't tested for unit tests since it might not be using the provided template, since appsettings.json is in headless, but in that case, in my opinion, the right way to resolve the issue is configure the logger separately for unit testing instead of abusing the library by using an unnecessary extra property.

@greymistcube greymistcube changed the title Reduce log clutter by getting rid of duplicate Exception.Messages Reduce log clutter by getting rid of duplicate Exception.ToString()s Dec 15, 2021
@greymistcube
Copy link
Contributor Author

Update/Errata: It's actually Exception.ToString() which contains the stack trace that gets printed not Exception.Message.

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Mar 2, 2022
@CodingPythonMan
Copy link

@stale stale bot removed the stale The issue is stale label Jul 21, 2022
@stale
Copy link

stale bot commented Jan 8, 2023

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

1 similar comment
@stale
Copy link

stale bot commented May 22, 2023

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers stale The issue is stale
Projects
Status: No status
Status: Todo
Development

No branches or pull requests

3 participants