[pbs-devel] [PATCH proxmox-backup] pxar-bin: remove `log` dependency, use `tracing` directly
Christian Ebner
c.ebner at proxmox.com
Wed Sep 4 10:06:39 CEST 2024
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 at proxmox.com>
Reviewed-by: Christian Ebner <c.ebner at 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 at 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()),
> }
> }
> }
More information about the pbs-devel
mailing list