Skip to content

Commit

Permalink
Add CLI flag to specify the format of logs written to the logfile (#3839
Browse files Browse the repository at this point in the history
)

## Proposed Changes

Decouple the stdout and logfile formats by adding the `--logfile-format` CLI flag.
This behaves identically to the existing `--log-format` flag, but instead will only affect the logs written to the logfile.
The `--log-format` flag will no longer have any effect on the contents of the logfile.

## Additional Info
This avoids being a breaking change by causing `logfile-format` to default to the value of `--log-format` if it is not provided. 
This means that users who were previously relying on being able to use a JSON formatted logfile will be able to continue to use `--log-format JSON`. 

Users who want to use JSON on stdout and default logs in the logfile, will need to pass the following flags: `--log-format JSON --logfile-format DEFAULT`
  • Loading branch information
macladson committed Jan 16, 2023
1 parent 912ea2a commit 6ac1c5b
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 2 deletions.
1 change: 1 addition & 0 deletions lcli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,7 @@ fn run<T: EthSpec>(
debug_level: String::from("trace"),
logfile_debug_level: String::from("trace"),
log_format: None,
logfile_format: None,
log_color: false,
disable_log_timestamp: false,
max_log_size: 0,
Expand Down
4 changes: 3 additions & 1 deletion lighthouse/environment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub struct LoggerConfig {
pub debug_level: String,
pub logfile_debug_level: String,
pub log_format: Option<String>,
pub logfile_format: Option<String>,
pub log_color: bool,
pub disable_log_timestamp: bool,
pub max_log_size: u64,
Expand All @@ -64,6 +65,7 @@ impl Default for LoggerConfig {
debug_level: String::from("info"),
logfile_debug_level: String::from("debug"),
log_format: None,
logfile_format: None,
log_color: false,
disable_log_timestamp: false,
max_log_size: 200,
Expand Down Expand Up @@ -252,7 +254,7 @@ impl<E: EthSpec> EnvironmentBuilder<E> {
let file_logger = FileLoggerBuilder::new(&path)
.level(logfile_level)
.channel_size(LOG_CHANNEL_SIZE)
.format(match config.log_format.as_deref() {
.format(match config.logfile_format.as_deref() {
Some("JSON") => Format::Json,
_ => Format::default(),
})
Expand Down
15 changes: 15 additions & 0 deletions lighthouse/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ fn main() {
.default_value("debug")
.global(true),
)
.arg(
Arg::with_name("logfile-format")
.long("logfile-format")
.value_name("FORMAT")
.help("Specifies the log format used when emitting logs to the logfile.")
.possible_values(&["DEFAULT", "JSON"])
.takes_value(true)
.global(true)
)
.arg(
Arg::with_name("logfile-max-size")
.long("logfile-max-size")
Expand Down Expand Up @@ -402,6 +411,11 @@ fn run<E: EthSpec>(
.value_of("logfile-debug-level")
.ok_or("Expected --logfile-debug-level flag")?;

let logfile_format = matches
.value_of("logfile-format")
// Ensure that `logfile-format` defaults to the value of `log-format`.
.or_else(|| matches.value_of("log-format"));

let logfile_max_size: u64 = matches
.value_of("logfile-max-size")
.ok_or("Expected --logfile-max-size flag")?
Expand Down Expand Up @@ -452,6 +466,7 @@ fn run<E: EthSpec>(
debug_level: String::from(debug_level),
logfile_debug_level: String::from(logfile_debug_level),
log_format: log_format.map(String::from),
logfile_format: logfile_format.map(String::from),
log_color,
disable_log_timestamp,
max_log_size: logfile_max_size * 1_024 * 1_024,
Expand Down
19 changes: 18 additions & 1 deletion lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,24 @@ fn logfile_no_restricted_perms_flag() {
assert!(config.logger_config.is_restricted == false);
});
}

#[test]
fn logfile_format_default() {
CommandLineTest::new()
.run_with_zero_port()
.with_config(|config| assert_eq!(config.logger_config.logfile_format, None));
}
#[test]
fn logfile_format_flag() {
CommandLineTest::new()
.flag("logfile-format", Some("JSON"))
.run_with_zero_port()
.with_config(|config| {
assert_eq!(
config.logger_config.logfile_format,
Some("JSON".to_string())
)
});
}
#[test]
fn sync_eth1_chain_default() {
CommandLineTest::new()
Expand Down
1 change: 1 addition & 0 deletions testing/simulator/src/eth1_sim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
debug_level: String::from("debug"),
logfile_debug_level: String::from("debug"),
log_format: None,
logfile_format: None,
log_color: false,
disable_log_timestamp: false,
max_log_size: 0,
Expand Down
1 change: 1 addition & 0 deletions testing/simulator/src/no_eth1_sim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
debug_level: String::from("debug"),
logfile_debug_level: String::from("debug"),
log_format: None,
logfile_format: None,
log_color: false,
disable_log_timestamp: false,
max_log_size: 0,
Expand Down
1 change: 1 addition & 0 deletions testing/simulator/src/sync_sim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ fn syncing_sim(
debug_level: String::from(log_level),
logfile_debug_level: String::from("debug"),
log_format: log_format.map(String::from),
logfile_format: None,
log_color: false,
disable_log_timestamp: false,
max_log_size: 0,
Expand Down

0 comments on commit 6ac1c5b

Please sign in to comment.