feat: introduce cleanups

consensus-shipyard · Jul 19, 2024 · 378d72c · 378d72c
1 parent 3d306c1
commit 378d72c
Show file tree

Hide file tree

Showing 11 changed files with 76 additions and 70 deletions.
diff --git a/fendermint/app/src/cmd/debug.rs b/fendermint/app/src/cmd/debug.rs
@@ -25,7 +25,7 @@ cmd! {
 
 cmd! {
   DebugIpcCommands(self) {
-    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default())?;
+    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default());
 
     match self {
         DebugIpcCommands::ExportTopDownEvents(args) => {

diff --git a/fendermint/app/src/cmd/eth.rs b/fendermint/app/src/cmd/eth.rs
@@ -16,7 +16,7 @@ use crate::{
 
 cmd! {
   EthArgs(self, settings: EthSettings) {
-    let _trace_file_guard = set_global_tracing_subscriber(&settings.tracing)?;
+    let _trace_file_guard = set_global_tracing_subscriber(&settings.tracing);
 
     match self.command.clone() {
       EthCommands::Run { ws_url, http_url, connect_retry_delay } => {

diff --git a/fendermint/app/src/cmd/genesis.rs b/fendermint/app/src/cmd/genesis.rs
@@ -24,7 +24,7 @@ use super::key::read_public_key;
 
 cmd! {
   GenesisArgs(self) {
-    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default())?;
+    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default());
     let genesis_file = self.genesis_file.clone();
 
     match &self.command {

diff --git a/fendermint/app/src/cmd/key.rs b/fendermint/app/src/cmd/key.rs
@@ -23,7 +23,7 @@ use crate::{
 
 cmd! {
     KeyArgs(self) {
-        let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default())?;
+        let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default());
 
         match &self.command {
             KeyCommands::Gen(args) => args.exec(()).await,

diff --git a/fendermint/app/src/cmd/materializer.rs b/fendermint/app/src/cmd/materializer.rs
@@ -23,7 +23,7 @@ use super::key::{read_secret_key, read_secret_key_hex};
 
 cmd! {
   MaterializerArgs(self) {
-    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default())?;
+    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default());
 
     let data_dir = expand_tilde(&self.data_dir);
     let dm = || DockerMaterializer::new(&data_dir, self.seed).map(|m| m.with_policy(DropPolicy::PERSISTENT));

diff --git a/fendermint/app/src/cmd/mod.rs b/fendermint/app/src/cmd/mod.rs
@@ -10,7 +10,7 @@ use crate::{
 use anyhow::{anyhow, Context};
 use async_trait::async_trait;
 
-use ipc_observability::traces::create_subscriber;
+use ipc_observability::traces::create_temporary_subscriber;
 use tracing::subscriber;
 
 pub mod config;
@@ -84,7 +84,7 @@ fn settings(opts: &Options) -> anyhow::Result<Settings> {
         d => d,
     };
 
-    subscriber::with_default(create_subscriber(), || {
+    subscriber::with_default(create_temporary_subscriber(), || {
         tracing::info!(
             path = config_dir.to_string_lossy().into_owned(),
             "reading configuration"

diff --git a/fendermint/app/src/cmd/rpc.rs b/fendermint/app/src/cmd/rpc.rs
@@ -42,7 +42,7 @@ use super::key::read_secret_key;
 
 cmd! {
   RpcArgs(self) {
-    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default())?;
+    let _trace_file_guard = set_global_tracing_subscriber(&TracesSettings::default());
 
     let client = FendermintClient::new_http(self.url.clone(), self.proxy_url.clone())?;
     match self.command.clone() {

diff --git a/fendermint/app/src/cmd/run.rs b/fendermint/app/src/cmd/run.rs
@@ -43,7 +43,7 @@ use ipc_observability::traces::set_global_tracing_subscriber;
 
 cmd! {
   RunArgs(self, settings) {
-    let _trace_file_guard = set_global_tracing_subscriber(&settings.tracing)?;
+    let _trace_file_guard = set_global_tracing_subscriber(&settings.tracing);
 
     run(settings).await
   }

diff --git a/fendermint/app/src/main.rs b/fendermint/app/src/main.rs
@@ -4,7 +4,7 @@
 pub use fendermint_app_options as options;
 pub use fendermint_app_settings as settings;
 
-use ipc_observability::traces::create_subscriber;
+use ipc_observability::traces::create_temporary_subscriber;
 use tracing::subscriber;
 
 mod cmd;
@@ -20,12 +20,12 @@ fn init_panic_handler() {
         // let stacktrace = std::backtrace::Backtrace::capture();
         let stacktrace = std::backtrace::Backtrace::force_capture();
 
-        subscriber::with_default(create_subscriber(), || {
+        subscriber::with_default(create_temporary_subscriber(), || {
             tracing::error!(
-                "panic occurred: {error:?}\n{stacktrace}",
-                error = info,
-                stacktrace = stacktrace
-            )
+                stacktrace = stacktrace.to_string(),
+                info = info.to_string(),
+                "panicking"
+            );
         });
 
         // We could exit the application if any of the background tokio tasks panic.
@@ -41,7 +41,7 @@ async fn main() {
     init_panic_handler();
 
     if let Err(e) = cmd::exec(&opts).await {
-        subscriber::with_default(create_subscriber(), || {
+        subscriber::with_default(create_temporary_subscriber(), || {
             tracing::error!("failed to execute {:?}: {e:?}", opts)
         });
         std::process::exit(fendermint_app::AppExitCode::UnknownError as i32);

diff --git a/ipc/observability/src/traces.rs b/ipc/observability/src/traces.rs
@@ -6,74 +6,63 @@ use tracing::Level;
 pub use tracing_appender::non_blocking;
 pub use tracing_appender::non_blocking::WorkerGuard;
 use tracing_appender::rolling::RollingFileAppender;
-use tracing_subscriber::{fmt, fmt::Subscriber, layer::SubscriberExt, Layer};
+use tracing_subscriber::{fmt, fmt::Subscriber, layer::SubscriberExt, Layer, LogLevel};
 
-use crate::traces_settings::{FileLayerSettings, LogLevel, TracesSettings};
+use crate::traces_settings::{FileLayerSettings, TracesSettings};
 use crate::tracing_layers::DomainEventFilterLayer;
-use anyhow::Result;
 
-pub fn create_subscriber() -> Subscriber {
+//
+pub fn create_temporary_subscriber() -> Subscriber {
     tracing_subscriber::FmtSubscriber::builder()
         .with_max_level(Level::TRACE)
         .with_target(false)
         .with_file(true)
         .with_line_number(true)
-        // builds the subscriber.
         .finish()
 }
 
-pub fn set_global_tracing_subscriber(config: &TracesSettings) -> Result<WorkerGuard> {
+pub fn set_global_tracing_subscriber(config: &TracesSettings) -> Option<WorkerGuard> {
+    let console_filter = match config.console {
+        Some(console_settings) => console_settings.level_to_filter(),
+        None => LogLevel::default().to_filter(),
+    };
+
     // log all traces to stderr (reserving stdout for any actual output such as from the CLI commands)
     let console_layer = fmt::layer()
         .with_writer(std::io::stderr)
         .with_target(false)
         .with_file(true)
         .with_line_number(true)
-        .with_filter(
-            config
-                .console
-                .level
+        .with_filter(console_filter);
+
+    let (file_layer, file_guard) = match config.file {
+        Some(file_settings) if file_settings.enabled => {
+            let (non_blocking, file_guard) = non_blocking(create_file_appender(&file_settings));
+
+            let file_layer = fmt::layer()
+                .json()
+                .with_writer(non_blocking)
+                .with_span_events(fmt::format::FmtSpan::CLOSE)
+                .with_target(false)
+                .with_file(true)
+                .with_line_number(true)
+                .with_filter(file_settings.level_to_filter());
+
+            let domains = file_settings
+                .domain_filter
                 .as_ref()
-                .unwrap_or(&LogLevel::default())
-                .to_filter()?,
-        );
+                .map(|v| v.iter().map(|s| s.to_string()).collect());
+
+            let events = file_settings
+                .events_filter
+                .as_ref()
+                .map(|v| v.iter().map(|s| s.to_string()).collect());
+
+            let file_layer = DomainEventFilterLayer::new(domains, events, file_layer);
 
-    let (file_layer, file_guard) = if config.file.enabled {
-        let (non_blocking, file_guard) = non_blocking(create_file_appender(&config.file));
-
-        let file_layer = fmt::layer()
-            .json()
-            .with_writer(non_blocking)
-            .with_span_events(fmt::format::FmtSpan::CLOSE)
-            .with_target(false)
-            .with_file(true)
-            .with_line_number(true)
-            .with_filter(
-                config
-                    .file
-                    .level
-                    .as_ref()
-                    .unwrap_or(&LogLevel::default())
-                    .to_filter()?,
-            );
-
-        let domains = config
-            .file
-            .domain_filter
-            .as_ref()
-            .map(|v| v.iter().map(|s| s.to_string()).collect());
-
-        let events = config
-            .file
-            .events_filter
-            .as_ref()
-            .map(|v| v.iter().map(|s| s.to_string()).collect());
-
-        let file_layer = DomainEventFilterLayer::new(domains, events, file_layer);
-
-        (Some(file_layer), Some(file_guard))
-    } else {
-        (None, None)
+            (Some(file_layer), Some(file_guard))
+        }
+        _ => (None, None),
     };
 
     let registry = tracing_subscriber::registry()
@@ -83,7 +72,7 @@ pub fn set_global_tracing_subscriber(config: &TracesSettings) -> Result<WorkerGu
     tracing::subscriber::set_global_default(registry)
         .expect("Unable to set a global tracing subscriber");
 
-    file_guard.ok_or_else(|| anyhow::anyhow!("file guard not created"))
+    file_guard
 }
 
 fn create_file_appender(settings: &FileLayerSettings) -> RollingFileAppender {

diff --git a/ipc/observability/src/traces_settings.rs b/ipc/observability/src/traces_settings.rs
@@ -77,21 +77,25 @@ impl FromStr for RotationKind {
 #[serde_as]
 #[derive(Debug, Deserialize, Clone, Default)]
 pub struct TracesSettings {
-    pub console: ConsoleLayerSettings,
-    pub file: FileLayerSettings,
+    pub console: Option<ConsoleLayerSettings>,
+    pub file: Option<FileLayerSettings>,
 }
 
 #[serde_as]
 #[derive(Debug, Deserialize, Clone)]
 pub struct ConsoleLayerSettings {
-    pub enabled: bool,
     pub level: Option<LogLevel>,
 }
 
+impl ConsoleLayerSettings {
+    pub fn level_to_filter(&self) -> EnvFilter {
+        level_to_filter(&self.level)
+    }
+}
+
 impl Default for ConsoleLayerSettings {
     fn default() -> Self {
         ConsoleLayerSettings {
-            enabled: true,
             level: Some(LogLevel::default()),
         }
     }
@@ -108,3 +112,16 @@ pub struct FileLayerSettings {
     pub domain_filter: Option<Vec<String>>,
     pub events_filter: Option<Vec<String>>,
 }
+
+impl FileLayerSettings {
+    pub fn level_to_filter(&self) -> EnvFilter {
+        level_to_filter(&self.level)
+    }
+}
+
+fn level_to_filter(level: &Option<LogLevel>) -> EnvFilter {
+    match level {
+        Some(level) => level.to_filter().unwrap_or_default(),
+        None => EnvFilter::default(),
+    }
+}