* [pbs-devel] [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel @ 2022-02-04 9:12 Dominik Csapak 2022-02-04 9:12 ` [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule Dominik Csapak 2022-02-04 10:22 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Thomas Lamprecht 0 siblings, 2 replies; 6+ messages in thread From: Dominik Csapak @ 2022-02-04 9:12 UTC (permalink / raw) To: pbs-devel so that we can use 'log::debug' to actually log debug messages Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- without this or something similar, we cannot actually see the messages from 'log::debug' (and we're using that sometimes) src/bin/proxmox-backup-proxy.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs index 9bacac97..73844935 100644 --- a/src/bin/proxmox-backup-proxy.rs +++ b/src/bin/proxmox-backup-proxy.rs @@ -188,18 +188,18 @@ async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<Body> } async fn run() -> Result<(), Error> { + // Note: To debug early connection error use + // PROXMOX_DEBUG=1 ./target/release/proxmox-backup-proxy + let debug = std::env::var("PROXMOX_DEBUG").is_ok(); + if let Err(err) = syslog::init( syslog::Facility::LOG_DAEMON, - log::LevelFilter::Info, + if debug { log::LevelFilter::Debug } else { log::LevelFilter::Info }, Some("proxmox-backup-proxy"), ) { bail!("unable to inititialize syslog - {}", err); } - // Note: To debug early connection error use - // PROXMOX_DEBUG=1 ./target/release/proxmox-backup-proxy - let debug = std::env::var("PROXMOX_DEBUG").is_ok(); - let _ = public_auth_key(); // load with lazy_static let _ = csrf_secret(); // load with lazy_static -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule 2022-02-04 9:12 [pbs-devel] [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Dominik Csapak @ 2022-02-04 9:12 ` Dominik Csapak 2022-02-04 10:05 ` Thomas Lamprecht 2022-02-04 10:22 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Thomas Lamprecht 1 sibling, 1 reply; 6+ messages in thread From: Dominik Csapak @ 2022-02-04 9:12 UTC (permalink / raw) To: pbs-devel Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- optional, at least one user in the forum has a problem with traffic control, this could help debug that in the future... src/cached_traffic_control.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs index 2f077d36..cd13bc1b 100644 --- a/src/cached_traffic_control.rs +++ b/src/cached_traffic_control.rs @@ -342,6 +342,7 @@ impl TrafficControlCache { Some((rule, _)) => { match self.limiter_map.get(&rule.config.name) { Some((read_limiter, write_limiter)) => { + log::debug!("found traffic control rule for {:?} : {}", peer_ip, &rule.config.name); (&rule.config.name, read_limiter.clone(), write_limiter.clone()) } None => ("", None, None), // should never happen -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule 2022-02-04 9:12 ` [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule Dominik Csapak @ 2022-02-04 10:05 ` Thomas Lamprecht 2022-02-04 10:09 ` Dominik Csapak 0 siblings, 1 reply; 6+ messages in thread From: Thomas Lamprecht @ 2022-02-04 10:05 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Dominik Csapak On 04.02.22 10:12, Dominik Csapak wrote: > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > optional, at least one user in the forum has a problem with traffic > control, this could help debug that in the future... Above needs to be in the commit message and actually linking to the relevant forum thread. in general sure, but I dislike the direction of the approach, as its again moving in the same direction as e.g., pmxcfs, a single boolean flag for all or nothing, which in practice will soon mean that's rather useless as its spamming so much stuff that relevant things get drowned even for experienced users. More fine grained approach it both, the verbosity and the topic axis would be much nicer, especially the latter as then a user could only enable traffic-control related logs. But just mentioning as this is a major pain point in pmxcfs that I get "hurt" by frequently.. > src/cached_traffic_control.rs | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs > index 2f077d36..cd13bc1b 100644 > --- a/src/cached_traffic_control.rs > +++ b/src/cached_traffic_control.rs > @@ -342,6 +342,7 @@ impl TrafficControlCache { > Some((rule, _)) => { > match self.limiter_map.get(&rule.config.name) { > Some((read_limiter, write_limiter)) => { > + log::debug!("found traffic control rule for {:?} : {}", peer_ip, &rule.config.name); > (&rule.config.name, read_limiter.clone(), write_limiter.clone()) > } > None => ("", None, None), // should never happen ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule 2022-02-04 10:05 ` Thomas Lamprecht @ 2022-02-04 10:09 ` Dominik Csapak 2022-02-04 10:21 ` Thomas Lamprecht 0 siblings, 1 reply; 6+ messages in thread From: Dominik Csapak @ 2022-02-04 10:09 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox Backup Server development discussion On 2/4/22 11:05, Thomas Lamprecht wrote: > On 04.02.22 10:12, Dominik Csapak wrote: >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> optional, at least one user in the forum has a problem with traffic >> control, this could help debug that in the future... > > Above needs to be in the commit message and actually linking to the relevant > forum thread. > > in general sure, but I dislike the direction of the approach, as its again > moving in the same direction as e.g., pmxcfs, a single boolean flag for all > or nothing, which in practice will soon mean that's rather useless as its > spamming so much stuff that relevant things get drowned even for experienced > users. > > More fine grained approach it both, the verbosity and the topic axis would > be much nicer, especially the latter as then a user could only enable > traffic-control related logs. > > But just mentioning as this is a major pain point in pmxcfs that I get "hurt" > by frequently.. makes total sense. did you already imagine any way to enable this? could we simply have some 'sections' (like tc,connections,etc.) and enable them like this: PROXMOX_DEBUG=tc=debug,conn=info,foo=none or should we avoid the environment variable at all, and put it in the node config? > >> src/cached_traffic_control.rs | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs >> index 2f077d36..cd13bc1b 100644 >> --- a/src/cached_traffic_control.rs >> +++ b/src/cached_traffic_control.rs >> @@ -342,6 +342,7 @@ impl TrafficControlCache { >> Some((rule, _)) => { >> match self.limiter_map.get(&rule.config.name) { >> Some((read_limiter, write_limiter)) => { >> + log::debug!("found traffic control rule for {:?} : {}", peer_ip, &rule.config.name); >> (&rule.config.name, read_limiter.clone(), write_limiter.clone()) >> } >> None => ("", None, None), // should never happen > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule 2022-02-04 10:09 ` Dominik Csapak @ 2022-02-04 10:21 ` Thomas Lamprecht 0 siblings, 0 replies; 6+ messages in thread From: Thomas Lamprecht @ 2022-02-04 10:21 UTC (permalink / raw) To: Dominik Csapak, Proxmox Backup Server development discussion On 04.02.22 11:09, Dominik Csapak wrote: > On 2/4/22 11:05, Thomas Lamprecht wrote: >> On 04.02.22 10:12, Dominik Csapak wrote: >>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>> --- >>> optional, at least one user in the forum has a problem with traffic >>> control, this could help debug that in the future... >> >> Above needs to be in the commit message and actually linking to the relevant >> forum thread. >> >> in general sure, but I dislike the direction of the approach, as its again >> moving in the same direction as e.g., pmxcfs, a single boolean flag for all >> or nothing, which in practice will soon mean that's rather useless as its >> spamming so much stuff that relevant things get drowned even for experienced >> users. >> >> More fine grained approach it both, the verbosity and the topic axis would >> be much nicer, especially the latter as then a user could only enable >> traffic-control related logs. >> >> But just mentioning as this is a major pain point in pmxcfs that I get "hurt" >> by frequently.. > > makes total sense. did you already imagine any way to enable this? I haven't thought out any specifics for our rust env yet, fwiw log provides a target and the module-path in the Record metadata, either or both could be used for employing some filtering. FWIW, we already depend transitively on the `tracing` crate, which could be also leveraged for a use case like this. Maintaining that is some work, IME having, somewhat thought out, tracing integrated in a system can make debugging and the like multiple orders of magnitude easier and faster. > could we simply have some 'sections' (like tc,connections,etc.) > and enable them like this: I would imagine that sections could be seen as what I called topics, so yes, something like that. > PROXMOX_DEBUG=tc=debug,conn=info,foo=none > > or should we avoid the environment variable at all, and put it in > the node config? > would be an option, but IMO not too relevant where the current level comes from, I'd figure that once we decide on a basic direction of how/what the setting source should be rather on the easy side. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel 2022-02-04 9:12 [pbs-devel] [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Dominik Csapak 2022-02-04 9:12 ` [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule Dominik Csapak @ 2022-02-04 10:22 ` Thomas Lamprecht 1 sibling, 0 replies; 6+ messages in thread From: Thomas Lamprecht @ 2022-02-04 10:22 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Dominik Csapak On 04.02.22 10:12, Dominik Csapak wrote: > so that we can use 'log::debug' to actually log debug messages > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > without this or something similar, we cannot actually see the messages > from 'log::debug' (and we're using that sometimes) > > src/bin/proxmox-backup-proxy.rs | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > applied, thanks! This is (for now) unrelated to the better debug-logging experience I replied at patch 2/2. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-04 10:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-04 9:12 [pbs-devel] [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Dominik Csapak 2022-02-04 9:12 ` [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule Dominik Csapak 2022-02-04 10:05 ` Thomas Lamprecht 2022-02-04 10:09 ` Dominik Csapak 2022-02-04 10:21 ` Thomas Lamprecht 2022-02-04 10:22 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] use PROXMOX_DEBUG env variable to control loglevel Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox