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 091371FF0B7 for ; Fri, 02 Oct 2026 12:59:10 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B739921690; Fri, 02 Oct 2026 12:59:09 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 02 Oct 2026 12:59:07 +0200 Message-Id: Subject: Re: [RFC datacenter-manager/proxmox v2 0/4] log: allow finegrained control logging levels From: "Lukas Wagner" To: "Thomas Ellmenreich" , "Lukas Wagner" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260923115815.211083-1-t.ellmenreich@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790938747200 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.363 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 Message-ID-Hash: AVP7WQUICDEFMGSO45SEI4LGITCC3BLY X-Message-ID-Hash: AVP7WQUICDEFMGSO45SEI4LGITCC3BLY 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 Fri Oct 2, 2026 at 12:00 PM CEST, Thomas Ellmenreich wrote: > On Fri Oct 2, 2026 at 11:44 AM CEST, Lukas Wagner wrote: >> On Fri Oct 2, 2026 at 11:34 AM CEST, Thomas Ellmenreich wrote: >>> Thank you very much for your review. I will address the changes in the >>> next version. I have one question below: >>> >>> On Fri Oct 2, 2026 at 10:10 AM CEST, Lukas Wagner wrote: >>> >>> [snip] >>> >>>>> - Initially, I found case 3. of the different logging cases quite con= fusing and >>>>> would have thought that our 'default_log_level' should apply as def= ault 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. >>> >>> Since you mentioned it, should I add some documentation somewhere >>> explaining how to use these env filters? If so, where would be the best >>> place for it? >> >> Since as I mentioned this is (mostly) a developer-feature, I think addin= g >> a small section about it in the 'dev-docs' (this is a folder in the PDM >> repo) would be a good start. >> >> It of course still could also make sense to mention in the user-facing >> product documentation, but as we have briefly discussed off-list, module >> paths are inherently unstable, so it's somewhat pointless to provide >> examples there. Familiarity with the code structure is required to >> properly use this feature. > > Yes, I totally understand. I was also thinking of adding it to the develo= per > focus documentation for the reasons you mentioned. > >> So for now, I'd just add it as developer documentation. >> >> I've noticed that PDM and PBS use different conventions for the >> environment variable, in PDM we use PROXMOX_DEBUG, while in PBS we use >> PBS_LOG -- maybe that's also something where we should find a common >> style to use. Just thinking out aloud here, no need to touch this as >> part of this series, of course. > > I was actually wondering about this when creating the series. In my mind,= all > log level variables should end with '_LOG', but we can't really replace > 'PROXMOX_DEBUG' since it is already in use. Could we also support a 'PDM_= LOG' > variable alongside it, and then first deprecate and later remove the old = one? > (Not for this series, but just as a thought) I grepped the user-facing documentation and did a quick google search, found no examples of the PROXMOX_DEBUG variable being described, so I *think* we might get away with just changing the variable. The approach you described is of course the 'cleaner' approach, but I guess that would require API changes in proxmox-log, not sure if that is worth it. What we would do is check on startup if the old variable is set, and if that is the case, we show a warning that guides the user to use the new one. A user caring about detailed logging would then hopefully notice this since they check the logs anyway. Probably best to get one or two other opinions on this before we change this. It's not super urgent anyway, this is also something we could do at the next major version (PDM 1.x -> PDM 2.x), then it would simply be a breaking change that is mentioned in the release notes, as usual. > >>> >>> [snip]