From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 8EBD41FF0B7 for ; Fri, 02 Oct 2026 10:10:07 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 94ECB21400; Fri, 02 Oct 2026 10:10:06 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 02 Oct 2026 10:10:02 +0200 Message-Id: Subject: Re: [RFC datacenter-manager/proxmox v2 0/4] log: allow finegrained control logging levels From: "Lukas Wagner" To: "Thomas Ellmenreich" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260923115815.211083-1-t.ellmenreich@proxmox.com> In-Reply-To: <20260923115815.211083-1-t.ellmenreich@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790928602892 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.399 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record WEIRD_PORT 0.001 Uses non-standard port number for HTTP Message-ID-Hash: L4JE72K4EFEZBMD2ZLBUVQBO5FHZV7NK X-Message-ID-Hash: L4JE72K4EFEZBMD2ZLBUVQBO5FHZV7NK X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 envir= onment > 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 la= tter > 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 With the minor comments addressed and the diff otherwise unchanged: Reviewed-by: Lukas Wagner > > Different Log Options > --------------------- > > Combining the different options of the EnvFilter as well as our own defau= lt > log level, we have the following cases for logging: > > 1. Env contains 'simple': > Meaning that the environment variable contains a simple 'info'... Thi= s > 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 ze= ro or > more module specific log levels. As expected the simple level is appl= ied > as a default and then for the specified modules the defined levels ap= ply. > > 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 fo= r 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 th= e > default log level. > > Open Questions > -------------- > > - I had the idea that instead of having all users of the proxmox-log crat= e > provide their own 'default_log_level' we could define that inside of th= e > proxmox-log crate. By doing so, we could have the default change betwee= n > 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 f= or ("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 f= or ("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 confusi= ng and > would have thought that our 'default_log_level' should apply as default= in > that case. Unfortunately, when setting 'default_log_level' and then app= lying > 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] >