From: "Thomas Ellmenreich" <t.ellmenreich@proxmox.com>
To: "Gabriel Goller" <g.goller@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox 1/6] log: replace per layer filtering by one global filter
Date: Tue, 13 Oct 2026 08:17:42 +0200 [thread overview]
Message-ID: <DM3HPDBL8GGO.2ODIP0FC63M3H@proxmox.com> (raw)
In-Reply-To: <aszdC1RNj1vq1HNx@luna.proxmox.com>
On Mon Oct 12, 2026 at 3:20 PM CEST, Gabriel Goller wrote:
>> [snip]
>> @@ -153,10 +135,13 @@ impl Logger {
>> ///
>> /// Also configures the `LogTracer` which will convert all `log` events to tracing events.
>> pub fn init(self) -> Result<(), anyhow::Error> {
>> - let registry = tracing_subscriber::registry().with(self.layer);
>> + let registry = tracing_subscriber::registry()
>> + .with(self.layer)
>> + .with(self.global_log_level);
>> +
>> tracing::subscriber::set_global_default(registry)?;
>>
>> - LogTracer::init_with_filter(self.global_log_level.as_log())?;
>> + LogTracer::init()?;
>
> Hmm any reason why this was changed?
> I'm asking because this means all the log invocations (there aren't many though)
> will build a whole tracing record before being filtered by the level filter.
> When passing a global log level to the LogTracer, all the levels below will be
> optimized away.
>
> I'm a bit afraid that there is a crate that has a log::debug! statement in the
> hot-path in a loop somewhere -- that would mean we create a whole tracing record
> and discard it on every debug! :)
>
> Note sure if this is a big issue though -- we have very few log invocations
> anyway and other crates shouldn't be such a big issue.
Thanks for pointing this out!
I removed it thinking that it was completely unnecessary, but what you
mentioned would indeed be interesting. My main reason for removing it was
to avoid duplicate filter addition, but I now realise that tracing_log
would not have respected the tracing_subscriber::filter::EnvFilter so
easily. I will check what the best approach here could be and then post
the next version.
Again, thanks for pointing this out ;)
>
>> Ok(())
>> }
>> }
>> --
next prev parent reply other threads:[~2026-10-13 6:17 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-07 7:45 [PATCH datacenter-manager/proxmox 0/6] log: allow finegrained control logging levels Thomas Ellmenreich
2026-10-07 7:45 ` [PATCH proxmox 1/6] log: replace per layer filtering by one global filter Thomas Ellmenreich
2026-10-12 12:16 ` Lukas Wagner
2026-10-12 13:20 ` Gabriel Goller
2026-10-13 6:17 ` Thomas Ellmenreich [this message]
2026-10-07 7:45 ` [PATCH proxmox 2/6] fix #6081: log: replace simple level filter with env filter Thomas Ellmenreich
2026-10-12 12:16 ` Lukas Wagner
2026-10-07 7:45 ` [PATCH proxmox 3/6] log: cleanup of logger components Thomas Ellmenreich
2026-10-12 12:16 ` Lukas Wagner
2026-10-07 7:45 ` [PATCH proxmox 4/6] log: add tests to the logger Thomas Ellmenreich
2026-10-12 12:16 ` Lukas Wagner
2026-10-07 7:45 ` [PATCH datacenter-manager 5/6] api: set REST server debug level based on actual log level Thomas Ellmenreich
2026-10-12 12:16 ` Lukas Wagner
2026-10-07 7:45 ` [PATCH datacenter-manager 6/6] api: update logging variable Thomas Ellmenreich
2026-10-12 12:15 ` 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=DM3HPDBL8GGO.2ODIP0FC63M3H@proxmox.com \
--to=t.ellmenreich@proxmox.com \
--cc=g.goller@proxmox.com \
--cc=pdm-devel@lists.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