Ensure logfile respects the validators-dir CLI flag (#3003)

## Issue Addressed

Closes #2990 

## Proposed Changes

Add a check to see if the `--validators-dir` CLI flag is set and if so store validator logs into it.
Ensure that if the log directory cannot be created, emit a `WARN` and disable file logging rather than panicking. 

## Additional Info

Panics associated with logfiles can still occur in these scenarios:
1. The `$datadir/validators/logs` directory already exists with the wrong permissions (or was changed after creation).
1. The logfile already exists with the wrong permissions (or was changed after creation).
> These panics are cosmetic only since only the logfile thread panics. Following the panics, LH will continue to function as normal. 

I believe this is due to the use of [`slog::Fuse`](https://docs.rs/slog/latest/slog/struct.Fuse.html) when initializing the logger.
I'm not sure if there a better way of handling logfile errors?
I think ideally, rather than panicking, we would emit a `WARN` to the stdout logger with the panic reason, then exit the logfile thread gracefully.
This commit is contained in:
Mac L 2022-02-24 00:31:35 +00:00
parent 696de58141
commit c1df5d29cb
2 changed files with 31 additions and 10 deletions

View File

@ -182,7 +182,21 @@ impl<E: EthSpec> EnvironmentBuilder<E> {
// Create the necessary directories for the correct service and network. // Create the necessary directories for the correct service and network.
if !dir.exists() { if !dir.exists() {
create_dir_all(dir).map_err(|e| format!("Unable to create directory: {:?}", e))?; let res = create_dir_all(dir);
// If the directories cannot be created, warn and disable the logger.
match res {
Ok(_) => (),
Err(e) => {
let log = stdout_logger;
warn!(
log,
"Background file logging is disabled";
"error" => e);
self.log = Some(log);
return Ok(self);
}
}
} }
} }

View File

@ -372,21 +372,28 @@ fn run<E: EthSpec>(
// Construct the path to the log file. // Construct the path to the log file.
let mut log_path: Option<PathBuf> = clap_utils::parse_optional(matches, "logfile")?; let mut log_path: Option<PathBuf> = clap_utils::parse_optional(matches, "logfile")?;
if log_path.is_none() { if log_path.is_none() {
log_path = match matches.subcommand_name() { log_path = match matches.subcommand() {
Some("beacon_node") => Some( ("beacon_node", _) => Some(
parse_path_or_default(matches, "datadir")? parse_path_or_default(matches, "datadir")?
.join(DEFAULT_BEACON_NODE_DIR) .join(DEFAULT_BEACON_NODE_DIR)
.join("logs") .join("logs")
.join("beacon") .join("beacon")
.with_extension("log"), .with_extension("log"),
), ),
Some("validator_client") => Some( ("validator_client", Some(vc_matches)) => {
parse_path_or_default(matches, "datadir")? let base_path = if vc_matches.is_present("validators-dir") {
.join(DEFAULT_VALIDATOR_DIR) parse_path_or_default(vc_matches, "validators-dir")?
} else {
parse_path_or_default(matches, "datadir")?.join(DEFAULT_VALIDATOR_DIR)
};
Some(
base_path
.join("logs") .join("logs")
.join("validator") .join("validator")
.with_extension("log"), .with_extension("log"),
), )
}
_ => None, _ => None,
}; };
} }