[pbs-devel] [RFC proxmox-backup 0/2] Tasklog rewrite with tracing

[Dominik Csapak]

On 10/23/23 11:09, Gabriel Goller wrote:
> On 10/18/23 15:12, Dominik Csapak wrote:
> 
>> hi, a high level comments on this series:
>>
>> you implemented a 'tasklog' = true filter, which is nice, but not filter for the syslog
>> in general we'd want to land things either in the task log (preferred) or in the syslog,
>> not in both (e.g. for some tasks this pollutes the syslog unnecessarily)
>>
> Ok, this should be easy, we just check in the `syslog_layer` if the `task_log` attribute
> is None in the Metadata. Like this we only log to task_log OR syslog.
>> so i'd like to see also a syslog = false (or syslog = true)  and maybe some
>> functionality to enable/disable that for whole block (if that's even possible?)
> As mentioned in the other patch, currently we can't inspect metadata values when enabling/
> disabling a layer :/ .
> What do you mean exactly by 'disabling in a whole block'?


a (theoretical) example:

fn some_function_in_a_worker() {
    ...
    log_to_tasklog("...");
    ...
    if (some_condition) {
       // low level operations+logs that don't belog into task log
       only_log_to_syslog("...");
       ...
       only_log_to_syslog("...");
       ...
    }
}

also having either syslog or tasklog is also not right everywhere, since sometimes
we explicitly want to have it in both the task and syslog (probably?)


> 
> We could enable/disable the filelog layer in specific rust modules (not so useful for us)
> or e.g, create a span on thread/task creation and then let all the logs in that span go to
> the task_log? The problem with that is that we would have to create an specific attribute
> to log to syslog while we are in a worker_task context (e.g. `info!(syslog = true, "to syslog")`)
> and it's not so clear anymore where we write (i.e. I'd have to look at the context to know
> if a `log::info!()` call writes to task_log or syslog).
> 
> IMO having a explicit `tasklog` attribute on every event is good (The name is obviously debatable,
> we could choose something shorter, or create a macro `task_info!` to make it easier to use).

having the 'tasklog' everytime make it harder to use imho, but if it's hidden behind some macro
can be ok.

in general what i'd like to have are three variants of logging:

* task log only
* syslog only
* task log and syslog

while the first and second will get the most use inside and outside a worker context respectively
so that should be most convenient to use (iow. short)

the second and third should also be possible in a worker context, but there it's not so important
that it's short

in my (naive) imagination i would have liked something like this:

info!("foo"); // logs to task log inside worker, syslog outside
info!(syslog = true, "foo"); // logs only to syslog, even in worker context
info!(syslog = true, tasklog = true, "foo"); // log in both task and syslog (if possible)

(the names are just placeholders, not suggestions. also i'm not against using
macros for either like you wrote: task_log!, syslog!, task_and_syslog! (probably not that))

does that make sense? (@Thomas, @Fabian?)