From 26c5ad4e838a1d7ebddc33e8c36b26380bd45d69 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 20 Jan 2021 13:33:30 +0100 Subject: [PATCH 1/9] Doc fixes for sc-telemetry --- client/cli/src/runner.rs | 2 +- client/service/src/builder.rs | 12 ++--------- client/service/src/config.rs | 7 ++---- client/service/src/task_manager/mod.rs | 2 +- client/telemetry/README.md | 26 +++++++++++----------- client/telemetry/src/endpoints.rs | 3 ++- client/telemetry/src/layer.rs | 2 +- client/telemetry/src/lib.rs | 30 +++++++++++++------------- 8 files changed, 37 insertions(+), 47 deletions(-) diff --git a/client/cli/src/runner.rs b/client/cli/src/runner.rs index 06676655581b1..61a7fe9b01454 100644 --- a/client/cli/src/runner.rs +++ b/client/cli/src/runner.rs @@ -239,7 +239,7 @@ impl Runner { /// Get a new [`TelemetryHandle`]. /// - /// This is used when you want to register a new telemetry for a Substrate node. + /// This is used when you want to register with the [`TelemetryWorker`]. pub fn telemetry_handle(&self) -> TelemetryHandle { self.telemetry_worker.handle() } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index a155899fbd99c..a47480e3a5688 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -308,11 +308,7 @@ pub fn new_full_parts( { let keystore_container = KeystoreContainer::new(&config.keystore)?; - let telemetry_span = if config.telemetry_endpoints.is_some() { - Some(TelemetrySpan::new()) - } else { - None - }; + let telemetry_span = config.telemetry_endpoints.as_ref().map(|_| TelemetrySpan::new()); let task_manager = { let registry = config.prometheus_config.as_ref().map(|cfg| &cfg.registry); TaskManager::new(config.task_executor.clone(), registry, telemetry_span.clone())? @@ -383,11 +379,7 @@ pub fn new_light_parts( TExecDisp: NativeExecutionDispatch + 'static, { let keystore_container = KeystoreContainer::new(&config.keystore)?; - let telemetry_span = if config.telemetry_endpoints.is_some() { - Some(TelemetrySpan::new()) - } else { - None - }; + let telemetry_span = config.telemetry_endpoints.as_ref().map(|_| TelemetrySpan::new()); let task_manager = { let registry = config.prometheus_config.as_ref().map(|cfg| &cfg.registry); TaskManager::new(config.task_executor.clone(), registry, telemetry_span.clone())? diff --git a/client/service/src/config.rs b/client/service/src/config.rs index c3be40e08397c..6c40d9ac305dc 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -213,11 +213,8 @@ impl Configuration { return None; } - match self.telemetry_endpoints.as_ref() { - // Don't initialise telemetry if `telemetry_endpoints` == Some([]) - Some(endpoints) if !endpoints.is_empty() => Some(endpoints), - _ => None, - } + // Don't initialise telemetry if `telemetry_endpoints` == Some([]) + self.telemetry_endpoints.as_ref().filter(|x| !x.is_empty()) } } diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index 4d9e16d900327..e910e2f3a32e2 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -233,7 +233,7 @@ pub struct TaskManager { /// terminates and gracefully shutdown. Also ends the parent `future()` if a child's essential /// task fails. children: Vec, - /// A telemetry handle used to enter the telemetry span when a task is spawned. + /// A `TelemetrySpan` used to enter the telemetry span when a task is spawned. telemetry_span: Option, } diff --git a/client/telemetry/README.md b/client/telemetry/README.md index a6b7b654508ae..2e3e19bd2f628 100644 --- a/client/telemetry/README.md +++ b/client/telemetry/README.md @@ -1,21 +1,21 @@ # sc-telemetry -Substrate's client telemetry is a part of substrate that allows logging telemetry information -with a [Polkadot telemetry](https://github.com/paritytech/substrate-telemetry). +Substrate's client telemetry is a part of substrate that allows ingesting telemetry data +with for example [Polkadot telemetry](https://github.com/paritytech/substrate-telemetry). -It works using Tokio's [tracing](https://github.com/tokio-rs/tracing/). The telemetry -information uses tracing's logging to report the telemetry which is then retrieved by a -tracing's `Layer`. This layer will then send the data through an asynchronous channel and to a -background task called [`TelemetryWorker`] which will send the information to the telemetry -server. +It works using Tokio's [tracing](https://github.com/tokio-rs/tracing/) library. The telemetry +information uses tracing's logging to report the telemetry data which is then retrieved by a +tracing `Layer`. This layer will then send the data through an asynchronous channel to a +background task called [`TelemetryWorker`] which will send the information to the configured +remote telemetry servers. -If multiple substrate nodes are running, it uses a tracing's `Span` to identify which substrate -node is reporting the telemetry. Every task spawned using sc-service's `TaskManager` -automatically inherit this span. +If multiple substrate nodes are running in the same process, it uses a `tracing::Span` to +identify which substrate node is reporting the telemetry. Every task spawned using sc-service's +`TaskManager` automatically inherit this span. -Substrate's nodes initialize/register to the [`TelemetryWorker`] using a [`TelemetryHandle`]. +Substrate's nodes initialize/register with the [`TelemetryWorker`] using a [`TelemetryHandle`]. This handle can be cloned and passed around. It uses an asynchronous channel to communicate with -the running [`TelemetryWorker`] dedicated to registration. Registering a telemetry can happen at -any point in time during the execution. +the running [`TelemetryWorker`] dedicated to registration. Registering can happen at any point +in time during the process execution. License: GPL-3.0-or-later WITH Classpath-exception-2.0 diff --git a/client/telemetry/src/endpoints.rs b/client/telemetry/src/endpoints.rs index 7d0338fb18e3c..fe4fa23974a64 100644 --- a/client/telemetry/src/endpoints.rs +++ b/client/telemetry/src/endpoints.rs @@ -25,7 +25,8 @@ use serde::{Deserialize, Deserializer, Serialize}; /// The URL string can be either a URL or a multiaddress. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] pub struct TelemetryEndpoints( - #[serde(deserialize_with = "url_or_multiaddr_deser")] pub(crate) Vec<(Multiaddr, u8)>, + #[serde(deserialize_with = "url_or_multiaddr_deser")] + pub(crate) Vec<(Multiaddr, u8)>, ); /// Custom deserializer for TelemetryEndpoints, used to convert urls or multiaddr to multiaddr. diff --git a/client/telemetry/src/layer.rs b/client/telemetry/src/layer.rs index eb5eee197770b..0ce3f97620da9 100644 --- a/client/telemetry/src/layer.rs +++ b/client/telemetry/src/layer.rs @@ -35,7 +35,7 @@ pub struct TelemetryLayer(Mutex>); impl TelemetryLayer { /// Create a new [`TelemetryLayer`] and [`TelemetryWorker`]. /// - /// If not provided, the `buffer_size` will be 16 by default. + /// The `buffer_size` defaults to 16. /// /// The [`ExtTransport`] is used in WASM contexts where we need some binding between the /// networking provided by the operating system or environment and libp2p. diff --git a/client/telemetry/src/lib.rs b/client/telemetry/src/lib.rs index 6a4533bb7bc40..7d50461bb9293 100644 --- a/client/telemetry/src/lib.rs +++ b/client/telemetry/src/lib.rs @@ -16,23 +16,23 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Substrate's client telemetry is a part of substrate that allows logging telemetry information -//! with a [Polkadot telemetry](https://github.com/paritytech/substrate-telemetry). +//! Substrate's client telemetry is a part of substrate that allows ingesting telemetry data +//! with for example [Polkadot telemetry](https://github.com/paritytech/substrate-telemetry). //! -//! It works using Tokio's [tracing](https://github.com/tokio-rs/tracing/). The telemetry -//! information uses tracing's logging to report the telemetry which is then retrieved by a -//! tracing's `Layer`. This layer will then send the data through an asynchronous channel and to a -//! background task called [`TelemetryWorker`] which will send the information to the telemetry -//! server. +//! It works using Tokio's [tracing](https://github.com/tokio-rs/tracing/) library. The telemetry +//! information uses tracing's logging to report the telemetry data which is then retrieved by a +//! tracing `Layer`. This layer will then send the data through an asynchronous channel to a +//! background task called [`TelemetryWorker`] which will send the information to the configured +//! remote telemetry servers. //! -//! If multiple substrate nodes are running, it uses a tracing's `Span` to identify which substrate -//! node is reporting the telemetry. Every task spawned using sc-service's `TaskManager` -//! automatically inherit this span. +//! If multiple substrate nodes are running in the same process, it uses a `tracing::Span` to +//! identify which substrate node is reporting the telemetry. Every task spawned using sc-service's +//! `TaskManager` automatically inherit this span. //! -//! Substrate's nodes initialize/register to the [`TelemetryWorker`] using a [`TelemetryHandle`]. +//! Substrate's nodes initialize/register with the [`TelemetryWorker`] using a [`TelemetryHandle`]. //! This handle can be cloned and passed around. It uses an asynchronous channel to communicate with -//! the running [`TelemetryWorker`] dedicated to registration. Registering a telemetry can happen at -//! any point in time during the execution. +//! the running [`TelemetryWorker`] dedicated to registration. Registering can happen at any point +//! in time during the process execution. #![warn(missing_docs)] @@ -115,7 +115,7 @@ pub struct ConnectionMessage { /// Telemetry worker. /// -/// It should be ran as a background task using the [`TelemetryWorker::run`] method. This method +/// It should run as a background task using the [`TelemetryWorker::run`] method. This method /// will consume the object and any further attempts of initializing a new telemetry through its /// handle will fail (without being fatal). #[derive(Debug)] @@ -143,7 +143,7 @@ impl TelemetryWorker { /// Get a new [`TelemetryHandle`]. /// - /// This is used when you want to register a new telemetry for a Substrate node. + /// This is used when you want to register with the [`TelemetryWorker`]. pub fn handle(&self) -> TelemetryHandle { TelemetryHandle { message_sender: self.register_sender.clone(), From 338c3bdcc91b80f2f31980cce7021d8704464f97 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 20 Jan 2021 14:42:36 +0100 Subject: [PATCH 2/9] Fix flag to disable log reloading --- client/tracing/src/logging/mod.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index ca4f74194bcc6..1def0ead73b69 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -67,7 +67,7 @@ pub enum Error { SetLoggerError(#[from] tracing_log::log_tracer::SetLoggerError), } -macro_rules! disable_log_reloading { +macro_rules! enable_log_reloading { ($builder:expr) => {{ let builder = $builder.with_filter_reloading(); let handle = builder.reload_handle(); @@ -201,7 +201,7 @@ pub struct GlobalLoggerBuilder { profiling: Option<(crate::TracingReceiver, String)>, telemetry_buffer_size: Option, telemetry_external_transport: Option, - disable_log_reloading: bool, + log_reloading: bool, force_colors: Option, } @@ -213,7 +213,7 @@ impl GlobalLoggerBuilder { profiling: None, telemetry_buffer_size: None, telemetry_external_transport: None, - disable_log_reloading: false, + log_reloading: true, force_colors: None, } } @@ -230,7 +230,7 @@ impl GlobalLoggerBuilder { /// Wether or not to disable log reloading. pub fn with_log_reloading(&mut self, enabled: bool) -> &mut Self { - self.disable_log_reloading = !enabled; + self.log_reloading = enabled; self } @@ -260,14 +260,14 @@ impl GlobalLoggerBuilder { // If profiling is activated, we require `trace` logging. let max_level = Some(log::LevelFilter::Trace); - if self.disable_log_reloading { + if !self.log_reloading { let (subscriber, telemetry_worker) = get_subscriber_internal( &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), max_level, self.force_colors, self.telemetry_buffer_size, self.telemetry_external_transport, - |builder| builder, + |builder| enable_log_reloading!(builder), )?; let profiling = crate::ProfilingLayer::new(tracing_receiver, &profiling_targets); @@ -281,7 +281,7 @@ impl GlobalLoggerBuilder { self.force_colors, self.telemetry_buffer_size, self.telemetry_external_transport, - |builder| disable_log_reloading!(builder), + |builder| builder, )?; let profiling = crate::ProfilingLayer::new(tracing_receiver, &profiling_targets); @@ -290,14 +290,14 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } } else { - if self.disable_log_reloading { + if !self.log_reloading { let (subscriber, telemetry_worker) = get_subscriber_internal( &self.pattern, None, self.force_colors, self.telemetry_buffer_size, self.telemetry_external_transport, - |builder| builder, + |builder| enable_log_reloading!(builder), )?; tracing::subscriber::set_global_default(subscriber)?; @@ -310,7 +310,7 @@ impl GlobalLoggerBuilder { self.force_colors, self.telemetry_buffer_size, self.telemetry_external_transport, - |builder| disable_log_reloading!(builder), + |builder| builder, )?; tracing::subscriber::set_global_default(subscriber)?; From 9af21dda8bcec8a5a13015f43f610d73700e5d4a Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 20 Jan 2021 14:46:58 +0100 Subject: [PATCH 3/9] Forgot to reverse the conditions --- client/tracing/src/logging/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index 1def0ead73b69..6b8b062cfbc8b 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -260,7 +260,7 @@ impl GlobalLoggerBuilder { // If profiling is activated, we require `trace` logging. let max_level = Some(log::LevelFilter::Trace); - if !self.log_reloading { + if self.log_reloading { let (subscriber, telemetry_worker) = get_subscriber_internal( &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), max_level, @@ -290,7 +290,7 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } } else { - if !self.log_reloading { + if self.log_reloading { let (subscriber, telemetry_worker) = get_subscriber_internal( &self.pattern, None, From fd8bbeca63ab8061b029406b8d28aed833841fcc Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 20 Jan 2021 15:41:15 +0100 Subject: [PATCH 4/9] Apply suggestion --- client/tracing/src/logging/directives.rs | 2 +- client/tracing/src/logging/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/tracing/src/logging/directives.rs b/client/tracing/src/logging/directives.rs index b108566bf2bce..446c3d239abf2 100644 --- a/client/tracing/src/logging/directives.rs +++ b/client/tracing/src/logging/directives.rs @@ -114,7 +114,7 @@ pub(crate) fn set_reload_handle(handle: Handle) { let _ = FILTER_RELOAD_HANDLE.set(handle); } -// The layered Subscriber as built up in `init_logger()`. +// The layered Subscriber as built up in `GlobalLogger::init()`. // Used in the reload `Handle`. type SCSubscriber< N = tracing_fmt::format::DefaultFields, diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index 6b8b062cfbc8b..a41c7e4fdd5ef 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -77,7 +77,7 @@ macro_rules! enable_log_reloading { } /// Common implementation to get the subscriber. -fn get_subscriber_internal( +fn prepare_subscriber( pattern: &str, max_level: Option, force_colors: Option, @@ -261,7 +261,7 @@ impl GlobalLoggerBuilder { let max_level = Some(log::LevelFilter::Trace); if self.log_reloading { - let (subscriber, telemetry_worker) = get_subscriber_internal( + let (subscriber, telemetry_worker) = prepare_subscriber( &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), max_level, self.force_colors, @@ -275,7 +275,7 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } else { - let (subscriber, telemetry_worker) = get_subscriber_internal( + let (subscriber, telemetry_worker) = prepare_subscriber( &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), max_level, self.force_colors, @@ -291,7 +291,7 @@ impl GlobalLoggerBuilder { } } else { if self.log_reloading { - let (subscriber, telemetry_worker) = get_subscriber_internal( + let (subscriber, telemetry_worker) = prepare_subscriber( &self.pattern, None, self.force_colors, @@ -304,7 +304,7 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } else { - let (subscriber, telemetry_worker) = get_subscriber_internal( + let (subscriber, telemetry_worker) = prepare_subscriber( &self.pattern, None, self.force_colors, From 3d07bed37a5919d7356ac958b2a7c9a841416026 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 20 Jan 2021 15:41:37 +0100 Subject: [PATCH 5/9] Rename pattern to directives --- client/tracing/src/logging/mod.rs | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index a41c7e4fdd5ef..b52ee4fc0ec88 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -78,7 +78,7 @@ macro_rules! enable_log_reloading { /// Common implementation to get the subscriber. fn prepare_subscriber( - pattern: &str, + directives: &str, max_level: Option, force_colors: Option, telemetry_buffer_size: Option, @@ -130,10 +130,10 @@ where } } - if pattern != "" { + if directives != "" { // 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, directives)?; } let max_level_hint = Layer::::max_level_hint(&env_filter); @@ -197,7 +197,7 @@ where /// A builder that is used to initialize the global logger. pub struct GlobalLoggerBuilder { - pattern: String, + directives: String, profiling: Option<(crate::TracingReceiver, String)>, telemetry_buffer_size: Option, telemetry_external_transport: Option, @@ -207,9 +207,9 @@ pub struct GlobalLoggerBuilder { impl GlobalLoggerBuilder { /// Create a new [`GlobalLoggerBuilder`] which can be used to initialize the global logger. - pub fn new>(pattern: S) -> Self { + pub fn new>(directives: S) -> Self { Self { - pattern: pattern.into(), + directives: directives.into(), profiling: None, telemetry_buffer_size: None, telemetry_external_transport: None, @@ -262,7 +262,7 @@ impl GlobalLoggerBuilder { if self.log_reloading { let (subscriber, telemetry_worker) = prepare_subscriber( - &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), + &format!("{},{},sc_tracing=trace", self.directives, profiling_targets), max_level, self.force_colors, self.telemetry_buffer_size, @@ -276,7 +276,7 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } else { let (subscriber, telemetry_worker) = prepare_subscriber( - &format!("{},{},sc_tracing=trace", self.pattern, profiling_targets), + &format!("{},{},sc_tracing=trace", self.directives, profiling_targets), max_level, self.force_colors, self.telemetry_buffer_size, @@ -292,7 +292,7 @@ impl GlobalLoggerBuilder { } else { if self.log_reloading { let (subscriber, telemetry_worker) = prepare_subscriber( - &self.pattern, + &self.directives, None, self.force_colors, self.telemetry_buffer_size, @@ -305,7 +305,7 @@ impl GlobalLoggerBuilder { Ok(telemetry_worker) } else { let (subscriber, telemetry_worker) = prepare_subscriber( - &self.pattern, + &self.directives, None, self.force_colors, self.telemetry_buffer_size, @@ -331,8 +331,8 @@ mod tests { const EXPECTED_LOG_MESSAGE: &'static str = "yeah logging works as expected"; const EXPECTED_NODE_NAME: &'static str = "THE_NODE"; - fn init_logger(pattern: &str) { - let _ = GlobalLoggerBuilder::new(pattern).init().unwrap(); + fn init_logger(directives: &str) { + let _ = GlobalLoggerBuilder::new(directives).init().unwrap(); } fn run_in_process(test_name: &str) { @@ -351,8 +351,8 @@ mod tests { fn test_logger_filters() { run_in_process("test_logger_filters"); - let test_pattern = "afg=debug,sync=trace,client=warn,telemetry,something-with-dash=error"; - init_logger(&test_pattern); + let test_directives = "afg=debug,sync=trace,client=warn,telemetry,something-with-dash=error"; + init_logger(&test_directives); tracing::dispatcher::get_default(|dispatcher| { let test_filter = |target, level| { @@ -410,8 +410,8 @@ mod tests { #[test] fn log_something_with_dash_target_name() { if env::var("ENABLE_LOGGING").is_ok() { - let test_pattern = "test-target=info"; - let _guard = init_logger(&test_pattern); + let test_directives = "test-target=info"; + let _guard = init_logger(&test_directives); log::info!(target: "test-target", "{}", EXPECTED_LOG_MESSAGE); } From f26a828988cf790be10a0caa115cf871a029e26c Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 08:10:24 +0100 Subject: [PATCH 6/9] Rename GlobalLoggerBuilder to LoggerBuilder --- client/cli/src/config.rs | 4 ++-- client/cli/src/lib.rs | 2 +- client/rpc/src/system/tests.rs | 2 +- client/tracing/src/logging/mod.rs | 8 ++++---- utils/browser/src/lib.rs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index ae43e8f334c6d..5bc14d52a5f30 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -34,7 +34,7 @@ use sc_service::config::{ }; use sc_service::{ChainSpec, TracingReceiver, KeepBlocks, TransactionStorageMode}; use sc_telemetry::TelemetryHandle; -use sc_tracing::logging::GlobalLoggerBuilder; +use sc_tracing::logging::LoggerBuilder; use std::net::SocketAddr; use std::path::PathBuf; @@ -576,7 +576,7 @@ pub trait CliConfiguration: Sized { fn init(&self) -> Result { sp_panic_handler::set(&C::support_url(), &C::impl_version()); - let mut logger = GlobalLoggerBuilder::new(self.log_filters()?); + let mut logger = LoggerBuilder::new(self.log_filters()?); logger.with_log_reloading(!self.is_log_filter_reloading_disabled()?); if let Some(transport) = self.telemetry_external_transport()? { diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs index a4b0bd45727e2..602c53272ea59 100644 --- a/client/cli/src/lib.rs +++ b/client/cli/src/lib.rs @@ -38,7 +38,7 @@ pub use runner::*; pub use sc_service::{ChainSpec, Role}; use sc_service::{Configuration, TaskExecutor}; use sc_telemetry::TelemetryHandle; -pub use sc_tracing::logging::GlobalLoggerBuilder; +pub use sc_tracing::logging::LoggerBuilder; pub use sp_version::RuntimeVersion; use std::io::Write; pub use structopt; diff --git a/client/rpc/src/system/tests.rs b/client/rpc/src/system/tests.rs index 89676acae26bf..c196403501035 100644 --- a/client/rpc/src/system/tests.rs +++ b/client/rpc/src/system/tests.rs @@ -344,7 +344,7 @@ fn test_add_reset_log_filter() { // Enter log generation / filter reload if std::env::var("TEST_LOG_FILTER").is_ok() { - sc_tracing::logging::GlobalLoggerBuilder::new("test_before_add=debug").init().unwrap(); + sc_tracing::logging::LoggerBuilder::new("test_before_add=debug").init().unwrap(); for line in std::io::stdin().lock().lines() { let line = line.expect("Failed to read bytes"); if line.contains("add_reload") { diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index b52ee4fc0ec88..8548ef01c1123 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -196,7 +196,7 @@ where } /// A builder that is used to initialize the global logger. -pub struct GlobalLoggerBuilder { +pub struct LoggerBuilder { directives: String, profiling: Option<(crate::TracingReceiver, String)>, telemetry_buffer_size: Option, @@ -205,8 +205,8 @@ pub struct GlobalLoggerBuilder { force_colors: Option, } -impl GlobalLoggerBuilder { - /// Create a new [`GlobalLoggerBuilder`] which can be used to initialize the global logger. +impl LoggerBuilder { + /// Create a new [`LoggerBuilder`] which can be used to initialize the global logger. pub fn new>(directives: S) -> Self { Self { directives: directives.into(), @@ -332,7 +332,7 @@ mod tests { const EXPECTED_NODE_NAME: &'static str = "THE_NODE"; fn init_logger(directives: &str) { - let _ = GlobalLoggerBuilder::new(directives).init().unwrap(); + let _ = LoggerBuilder::new(directives).init().unwrap(); } fn run_in_process(test_name: &str) { diff --git a/utils/browser/src/lib.rs b/utils/browser/src/lib.rs index 5e1e8db316688..b72f2e973b682 100644 --- a/utils/browser/src/lib.rs +++ b/utils/browser/src/lib.rs @@ -25,7 +25,7 @@ use sc_service::{ KeepBlocks, TransactionStorageMode, }; use sc_telemetry::TelemetryHandle; -use sc_tracing::logging::GlobalLoggerBuilder; +use sc_tracing::logging::LoggerBuilder; use wasm_bindgen::prelude::*; use futures::{ prelude::*, channel::{oneshot, mpsc}, compat::*, future::{ready, ok, select} @@ -41,7 +41,7 @@ pub fn init_logging_and_telemetry( pattern: &str, ) -> Result { let transport = ExtTransport::new(ffi::websocket_transport()); - let mut logger = GlobalLoggerBuilder::new(pattern); + let mut logger = LoggerBuilder::new(pattern); logger.with_transport(transport); logger.init() } From 5efdec66257902e39f586cc33eb80e722b1a47a1 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 06:06:21 +0100 Subject: [PATCH 7/9] Return instead of expect --- client/tracing/src/logging/layers/prefix_layer.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/client/tracing/src/logging/layers/prefix_layer.rs b/client/tracing/src/logging/layers/prefix_layer.rs index 6aa7e6d436e1a..0c8f25c24100c 100644 --- a/client/tracing/src/logging/layers/prefix_layer.rs +++ b/client/tracing/src/logging/layers/prefix_layer.rs @@ -33,9 +33,18 @@ where S: Subscriber + for<'a> LookupSpan<'a>, { fn new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { - let span = ctx - .span(id) - .expect("new_span has been called for this span; qed"); + let span = match ctx.span(id) { + Some(span) => span, + None => { + // this shouldn't happen! + debug_assert!( + false, + "newly created span with ID {:?} did not exist in the registry; this is a bug!", + id + ); + return; + } + }; if span.name() != PREFIX_LOG_SPAN { return; From cb2cde970c6910d0efb15eaedfa940e7ec699fec Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 06:09:08 +0100 Subject: [PATCH 8/9] Use transparent outside the enum --- client/tracing/src/logging/mod.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/client/tracing/src/logging/mod.rs b/client/tracing/src/logging/mod.rs index 8548ef01c1123..f74c3e6646073 100644 --- a/client/tracing/src/logging/mod.rs +++ b/client/tracing/src/logging/mod.rs @@ -53,17 +53,11 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] #[non_exhaustive] +#[error(transparent)] pub enum Error { - #[error(transparent)] IoError(#[from] io::Error), - - #[error(transparent)] SetGlobalDefaultError(#[from] tracing::subscriber::SetGlobalDefaultError), - - #[error(transparent)] DirectiveParseError(#[from] tracing_subscriber::filter::ParseError), - - #[error(transparent)] SetLoggerError(#[from] tracing_log::log_tracer::SetLoggerError), } From daa602116da5d9aba1adad33cd6891db7b33c1c6 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 27 Jan 2021 12:33:50 +0100 Subject: [PATCH 9/9] Update client/tracing/src/logging/directives.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/tracing/src/logging/directives.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/tracing/src/logging/directives.rs b/client/tracing/src/logging/directives.rs index 446c3d239abf2..39dee2b061f0a 100644 --- a/client/tracing/src/logging/directives.rs +++ b/client/tracing/src/logging/directives.rs @@ -114,7 +114,7 @@ pub(crate) fn set_reload_handle(handle: Handle) { let _ = FILTER_RELOAD_HANDLE.set(handle); } -// The layered Subscriber as built up in `GlobalLogger::init()`. +// The layered Subscriber as built up in `LoggerBuilder::init()`. // Used in the reload `Handle`. type SCSubscriber< N = tracing_fmt::format::DefaultFields,