Migrate from log to tracing
#59
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| GracefulShutdownError::ShutdownTimeout(_) => { | ||
| log::warn!("Shutdown timed out.") | ||
| sum.push_str(format!("Shutdown timed out.\n").as_str()); |
Check warning
Code scanning / clippy
useless use of `format!`
| } | ||
| MyError::WithoutData => { | ||
| log::warn!(" It failed with MyError::WithoutData") | ||
| sum.push_str( | ||
| format!(" It failed with MyError::WithoutData\n").as_str(), |
Check warning
Code scanning / clippy
useless use of `format!`
| if let Err(e) = &errors { | ||
| match e { | ||
| GracefulShutdownError::SubsystemsFailed(_) => { | ||
| log::warn!("Subsystems failed.") | ||
| sum.push_str(format!("Subsystems failed.\n").as_str()); |
Check warning
Code scanning / clippy
useless use of `format!`
| @@ -94,38 +101,45 @@ async fn main() -> Result<(), miette::Report> { | |||
| .handle_shutdown_requests(Duration::from_millis(500)) | |||
| .await; | |||
|
|
|||
| let mut sum = String::new(); | |||
There was a problem hiding this comment.
After reading a bit online, it seems that multiple messages are preferred over one multi-line message. Multi-line messages seem quite discouraged.
| @@ -22,20 +22,20 @@ | |||
| //! ``` | |||
| //! use miette::Result; | |||
| //! use tokio_graceful_shutdown::{SubsystemHandle, Toplevel}; | |||
| //! use env_logger::{Builder, Env}; | |||
| //! | |||
| @@ -48,7 +48,7 @@ | |||
| //! #[tokio::main] | |||
| //! async fn main() -> Result<()> { | |||
| //! // Init logging | |||
| //! Builder::from_env(Env::default().default_filter_or("debug")).init(); | |||
| //! tracing_subscriber::fmt().pretty().with_max_level(tracing::Level::DEBUG).init(); | |||
| Err(SubsystemError::Panicked(name)) => { | ||
| log::error!("Subsystem '{}' panicked", name); | ||
| Err(SubsystemError::Panicked(_)) => { | ||
| tracing::error!("Subsystem panicked"); |
There was a problem hiding this comment.
Revert those changes, spans won't cover this one
| tracing_subscriber::fmt() | ||
| .pretty() | ||
| .with_max_level(tracing::Level::DEBUG) | ||
| .init(); |
There was a problem hiding this comment.
I'm not sure, but I think this one specifically disabled logging, so the behaviour changed here.
| @@ -251,7 +254,8 @@ impl<ErrType: ErrTypeTraits> Toplevel<ErrType> { | |||
| let cancel_on_timeout = async { | |||
| // Wait for the timeout to happen | |||
| tokio::time::sleep(shutdown_timeout).await; | |||
| log::error!("Shutdown timed out. Attempting to cleanup stale subsystems ..."); | |||
|
|
|||
| tracing::error!("Shutdown timed out. Attempting to cleanup stale subsystems ...",); | |||
| } | ||
| tracing::debug!("{}", log_message); |
There was a problem hiding this comment.
Leave as-is. After doing some research it seems that multi-line logs are quite discouraged, and multiple single-line logs are preferred.
|
I will leave this PR stale for now, as I don't have much time. Might come back to it at some point. |
|
What is remaining to do in this PR? |
Are you interested in it? I primarily stopped implementing it because nobody depended on it any more. But I can continue if people want it ;) |
|
Checking out your crate, but I am using tracing. |
I see. So the tracing-log adapter would not be an option for you? |
|
Will close this for now due to lack of time and interest. |
|
@h7kanna I did a full rewrite, and it is now |
|
Yes I was watching your rewrite 👏 |
Nice :) feel free to give me some feedback regarding whether you run into any problems with it. |
This is a continuation of #58.