From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ronja.mits.lan by ronja.mits.lan with LMTP id SKHfGKKDemZ+QwAAxxbTJA (envelope-from ); Tue, 25 Jun 2024 10:45:22 +0200 Received: from proxmox-new.maurer-it.com (unknown [192.168.2.33]) by ronja.mits.lan (Postfix) with ESMTPS id 49416F6462E; Tue, 25 Jun 2024 10:45:22 +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 2D2C0484FB; Tue, 25 Jun 2024 10:45:22 +0200 (CEST) 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 (4096 bits)) (No client certificate requested) by proxmox-new.maurer-it.com (Proxmox) with ESMTPS; Tue, 25 Jun 2024 10:45:21 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9B573C3D8; Tue, 25 Jun 2024 10:45:20 +0200 (CEST) Date: Tue, 25 Jun 2024 10:44:45 +0200 From: Gabriel Goller To: Lukas Wagner Message-ID: <20240625084445.ippvlffwcdniuav7@luna.proxmox.com> References: <20240613135623.279167-1-g.goller@proxmox.com> <20240613135623.279167-2-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Subject: Re: [pbs-devel] [PATCH proxmox v5 1/4] proxmox-log: add tracing infrastructure X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Cc: Proxmox Backup Server development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods MAILING_LIST_MULTI -2 Multiple indicators imply a widely-seen list manager 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_NONE 0.001 SPF: sender does not publish an SPF Record Thanks for the review! On 24.06.2024 13:10, Lukas Wagner wrote: >Just wondering, have you seen/checked out tracing-journald? >Would that be interesting for us? > >https://docs.rs/tracing-journald/latest/tracing_journald/ I packaged it for debian some time ago. It basically does the same as the current `syslog` lib and custom subscriber, so we should just be able to drop it in. Don't know if that's too much for this series though :). Maybe as a follow-up? What do you think? >> + }; >> + if let Err(err) = self.file.write_all(line.as_bytes()) { >> + // avoid panicking, log methods should not do that >> + // FIXME: or, return result??? >> + log::error!("error writing to log file - {}", err); >Since you use `FileLogger` in `SyslogAndTaskLogger` as a tracing logging layer and >also setup the `tracing-log` bridge to convert log records to tracing events, >wouldn't this lead to recursion? Good catch! >> diff --git a/proxmox-log/src/syslog_tasklog_layer.rs b/proxmox-log/src/syslog_tasklog_layer.rs >> new file mode 100644 >> index 00000000..89437caf >> --- /dev/null >> +++ b/proxmox-log/src/syslog_tasklog_layer.rs >> @@ -0,0 +1,106 @@ >> +use std::fmt::Write as _; >> +use std::sync::Arc; >> +use std::sync::Mutex; >> + >> +use syslog::{Formatter3164, LoggerBackend}; >> +use tracing::field::Field; >> +use tracing::field::Visit; >> +use tracing::Event; >> +use tracing::Level; >> +use tracing::Subscriber; >> +use tracing_subscriber::layer::Context; >> +use tracing_subscriber::Layer; >> + >> +use crate::FileLogger; >> +use crate::LOGGER; >> +use crate::WARN_COUNTER; >> + >> +pub struct SyslogAndTasklogLayer { >> + syslog_logger: Arc>>, >> +} >> + >> +impl SyslogAndTasklogLayer { >> + pub fn new(application_name: String) -> Self { >> + let formatter = Formatter3164 { >> + facility: syslog::Facility::LOG_DAEMON, >> + process: application_name, >> + ..Formatter3164::default() >> + }; >> + >> + // we panic here if we can't initialize the syslogger >> + let logger = syslog::unix(formatter) > >Maybe explain why it is okay to panic in this case? (is it?) I think it's reasonable to not start if we can't write to the syslog. Either way this was done a long time before me, so I didn't change it :) Also incorporated all the other changes! _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel