public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Thomas Ellmenreich" <t.ellmenreich@proxmox.com>,
	<pdm-devel@lists.proxmox.com>
Subject: Re: [RFC datacenter-manager/proxmox v2 0/4] log: allow finegrained control logging levels
Date: Fri, 02 Oct 2026 10:10:02 +0200	[thread overview]
Message-ID: <DLU77E1N0NKI.2ELPWL0RMIP8Z@proxmox.com> (raw)
In-Reply-To: <20260923115815.211083-1-t.ellmenreich@proxmox.com>

On Wed Sep 23, 2026 at 1:57 PM CEST, Thomas Ellmenreich wrote:
> The core of this change is a simple reinterpretation of the logging environment
> variables that we are already using. Instead of parsing them as a single level
> filter [0], they will now be interpreted as env filters [1]. Since the latter
> can still parse the former, this change is backwards compatible and will not
> break with any logging environment variables that have been set.
>

Thanks for these patches, looking very good so far. I only had very
minor complaints -- see the patch replies for these.

Tested-by: Lukas Wagner <l.wagner@proxmox.com>

With the minor comments addressed and the diff otherwise unchanged:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

>
> Different Log Options
> ---------------------
>
> Combining the different options of the EnvFilter as well as our own default
> log level, we have the following cases for logging:
>
> 1. Env contains 'simple':
>     Meaning that the environment variable contains a simple 'info'... This
>     means that that level is taken as the expected one.
>
> 2. Env contains 'simple', and 'module':
>     Means that the env variable contains a simple log level as well as zero or
>     more module specific log levels. As expected the simple level is applied
>     as a default and then for the specified modules the defined levels apply.
>
> 3. Env only contains 'module':
>     If the env only contains a module (or multiple), that is interpreted by
>     tracing as disabling the default logging and only enabling logging for the
>     module/s
>
> 4. Env is empty:
>     No logs are printed at all, everything is hidden.
>
> 5. Env is not set:
>     This is the only time the default comes into play, by being set as the
>     default log level.
>
> Open Questions
> --------------
>
> - I had the idea that instead of having all users of the proxmox-log crate
>   provide their own 'default_log_level' we could define that inside of the
>   proxmox-log crate. By doing so, we could have the default change between
>   DEBUG and INFO depending on if we are in a normal or release build.

I think keeping the default log level inside the application code is
fine, to me this seems to be inherently application-specific, even if we
are going to use the same default for all rust-based product code for
now.

I'm also not sure if we'd want to automatically switch to DEBUG for
debug builds, there are some crates in our dependency graph that are
*very* noisy at the debug level. E.g.:

Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: pooling idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: pooling idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: reuse idle connection for ("https", xxx.xxx.xxx.xxx:8006)
Oct 02 09:55:10 pdm proxmox-datacenter-api[1451]: connecting to xxx.xxx.xxx.xxx:8007


>
> - Initially, I found case 3. of the different logging cases quite confusing and
>   would have thought that our 'default_log_level' should apply as default in
>   that case. Unfortunately, when setting 'default_log_level' and then applying
>   the EnvFilter string, tracing applies the modules as expected, but then also
>   overrides the default as '', which means off.

Indeed a bit confusing, but since this is (mostly) a
developer/supporter feature, as long at this behavior is documented
somewhere, it should be manageable.

>
> Notes for the Maintainer
> ------------------------
>
> For all tracing backed logging in Proxmox products to support EnvFilters,
> only patches 1 and 2 have to be applied. The 3 Patch only contains tests
> which I mostly used for understanding, but do still test our defaulting
> behaviour.
>
> Patch 4 is just something I noticed and fixed, but is not necessary to
> fix the Bugzilla issue: [2]
>





  parent reply	other threads:[~2026-10-02  8:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 11:57 [RFC datacenter-manager/proxmox v2 0/4] log: allow finegrained control logging levels Thomas Ellmenreich
2026-09-23 11:57 ` [PATCH proxmox v2 1/4] log: replace per layer filtering by one global filter Thomas Ellmenreich
2026-10-02  8:10   ` Lukas Wagner
2026-09-23 11:58 ` [PATCH proxmox v2 2/4] fix #6081: log: replace simple level filter with env filter Thomas Ellmenreich
2026-10-02  8:10   ` Lukas Wagner
2026-09-23 11:58 ` [PATCH proxmox v2 3/4] log: add tests to the logger Thomas Ellmenreich
2026-10-02  8:10   ` Lukas Wagner
2026-09-23 11:58 ` [PATCH datacenter-manager v2 4/4] api: set REST server debug level based on actual log level Thomas Ellmenreich
2026-10-02  8:10 ` Lukas Wagner [this message]
2026-10-02  9:34   ` [RFC datacenter-manager/proxmox v2 0/4] log: allow finegrained control logging levels Thomas Ellmenreich
2026-10-02  9:44     ` Lukas Wagner
2026-10-02 10:00       ` Thomas Ellmenreich
2026-10-02 10:59         ` Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLU77E1N0NKI.2ELPWL0RMIP8Z@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=t.ellmenreich@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal