* [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel @ 2023-08-07 12:57 Gabriel Goller 2023-08-08 10:38 ` Wolfgang Bumiller 0 siblings, 1 reply; 6+ messages in thread From: Gabriel Goller @ 2023-08-07 12:57 UTC (permalink / raw) To: pbs-devel 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 <g.goller@proxmox.com> --- 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<Value, Error> { 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<OwnedFd>) -> Result<Value, Error> { 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 2023-08-07 12:57 [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel Gabriel Goller @ 2023-08-08 10:38 ` Wolfgang Bumiller 2023-08-09 15:06 ` Gabriel Goller 2023-08-11 12:53 ` Gabriel Goller 0 siblings, 2 replies; 6+ messages in thread From: Wolfgang Bumiller @ 2023-08-08 10:38 UTC (permalink / raw) To: Gabriel Goller; +Cc: pbs-devel 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 <g.goller@proxmox.com> > --- > 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<Value, Error> { > 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<OwnedFd>) -> Result<Value, Error> { > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 2023-08-08 10:38 ` Wolfgang Bumiller @ 2023-08-09 15:06 ` Gabriel Goller 2023-08-11 12:53 ` Gabriel Goller 1 sibling, 0 replies; 6+ messages in thread From: Gabriel Goller @ 2023-08-09 15:06 UTC (permalink / raw) To: Wolfgang Bumiller; +Cc: pbs-devel Yes, this controls the log level on the remote side, which is a bit weird (as discussed here [0]). The CLI output locally won't change (we use `env_logger` there and get the loglevel from `PBS_LOG`). [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=4646 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 <g.goller@proxmox.com> >> --- >> 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<Value, Error> { >> 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<OwnedFd>) -> Result<Value, Error> { >> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 2023-08-08 10:38 ` Wolfgang Bumiller 2023-08-09 15:06 ` Gabriel Goller @ 2023-08-11 12:53 ` Gabriel Goller 2023-08-17 12:42 ` Wolfgang Bumiller 1 sibling, 1 reply; 6+ messages in thread From: Gabriel Goller @ 2023-08-11 12:53 UTC (permalink / raw) To: Wolfgang Bumiller; +Cc: pbs-devel 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 <g.goller@proxmox.com> >> --- >> 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<Value, Error> { >> 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<OwnedFd>) -> Result<Value, Error> { >> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 2023-08-11 12:53 ` Gabriel Goller @ 2023-08-17 12:42 ` Wolfgang Bumiller 2023-08-21 11:20 ` Gabriel Goller 0 siblings, 1 reply; 6+ messages in thread From: Wolfgang Bumiller @ 2023-08-17 12:42 UTC (permalink / raw) To: Gabriel Goller; +Cc: pbs-devel On Fri, Aug 11, 2023 at 02:53:17PM +0200, Gabriel Goller wrote: > 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). A while ago we Hannes added `init_cli_logger` to proxmox-router which the CLI binaries call with the `$PBS_LOG` env var. Having the same mechanism for the daemons' log levels would probably make sense. We should probably also call it PBS_LOG then. People could probably temporarily change it via `systemctl set-environment` or more permanently via an `Environment=` systemd service snippet file. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel 2023-08-17 12:42 ` Wolfgang Bumiller @ 2023-08-21 11:20 ` Gabriel Goller 0 siblings, 0 replies; 6+ messages in thread From: Gabriel Goller @ 2023-08-21 11:20 UTC (permalink / raw) To: Wolfgang Bumiller; +Cc: pbs-devel Submitted a patch with the proposed change. On 8/17/23 14:42, Wolfgang Bumiller wrote: > On Fri, Aug 11, 2023 at 02:53:17PM +0200, Gabriel Goller wrote: >> 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). > A while ago we Hannes added `init_cli_logger` to proxmox-router which > the CLI binaries call with the `$PBS_LOG` env var. > Having the same mechanism for the daemons' log levels would probably > make sense. We should probably also call it PBS_LOG then. > People could probably temporarily change it via `systemctl > set-environment` or more permanently via an `Environment=` systemd > service snippet file. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-21 11:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-07 12:57 [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel Gabriel Goller 2023-08-08 10:38 ` Wolfgang Bumiller 2023-08-09 15:06 ` Gabriel Goller 2023-08-11 12:53 ` Gabriel Goller 2023-08-17 12:42 ` Wolfgang Bumiller 2023-08-21 11:20 ` Gabriel Goller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox