-
Notifications
You must be signed in to change notification settings - Fork 17
Update logging #72
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
Update logging #72
Conversation
7c072ab to
43a8295
Compare
|
Missing docs, so not clear what is the behavior change. Can you add the missing docs changes we can start with the user experience and how this affects programs running krunkit? I see this is added in the last commit, will check it. |
|
|
||
| - `--krun-log-level` | ||
|
|
||
| Set the log level for libkrun. Supported values: 0=off, 1=error, 2=warn, 3=info (default), 4=debug, 5 or more=trace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that this was discussed in the past, but this log level is not compatible with vfkit, and not easy to use. We should really support the standard log level as strings:
- trace
- debug
- info
- warn
- error
I don't see a need for 0 off. Why would someone want to disable errors in the log? We should not help to user to shoot themself in the foot. They can always redirect the log to /dev/null or with the new --log-file use /dev/null.
To keep the option to use numbers, we can support strings values and any integer. if the value is not one of the level names, convert the int. If not an int, fail. If and int, just use the int. This will keep everyone happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not really related to this change, I can open an issue to discuss this.
43a8295 to
d4c5325
Compare
ae8cf1a to
fb27d71
Compare
Moves away from using the `krun_set_log_level` API in favor of the newer, and more configurable, `krun_init_log`. Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
Add a new `--log-file` option that allows the user to specify a path in which krunkit and libkrun will write the logs. Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
Rather than printing out any logs to stdout through `println!`, we should be using the log crate. Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
fb27d71 to
96c8d1a
Compare
tylerfanelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
The goal of this PR is to update how krunkit performs logging. It moves away from using the
krun_set_log_levelAPI in favor of the newer, and more configurable,krun_init_logsto configure the libkrun logging behavior. The intent of these changes is for krunkit to follow similar behavior by exposing to the user a--log-fileoption to specify where they want their logs output to.Fixes: #52