From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 696D81FF15C for ; Wed, 13 Nov 2024 12:41:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E5525146BD; Wed, 13 Nov 2024 12:41:54 +0100 (CET) Mime-Version: 1.0 Date: Wed, 13 Nov 2024 12:41:21 +0100 Message-Id: To: "Proxmox Backup Server development discussion" From: "Shannon Sterz" X-Mailer: aerc 0.17.0-69-g65571b67d7d3-dirty References: <20241111130822.124584-1-f.schauer@proxmox.com> <20241111130822.124584-4-f.schauer@proxmox.com> In-Reply-To: <20241111130822.124584-4-f.schauer@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.043 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH vma-to-pbs v5 3/4] use level-based logging instead of println X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" comments in-line: On Mon Nov 11, 2024 at 2:08 PM CET, Filip Schauer wrote: > Use log level "info" by default and prevent spamming messages for every > single chunk uploaded. To re-enable these messages, set the RUST_LOG > environment variable to "debug". > > Signed-off-by: Filip Schauer > --- > Cargo.toml | 2 ++ > src/main.rs | 28 ++++++++++++++++++++++------ > src/vma2pbs.rs | 38 ++++++++++++++++++++------------------ > 3 files changed, 44 insertions(+), 24 deletions(-) > > diff --git a/Cargo.toml b/Cargo.toml > index ad80304..7951bbc 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -8,7 +8,9 @@ edition = "2021" > anyhow = "1.0" > bincode = "1.3" > chrono = "0.4" > +env_logger = "0.10" > hyper = "0.14.5" > +log = "0.4" > pico-args = "0.5" > md5 = "0.7.0" > regex = "1.7" > diff --git a/src/main.rs b/src/main.rs > index d4b36fa..203196b 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -6,6 +6,7 @@ use std::path::PathBuf; > > use anyhow::{bail, Context, Error}; > use chrono::NaiveDateTime; > +use env_logger::Target; > use proxmox_sys::linux::tty; > use proxmox_time::epoch_i64; > use regex::Regex; > @@ -128,7 +129,7 @@ fn parse_args() -> Result { > > match (encrypt, keyfile.is_some()) { > (true, false) => bail!("--encrypt requires a --keyfile!"), > - (false, true) => println!( > + (false, true) => log::info!( > "--keyfile given, but --encrypt not set -> backup will be signed, but not encrypted!" > ), > _ => {} > @@ -190,7 +191,7 @@ fn parse_args() -> Result { > > Some(key_password) > } else if vma_file_path.is_none() { > - println!( > + log::info!( > "Please use --key-password-file to provide the password when passing the VMA file \ > to stdin, if required." > ); > @@ -246,13 +247,17 @@ fn parse_args() -> Result { > let Some((_, [backup_id, timestr, ext])) = > re.captures(file_name).map(|c| c.extract()) > else { > - // Skip the file, since it is not a VMA backup > + log::debug!("Skip \"{file_name}\", since it is not a VMA backup"); > continue; > }; > > if let Some(ref vmid) = vmid { > if backup_id != vmid { > - // Skip the backup, since it does not match the specified vmid > + log::debug!( > + "Skip backup with VMID {}, since it does not match specified VMID {}", > + backup_id, > + vmid nit: you can use format strings here > + ); > continue; > } > } > @@ -308,14 +313,14 @@ fn parse_args() -> Result { > bail!("Did not find any backup archives"); > } > > - println!( > + log::info!( > "Found {} backup archive(s) of {} different VMID(s):", > total_vma_count, > grouped_vmas.len() > ); > > for (backup_id, vma_group) in &grouped_vmas { > - println!("- VMID {}: {} backups", backup_id, vma_group.len()); > + log::info!("- VMID {}: {} backups", backup_id, vma_group.len()); > } nit: if you are already touching this, move this over to format strings as well > > if !yes { > @@ -358,7 +363,18 @@ fn parse_args() -> Result { > Ok(options) > } > > +fn init_cli_logger() { > + env_logger::Builder::from_env(env_logger::Env::new().filter_or("RUST_LOG", "info")) > + .format_level(false) > + .format_target(false) > + .format_timestamp(None) > + .target(Target::Stdout) > + .init(); > +} > + > fn main() -> Result<(), Error> { > + init_cli_logger(); > + > let args = parse_args()?; > vma2pbs(args)?; > > diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs > index a5b4027..0517251 100644 > --- a/src/vma2pbs.rs > +++ b/src/vma2pbs.rs > @@ -82,8 +82,8 @@ fn create_pbs_backup_task( > pbs_args: &PbsArgs, > backup_args: &VmaBackupArgs, > ) -> Result<*mut ProxmoxBackupHandle, Error> { > - println!( > - "backup time: {}", > + log::info!( > + "\tbackup time: {}", > epoch_to_rfc3339(backup_args.backup_time)? > ); > > @@ -152,7 +152,7 @@ where > let config_name = config.name; > let config_data = config.content; > > - println!("CFG: size: {} name: {}", config_data.len(), config_name); > + log::info!("\tCFG: size: {} name: {}", config_data.len(), config_name); nit: move `config_name` into the format string > > let config_name_cstr = CString::new(config_name)?; > > @@ -190,9 +190,11 @@ where > let device_name = vma_reader.get_device_name(device_id.try_into()?)?; > let device_size = vma_reader.get_device_size(device_id.try_into()?)?; > > - println!( > - "DEV: dev_id={} size: {} devname: {}", > - device_id, device_size, device_name > + log::info!( > + "\tDEV: dev_id={} size: {} devname: {}", nit: format string > + device_id, > + device_size, > + device_name > ); > > let device_name_cstr = CString::new(device_name)?; > @@ -276,8 +278,8 @@ where > }; > > let pbs_upload_chunk = |pbs_chunk_buffer: Option<&[u8]>| { > - println!( > - "Uploading dev_id: {} offset: {:#0X} - {:#0X}", > + log::debug!( > + "\tUploading dev_id: {} offset: {:#0X} - {:#0X}", nit: format string, for example: `\tUploading dev_id: {dev_id} offset: {pbs_chunk_offset:#0X} - {:#0X}` > dev_id, > pbs_chunk_offset, > pbs_chunk_offset + pbs_chunk_size, > @@ -466,13 +468,13 @@ fn set_notes( > > pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > let pbs_args = &args.pbs_args; > - println!("PBS repository: {}", pbs_args.pbs_repository); > + log::info!("PBS repository: {}", pbs_args.pbs_repository); > if let Some(ns) = &pbs_args.namespace { > - println!("PBS namespace: {}", ns); > + log::info!("PBS namespace: {}", ns); nit: format string > } > - println!("PBS fingerprint: {}", pbs_args.fingerprint); > - println!("compress: {}", pbs_args.compress); > - println!("encrypt: {}", pbs_args.encrypt); > + log::info!("PBS fingerprint: {}", pbs_args.fingerprint); > + log::info!("compress: {}", pbs_args.compress); > + log::info!("encrypt: {}", pbs_args.encrypt); > > let start_transfer_time = SystemTime::now(); > > @@ -486,8 +488,8 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > ); > > if args.skip_failed { > - eprintln!("{}", err_msg); > - println!("Skipping VMID {}", backup_args.backup_id); > + log::warn!("{}", err_msg); > + log::info!("Skipping VMID {}", backup_args.backup_id); > break; > } else { > bail!(err_msg); > @@ -501,15 +503,15 @@ pub fn vma2pbs(args: BackupVmaToPbsArgs) -> Result<(), Error> { > let minutes = total_seconds / 60; > let seconds = total_seconds % 60; > let milliseconds = transfer_duration.as_millis() % 1000; > - println!("Backup finished within {minutes} minutes, {seconds} seconds and {milliseconds} ms"); > + log::info!("Backup finished within {minutes} minutes, {seconds} seconds and {milliseconds} ms"); > > Ok(()) > } > > fn upload_vma_file(pbs_args: &PbsArgs, backup_args: &VmaBackupArgs) -> Result<(), Error> { > match &backup_args.vma_file_path { > - Some(vma_file_path) => println!("Uploading VMA backup from {:?}", vma_file_path), > - None => println!("Uploading VMA backup from (stdin)"), > + Some(vma_file_path) => log::info!("Uploading VMA backup from {:?}", vma_file_path), nit: format string > + None => log::info!("Uploading VMA backup from (stdin)"), > }; > > let vma_file: Box = match &backup_args.compression { _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel