* [pbs-devel] [PATCH proxmox] log: write to stderr when using init_cli_logger, export tracing::Level @ 2024-09-03 12:38 Gabriel Goller 2024-09-03 12:38 ` [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly Gabriel Goller 0 siblings, 1 reply; 5+ messages in thread From: Gabriel Goller @ 2024-09-03 12:38 UTC (permalink / raw) To: pbs-devel Previously when using `env_logger` all of our cli-tools logged to stderr, make tracing do the same. Export `tracing::Level` so that we can use the `tracing::enabled!` macro. Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- proxmox-log/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs index bcde5495dead..0cf5cb7a50b2 100644 --- a/proxmox-log/src/lib.rs +++ b/proxmox-log/src/lib.rs @@ -6,7 +6,6 @@ use std::future::Future; use std::sync::{Arc, Mutex}; use tokio::task::futures::TaskLocalFuture; -use tracing::Level; use tracing_log::{AsLog, LogTracer}; use tracing_subscriber::filter::filter_fn; use tracing_subscriber::prelude::*; @@ -32,6 +31,7 @@ pub use tracing::trace; pub use tracing::trace_span; pub use tracing::warn; pub use tracing::warn_span; +pub use tracing::Level; pub use tracing_subscriber::filter::LevelFilter; tokio::task_local! { @@ -154,6 +154,7 @@ pub fn init_cli_logger( .with( tracing_subscriber::fmt::layer() .event_format(format) + .with_writer(std::io::stderr) .with_filter(filter_fn(|metadata| { !LogContext::exists() || *metadata.level() >= Level::ERROR })) -- 2.39.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly 2024-09-03 12:38 [pbs-devel] [PATCH proxmox] log: write to stderr when using init_cli_logger, export tracing::Level Gabriel Goller @ 2024-09-03 12:38 ` Gabriel Goller 2024-09-04 8:06 ` Christian Ebner 0 siblings, 1 reply; 5+ messages in thread From: Gabriel Goller @ 2024-09-03 12:38 UTC (permalink / raw) To: pbs-devel When using the `log` to `tracing` translation layer, the messages get padded with whitespaces. This bug will get fixed upstream [0], but in the meantime we switch to the `tracing` macros. [0]: https://github.com/tokio-rs/tracing/pull/3070 Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- pxar-bin/Cargo.toml | 1 - pxar-bin/src/main.rs | 24 ++++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pxar-bin/Cargo.toml b/pxar-bin/Cargo.toml index 64dcf1496b4d..d0d7ab24d9b9 100644 --- a/pxar-bin/Cargo.toml +++ b/pxar-bin/Cargo.toml @@ -11,7 +11,6 @@ path = "src/main.rs" [dependencies] anyhow.workspace = true futures.workspace = true -log.workspace = true nix.workspace = true serde_json.workspace = true tokio = { workspace = true, features = [ "rt", "rt-multi-thread" ] } diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs index 71ce3bf78f52..70e627775c26 100644 --- a/pxar-bin/src/main.rs +++ b/pxar-bin/src/main.rs @@ -19,7 +19,7 @@ use pbs_client::pxar::{ use pxar::EntryKind; use proxmox_human_byte::HumanByte; -use proxmox_log::init_cli_logger; +use proxmox_log::{init_cli_logger, info, error, debug, enabled, Level}; use proxmox_router::cli::*; use proxmox_schema::api; @@ -40,7 +40,7 @@ fn extract_archive_from_reader<R: std::io::Read>( Path::new(target), feature_flags, |path| { - log::debug!("{:?}", path); + debug!("{:?}", path); }, options, ) @@ -222,7 +222,7 @@ fn extract_archive( // otherwise we want to log them but not act on them Some(Box::new(move |err| { was_ok.store(false, Ordering::Release); - log::error!("error: {}", err); + error!("error: {}", err); Ok(()) }) as Box<dyn FnMut(Error) -> Result<(), Error> + Send>) @@ -243,7 +243,7 @@ fn extract_archive( extract_archive_from_reader(&mut reader, target, feature_flags, options, None) .map_err(|err| format_err!("error extracting archive - {err:#}"))?; } else { - log::debug!("PXAR extract: {}", archive); + debug!("PXAR extract: {}", archive); let file = std::fs::File::open(archive)?; let mut reader = std::io::BufReader::new(file); let mut payload_reader = if let Some(payload_input) = payload_input { @@ -439,7 +439,7 @@ async fn create_archive( PxarWriters::new(writer, None), feature_flags, move |path| { - log::debug!("{:?}", path); + debug!("{:?}", path); Ok(()) }, options, @@ -495,7 +495,7 @@ async fn mount_archive( select! { res = session.fuse() => res?, _ = interrupt.recv().fuse() => { - log::debug!("interrupted"); + debug!("interrupted"); } } @@ -531,14 +531,14 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er for entry in pxar::decoder::Decoder::open(input)? { let entry = entry?; - if log::log_enabled!(log::Level::Debug) { + if enabled!(Level::DEBUG) { match entry.kind() { EntryKind::Version(version) => { - log::debug!("pxar format version '{version:?}'"); + debug!("pxar format version '{version:?}'"); continue; } EntryKind::Prelude(prelude) => { - log::debug!("prelude of size {}", HumanByte::from(prelude.data.len())); + debug!("prelude of size {}", HumanByte::from(prelude.data.len())); continue; } EntryKind::File { @@ -549,7 +549,7 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er if let Some(last) = last { let skipped = offset - last; if skipped > 0 { - log::debug!("Encountered padding of {skipped} bytes"); + debug!("Encountered padding of {skipped} bytes"); } } last = Some(offset + size + std::mem::size_of::<pxar::format::Header>() as u64); @@ -557,11 +557,11 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er _ => (), } - log::debug!("{}", format_single_line_entry(&entry)); + debug!("{}", format_single_line_entry(&entry)); } else { match entry.kind() { EntryKind::Version(_) | EntryKind::Prelude(_) => continue, - _ => log::info!("{:?}", entry.path()), + _ => info!("{:?}", entry.path()), } } } -- 2.39.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly 2024-09-03 12:38 ` [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly Gabriel Goller @ 2024-09-04 8:06 ` Christian Ebner 2024-09-04 12:31 ` Christian Ebner 2024-09-04 13:27 ` Gabriel Goller 0 siblings, 2 replies; 5+ messages in thread From: Christian Ebner @ 2024-09-04 8:06 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller small nits inline As you already discussed with Wolfgang in the other thread, any user facing output not intended for debugging, e.g. the output of the `pxar list` might be better displayed by writing to stdout using a `println` instead. Tested by listing and extracting a pxar archive, with and without the `PXAR_LOG=debug` environment variable set. Other than that, please consider this and the patch to `proxmox-log`: Tested-by: Christian Ebner <c.ebner@proxmox.com> Reviewed-by: Christian Ebner <c.ebner@proxmox.com> On 9/3/24 14:38, Gabriel Goller wrote: > When using the `log` to `tracing` translation layer, the messages get > padded with whitespaces. This bug will get fixed upstream [0], but in > the meantime we switch to the `tracing` macros. > > [0]: https://github.com/tokio-rs/tracing/pull/3070 > > Signed-off-by: Gabriel Goller <g.goller@proxmox.com> > --- > pxar-bin/Cargo.toml | 1 - > pxar-bin/src/main.rs | 24 ++++++++++++------------ > 2 files changed, 12 insertions(+), 13 deletions(-) > > diff --git a/pxar-bin/Cargo.toml b/pxar-bin/Cargo.toml > index 64dcf1496b4d..d0d7ab24d9b9 100644 > --- a/pxar-bin/Cargo.toml > +++ b/pxar-bin/Cargo.toml > @@ -11,7 +11,6 @@ path = "src/main.rs" > [dependencies] > anyhow.workspace = true > futures.workspace = true > -log.workspace = true > nix.workspace = true > serde_json.workspace = true > tokio = { workspace = true, features = [ "rt", "rt-multi-thread" ] } > diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs > index 71ce3bf78f52..70e627775c26 100644 > --- a/pxar-bin/src/main.rs > +++ b/pxar-bin/src/main.rs > @@ -19,7 +19,7 @@ use pbs_client::pxar::{ > use pxar::EntryKind; > > use proxmox_human_byte::HumanByte; > -use proxmox_log::init_cli_logger; > +use proxmox_log::{init_cli_logger, info, error, debug, enabled, Level}; > use proxmox_router::cli::*; > use proxmox_schema::api; > > @@ -40,7 +40,7 @@ fn extract_archive_from_reader<R: std::io::Read>( > Path::new(target), > feature_flags, > |path| { > - log::debug!("{:?}", path); > + debug!("{:?}", path); `path` can be inlined here > }, > options, > ) > @@ -222,7 +222,7 @@ fn extract_archive( > // otherwise we want to log them but not act on them > Some(Box::new(move |err| { > was_ok.store(false, Ordering::Release); > - log::error!("error: {}", err); > + error!("error: {}", err); > Ok(()) > }) > as Box<dyn FnMut(Error) -> Result<(), Error> + Send>) > @@ -243,7 +243,7 @@ fn extract_archive( > extract_archive_from_reader(&mut reader, target, feature_flags, options, None) > .map_err(|err| format_err!("error extracting archive - {err:#}"))?; > } else { > - log::debug!("PXAR extract: {}", archive); > + debug!("PXAR extract: {}", archive); same for `archive` > let file = std::fs::File::open(archive)?; > let mut reader = std::io::BufReader::new(file); > let mut payload_reader = if let Some(payload_input) = payload_input { > @@ -439,7 +439,7 @@ async fn create_archive( > PxarWriters::new(writer, None), > feature_flags, > move |path| { > - log::debug!("{:?}", path); > + debug!("{:?}", path); same here > Ok(()) > }, > options, > @@ -495,7 +495,7 @@ async fn mount_archive( > select! { > res = session.fuse() => res?, > _ = interrupt.recv().fuse() => { > - log::debug!("interrupted"); > + debug!("interrupted"); > } > } > > @@ -531,14 +531,14 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er > for entry in pxar::decoder::Decoder::open(input)? { > let entry = entry?; > > - if log::log_enabled!(log::Level::Debug) { > + if enabled!(Level::DEBUG) { > match entry.kind() { > EntryKind::Version(version) => { > - log::debug!("pxar format version '{version:?}'"); > + debug!("pxar format version '{version:?}'"); > continue; > } > EntryKind::Prelude(prelude) => { > - log::debug!("prelude of size {}", HumanByte::from(prelude.data.len())); > + debug!("prelude of size {}", HumanByte::from(prelude.data.len())); > continue; > } > EntryKind::File { > @@ -549,7 +549,7 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er > if let Some(last) = last { > let skipped = offset - last; > if skipped > 0 { > - log::debug!("Encountered padding of {skipped} bytes"); > + debug!("Encountered padding of {skipped} bytes"); > } > } > last = Some(offset + size + std::mem::size_of::<pxar::format::Header>() as u64); > @@ -557,11 +557,11 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er > _ => (), > } > > - log::debug!("{}", format_single_line_entry(&entry)); > + debug!("{}", format_single_line_entry(&entry)); > } else { > match entry.kind() { > EntryKind::Version(_) | EntryKind::Prelude(_) => continue, > - _ => log::info!("{:?}", entry.path()), > + _ => info!("{:?}", entry.path()), > } > } > } _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly 2024-09-04 8:06 ` Christian Ebner @ 2024-09-04 12:31 ` Christian Ebner 2024-09-04 13:27 ` Gabriel Goller 1 sibling, 0 replies; 5+ messages in thread From: Christian Ebner @ 2024-09-04 12:31 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller On 9/4/24 10:06, Christian Ebner wrote: > small nits inline > > As you already discussed with Wolfgang in the other thread, any user > facing output not intended for debugging, e.g. the output of the `pxar > list` might be better displayed by writing to stdout using a `println` > instead. > > Tested by listing and extracting a pxar archive, with and without the > `PXAR_LOG=debug` environment variable set. > > Other than that, please consider this and the patch to `proxmox-log`: > > Tested-by: Christian Ebner <c.ebner@proxmox.com> > Reviewed-by: Christian Ebner <c.ebner@proxmox.com> > Also, this needs a `cargo fmt` to correctly sort the entities in the proxmox_log use statement. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly 2024-09-04 8:06 ` Christian Ebner 2024-09-04 12:31 ` Christian Ebner @ 2024-09-04 13:27 ` Gabriel Goller 1 sibling, 0 replies; 5+ messages in thread From: Gabriel Goller @ 2024-09-04 13:27 UTC (permalink / raw) To: Christian Ebner; +Cc: Proxmox Backup Server development discussion On 04.09.2024 10:06, Christian Ebner wrote: >small nits inline > >As you already discussed with Wolfgang in the other thread, any user >facing output not intended for debugging, e.g. the output of the `pxar >list` might be better displayed by writing to stdout using a `println` >instead. I think it's better if we do this later in a more comprehensive series where we change the behavior in all proxmox clients. >Tested by listing and extracting a pxar archive, with and without the >`PXAR_LOG=debug` environment variable set. > >Other than that, please consider this and the patch to `proxmox-log`: > >Tested-by: Christian Ebner <c.ebner@proxmox.com> >Reviewed-by: Christian Ebner <c.ebner@proxmox.com> Thanks a lot for the review! Sent a v2! _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-04 13:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-09-03 12:38 [pbs-devel] [PATCH proxmox] log: write to stderr when using init_cli_logger, export tracing::Level Gabriel Goller 2024-09-03 12:38 ` [pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly Gabriel Goller 2024-09-04 8:06 ` Christian Ebner 2024-09-04 12:31 ` Christian Ebner 2024-09-04 13:27 ` Gabriel Goller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox