Skip to content

better logs #4255

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 9, 2024
Merged

better logs #4255

merged 3 commits into from
Jul 9, 2024

Conversation

Byron
Copy link
Collaborator

@Byron Byron commented Jul 7, 2024 •
edited
Loading

This PR improves on application logging in various ways, see tasks.

Tasks

  • make sure we don't keep all the logs (max_log_files(14)) seems ineffective
    • The problem is that this is only activated on roll-over, not on initialization or other events we actually have
  • make log filename simpler and purge all non-conforming logs

Notes for the Reviewer

  • I performed manual testing on each commit, which worked well as there were many stray log files in my case. For safety, I compressed the com.gitbutler.app.dev folder to be able to restore and re-test if necessary, but didn't end up using it.
  • Logs are now named GitButler.<date>.log consistently.
  • This needs a documentation change which needs to be timed with a release, or be written so it's flexible enough.
  • The docs-update is available here, and ideally it's synced with the release of this PR in stable. But even if it was released before people will probably still find their logs without problems.

Byron added 3 commits July 9, 2024 10:53
@Byron
assure not too many log files are retained.
b1b04ad 
Previously, despite `max_log_files()` configured, it would still
retain more than that, and effectively never delete any as that
would only happen on log-rotation.

Now we do it ourselves just once.
@Byron
consistent extension for .log files should help in reading them.
5f192d0 
This might particularly be true on Windows.
Note that we don't call them `txt` to make opening them with
a specialized program easier.
@Byron
specifically remove log files with old name
bb880cf 
This also assures that previously spilled secrets will be removed.
@github-actions github-actions bot added the rust Pull requests that update Rust code label Jul 9, 2024
@Byron Byron marked this pull request as ready for review July 9, 2024 09:12
@Byron Byron requested a review from krlvi July 9, 2024 10:48
Byron added a commit to Byron/docs that referenced this pull request Jul 9, 2024
@Byron
update log format to match the latest iteration.
c1d5d3d 
It was changed in gitbutlerapp/gitbutler#4255.

Note that the log without date, `GitButler.log`, could not be observed
and was removed for that reason.
@Byron Byron mentioned this pull request Jul 9, 2024
Open
krlvi
krlvi approved these changes Jul 9, 2024
View reviewed changes
Copy link
Member

@krlvi krlvi left a comment

Choose a reason for hiding this comment

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

👍

@krlvi krlvi merged commit 164ca5e into gitbutlerapp:master Jul 9, 2024
@Byron Byron deleted the log-retention branch July 9, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@krlvi krlvi krlvi approved these changes

Assignees

No one assigned

Labels

rust Pull requests that update Rust code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Byron @krlvi