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

Replace Console.WriteLine with proper logging #195

Conversation

DaleStan
Copy link
Collaborator

This closes #54 by replacing the calls to Console.WriteLine with calls to calls into Serilog. In cases where Yafc currently produces console output, it should not change that output.

I was briefly tempted to write the logs to a SQLite database, but decided not to do that so the logs would be somewhat user-readable. The log files will look something like this, and will be deleted is they're over a week old: yafc20240717.zip

This was prompted by #178 (comment), which made me think that it might be useful to log the transitions to and from the UI thread, along with their call stacks.

@Dorus, I used Serilog instead of Microsoft.Extensions.Logging because there does not appear to be a Microsoft.Extensions.Logging.File nuget package. If I'm wrong there, I'm happy to use a Microsoft.Extensions.Logging-based package instead, as long as it provides structured logging. That is, something where I can easily distinguish between "Disk quota {} set for user {}" and "Disk quota {} exceed by user {}".

@veger
Copy link
Collaborator

veger commented Jul 17, 2024

Looks like this contains the commits of your previous PR as well?

@DaleStan DaleStan force-pushed the 54-discussion-replace-consolewriteline-with-proper-logging branch from b378230 to 16735be Compare July 17, 2024 22:57
@DaleStan
Copy link
Collaborator Author

It did. I thought githib would remove the merged commits after #194 was merged, but I guess not.

Copy link
Owner

@shpaass shpaass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shpaass shpaass merged commit 1a6fa44 into shpaass:master Jul 18, 2024
1 check passed
@Dorus
Copy link
Collaborator

Dorus commented Jul 18, 2024

@Dorus, I used Serilog instead of Microsoft.Extensions.Logging because there does not appear to be a Microsoft.Extensions.Logging.File nuget package. If I'm wrong there, I'm happy to use a Microsoft.Extensions.Logging-based package instead, as long as it provides structured logging. That is, something where I can easily distinguish between "Disk quota {} set for user {}" and "Disk quota {} exceed by user {}".

Yes this is the default for microsoft logging too, you get a lot template and the objects logged separate.

There is no official file logger package apparently, but a quick google shows there are custom packages like this one: https://github.com/nreco/logging

@DaleStan DaleStan deleted the 54-discussion-replace-consolewriteline-with-proper-logging branch July 21, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discussion] Replace Console.WriteLine with proper logging
4 participants