From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 05BD2B4264 for ; Fri, 1 Dec 2023 10:56:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D971214C08 for ; Fri, 1 Dec 2023 10:56:49 +0100 (CET) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 1 Dec 2023 10:56:49 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 24CF24356D for ; Fri, 1 Dec 2023 10:56:48 +0100 (CET) Message-ID: <48fb398e-d7d4-4e7b-a313-057953b82aca@proxmox.com> Date: Fri, 1 Dec 2023 10:56:47 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: de-AT, en-US To: Proxmox Backup Server development discussion , Gabriel Goller References: <20231103112849.71953-1-g.goller@proxmox.com> <20231103112849.71953-2-g.goller@proxmox.com> From: Lukas Wagner Cc: Wolfgang Bumiller In-Reply-To: <20231103112849.71953-2-g.goller@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.008 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [lib.rs] Subject: Re: [pbs-devel] [PATCH proxmox 1/2] proxmox-log: added tracing infra 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: , X-List-Received-Date: Fri, 01 Dec 2023 09:56:50 -0000 Hello, I'm currently evaluating how this crate could be used from the proxmox-perl-rs bindings. The status quo there is that we initialize an envlogger, but that leads to the problem that logs are only visible for tasks, since envlogger does log to stdout (or was it stderr?). To differentiate between 'daemon context' and 'fork worker context' we obviously cannot rely on a tokio task-local var there. Maybe it would make sense to have an alternative mode (gated via feature-flags) with a static flag, which is then set in RestEnvironment::fork_worker via a setter? The dependency on tokio should then also be feature-gated, since we don't want to have to pull in tokio as a dep for proxmox-perl-rs yet. @Wolfgang what do you think about this? On 11/3/23 12:28, Gabriel Goller wrote: > diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs > new file mode 100644 > index 0000000..15fa22d > --- /dev/null > +++ b/proxmox-log/src/lib.rs > @@ -0,0 +1,34 @@ > +use std::{cell::RefCell, env}; > +use syslog_tasklog_layer::SyslogAndTasklogLayer; > +use tracing_log::{AsLog, LogTracer}; > +use tracing_subscriber::filter::LevelFilter; > +use tracing_subscriber::prelude::*; > + > +mod file_logger; > +pub use file_logger::{FileLogOptions, FileLogger}; > + > +mod syslog_tasklog_layer; > + > +tokio::task_local! { > + pub static LOGGER: RefCell; > + pub static WARN_COUNTER: RefCell; > +} > + > +pub fn init_logger( > + env_var_name: &str, > + default_log_level: LevelFilter, > + application_name: &str, > +) -> Result<(), anyhow::Error> { > + let mut log_level = default_log_level; > + if let Ok(v) = env::var(env_var_name) { > + if let Ok(l) = v.parse::() { > + log_level = l; > + } > + } > + let registry = tracing_subscriber::registry() > + .with(SyslogAndTasklogLayer::new(application_name.to_string()).with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > diff --git a/proxmox-log/src/syslog_tasklog_layer.rs b/proxmox-log/src/syslog_tasklog_layer.rs > new file mode 100644 > index 0000000..344a514 > --- /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) > + .map_err(|e| { > + anyhow::Error::new(std::io::Error::new( > + std::io::ErrorKind::Other, > + e.description(), > + )) > + }) > + .unwrap(); > + > + let logger = Arc::new(Mutex::new(logger)); > + > + Self { > + syslog_logger: logger, > + } > + } > +} > + > +impl Layer for SyslogAndTasklogLayer { > + fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { > + let mut buf = String::new(); > + event.record(&mut EventVisitor::new(&mut buf)); > + let level = event.metadata().level(); > + > + let result = LOGGER.try_with(|logger| { > + log_to_file(&mut logger.borrow_mut(), level, &buf); > + }); > + if result.is_err() || *level == Level::ERROR { ^ Shouldn't this be configurable? Or, alternatively would it make sense of drop this comparison and only rely on the env var (e.g. PBS_LOG)? > + log_to_syslog(&mut self.syslog_logger.lock().unwrap(), level, &buf); > + } > + } > +} > + > +fn log_to_syslog( > + logger: &mut syslog::Logger, > + level: &Level, > + buf: &String, > +) { (...) -- - Lukas