Add initial logging capability to agama-dbus-server#629
Conversation
|
Changes unknown |
No. AFAIK the Ruby part simply logs to stderr and it is the controlling daemons, See also this relevant example that we may want to copy, it uses more metadata when using the journal and falls back to stderr if run directly: https://codeberg.org/swsnr/systemd-journal-logger.rs/src/branch/main/examples/systemd_service.rs |
mvidner
changed the title
rust/agama-dbus-server/src/main.rs
Outdated
|
|
||
| #[async_std::main] | ||
| async fn main() -> Result<(), Box<dyn std::error::Error>> { | ||
| JournalLog::default().install().unwrap(); |
There was a problem hiding this comment.
unwrap...
should be used when you know better than the compiler that it cannot fail
The function is already returning a Result
| JournalLog::default().install().unwrap(); | |
| JournalLog::default().install()?; |
There was a problem hiding this comment.
... and it turns out that you are right, it fails only if the logger has been set already
https://docs.rs/log/0.4.17/log/fn.set_boxed_logger.html
In that case I'd add a comment explaining why it can't fail. Might as well keep the ?.
There was a problem hiding this comment.
ok, will document it. As I am not big fan of Box due to its wide scope and with unwrap it will make it change to more precise error definition easier.
There was a problem hiding this comment.
As I am not big fan of Box due to its wide scope and with unwrap it will make it change to more precise error definition easier.
Just some nitpicking while I wait for the rustfmt CI run 😄
I think it is the dyn part, not the Box, that you object to :)
In general yes, it makes the error API too catch-all... but then we are in fn main and we can only crash... which also refutes my initial objection 🤣
rust/agama-dbus-server/src/main.rs
Outdated
| #[async_std::main] | ||
| async fn main() -> Result<(), Box<dyn std::error::Error>> { | ||
| // be smart with logging and log directly to journal if connected to it | ||
| if systemd_journal_logger::connected_to_journal(){ |
There was a problem hiding this comment.
| if systemd_journal_logger::connected_to_journal(){ | |
| if systemd_journal_logger::connected_to_journal() { |
That reminds me, I don't know how to check for formatting problems. How? Should we make it mandatory?
There was a problem hiding this comment.
well, I again forget to run rustfmt before submit. I will try to configure my IDE to do it
There was a problem hiding this comment.
btw I can add it to CI with something like find rust/agama-*/src/ | grep '.rs$' | xargs rustfmt --check --edition 2021
|
So it turns out automatic |
3 participants
Problem
Currently agama-dbus-service does not provide any logging. So troubleshooting is hard.
trello: https://trello.com/c/qjrhMrmi/3378-2-agama-enable-a-logging-mechanism-in-agama-dbus-server
Solution
Add initial logging capability with systemd logger to be consistent with ruby counterpart that also logs to journal.
Testing