Add CLI argument to disable log color output (#7795)

Author: bkchr

client/cli/src/config.rs

@@ -22,7 +22,7 @@ use crate::arg_enums::Database;
 use crate::error::Result;
 use crate::{
 	init_logger, DatabaseParams, ImportParams, KeystoreParams, NetworkParams, NodeKeyParams,
-	OffchainWorkerParams, PruningParams, SharedParams, SubstrateCli,
+	OffchainWorkerParams, PruningParams, SharedParams, SubstrateCli, InitLoggerParams,
 };
 use log::warn;
 use names::{Generator, Name};
@@ -538,6 +538,11 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		Ok(self.shared_params().is_log_filter_reloading_disabled())
 	}
 
+	/// Should the log color output be disabled?
+	fn disable_log_color(&self) -> Result<bool> {
+		Ok(self.shared_params().disable_log_color())
+	}
+
 	/// Initialize substrate. This must be done only once per process.
 	///
 	/// This method:
@@ -550,15 +555,17 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		let tracing_receiver = self.tracing_receiver()?;
 		let tracing_targets = self.tracing_targets()?;
 		let disable_log_reloading = self.is_log_filter_reloading_disabled()?;
+		let disable_log_color = self.disable_log_color()?;
 
 		sp_panic_handler::set(&C::support_url(), &C::impl_version());
 
-		init_logger(
-			&logger_pattern,
+		init_logger(InitLoggerParams {
+			pattern: logger_pattern,
 			tracing_receiver,
 			tracing_targets,
 			disable_log_reloading,
-		)?;
+			disable_log_color,
+		})?;
 
 		if let Some(new_limit) = fdlimit::raise_fd_limit() {
 			if new_limit < RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT {

client/cli/src/lib.rs

@@ -237,30 +237,50 @@ pub trait SubstrateCli: Sized {
 	fn native_runtime_version(chain_spec: &Box<dyn ChainSpec>) -> &'static RuntimeVersion;
 }
 
+/// The parameters for [`init_logger`].
+#[derive(Default)]
+pub struct InitLoggerParams {
+	/// A comma seperated list of logging patterns.
+	///
+	/// E.g.: `test-crate=debug`
+	pub pattern: String,
+	/// The tracing receiver.
+	pub tracing_receiver: sc_tracing::TracingReceiver,
+	/// Optional comma seperated list of tracing targets.
+	pub tracing_targets: Option<String>,
+	/// Should log reloading be disabled?
+	pub disable_log_reloading: bool,
+	/// Should the log color output be disabled?
+	pub disable_log_color: bool,
+}
+
 /// Initialize the global logger
 ///
 /// This sets various global logging and tracing instances and thus may only be called once.
 pub fn init_logger(
-	pattern: &str,
-	tracing_receiver: sc_tracing::TracingReceiver,
-	profiling_targets: Option<String>,
-	disable_log_reloading: bool,
+	InitLoggerParams {
+		pattern,
+		tracing_receiver,
+		tracing_targets,
+		disable_log_reloading,
+		disable_log_color,
+	}: InitLoggerParams,
 ) -> std::result::Result<(), String> {
 	use sc_tracing::parse_default_directive;
 
 	// Accept all valid directives and print invalid ones
-	fn parse_user_directives(mut env_filter: EnvFilter, dirs: &str) -> std::result::Result<EnvFilter, String> {
+	fn parse_user_directives(
+		mut env_filter: EnvFilter,
+		dirs: &str,
+	) -> std::result::Result<EnvFilter, String> {
 		for dir in dirs.split(',') {
 			env_filter = env_filter.add_directive(parse_default_directive(&dir)?);
 		}
 		Ok(env_filter)
 	}
 
-	if let Err(e) = tracing_log::LogTracer::init() {
-		return Err(format!(
-			"Registering Substrate logger failed: {:}!", e
-		))
-	}
+	tracing_log::LogTracer::init()
+		.map_err(|e| format!("Registering Substrate logger failed: {:}!", e))?;
 
 	// Initialize filter - ensure to use `parse_default_directive` for any defaults to persist
 	// after log filter reloading by RPC
@@ -290,7 +310,7 @@ pub fn init_logger(
 	if pattern != "" {
 		// We're not sure if log or tracing is available at this moment, so silently ignore the
 		// parse error.
-		env_filter = parse_user_directives(env_filter, pattern)?;
+		env_filter = parse_user_directives(env_filter, &pattern)?;
 	}
 
 	// If we're only logging `INFO` entries then we'll use a simplified logging format.
@@ -308,11 +328,11 @@ pub fn init_logger(
 	);
 
 	// Make sure to include profiling targets in the filter
-	if let Some(profiling_targets) = profiling_targets.clone() {
-		env_filter = parse_user_directives(env_filter, &profiling_targets)?;
+	if let Some(tracing_targets) = tracing_targets.clone() {
+		env_filter = parse_user_directives(env_filter, &tracing_targets)?;
 	}
 
-	let enable_color = atty::is(atty::Stream::Stderr);
+	let enable_color = atty::is(atty::Stream::Stderr) && !disable_log_color;
 	let timer = ChronoLocal::with_format(if simple {
 		"%Y-%m-%d %H:%M:%S".to_string()
 	} else {
@@ -333,15 +353,15 @@ pub fn init_logger(
 		let subscriber = subscriber_builder
 			.finish()
 			.with(logging::NodeNameLayer);
-		initialize_tracing(subscriber, tracing_receiver, profiling_targets)
+		initialize_tracing(subscriber, tracing_receiver, tracing_targets)
 	} else {
 		let subscriber_builder = subscriber_builder.with_filter_reloading();
 		let handle = subscriber_builder.reload_handle();
 		sc_tracing::set_reload_handle(handle);
 		let subscriber = subscriber_builder
 			.finish()
 			.with(logging::NodeNameLayer);
-		initialize_tracing(subscriber, tracing_receiver, profiling_targets)
+		initialize_tracing(subscriber, tracing_receiver, tracing_targets)
 	}
 }
 
@@ -380,7 +400,9 @@ mod tests {
 	#[test]
 	fn test_logger_filters() {
 		let test_pattern = "afg=debug,sync=trace,client=warn,telemetry,something-with-dash=error";
-		init_logger(&test_pattern, Default::default(), Default::default(), false).unwrap();
+		init_logger(
+			InitLoggerParams { pattern: test_pattern.into(), ..Default::default() },
+		).unwrap();
 
 		tracing::dispatcher::get_default(|dispatcher| {
 			let test_filter = |target, level| {
@@ -439,7 +461,9 @@ mod tests {
 	fn log_something_with_dash_target_name() {
 		if env::var("ENABLE_LOGGING").is_ok() {
 			let test_pattern = "test-target=info";
-			init_logger(&test_pattern, Default::default(), Default::default(), false).unwrap();
+			init_logger(
+				InitLoggerParams { pattern: test_pattern.into(), ..Default::default() },
+			).unwrap();
 
 			log::info!(target: "test-target", "{}", EXPECTED_LOG_MESSAGE);
 		}
@@ -475,7 +499,9 @@ mod tests {
 	fn prefix_in_log_lines_entrypoint() {
 		if env::var("ENABLE_LOGGING").is_ok() {
 			let test_pattern = "test-target=info";
-			init_logger(&test_pattern, Default::default(), Default::default(), false).unwrap();
+			init_logger(
+				InitLoggerParams { pattern: test_pattern.into(), ..Default::default() },
+			).unwrap();
 			prefix_in_log_lines_process();
 		}
 	}
@@ -491,7 +517,7 @@ mod tests {
 	#[test]
 	fn do_not_write_with_colors_on_tty_entrypoint() {
 		if env::var("ENABLE_LOGGING").is_ok() {
-			init_logger("", Default::default(), Default::default(), false).unwrap();
+			init_logger(InitLoggerParams::default()).unwrap();
 			log::info!("{}", ansi_term::Colour::Yellow.paint(EXPECTED_LOG_MESSAGE));
 		}
 	}

client/cli/src/params/shared_params.rs

@@ -46,6 +46,10 @@ pub struct SharedParams {
 	#[structopt(short = "l", long, value_name = "LOG_PATTERN")]
 	pub log: Vec<String>,
 
+	/// Disable log color output.
+	#[structopt(long)]
+	pub disable_log_color: bool,
+
 	/// Disable feature to dynamically update and reload the log filter.
 	///
 	/// By default this feature is enabled, however it leads to a small performance decrease.
@@ -99,6 +103,11 @@ impl SharedParams {
 		&self.log
 	}
 
+	/// Should the log color output be disabled?
+	pub fn disable_log_color(&self) -> bool {
+		self.disable_log_color
+	}
+
 	/// Is log reloading disabled
 	pub fn is_log_filter_reloading_disabled(&self) -> bool {
 		self.disable_log_reloading

client/rpc/src/system/tests.rs

@@ -344,13 +344,15 @@ fn test_add_reset_log_filter() {
 
 	// Enter log generation / filter reload
 	if std::env::var("TEST_LOG_FILTER").is_ok() {
-		sc_cli::init_logger("test_before_add=debug", Default::default(), Default::default(), false).unwrap();
+		sc_cli::init_logger(
+			sc_cli::InitLoggerParams { pattern: "test_before_add=debug".into(), ..Default::default() },
+		).unwrap();
 		for line in std::io::stdin().lock().lines() {
 			let line = line.expect("Failed to read bytes");
 			if line.contains("add_reload") {
-				assert!(api(None).system_add_log_filter("test_after_add".to_owned()).is_ok(), "`system_add_log_filter` failed");
+				api(None).system_add_log_filter("test_after_add".into()).expect("`system_add_log_filter` failed");
 			} else if line.contains("reset") {
-				assert!(api(None).system_reset_log_filter().is_ok(), "`system_reset_log_filter` failed");
+				api(None).system_reset_log_filter().expect("`system_reset_log_filter` failed");
 			} else if line.contains("exit") {
 				return;
 			}