* [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