[pbs-devel] [PATCH proxmox-backup] log: retrieve `ReaderEnvironment` debug flag from tracing

[Gabriel Goller]

On 16.07.2024 10:21, Christian Ebner wrote:
>one comment inline
>
>On 7/15/24 17:13, Gabriel Goller wrote:
>>@@ -687,12 +688,16 @@ impl BackupEnvironment {
>>      }
>>      pub fn log<S: AsRef<str>>(&self, msg: S) {
>>-        self.worker.log_message(msg);
>>+        info!("{}", msg.as_ref());
>>      }
>>      pub fn debug<S: AsRef<str>>(&self, msg: S) {
>>          if self.debug {
>>-            self.worker.log_message(msg);
>>+            // This is kinda weird, we would like to use debug! here and automatically filter it,
>>+            // but self.debug is set from the client-side and the logs are printed on client and
>>+            // server side. This means that if the client sets the log level to debug, both server
>>+            // and client need to have 'debug' logs printed.
>>+            info!("{}", msg.as_ref());
>
>This is indeed a bit odd: The fact that the client can set the `debug` 
>flag when upgrading to the backup protocol, but cannot and should not 
>control the filter level makes this a bit strange.

Yeah, this is indeed kind of weird.
But I don't even think it's that stupid, because the output will be
printed on both server and client, so the client setting the loglevel is
fine I guess.

>So I would suggest to rename this method to something like 
>`client_debug` (or something better fitting?) 

Hmm, I don't know, the function doesn't only print to the client, it
also prints to the tasklog and thus on the server.

>and instead of calling the `info` macro directly, call `self.log`
>instead if `self.debug` is set. By this, it should at least be a bit
>more clear what is going on and one cannot accidentally loose these
>debug messages as the same codepath is then used for generating the
>event.

Yeah, that's a good idea...


I also though about adding the same stuff to the ReaderEnvironment as it
basically does the same thing.