From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 8729F9C09F
 for <pbs-devel@lists.proxmox.com>; Mon, 23 Oct 2023 11:29:46 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 67BD81179A
 for <pbs-devel@lists.proxmox.com>; Mon, 23 Oct 2023 11:29:46 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pbs-devel@lists.proxmox.com>; Mon, 23 Oct 2023 11:29:45 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9420B444F7;
 Mon, 23 Oct 2023 11:29:45 +0200 (CEST)
Message-ID: <030d3735-02c2-4a6e-860f-e8b6521e7e0e@proxmox.com>
Date: Mon, 23 Oct 2023 11:29:44 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Gabriel Goller <g.goller@proxmox.com>,
 Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>
References: <20231011140102.273423-1-g.goller@proxmox.com>
 <4e0de017-9fbf-4a5e-b735-62b955fe8a8b@proxmox.com>
 <0fd77836-896a-4017-9300-b74be824f346@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>,
 =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
In-Reply-To: <0fd77836-896a-4017-9300-b74be824f346@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.012 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [RFC proxmox-backup 0/2] Tasklog rewrite with
 tracing
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 23 Oct 2023 09:29:46 -0000

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?)