Re: [pbs-devel] [RFC proxmox v2 2/2] proxmox-log: added tracing infra

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Fri, Nov 03, 2023 at 11:27:11AM +0100, Gabriel Goller wrote:
> On 11/3/23 10:52, Wolfgang Bumiller wrote:
> 
> > [..]
> > > To be honest, I don't really get the advantage of a Future-Wrapper?
> > > Why not just have a:
> > > ```rust
> > > tokio::spawn(async move {
> > ^ this async{} block up here should not be necessary, `.scope()` returns
> > a future.
> > 
> > >      LOGGER.scope(logger, async move {
> > >          // worker logic
> > >      })
> > > })
> > > ```
> > > and
> > > ```rust
> > > let _child = std::thread::Builder::new()
> > >      .name(upid.clone())
> > >      .spawn(move || {
> > >          LOGGER.sync_scope(logger, || {
> > >              // worker logic
> > >          })
> > >      });
> > It's essentially the same, so that's fine too.
> > I'm guessing you intend to skip the boolean part then and only go with
> > whether a logger is set?
> Yes, kinda.
> I think I'll check if a FileLogger exists, then print to syslog or tasklog.
> The thing is I don't know if I should keep it simple and merge everything
> in a single layer, or if I should do the check in the Filters (of which
> there are
> two, one for each layer).
> If we merge it into a single layer, it's easy and fast, but it's not that
> clean
> anymore, we can't reuse the single layers etc..

But do we even want to right now?
If so we can still split them later.

> If we keep them separated, we have to check twice if the FileLogger exists
> (in each filter)

I'm confused, I thought one layer was only for syslog, why would we
check the file logger there?

> (it's probably not so expensive, but still) but we'll keep the FileLogLayer
> and the
> SyslogLayer separated and clean.
> What do you think?

I'd lean towards a single layer, mainly because I don't currently see
the immediate use of multiple ones.

I mean, we can still change things later if we need to.
But IMO the current set of requirements should be covered fine by one
layer.