From 073066f923fe5a3cbea455c9d4ea9348473c9694 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Fri, 23 Oct 2020 12:21:36 -0700 Subject: [PATCH 1/2] Make a stderr configuration of the logger module, and use it in tests Sometimes, it isn't acceptable to log on stdout, because a CLI tool may be required to return structured output on stdout. Typically, the stderr channel is reserved for logging in unix, and this is what glog does by default. I'm not sure why we log to stdout. To avoid breaking any of our partners who may be depending on the stdout behavior, this creates a configuration for stderr, but keeps the defaults with the current behavior. --- common/src/logger/loggers/mod.rs | 34 ++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/common/src/logger/loggers/mod.rs b/common/src/logger/loggers/mod.rs index ac149b53d6..a23b6aedd2 100644 --- a/common/src/logger/loggers/mod.rs +++ b/common/src/logger/loggers/mod.rs @@ -43,6 +43,22 @@ fn create_stdout_logger() -> slog::Fuse { .fuse() } +/// Create a basic stderr logger. +fn create_stderr_logger() -> slog::Fuse { + let decorator = slog_term::TermDecorator::new().stderr().build(); + let drain = slog_envlogger::new( + slog_term::FullFormat::new(decorator) + .use_custom_timestamp(custom_timestamp) + .build() + .fuse(), + ); + slog_async::Async::new(drain) + .thread_name("slog-stderr".into()) + .chan_size(STDOUT_CHANNEL_SIZE) + .build() + .fuse() +} + /// Create a GELF (https://docs.graylog.org/en/3.0/pages/gelf.html) logger. fn create_gelf_logger() -> Option> { env::var("MC_LOG_GELF").ok().map(|remote_host_port| { @@ -123,7 +139,13 @@ pub fn create_root_logger() -> Logger { (None, Some(udp_json)) => Some(udp_json), (Some(_), Some(_)) => panic!("MC_LOG_GELF and MC_LOG_UDP_JSON are mutually exclusive!"), }; - let stdout_logger = create_stdout_logger(); + + // Create stdout / stderr sink + let std_logger = if env::var("MC_LOG_STDERR").is_ok() { + create_stderr_logger() + } else { + create_stdout_logger() + }; // Extra context that always gets added to each log message. let extra_kv = o!( @@ -133,12 +155,9 @@ pub fn create_root_logger() -> Logger { // Create root logger. let mut root_logger = if let Some(network_logger) = network_logger { - Logger::root( - slog::Duplicate(stdout_logger, network_logger).fuse(), - extra_kv, - ) + Logger::root(slog::Duplicate(std_logger, network_logger).fuse(), extra_kv) } else { - Logger::root(stdout_logger, extra_kv) + Logger::root(std_logger, extra_kv) }; // Add extra context if it is available. @@ -165,6 +184,9 @@ pub fn create_root_logger() -> Logger { /// Create a logger that is suitable for use during test execution. pub fn create_test_logger(test_name: String) -> Logger { + if env::var("MC_LOG_STDERR").is_err() { + env::set_var("MC_LOG_STDERR", "1"); + } create_root_logger().new(o!( "mc.test_name" => test_name, )) From 409a776bacd16f3e9e6943eae7b79adb7ada6bbd Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Fri, 23 Oct 2020 12:44:59 -0700 Subject: [PATCH 2/2] review comments --- common/src/logger/loggers/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/src/logger/loggers/mod.rs b/common/src/logger/loggers/mod.rs index a23b6aedd2..942d867e57 100644 --- a/common/src/logger/loggers/mod.rs +++ b/common/src/logger/loggers/mod.rs @@ -2,6 +2,7 @@ /// Sets chan_size for stdout, gelf, and UDP loggers const STDOUT_CHANNEL_SIZE: usize = 100_000; +const STDERR_CHANNEL_SIZE: usize = 100_000; const GELF_CHANNEL_SIZE: usize = 100_000; const UDP_CHANNEL_SIZE: usize = 100_000; @@ -54,7 +55,7 @@ fn create_stderr_logger() -> slog::Fuse { ); slog_async::Async::new(drain) .thread_name("slog-stderr".into()) - .chan_size(STDOUT_CHANNEL_SIZE) + .chan_size(STDERR_CHANNEL_SIZE) .build() .fuse() } @@ -141,7 +142,7 @@ pub fn create_root_logger() -> Logger { }; // Create stdout / stderr sink - let std_logger = if env::var("MC_LOG_STDERR").is_ok() { + let std_logger = if env::var("MC_LOG_STDERR") == Ok("1".to_string()) { create_stderr_logger() } else { create_stdout_logger() @@ -184,6 +185,9 @@ pub fn create_root_logger() -> Logger { /// Create a logger that is suitable for use during test execution. pub fn create_test_logger(test_name: String) -> Logger { + // Make it so that tests log to stderr by default. + // This can be overrided by setting MC_LOG_STDERR to 0, + // but that isn't expected to be necessary if env::var("MC_LOG_STDERR").is_err() { env::set_var("MC_LOG_STDERR", "1"); }