From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 2671FBF35 for ; Fri, 11 Aug 2023 14:53:21 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0F2ADCCBF for ; Fri, 11 Aug 2023 14:53:21 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 11 Aug 2023 14:53:19 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6718346039 for ; Fri, 11 Aug 2023 14:53:19 +0200 (CEST) Message-ID: Date: Fri, 11 Aug 2023 14:53:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.1 Content-Language: en-US To: Wolfgang Bumiller Cc: pbs-devel@lists.proxmox.com References: <20230807125737.168352-1-g.goller@proxmox.com> From: Gabriel Goller In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.701 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 NICE_REPLY_A -2.156 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [main.rs, pull.rs, catalog.rs, mount.rs, diff.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 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: , X-List-Received-Date: Fri, 11 Aug 2023 12:53:21 -0000 On a second note, how about we also introduce more fine-grained logging settings to the api/proxy? Currently on the api we hardcode the `log::max_level` to `LevelFilter::Info` and on the proxy we check if the environment variable `PROXMOX_DEBUG` is set and use `LevelFilter::Debug` if it exists. We could do the same as on the cli and allow `PROXMOX_DEBUG` (probably renaming it to `PROXMOX_LOG_LEVEL` or something) to be set to `trace`, `debug`, `info`, `warning` or `error`. Then we could also decide if we want to use the server `log::max_level` or the client-side `debug` flag in the `download_file` and `download_chunk` functions (where the original issue #4646 comes from). On 8/8/23 12:38, Wolfgang Bumiller wrote: > Oof. This controls the debug parameter for the remote side. > > IMO it should be a separate parameter passed through from the CLI, > rather than the current log level. > I mean, AFAICT this controls task logs which you can view eg. via the > UI, and not the actual console output locally? Or do we display the > remote task output in all of the affected commands? > If I set a log level and don't see anything because it just happens > remotely, that would be weird :-) > > On Mon, Aug 07, 2023 at 02:57:37PM +0200, Gabriel Goller wrote: >> Previously the BackupReader debug parameter was always hardcoded. Now we >> check the current loglevel with the `log_enabled!()` macro and set debug >> to true if we are on Trace or Debug. This allow the user to set the loglevel >> using `PBS_LOG`. >> >> Signed-off-by: Gabriel Goller >> --- >> proxmox-backup-client/src/catalog.rs | 5 +++-- >> proxmox-backup-client/src/main.rs | 3 ++- >> proxmox-backup-client/src/mount.rs | 3 ++- >> proxmox-file-restore/src/main.rs | 3 ++- >> src/bin/proxmox_backup_debug/diff.rs | 3 ++- >> src/server/pull.rs | 3 ++- >> 6 files changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/proxmox-backup-client/src/catalog.rs b/proxmox-backup-client/src/catalog.rs >> index 8c8c1458..c9cf1b5c 100644 >> --- a/proxmox-backup-client/src/catalog.rs >> +++ b/proxmox-backup-client/src/catalog.rs >> @@ -3,6 +3,7 @@ use std::os::unix::fs::OpenOptionsExt; >> use std::sync::Arc; >> >> use anyhow::{bail, format_err, Error}; >> +use log::{log_enabled, Level}; >> use serde_json::Value; >> >> use proxmox_router::cli::*; >> @@ -80,7 +81,7 @@ async fn dump_catalog(param: Value) -> Result { >> repo.store(), >> &backup_ns, >> &snapshot, >> - true, >> + log_enabled!(Level::Debug) || log_enabled!(Level::Trace), >> ) >> .await?; >> >> @@ -192,7 +193,7 @@ async fn catalog_shell(param: Value) -> Result<(), Error> { >> repo.store(), >> &backup_ns, >> &backup_dir, >> - true, >> + log_enabled!(Level::Debug) || log_enabled!(Level::Trace), >> ) >> .await?; >> >> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs >> index d9e7b899..f89823c2 100644 >> --- a/proxmox-backup-client/src/main.rs >> +++ b/proxmox-backup-client/src/main.rs >> @@ -7,6 +7,7 @@ use std::task::Context; >> >> use anyhow::{bail, format_err, Error}; >> use futures::stream::{StreamExt, TryStreamExt}; >> +use log::{log_enabled, Level}; >> use serde::Deserialize; >> use serde_json::{json, Value}; >> use tokio::sync::mpsc; >> @@ -1300,7 +1301,7 @@ async fn restore( >> repo.store(), >> &ns, >> &backup_dir, >> - true, >> + log_enabled!(Level::Trace) || log_enabled!(Level::Debug), >> ) >> .await?; >> >> diff --git a/proxmox-backup-client/src/mount.rs b/proxmox-backup-client/src/mount.rs >> index 242556d0..aacd4362 100644 >> --- a/proxmox-backup-client/src/mount.rs >> +++ b/proxmox-backup-client/src/mount.rs >> @@ -9,6 +9,7 @@ use anyhow::{bail, format_err, Error}; >> use futures::future::FutureExt; >> use futures::select; >> use futures::stream::{StreamExt, TryStreamExt}; >> +use log::{log_enabled, Level}; >> use nix::unistd::{fork, ForkResult}; >> use serde_json::Value; >> use tokio::signal::unix::{signal, SignalKind}; >> @@ -239,7 +240,7 @@ async fn mount_do(param: Value, pipe: Option) -> Result { >> repo.store(), >> &backup_ns, >> &backup_dir, >> - true, >> + log_enabled!(Level::Debug) || log_enabled!(Level::Trace), >> ) >> .await?; >> >> diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs >> index 9c74a476..96c2a802 100644 >> --- a/proxmox-file-restore/src/main.rs >> +++ b/proxmox-file-restore/src/main.rs >> @@ -5,6 +5,7 @@ use std::sync::Arc; >> >> use anyhow::{bail, format_err, Error}; >> use futures::StreamExt; >> +use log::{log_enabled, Level}; >> use serde_json::{json, Value}; >> use tokio::io::AsyncWriteExt; >> >> @@ -112,7 +113,7 @@ async fn list_files( >> repo.store(), >> &namespace, >> &snapshot, >> - true, >> + log_enabled!(Level::Debug) || log_enabled!(Level::Trace), >> ) >> .await?; >> >> diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs >> index 9924fb7b..c4cf70ba 100644 >> --- a/src/bin/proxmox_backup_debug/diff.rs >> +++ b/src/bin/proxmox_backup_debug/diff.rs >> @@ -9,6 +9,7 @@ use anyhow::{bail, Context as AnyhowContext, Error}; >> use futures::future::BoxFuture; >> use futures::FutureExt; >> >> +use log::{log_enabled, Level}; >> use proxmox_human_byte::HumanByte; >> use proxmox_router::cli::{CliCommand, CliCommandMap, CommandLineInterface}; >> use proxmox_schema::api; >> @@ -299,7 +300,7 @@ async fn create_backup_reader( >> params.repo.store(), >> ¶ms.namespace, >> &backup_dir, >> - false, >> + log_enabled!(Level::Trace) || log_enabled!(Level::Debug), >> ) >> .await?; >> Ok(backup_reader) >> diff --git a/src/server/pull.rs b/src/server/pull.rs >> index a973a10e..bbaa9681 100644 >> --- a/src/server/pull.rs >> +++ b/src/server/pull.rs >> @@ -8,6 +8,7 @@ use std::time::SystemTime; >> >> use anyhow::{bail, format_err, Error}; >> use http::StatusCode; >> +use log::{log_enabled, Level}; >> use pbs_config::CachedUserInfo; >> use serde_json::json; >> >> @@ -743,7 +744,7 @@ async fn pull_group( >> params.source.store(), >> &remote_ns, >> &snapshot, >> - true, >> + log_enabled!(Level::Debug) || log_enabled!(Level::Trace), >> ) >> .await?; >> >> -- >> 2.39.2