This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Rework telemetry to replace the use of tracing with an object we pass around #8143
Merged
Merged
Changes from 76 commits
Commits
Show all changes
108 commits
Select commit
Hold shift + click to select a range
39e3c9a
Rework sc-telemetry to not use tracing internally
cecton 7d94122
Issue with telemetry usage in ProfilingLayer
cecton 9aa9a6a
WIP
cecton af2c532
WIP
cecton 942addb
WIP
cecton 29bda72
WIP
cecton faad155
WIP
cecton d09bef4
Oh no this is terrible
cecton 4f3d24b
WIP
cecton 6a8040f
WIP
cecton 244982a
WIP
cecton 5fbc85c
WIP
cecton e06d06d
Rename TelemetryHandle to TelemetryWorkerHandle
cecton e3a4c64
Rename Telemetry to TelemetryHandle
cecton c9a60aa
WIP
cecton 1b099fe
CLEANUP
cecton 98ccd2a
CLEANUP
cecton d1055f4
CLEANUP
cecton 280bd25
CLEANUP
cecton 14bdec1
CLEANUP
cecton 9ed97f1
CLEANUP
cecton b88989d
CLEANUP
cecton 420f357
Fix todo
cecton 391efa2
CLEANUP
cecton 69aa516
fmt
cecton 9148541
Adapt more code
cecton 8210e35
Adapt more code
cecton a7bec3d
Adapt even MORE code
cecton f399174
Adapt more code...
cecton eb180de
Doc
cecton f3f7c78
Instantiate telemetry worker from sc-cli instead of sc-tracing
cecton 985eb93
Update doc
cecton 4eca8bf
Rename send() to send_telemetry()
cecton dabdb14
Fix doc
cecton 3f9e73e
Remove IdGenerator and global variable
cecton eaeb79a
Pass the telemetry worker handle down from the runner instead of the …
cecton 6f0563f
Moving TelemetryWorker creation to the user
cecton 81f7df8
Make Tracing bounded channel and use it in telemetry
cecton 6dd32f4
start_telemetry now return errors instead of logging
cecton c7139fd
Automatic split line
cecton 993040b
Forgot /g at the end again...
cecton 5581a85
Missed replacements
cecton 54390e7
Use telemetry in trait fn instead of fn argument
cecton 30d11b5
Remove ClientTelemetry
cecton 147d5d4
Adapt code
cecton 2759a3f
Merge commit 2c99434c8bab9230040cebe727e687f5a4c6d229 (no conflict)
cecton 9d88fb2
Merge commit f53d72f0fdadf56c34f3199bea96c652633f82a9 (conflicts)
cecton f708211
Apply suggestions from code review
cecton 1401067
Added missing license
cecton afd6056
Fix indentation with spaces
cecton 526d877
Improve doc
cecton fc1ce16
Use Mutex and make Telemetry work with immutable
cecton c1fac06
Optimization?
cecton 8aebf81
Replace code using mem with Vec::retain
cecton 87cc352
Merge commit fd88fbf7e05b3bea3e8324e2661c586353b48a43 (no conflict)
cecton 71c0193
Merge commit 59494855205374e8f27fb131425dfd78897a9298 (conflicts)
cecton 2627e25
Merge commit 1c34ccf6c88188b173286bf28c279409eff0eebb (no conflict)
cecton 640e4e3
Merge commit 7a6d60de2d19c67831859a9014e315f5ec7a3870 (conflicts)
cecton c1d2eb2
Merge commit 1febf99d6385ff9ff934a9b16a53234b4858bf5c (no conflict)
cecton e306664
Revert "Optimization?"
cecton a95c2c4
Fixing stuff
cecton ab5179e
Fixing other stuff
cecton 1a47b96
Fix more stuff
cecton 0587055
Fix browser telemetry
cecton ee8eae6
Temporary changes
cecton 0f6bc8f
WIP
cecton 3e2c066
debug
cecton 7ac7728
Forgot to wrap the payload
cecton 49541ab
Revert "Temporary changes"
cecton 133770f
Fix some tests
cecton 7bb9bf5
Fix doc test
cecton cd8d77b
Fix tests
cecton 0808536
Fix more tests
cecton 3b23031
Fixes
cecton 4d02b5f
Fix tests
cecton 462b20e
Merge commit a6ff3d3be40de2496a705a237483b2c3a541b143 (no conflict)
cecton 1b5df60
Revert TracingBounded channel
cecton 9dae7e4
Replace channel with futures' mpsc channel
cecton 4ad1643
Apply suggestions from code review
cecton 31e08fa
Apply suggestion
cecton 0650ca9
Apply suggestions from code review
cecton 13c62f3
WIP
cecton 051985e
Fix invalid unreachable!()
cecton 8d68021
Remove global_counter
cecton 0195902
Do not create telemetry worker if not necessary
cecton d3468c9
Merge commit e5e8197f4530a2ea6b0ee419699ad45379e96774 (no conflict)
cecton 5f61065
Merge commit a8c2bc66ea8667a1dcbafa13ce184b020bd6f84b (no conflict)
cecton 8e0dfd5
Revert finality-grandpa
cecton 560bb31
WIP
cecton ddd2bf7
WIP
cecton a67f8f4
WIP
cecton 9b6851c
WIP
cecton 3dde350
WIP
cecton ae35cf7
WIP
cecton b48c01e
WIP
cecton d591bb4
WIP
cecton 8228e0c
WIP
cecton f10ae74
WIP
cecton 450cebe
WIP
cecton 9fad6f8
WIP
cecton a0d2206
Fix doc test
cecton e1881e0
Add a method with_transport for when transport is needed
cecton 0c6e384
Merge commit adca4983ce51652600fdee4415a81e621fceee6f (no conflict)
cecton d545ebd
Merge commit 79ddc796ed130810ac9e620493154c3cac9f290a (conflicts)
cecton f8df9a4
Merge commit a94749cb5321cbc43403ead66a1c915236720f8d (no conflict)
cecton 0fcce2e
Merge commit 8fca15b79bef42c53faad849652c8f5a9936b369 (conflicts)
cecton 81ba8cb
Merge commit ef50a44531b807685c9fc17adbaded9998acce39 (no conflict)
cecton 179f310
Merge commit 3adefdcea47a26c4904651e108129fd6b6ce132a (conflicts)
cecton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ 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; | ||
use sc_telemetry::{Telemetry, TelemetryWorker}; | ||
|
||
// Our native executor instance. | ||
native_executor_instance!( | ||
|
@@ -37,6 +37,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen | |
AuraPair | ||
>, | ||
sc_finality_grandpa::LinkHalf<Block, FullClient, FullSelectChain>, | ||
Option<Telemetry>, | ||
) | ||
>, ServiceError> { | ||
if config.keystore_remote.is_some() { | ||
|
@@ -45,10 +46,20 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen | |
} | ||
let inherent_data_providers = sp_inherents::InherentDataProviders::new(); | ||
|
||
let telemetry_worker = TelemetryWorker::new(16, None)?; | ||
let telemetry = config.telemetry_endpoints.clone() | ||
.filter(|x| !x.is_empty()) | ||
.map(|endpoints| telemetry_worker.handle().new_telemetry(endpoints)); | ||
|
||
let (client, backend, keystore_container, task_manager) = | ||
sc_service::new_full_parts::<Block, RuntimeApi, Executor>(&config)?; | ||
sc_service::new_full_parts::<Block, RuntimeApi, Executor>( | ||
&config, | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
let client = Arc::new(client); | ||
|
||
task_manager.spawn_handle().spawn("telemetry", telemetry_worker.run()); | ||
|
||
let select_chain = sc_consensus::LongestChain::new(backend.clone()); | ||
|
||
let transaction_pool = sc_transaction_pool::BasicPool::new_full( | ||
|
@@ -60,7 +71,10 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen | |
); | ||
|
||
let (grandpa_block_import, grandpa_link) = sc_finality_grandpa::block_import( | ||
client.clone(), &(client.clone() as Arc<_>), select_chain.clone(), | ||
client.clone(), | ||
&(client.clone() as Arc<_>), | ||
select_chain.clone(), | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
let aura_block_import = sc_consensus_aura::AuraBlockImport::<_, _, _, AuraPair>::new( | ||
|
@@ -76,6 +90,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen | |
&task_manager.spawn_essential_handle(), | ||
config.prometheus_registry(), | ||
sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone()), | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
Ok(sc_service::PartialComponents { | ||
|
@@ -87,7 +102,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen | |
select_chain, | ||
transaction_pool, | ||
inherent_data_providers, | ||
other: (aura_block_import, grandpa_link), | ||
other: (aura_block_import, grandpa_link, telemetry), | ||
}) | ||
} | ||
|
||
|
@@ -109,7 +124,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
select_chain, | ||
transaction_pool, | ||
inherent_data_providers, | ||
other: (block_import, grandpa_link), | ||
other: (block_import, grandpa_link, mut telemetry), | ||
} = new_partial(&config)?; | ||
|
||
if let Some(url) = &config.keystore_remote { | ||
|
@@ -163,10 +178,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
}) | ||
}; | ||
|
||
let telemetry_span = TelemetrySpan::new(); | ||
let _telemetry_span_entered = telemetry_span.enter(); | ||
|
||
let (_rpc_handlers, telemetry_connection_notifier) = sc_service::spawn_tasks( | ||
let _rpc_handlers = sc_service::spawn_tasks( | ||
sc_service::SpawnTasksParams { | ||
network: network.clone(), | ||
client: client.clone(), | ||
|
@@ -180,7 +192,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
network_status_sinks, | ||
system_rpc_tx, | ||
config, | ||
telemetry_span: Some(telemetry_span.clone()), | ||
telemetry: telemetry.as_mut(), | ||
}, | ||
)?; | ||
|
||
|
@@ -190,6 +202,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
client.clone(), | ||
transaction_pool, | ||
prometheus_registry.as_ref(), | ||
telemetry.as_ref().map(|x| x.handle()), | ||
); | ||
|
||
let can_author_with = | ||
|
@@ -207,6 +220,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
backoff_authoring_blocks, | ||
keystore_container.sync_keystore(), | ||
can_author_with, | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
// the AURA authoring task is considered essential, i.e. if it | ||
|
@@ -230,6 +244,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
observer_enabled: false, | ||
keystore, | ||
is_authority: role.is_authority(), | ||
telemetry: telemetry.as_ref().map(|x| x.handle()), | ||
}; | ||
|
||
if enable_grandpa { | ||
|
@@ -243,10 +258,10 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
config: grandpa_config, | ||
link: grandpa_link, | ||
network, | ||
telemetry_on_connect: telemetry_connection_notifier.map(|x| x.on_connect_stream()), | ||
voting_rule: sc_finality_grandpa::VotingRulesBuilder::default().build(), | ||
prometheus_registry, | ||
shared_voter_state: SharedVoterState::empty(), | ||
telemetry: telemetry.as_ref().map(|x| x.handle()), | ||
}; | ||
|
||
// the GRANDPA voter task is considered infallible, i.e. | ||
|
@@ -263,8 +278,18 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
|
||
/// Builds a new service for a light client. | ||
pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> { | ||
let telemetry_worker = TelemetryWorker::new(16, None)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
let mut telemetry = config.telemetry_endpoints.clone() | ||
.filter(|x| !x.is_empty()) | ||
.map(|endpoints| telemetry_worker.handle().new_telemetry(endpoints)); | ||
|
||
let (client, backend, keystore_container, mut task_manager, on_demand) = | ||
sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?; | ||
sc_service::new_light_parts::<Block, RuntimeApi, Executor>( | ||
&config, | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
task_manager.spawn_handle().spawn("telemetry", telemetry_worker.run()); | ||
|
||
config.network.extra_sets.push(sc_finality_grandpa::grandpa_peers_set_config()); | ||
|
||
|
@@ -282,6 +307,7 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
client.clone(), | ||
&(client.clone() as Arc<_>), | ||
select_chain.clone(), | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
let aura_block_import = sc_consensus_aura::AuraBlockImport::<_, _, _, AuraPair>::new( | ||
|
@@ -298,6 +324,7 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
&task_manager.spawn_essential_handle(), | ||
config.prometheus_registry(), | ||
sp_consensus::NeverCanAuthor, | ||
telemetry.as_ref().map(|x| x.handle()), | ||
)?; | ||
|
||
let (network, network_status_sinks, system_rpc_tx, network_starter) = | ||
|
@@ -317,9 +344,6 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
); | ||
} | ||
|
||
let telemetry_span = TelemetrySpan::new(); | ||
let _telemetry_span_entered = telemetry_span.enter(); | ||
|
||
sc_service::spawn_tasks(sc_service::SpawnTasksParams { | ||
remote_blockchain: Some(backend.remote_blockchain()), | ||
transaction_pool, | ||
|
@@ -333,7 +357,7 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> | |
network, | ||
network_status_sinks, | ||
system_rpc_tx, | ||
telemetry_span: Some(telemetry_span.clone()), | ||
telemetry: telemetry.as_mut(), | ||
})?; | ||
|
||
network_starter.start_network(); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we create a worker, if there are maybe no endpoints to connect to?