Disable log reloading by default (#9966)

* Disable log reloading by default

This disables the log reloading that was enabled by default. The problem
is that the log reload implementation of `tracing` is using a lock to
make the layer replaceable. This lock needs to be locked every time we
need to check if a particular target is enabled (assuming the log level
is high enough). This kills the performance when for example
`sometarget=trace` logging is enabled.

* 🤦

* Remove unused parameter

* Fix test

* Fix

bin/node/cli/benches/transaction_pool.rs

@@ -99,7 +99,6 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
 		base_path: Some(base_path),
 		informant_output_format: Default::default(),
 		wasm_runtime_overrides: None,
-		disable_log_reloading: false,
 	};
 
 	node_cli::service::new_full_base(config, |_, _| ()).expect("Creates node")

client/cli/src/config.rs

@@ -522,7 +522,6 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 			dev_key_seed: self.dev_key_seed(is_dev)?,
 			tracing_targets: self.tracing_targets()?,
 			tracing_receiver: self.tracing_receiver()?,
-			disable_log_reloading: self.is_log_filter_reloading_disabled()?,
 			chain_spec,
 			max_runtime_instances,
 			announce_block: self.announce_block()?,
@@ -542,9 +541,9 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		Ok(self.shared_params().log_filters().join(","))
 	}
 
-	/// Is log reloading disabled (enabled by default)
-	fn is_log_filter_reloading_disabled(&self) -> Result<bool> {
-		Ok(self.shared_params().is_log_filter_reloading_disabled())
+	/// Is log reloading enabled?
+	fn enable_log_reloading(&self) -> Result<bool> {
+		Ok(self.shared_params().enable_log_reloading())
 	}
 
 	/// Should the log color output be disabled?
@@ -563,7 +562,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		sp_panic_handler::set(&C::support_url(), &C::impl_version());
 
 		let mut logger = LoggerBuilder::new(self.log_filters()?);
-		logger.with_log_reloading(!self.is_log_filter_reloading_disabled()?);
+		logger.with_log_reloading(self.enable_log_reloading()?);
 
 		if let Some(tracing_targets) = self.tracing_targets()? {
 			let tracing_receiver = self.tracing_receiver()?;

client/cli/src/params/shared_params.rs

@@ -50,13 +50,15 @@ pub struct SharedParams {
 	#[structopt(long)]
 	pub disable_log_color: bool,
 
-	/// Disable feature to dynamically update and reload the log filter.
+	/// Enable feature to dynamically update and reload the log filter.
+	///
+	/// Be aware that enabling this feature can lead to a performance decrease up to factor six or
+	/// more. Depending on the global logging level the performance decrease changes.
 	///
-	/// By default this feature is enabled, however it leads to a small performance decrease.
 	/// The `system_addLogFilter` and `system_resetLogFilter` RPCs will have no effect with this
-	/// option set.
-	#[structopt(long = "disable-log-reloading")]
-	pub disable_log_reloading: bool,
+	/// option not being set.
+	#[structopt(long)]
+	pub enable_log_reloading: bool,
 
 	/// Sets a custom profiling filter. Syntax is the same as for logging: <target>=<level>
 	#[structopt(long = "tracing-targets", value_name = "TARGETS")]
@@ -107,9 +109,9 @@ impl SharedParams {
 		self.disable_log_color
 	}
 
-	/// Is log reloading disabled
-	pub fn is_log_filter_reloading_disabled(&self) -> bool {
-		self.disable_log_reloading
+	/// Is log reloading enabled
+	pub fn enable_log_reloading(&self) -> bool {
+		self.enable_log_reloading
 	}
 
 	/// Receiver to process tracing messages.

client/rpc/src/system/tests.rs

@@ -308,7 +308,10 @@ fn test_add_reset_log_filter() {
 
 	// Enter log generation / filter reload
 	if std::env::var("TEST_LOG_FILTER").is_ok() {
-		sc_tracing::logging::LoggerBuilder::new("test_before_add=debug").init().unwrap();
+		let mut builder = sc_tracing::logging::LoggerBuilder::new("test_before_add=debug");
+		builder.with_log_reloading(true);
+		builder.init().unwrap();
+
 		for line in std::io::stdin().lock().lines() {
 			let line = line.expect("Failed to read bytes");
 			if line.contains("add_reload") {

client/service/src/config.rs

@@ -118,8 +118,6 @@ pub struct Configuration {
 	pub dev_key_seed: Option<String>,
 	/// Tracing targets
 	pub tracing_targets: Option<String>,
-	/// Is log filter reloading disabled
-	pub disable_log_reloading: bool,
 	/// Tracing receiver
 	pub tracing_receiver: sc_tracing::TracingReceiver,
 	/// The size of the instances cache.

client/service/test/src/lib.rs

@@ -274,7 +274,6 @@ fn node_config<
 		announce_block: true,
 		base_path: Some(BasePath::new(root)),
 		informant_output_format: Default::default(),
-		disable_log_reloading: false,
 	}
 }
 

client/tracing/src/logging/mod.rs

@@ -197,7 +197,7 @@ impl LoggerBuilder {
 		Self {
 			directives: directives.into(),
 			profiling: None,
-			log_reloading: true,
+			log_reloading: false,
 			force_colors: None,
 		}
 	}

test-utils/test-runner/src/utils.rs

@@ -108,7 +108,6 @@ pub fn default_config(tokio_handle: Handle, mut chain_spec: Box<dyn ChainSpec>)
 		base_path: Some(base_path),
 		wasm_runtime_overrides: None,
 		informant_output_format,
-		disable_log_reloading: false,
 		keystore_remote: None,
 		keep_blocks: KeepBlocks::All,
 		state_pruning: Default::default(),