From 4e51e9adfdf0dc7cf37b562b60a0e83ca1d0b00d Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 15:53:35 +0100 Subject: [PATCH 01/16] WIP --- client/telemetry/src/layer.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/telemetry/src/layer.rs b/client/telemetry/src/layer.rs index eb5eee197770b..d08290ea69add 100644 --- a/client/telemetry/src/layer.rs +++ b/client/telemetry/src/layer.rs @@ -100,6 +100,11 @@ where event, ); } + } else { + let mut attrs = TelemetryAttrs::new(Id::from_u64(42_u64)); + let mut vis = TelemetryAttrsVisitor(&mut attrs); + event.record(&mut vis); + eprintln!("###### no telemetry span found: {:?}", attrs.json); } } } From 234402343c4811287cb049c01d2b19e00d9efeba Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 16:26:24 +0100 Subject: [PATCH 02/16] WIP --- client/service/src/task_manager/mod.rs | 11 +++++++---- client/telemetry/src/lib.rs | 4 ++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index 4d9e16d900327..61a0aae362db9 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -91,10 +91,7 @@ impl SpawnTaskHandle { metrics.tasks_ended.with_label_values(&[name, "finished"]).inc_by(0); } - let telemetry_span = self.telemetry_span.clone(); let future = async move { - let _telemetry_entered = telemetry_span.as_ref().map(|x| x.enter()); - if let Some(metrics) = metrics { // Add some wrappers around `task`. let task = { @@ -127,7 +124,13 @@ impl SpawnTaskHandle { } }; - let join_handle = self.executor.spawn(Box::pin(future.in_current_span()), task_type); + let future: Pin + Send>> = if let Some(telemetry_span) = self.telemetry_span.clone() { + Box::pin(future.instrument(telemetry_span.span())) + } else { + Box::pin(future.in_current_span()) + }; + + let join_handle = self.executor.spawn(future, task_type); let mut task_notifier = self.task_notifier.clone(); self.executor.spawn( Box::pin(async move { diff --git a/client/telemetry/src/lib.rs b/client/telemetry/src/lib.rs index 6a4533bb7bc40..4f68979385576 100644 --- a/client/telemetry/src/lib.rs +++ b/client/telemetry/src/lib.rs @@ -88,6 +88,10 @@ impl TelemetrySpan { pub fn new() -> Self { Self(tracing::info_span!(TELEMETRY_LOG_SPAN)) } + + pub fn span(&self) -> tracing::Span { + self.0.clone() + } } /// Message sent when the connection (re-)establishes. From 829a374e997eb259fd3f3c33365cbf0e7a8badbf Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 16:42:26 +0100 Subject: [PATCH 03/16] Test --- client/service/src/task_manager/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index 61a0aae362db9..6da8af0c32399 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -127,7 +127,7 @@ impl SpawnTaskHandle { let future: Pin + Send>> = if let Some(telemetry_span) = self.telemetry_span.clone() { Box::pin(future.instrument(telemetry_span.span())) } else { - Box::pin(future.in_current_span()) + Box::pin(future) }; let join_handle = self.executor.spawn(future, task_type); From 6fca0b02afa7e05451988b88c39eb08f8196f453 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 18:35:33 +0100 Subject: [PATCH 04/16] bug fix --- client/service/src/builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index a155899fbd99c..fe46d03757302 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -308,7 +308,7 @@ pub fn new_full_parts( { let keystore_container = KeystoreContainer::new(&config.keystore)?; - let telemetry_span = if config.telemetry_endpoints.is_some() { + let telemetry_span = if config.telemetry_endpoints().is_some() { Some(TelemetrySpan::new()) } else { None @@ -383,7 +383,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() { + let telemetry_span = if config.telemetry_endpoints().is_some() { Some(TelemetrySpan::new()) } else { None From 9b8511cd884ed7de1856f6d324ec9abb0bf6a918 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 19:15:47 +0100 Subject: [PATCH 05/16] WIP --- client/service/src/task_manager/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index 6da8af0c32399..fb332ab1e000a 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -125,9 +125,13 @@ impl SpawnTaskHandle { }; let future: Pin + Send>> = if let Some(telemetry_span) = self.telemetry_span.clone() { + // this will preserve the current spans entered & enter the telemetry span + // this is used by sc_telemetry::layer::TelemetryLayer Box::pin(future.instrument(telemetry_span.span())) } else { - Box::pin(future) + // this will preserve the current spans entered + // this is used by sc_tracing::logging::prefix_logs_with + Box::pin(future.in_current_span()) }; let join_handle = self.executor.spawn(future, task_type); From f01b1298829e7ed9f4767735fbb70f0e8827c4e0 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 19:16:44 +0100 Subject: [PATCH 06/16] Revert "WIP" This reverts commit 4e51e9adfdf0dc7cf37b562b60a0e83ca1d0b00d. --- client/telemetry/src/layer.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/client/telemetry/src/layer.rs b/client/telemetry/src/layer.rs index d08290ea69add..eb5eee197770b 100644 --- a/client/telemetry/src/layer.rs +++ b/client/telemetry/src/layer.rs @@ -100,11 +100,6 @@ where event, ); } - } else { - let mut attrs = TelemetryAttrs::new(Id::from_u64(42_u64)); - let mut vis = TelemetryAttrsVisitor(&mut attrs); - event.record(&mut vis); - eprintln!("###### no telemetry span found: {:?}", attrs.json); } } } From df7e630cdab8994f8bf076a92caa9e20dcdd156b Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Thu, 21 Jan 2021 19:20:55 +0100 Subject: [PATCH 07/16] doc --- client/telemetry/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/telemetry/src/lib.rs b/client/telemetry/src/lib.rs index 4f68979385576..1a0e0fde0db42 100644 --- a/client/telemetry/src/lib.rs +++ b/client/telemetry/src/lib.rs @@ -89,6 +89,7 @@ impl TelemetrySpan { Self(tracing::info_span!(TELEMETRY_LOG_SPAN)) } + /// Return a clone of the underlying `tracing::Span` instance. pub fn span(&self) -> tracing::Span { self.0.clone() } From a897dbc61f454fe17545f01761b82a3c0ff84e37 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 07:28:15 +0100 Subject: [PATCH 08/16] Improve comment on why all spans are preserved --- client/service/src/task_manager/mod.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index fb332ab1e000a..bfb81e680206d 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -124,15 +124,18 @@ impl SpawnTaskHandle { } }; - let future: Pin + Send>> = if let Some(telemetry_span) = self.telemetry_span.clone() { - // this will preserve the current spans entered & enter the telemetry span - // this is used by sc_telemetry::layer::TelemetryLayer - Box::pin(future.instrument(telemetry_span.span())) - } else { - // this will preserve the current spans entered - // this is used by sc_tracing::logging::prefix_logs_with - Box::pin(future.in_current_span()) - }; + let future: Pin + Send>> = + if let Some(telemetry_span) = self.telemetry_span.clone() { + // this will preserve the telemetry span and by extension all of its parents as they + // are linked together upon creation + // this is used by sc_telemetry::layer::TelemetryLayer + Box::pin(future.instrument(telemetry_span.span())) + } else { + // this will preserve the current span (if any) and by extension all of its parents + // as they are linked together upon creation + // this is used by sc_tracing::logging::prefix_logs_with + Box::pin(future.in_current_span()) + }; let join_handle = self.executor.spawn(future, task_type); let mut task_notifier = self.task_notifier.clone(); From 783d1dda961829f52fcf6c54a463d371260d5862 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 09:45:32 +0100 Subject: [PATCH 09/16] Added missing suggestion from previous PR --- client/service/src/builder.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index e4209eb9665fb..8a1514b48f618 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().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().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())? From d3504f45b1fe6522551492bdbc8ed28b48d89483 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 10:54:31 +0100 Subject: [PATCH 10/16] Use BoxFuture --- client/service/src/task_manager/mod.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index bfb81e680206d..cb66e2b431f0d 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -124,18 +124,17 @@ impl SpawnTaskHandle { } }; - let future: Pin + Send>> = - if let Some(telemetry_span) = self.telemetry_span.clone() { - // this will preserve the telemetry span and by extension all of its parents as they - // are linked together upon creation - // this is used by sc_telemetry::layer::TelemetryLayer - Box::pin(future.instrument(telemetry_span.span())) - } else { - // this will preserve the current span (if any) and by extension all of its parents - // as they are linked together upon creation - // this is used by sc_tracing::logging::prefix_logs_with - Box::pin(future.in_current_span()) - }; + let future: BoxFuture<()> = if let Some(telemetry_span) = self.telemetry_span.clone() { + // this will preserve the telemetry span and by extension all of its parents as they + // are linked together upon creation + // this is used by sc_telemetry::layer::TelemetryLayer + Box::pin(future.instrument(telemetry_span.span())) + } else { + // this will preserve the current span (if any) and by extension all of its parents + // as they are linked together upon creation + // this is used by sc_tracing::logging::prefix_logs_with + Box::pin(future.in_current_span()) + }; let join_handle = self.executor.spawn(future, task_type); let mut task_notifier = self.task_notifier.clone(); From 827b83cc1be23af4f50e32deb8bdbfa497408f3d Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 20:26:19 +0100 Subject: [PATCH 11/16] Move TelemetrySpan creation to sc-cli, need to test... --- client/cli/src/config.rs | 12 ++++++++++-- client/service/src/builder.rs | 31 ++++++++++--------------------- client/service/src/config.rs | 17 ++++------------- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index ae43e8f334c6d..066a4a734d475 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -33,7 +33,7 @@ use sc_service::config::{ TaskExecutor, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, }; use sc_service::{ChainSpec, TracingReceiver, KeepBlocks, TransactionStorageMode}; -use sc_telemetry::TelemetryHandle; +use sc_telemetry::{TelemetryHandle, TelemetrySpan}; use sc_tracing::logging::GlobalLoggerBuilder; use std::net::SocketAddr; use std::path::PathBuf; @@ -488,6 +488,13 @@ pub trait CliConfiguration: Sized { let max_runtime_instances = self.max_runtime_instances()?.unwrap_or(8); let is_validator = role.is_network_authority(); let (keystore_remote, keystore) = self.keystore_config(&config_dir)?; + let telemetry_endpoints = telemetry_handle + .as_ref() + .and_then(|_| self.telemetry_endpoints(&chain_spec).transpose()) + .transpose()? + // Don't initialise telemetry if `telemetry_endpoints` == Some([]) + .filter(|x| !x.is_empty()); + let telemetry_span = telemetry_endpoints.as_ref().map(|_| TelemetrySpan::new()); let unsafe_pruning = self .import_params() @@ -526,7 +533,8 @@ pub trait CliConfiguration: Sized { rpc_ws_max_connections: self.rpc_ws_max_connections()?, rpc_cors: self.rpc_cors(is_dev)?, prometheus_config: self.prometheus_config(DCV::prometheus_listen_port())?, - telemetry_endpoints: self.telemetry_endpoints(&chain_spec)?, + telemetry_endpoints, + telemetry_span, telemetry_external_transport: self.telemetry_external_transport()?, default_heap_pages: self.default_heap_pages()?, offchain_worker: self.offchain_worker(&role)?, diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 8a1514b48f618..2ee95bd24d324 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -55,7 +55,6 @@ use sc_telemetry::{ telemetry, ConnectionMessage, TelemetryConnectionNotifier, - TelemetrySpan, SUBSTRATE_INFO, }; use sp_transaction_pool::MaintainedTransactionPool; @@ -184,7 +183,6 @@ type TFullParts = ( Arc>, KeystoreContainer, TaskManager, - Option, ); type TLightParts = ( @@ -193,7 +191,6 @@ type TLightParts = ( KeystoreContainer, TaskManager, Arc>, - Option, ); /// Light client backend type with a specific hash type. @@ -308,10 +305,9 @@ pub fn new_full_parts( { let keystore_container = KeystoreContainer::new(&config.keystore)?; - let telemetry_span = config.telemetry_endpoints().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())? + TaskManager::new(config.task_executor.clone(), registry, config.telemetry_span.clone())? }; let executor = NativeExecutor::::new( @@ -367,7 +363,6 @@ pub fn new_full_parts( backend, keystore_container, task_manager, - telemetry_span, )) } @@ -379,10 +374,9 @@ pub fn new_light_parts( TExecDisp: NativeExecutionDispatch + 'static, { let keystore_container = KeystoreContainer::new(&config.keystore)?; - let telemetry_span = config.telemetry_endpoints().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())? + TaskManager::new(config.task_executor.clone(), registry, config.telemetry_span.clone())? }; let executor = NativeExecutor::::new( @@ -421,7 +415,7 @@ pub fn new_light_parts( config.prometheus_config.as_ref().map(|config| config.registry.clone()), )?); - Ok((client, backend, keystore_container, task_manager, on_demand, telemetry_span)) + Ok((client, backend, keystore_container, task_manager, on_demand)) } /// Create an instance of db-backed client. @@ -473,8 +467,6 @@ pub fn new_client( pub struct SpawnTasksParams<'a, TBl: BlockT, TCl, TExPool, TRpc, Backend> { /// The service configuration. pub config: Configuration, - /// Telemetry span, if any. - pub telemetry_span: Option, /// A shared client returned by `new_full_parts`/`new_light_parts`. pub client: Arc, /// A shared backend returned by `new_full_parts`/`new_light_parts`. @@ -567,7 +559,6 @@ pub fn spawn_tasks( let SpawnTasksParams { mut config, task_manager, - telemetry_span, client, on_demand, backend, @@ -588,13 +579,11 @@ pub fn spawn_tasks( config.dev_key_seed.clone().map(|s| vec![s]).unwrap_or_default(), )?; - let telemetry_connection_notifier = telemetry_span - .and_then(|span| init_telemetry( - &mut config, - span, - network.clone(), - client.clone(), - )); + let telemetry_connection_notifier = init_telemetry( + &mut config, + network.clone(), + client.clone(), + ); info!("📦 Highest known block at #{}", chain_info.best_number); @@ -692,11 +681,11 @@ async fn transaction_notifications( fn init_telemetry>( config: &mut Configuration, - telemetry_span: TelemetrySpan, network: Arc::Hash>>, client: Arc, ) -> Option { - let endpoints = config.telemetry_endpoints()?.clone(); + let telemetry_span = config.telemetry_span.clone()?; + let endpoints = config.telemetry_endpoints.clone()?; let genesis_hash = client.block_hash(Zero::zero()).ok().flatten().unwrap_or_default(); let connection_message = ConnectionMessage { name: config.network.node_name.to_owned(), diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 74d15cb3fb922..1e316c37dc9a4 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -101,6 +101,10 @@ pub struct Configuration { /// This is a handle to a `TelemetryWorker` instance. It is used to initialize the telemetry for /// a substrate node. pub telemetry_handle: Option, + /// Telemetry span. + /// + /// This span is entered for every background task spawned using the TaskManager. + pub telemetry_span: Option, /// The default number of 64KB pages to allocate for Wasm execution pub default_heap_pages: Option, /// Should offchain workers be executed. @@ -207,19 +211,6 @@ impl Configuration { self.prometheus_config.as_ref().map(|config| &config.registry) } - /// Returns the telemetry endpoints if any and if the telemetry handle exists. - pub(crate) fn telemetry_endpoints(&self) -> Option<&TelemetryEndpoints> { - if self.telemetry_handle.is_none() { - 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, - } - } - /// Returns the network protocol id from the chain spec, or the default. pub fn protocol_id(&self) -> sc_network::config::ProtocolId { let protocol_id_full = match self.chain_spec.protocol_id() { From 333806b2fe1626efaa2691f9f44d0b4dd979bc36 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 21:01:42 +0100 Subject: [PATCH 12/16] Test code --- bin/node/cli/src/command.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index fcb8b6f0085e0..fef30387b0806 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -73,10 +73,18 @@ pub fn run() -> Result<()> { None => { let runner = cli.create_runner(&cli.run)?; runner.run_node_until_exit(|config| async move { - match config.role { - Role::Light => service::new_light(config), - _ => service::new_full(config), - }.map_err(sc_cli::Error::Service) + // TODO revert after test + let config2 = cli.create_configuration(&cli.run, config.task_executor.clone(), config.telemetry_handle.clone()).unwrap(); + let mut task_manager = match config.role { + Role::Light => service::new_light(config).unwrap(), + _ => service::new_full(config).unwrap(), + }; + let task_manager2 = match config2.role { + Role::Light => service::new_light(config2).unwrap(), + _ => service::new_full(config2).unwrap(), + }; + task_manager.add_child(task_manager2); + Ok(task_manager) }) } Some(Subcommand::Inspect(cmd)) => { From 43ea7bf34c449a72a675f7ea66cbc9bbee85ff8d Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 22 Jan 2021 21:02:16 +0100 Subject: [PATCH 13/16] Adapt user code --- bin/node-template/node/src/service.rs | 12 ++++-------- bin/node/cli/src/service.rs | 13 +++++-------- client/service/test/src/lib.rs | 1 + utils/browser/src/lib.rs | 4 +++- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index b9e5705333e70..a3aca89ef7468 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -11,7 +11,6 @@ pub use sc_executor::NativeExecutor; use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair}; use sc_finality_grandpa::SharedVoterState; use sc_keystore::LocalKeystore; -use sc_telemetry::TelemetrySpan; // Our native executor instance. native_executor_instance!( @@ -37,7 +36,6 @@ pub fn new_partial(config: &Configuration) -> Result, sc_finality_grandpa::LinkHalf, - Option, ) >, ServiceError> { if config.keystore_remote.is_some() { @@ -46,7 +44,7 @@ pub fn new_partial(config: &Configuration) -> Result(&config)?; let client = Arc::new(client); @@ -87,7 +85,7 @@ pub fn new_partial(config: &Configuration) -> Result Result select_chain, transaction_pool, inherent_data_providers, - other: (block_import, grandpa_link, telemetry_span), + other: (block_import, grandpa_link), } = new_partial(&config)?; if let Some(url) = &config.keystore_remote { @@ -177,7 +175,6 @@ pub fn new_full(mut config: Configuration) -> Result network_status_sinks, system_rpc_tx, config, - telemetry_span, }, )?; @@ -260,7 +257,7 @@ pub fn new_full(mut config: Configuration) -> Result /// Builds a new service for a light client. pub fn new_light(mut config: Configuration) -> Result { - let (client, backend, keystore_container, mut task_manager, on_demand, telemetry_span) = + let (client, backend, keystore_container, mut task_manager, on_demand) = sc_service::new_light_parts::(&config)?; config.network.extra_sets.push(sc_finality_grandpa::grandpa_peers_set_config()); @@ -327,7 +324,6 @@ pub fn new_light(mut config: Configuration) -> Result network, network_status_sinks, system_rpc_tx, - telemetry_span, })?; network_starter.start_network(); diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 838a6fb90219a..2f6a1339da412 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -34,7 +34,7 @@ use sp_runtime::traits::Block as BlockT; use futures::prelude::*; use sc_client_api::{ExecutorProvider, RemoteBackend}; use node_executor::Executor; -use sc_telemetry::{TelemetryConnectionNotifier, TelemetrySpan}; +use sc_telemetry::TelemetryConnectionNotifier; type FullClient = sc_service::TFullClient; type FullBackend = sc_service::TFullBackend; @@ -58,10 +58,9 @@ pub fn new_partial(config: &Configuration) -> Result, ), grandpa::SharedVoterState, - Option, ) >, ServiceError> { - let (client, backend, keystore_container, task_manager, telemetry_span) = + let (client, backend, keystore_container, task_manager) = sc_service::new_full_parts::(&config)?; let client = Arc::new(client); @@ -160,7 +159,7 @@ pub fn new_partial(config: &Configuration) -> Result Result<( Arc::Hash>>, Arc>> ), ServiceError> { - let (client, backend, keystore_container, mut task_manager, on_demand, telemetry_span) = + let (client, backend, keystore_container, mut task_manager, on_demand) = sc_service::new_light_parts::(&config)?; config.network.extra_sets.push(grandpa::grandpa_peers_set_config()); @@ -446,7 +444,6 @@ pub fn new_light_base(mut config: Configuration) -> Result<( config, backend, network_status_sinks, system_rpc_tx, network: network.clone(), task_manager: &mut task_manager, - telemetry_span, })?; Ok(( diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index f1d5c6a86b06e..a42dba84dfeae 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -268,6 +268,7 @@ fn node_config Date: Fri, 22 Jan 2021 21:02:50 +0100 Subject: [PATCH 14/16] Revert "Test code" This reverts commit 333806b2fe1626efaa2691f9f44d0b4dd979bc36. --- bin/node/cli/src/command.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index fef30387b0806..fcb8b6f0085e0 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -73,18 +73,10 @@ pub fn run() -> Result<()> { None => { let runner = cli.create_runner(&cli.run)?; runner.run_node_until_exit(|config| async move { - // TODO revert after test - let config2 = cli.create_configuration(&cli.run, config.task_executor.clone(), config.telemetry_handle.clone()).unwrap(); - let mut task_manager = match config.role { - Role::Light => service::new_light(config).unwrap(), - _ => service::new_full(config).unwrap(), - }; - let task_manager2 = match config2.role { - Role::Light => service::new_light(config2).unwrap(), - _ => service::new_full(config2).unwrap(), - }; - task_manager.add_child(task_manager2); - Ok(task_manager) + match config.role { + Role::Light => service::new_light(config), + _ => service::new_full(config), + }.map_err(sc_cli::Error::Service) }) } Some(Subcommand::Inspect(cmd)) => { From 264a9b8125ec3c69d94451499d591e71ca4bba47 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Wed, 27 Jan 2021 12:31:11 +0100 Subject: [PATCH 15/16] Update client/service/src/task_manager/mod.rs Co-authored-by: David --- client/service/src/task_manager/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index cb66e2b431f0d..097eeac79e311 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -125,14 +125,14 @@ impl SpawnTaskHandle { }; let future: BoxFuture<()> = if let Some(telemetry_span) = self.telemetry_span.clone() { - // this will preserve the telemetry span and by extension all of its parents as they - // are linked together upon creation - // this is used by sc_telemetry::layer::TelemetryLayer + // This will preserve the telemetry span and by extension all of its + // parents as they are linked together upon creation. + // Used by [`sc_telemetry::layer::TelemetryLayer`]. Box::pin(future.instrument(telemetry_span.span())) } else { - // this will preserve the current span (if any) and by extension all of its parents - // as they are linked together upon creation - // this is used by sc_tracing::logging::prefix_logs_with + // This will preserve the current span (if any) and by extension all + // of its parents as they are linked together upon creation. + // Used by [`sc_tracing::logging::prefix_logs_with`]. Box::pin(future.in_current_span()) }; From 92d83421b4bda9c3ca404b76c8a22345c51660f0 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Fri, 29 Jan 2021 10:58:36 +0100 Subject: [PATCH 16/16] Better & simpler solution --- client/service/src/task_manager/mod.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/client/service/src/task_manager/mod.rs b/client/service/src/task_manager/mod.rs index 097eeac79e311..c8b7e2547c773 100644 --- a/client/service/src/task_manager/mod.rs +++ b/client/service/src/task_manager/mod.rs @@ -124,19 +124,11 @@ impl SpawnTaskHandle { } }; - let future: BoxFuture<()> = if let Some(telemetry_span) = self.telemetry_span.clone() { - // This will preserve the telemetry span and by extension all of its - // parents as they are linked together upon creation. - // Used by [`sc_telemetry::layer::TelemetryLayer`]. - Box::pin(future.instrument(telemetry_span.span())) - } else { - // This will preserve the current span (if any) and by extension all - // of its parents as they are linked together upon creation. - // Used by [`sc_tracing::logging::prefix_logs_with`]. - Box::pin(future.in_current_span()) + let join_handle = { + let _span = self.telemetry_span.as_ref().map(|s| s.enter()); + self.executor.spawn(Box::pin(future.in_current_span()), task_type) }; - let join_handle = self.executor.spawn(future, task_type); let mut task_notifier = self.task_notifier.clone(); self.executor.spawn( Box::pin(async move {