Re: [pbs-devel] [PATCH proxmox-backup] fix #4646: Set BackupReader debug to current loglevel

[Gabriel Goller <g.goller@proxmox.com>]

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(),
>>           &params.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