Fix rolling file appender bug (#6266)

* ensure file path passed to the rolling file appender only contains directories * handle edge case * handle edge case * fix based on feedback * Merge branch 'unstable' of https://github.com/sigp/lighthouse into rolling-file-apender-bug * fmt * linting * ensure only bn inits tracing logfile * ensure only bn inits tracing logfile * Merge branch 'unstable' into rolling-file-apender-bug * Get the metadata of `tracing_log_path` instead of `p`, which is a part of the path * Merge pull request #11 from ackintosh/rolling-file-apender-bug-ackintosh Get the metadata of `tracing_log_path` instead of `p` * Merge branch 'unstable' of https://github.com/sigp/lighthouse into rolling-file-apender-bug * fmt
sigp · Oct 3, 2024 · f6d46fd · f6d46fd
1 parent 82faf97
commit f6d46fd
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 16 deletions.
diff --git a/common/logging/src/lib.rs b/common/logging/src/lib.rs
@@ -217,6 +217,19 @@ impl TimeLatch {
 }
 
 pub fn create_tracing_layer(base_tracing_log_path: PathBuf) {
+    let mut tracing_log_path = PathBuf::new();
+
+    // Ensure that `tracing_log_path` only contains directories.
+    for p in base_tracing_log_path.iter() {
+        tracing_log_path = tracing_log_path.join(p);
+        if let Ok(metadata) = tracing_log_path.metadata() {
+            if !metadata.is_dir() {
+                tracing_log_path.pop();
+                break;
+            }
+        }
+    }
+
     let filter_layer = match tracing_subscriber::EnvFilter::try_from_default_env()
         .or_else(|_| tracing_subscriber::EnvFilter::try_new("warn"))
     {
@@ -232,7 +245,7 @@ pub fn create_tracing_layer(base_tracing_log_path: PathBuf) {
         .max_log_files(2)
         .filename_prefix("libp2p")
         .filename_suffix("log")
-        .build(base_tracing_log_path.clone())
+        .build(tracing_log_path.clone())
     else {
         eprintln!("Failed to initialize libp2p rolling file appender");
         return;
@@ -243,7 +256,7 @@ pub fn create_tracing_layer(base_tracing_log_path: PathBuf) {
         .max_log_files(2)
         .filename_prefix("discv5")
         .filename_suffix("log")
-        .build(base_tracing_log_path.clone())
+        .build(tracing_log_path)
     else {
         eprintln!("Failed to initialize discv5 rolling file appender");
         return;

diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs
@@ -626,20 +626,6 @@ fn run<E: EthSpec>(
         }));
     }
 
-    let mut tracing_log_path: Option<PathBuf> = clap_utils::parse_optional(matches, "logfile")?;
-
-    if tracing_log_path.is_none() {
-        tracing_log_path = Some(
-            parse_path_or_default(matches, "datadir")?
-                .join(DEFAULT_BEACON_NODE_DIR)
-                .join("logs"),
-        )
-    }
-
-    let path = tracing_log_path.clone().unwrap();
-
-    logging::create_tracing_layer(path);
-
     // Allow Prometheus to export the time at which the process was started.
     metrics::expose_process_start_time(&log);
 
@@ -724,6 +710,21 @@ fn run<E: EthSpec>(
                 return Ok(());
             }
 
+            let mut tracing_log_path: Option<PathBuf> =
+                clap_utils::parse_optional(matches, "logfile")?;
+
+            if tracing_log_path.is_none() {
+                tracing_log_path = Some(
+                    parse_path_or_default(matches, "datadir")?
+                        .join(DEFAULT_BEACON_NODE_DIR)
+                        .join("logs"),
+                )
+            }
+
+            let path = tracing_log_path.clone().unwrap();
+
+            logging::create_tracing_layer(path);
+
             executor.clone().spawn(
                 async move {
                     if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await {