From 5427664cf4f5d9ad914fb2f8d93445d84d937af3 Mon Sep 17 00:00:00 2001 From: pscott <30843220+pscott@users.noreply.github.com> Date: Mon, 6 Jan 2020 02:26:30 +0100 Subject: [PATCH] Add log-format option to CLI (#744) * Add log-format CLI option * Cargo fmt * Add log format logic for file logging. Add doc * Review comment * Fix compilation errors * Remove Mutex from logger --- lcli/src/main.rs | 2 +- lighthouse/environment/src/lib.rs | 45 +++++++++++++++++++++++------- lighthouse/src/main.rs | 18 ++++++++++-- tests/beacon_chain_sim/src/main.rs | 5 +++- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/lcli/src/main.rs b/lcli/src/main.rs index 93100c01d..2213f89c6 100644 --- a/lcli/src/main.rs +++ b/lcli/src/main.rs @@ -249,7 +249,7 @@ fn run(env_builder: EnvironmentBuilder, matches: &ArgMatches) { let env = env_builder .multi_threaded_tokio_runtime() .expect("should start tokio runtime") - .async_logger("trace") + .async_logger("trace", None) .expect("should start null logger") .build() .expect("should build env"); diff --git a/lighthouse/environment/src/lib.rs b/lighthouse/environment/src/lib.rs index cba77a03f..249a0d28e 100644 --- a/lighthouse/environment/src/lib.rs +++ b/lighthouse/environment/src/lib.rs @@ -15,7 +15,6 @@ use std::cell::RefCell; use std::ffi::OsStr; use std::fs::{rename as FsRename, OpenOptions}; use std::path::PathBuf; -use std::sync::Mutex; use std::time::{SystemTime, UNIX_EPOCH}; use tokio::runtime::{Builder as RuntimeBuilder, Runtime, TaskExecutor}; use types::{EthSpec, InteropEthSpec, MainnetEthSpec, MinimalEthSpec}; @@ -99,12 +98,27 @@ impl EnvironmentBuilder { /// The logger is "async" because it has a dedicated thread that accepts logs and then /// asynchronously flushes them to stdout/files/etc. This means the thread that raised the log /// does not have to wait for the logs to be flushed. - pub fn async_logger(mut self, debug_level: &str) -> Result { - // Build the initial logger. - let decorator = slog_term::TermDecorator::new().build(); - let decorator = logging::AlignedTermDecorator::new(decorator, logging::MAX_MESSAGE_WIDTH); - let drain = slog_term::FullFormat::new(decorator).build().fuse(); - let drain = slog_async::Async::new(drain).build(); + pub fn async_logger( + mut self, + debug_level: &str, + log_format: Option<&str>, + ) -> Result { + // Setting up the initial logger format and building it. + let drain = if let Some(format) = log_format { + match format.to_uppercase().as_str() { + "JSON" => { + let drain = slog_json::Json::default(std::io::stdout()).fuse(); + slog_async::Async::new(drain).build() + } + _ => return Err("Logging format provided is not supported".to_string()), + } + } else { + let decorator = slog_term::TermDecorator::new().build(); + let decorator = + logging::AlignedTermDecorator::new(decorator, logging::MAX_MESSAGE_WIDTH); + let drain = slog_term::FullFormat::new(decorator).build().fuse(); + slog_async::Async::new(drain).build() + }; let drain = match debug_level { "info" => drain.filter_level(Level::Info), @@ -230,7 +244,12 @@ impl Environment { } /// Sets the logger (and all child loggers) to log to a file. - pub fn log_to_json_file(&mut self, path: PathBuf, debug_level: &str) -> Result<(), String> { + pub fn log_to_json_file( + &mut self, + path: PathBuf, + debug_level: &str, + log_format: Option<&str>, + ) -> Result<(), String> { // Creating a backup if the logfile already exists. if path.exists() { let start = SystemTime::now(); @@ -256,8 +275,14 @@ impl Environment { .open(&path) .map_err(|e| format!("Unable to open logfile: {:?}", e))?; - let drain = Mutex::new(slog_json::Json::default(file)).fuse(); - let drain = slog_async::Async::new(drain).build(); + let log_format = log_format.unwrap_or("JSON"); + let drain = match log_format.to_uppercase().as_str() { + "JSON" => { + let drain = slog_json::Json::default(file).fuse(); + slog_async::Async::new(drain).build() + } + _ => return Err("Logging format provided is not supported".to_string()), + }; let drain = match debug_level { "info" => drain.filter_level(Level::Info), diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 6de49ddf0..9b04d015d 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -42,7 +42,17 @@ fn main() { Arg::with_name("logfile") .long("logfile") .value_name("FILE") - .help("File path where output will be written.") + .help( + "File path where output will be written. Default file logging format is JSON.", + ) + .takes_value(true), + ) + .arg( + Arg::with_name("log-format") + .long("log-format") + .value_name("FORMAT") + .help("Specifies the format used for logging.") + .possible_values(&["JSON"]) .takes_value(true), ) .arg( @@ -99,8 +109,10 @@ fn run( .value_of("debug-level") .ok_or_else(|| "Expected --debug-level flag".to_string())?; + let log_format = matches.value_of("log-format"); + let mut environment = environment_builder - .async_logger(debug_level)? + .async_logger(debug_level, log_format)? .multi_threaded_tokio_runtime()? .build()?; @@ -110,7 +122,7 @@ fn run( let path = log_path .parse::() .map_err(|e| format!("Failed to parse log path: {:?}", e))?; - environment.log_to_json_file(path, debug_level)?; + environment.log_to_json_file(path, debug_level, log_format)?; } if std::mem::size_of::() != 8 { diff --git a/tests/beacon_chain_sim/src/main.rs b/tests/beacon_chain_sim/src/main.rs index 99e5414b3..4eea1cd24 100644 --- a/tests/beacon_chain_sim/src/main.rs +++ b/tests/beacon_chain_sim/src/main.rs @@ -41,6 +41,7 @@ fn main() { let nodes = 4; let validators_per_node = 20; let log_level = "debug"; + let log_format = None; let speed_up_factor = 4; let end_after_checks = true; @@ -49,6 +50,7 @@ fn main() { validators_per_node, speed_up_factor, log_level, + log_format, end_after_checks, ) { Ok(()) => println!("Simulation exited successfully"), @@ -64,10 +66,11 @@ fn async_sim( validators_per_node: usize, speed_up_factor: u64, log_level: &str, + log_format: Option<&str>, end_after_checks: bool, ) -> Result<(), String> { let mut env = EnvironmentBuilder::minimal() - .async_logger(log_level)? + .async_logger(log_level, log_format)? .multi_threaded_tokio_runtime()? .build()?;