-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Allow adding a prefix to the informant #6174
Changes from 19 commits
6c3a2c0
9c2e726
c966d4b
a3c306e
d3c1d52
cd86c58
6c0029b
ed89283
ad18c29
ff258dc
f5cf997
1b02761
b88ed95
b81f8ff
a04e788
cb8e402
5dceb40
bf7e144
d8daabb
4133e3c
23d775e
f006ccf
b5dd8d8
e32866a
1b38e33
5523518
36c4148
9728c25
036a1ea
f494058
6ba7627
c1ec56a
d334904
f73afa4
89a9c2b
a82f2cb
f75b06a
f1bcacb
38a4f27
bb7c7c2
89b5df8
File filter
Filter by extension
Conversations
Jump to
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.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,90 +19,138 @@ | |
//! Console informant. Prints sync progress and block events. Runs on the calling thread. | ||
|
||
use ansi_term::Colour; | ||
use sc_client_api::{BlockchainEvents, UsageProvider}; | ||
use futures::prelude::*; | ||
use log::{info, warn, trace}; | ||
use sp_runtime::traits::Header; | ||
use sc_service::AbstractService; | ||
use log::{info, trace, warn}; | ||
use parity_util_mem::MallocSizeOf; | ||
use sc_client_api::{BlockchainEvents, UsageProvider}; | ||
use sc_network::{network_state::NetworkState, NetworkStatus}; | ||
use sp_blockchain::HeaderMetadata; | ||
use sp_runtime::traits::{Block as BlockT, Header}; | ||
use sp_transaction_pool::TransactionPool; | ||
use sp_utils::mpsc::TracingUnboundedReceiver; | ||
use std::fmt::Display; | ||
use std::sync::Arc; | ||
use std::time::Duration; | ||
|
||
mod display; | ||
|
||
/// The format to print telemetry output in. | ||
#[derive(PartialEq)] | ||
pub enum OutputFormat { | ||
Coloured, | ||
Plain, | ||
#[derive(Clone)] | ||
pub struct OutputFormat { | ||
/// Use colors to display the logs | ||
pub colors: bool, | ||
cecton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// Add a prefix before every log line | ||
pub prefix: String, | ||
Comment on lines
+42
to
+43
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. It should here also again should be made aware that this does not uses a whitespace in front of. While I'm still not sure if we should not add the whitespace by default for a prefix. Another point, please create the following:
This will make it possible for the user to supply a style as well. 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. We should not add the whitespace imo. People might want to use something like 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. I think the style is not really useful as people can put it in the String already. Do you really want it? 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. Fair point |
||
} | ||
|
||
/// Creates an informant in the form of a `Future` that must be polled regularly. | ||
pub fn build(service: &impl AbstractService, format: OutputFormat) -> impl futures::Future<Output = ()> { | ||
let client = service.client(); | ||
let pool = service.transaction_pool(); | ||
|
||
let mut display = display::InformantDisplay::new(format); | ||
|
||
let display_notifications = service | ||
.network_status(Duration::from_millis(5000)) | ||
.for_each(move |(net_status, _)| { | ||
let info = client.usage_info(); | ||
if let Some(ref usage) = info.usage { | ||
trace!(target: "usage", "Usage statistics: {}", usage); | ||
} else { | ||
macro_rules! build { | ||
($client:expr, $network_status_stream_builder:expr, $pool:expr, $format:expr) => {{ | ||
let mut display = display::InformantDisplay::new($format.clone()); | ||
|
||
let client_1 = $client.clone(); | ||
let display_notifications = $network_status_stream_builder(Duration::from_millis(5000)) | ||
.for_each(move |(net_status, _)| { | ||
let info = client_1.usage_info(); | ||
if let Some(ref usage) = info.usage { | ||
trace!(target: "usage", "Usage statistics: {}", usage); | ||
} else { | ||
trace!( | ||
target: "usage", | ||
"Usage statistics not displayed as backend does not provide it", | ||
) | ||
} | ||
#[cfg(not(target_os = "unknown"))] | ||
trace!( | ||
target: "usage", | ||
"Usage statistics not displayed as backend does not provide it", | ||
) | ||
"Subsystems memory [txpool: {} kB]", | ||
parity_util_mem::malloc_size(&*$pool) / 1024, | ||
); | ||
display.display(&info, net_status); | ||
future::ready(()) | ||
}); | ||
|
||
let mut last_best = { | ||
let info = $client.usage_info(); | ||
Some((info.chain.best_number, info.chain.best_hash)) | ||
}; | ||
|
||
let display_block_import = $client.import_notification_stream().for_each(move |n| { | ||
// detect and log reorganizations. | ||
if let Some((ref last_num, ref last_hash)) = last_best { | ||
if n.header.parent_hash() != last_hash && n.is_new_best { | ||
let maybe_ancestor = sp_blockchain::lowest_common_ancestor( | ||
&*$client, | ||
last_hash.clone(), | ||
n.hash, | ||
); | ||
|
||
match maybe_ancestor { | ||
Ok(ref ancestor) if ancestor.hash != *last_hash => info!( | ||
"♻️ Reorg on #{},{} to #{},{}, common ancestor #{},{}", | ||
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. Prefix? And also does not checks if color output is enabled. 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. Prefix => done in 36c4148
That I wanted to discuss with you. I propose that we drop the color check entirely and use only colors. The reason is that the colors are strip anyway. It will reduce the code size and maintenance a bit. |
||
Colour::Red.bold().paint(format!("{}", last_num)), last_hash, | ||
Colour::Green.bold().paint(format!("{}", n.header.number())), n.hash, | ||
Colour::White.bold().paint(format!("{}", ancestor.number)), ancestor.hash, | ||
), | ||
Ok(_) => {}, | ||
Err(e) => warn!("Error computing tree route: {}", e), | ||
} | ||
} | ||
} | ||
|
||
if n.is_new_best { | ||
last_best = Some((n.header.number().clone(), n.hash.clone())); | ||
} | ||
#[cfg(not(target_os = "unknown"))] | ||
trace!( | ||
target: "usage", | ||
"Subsystems memory [txpool: {} kB]", | ||
parity_util_mem::malloc_size(&*pool) / 1024, | ||
|
||
info!( | ||
target: "substrate", "✨ {}Imported #{} ({})", | ||
$format.prefix, Colour::White.bold().paint(format!("{}", n.header.number())), n.hash, | ||
); | ||
display.display(&info, net_status); | ||
future::ready(()) | ||
}); | ||
|
||
let client = service.client(); | ||
let mut last_best = { | ||
let info = client.usage_info(); | ||
Some((info.chain.best_number, info.chain.best_hash)) | ||
}; | ||
|
||
let display_block_import = client.import_notification_stream().for_each(move |n| { | ||
// detect and log reorganizations. | ||
if let Some((ref last_num, ref last_hash)) = last_best { | ||
if n.header.parent_hash() != last_hash && n.is_new_best { | ||
let maybe_ancestor = sp_blockchain::lowest_common_ancestor( | ||
&*client, | ||
last_hash.clone(), | ||
n.hash, | ||
); | ||
|
||
match maybe_ancestor { | ||
Ok(ref ancestor) if ancestor.hash != *last_hash => info!( | ||
"♻️ Reorg on #{},{} to #{},{}, common ancestor #{},{}", | ||
Colour::Red.bold().paint(format!("{}", last_num)), last_hash, | ||
Colour::Green.bold().paint(format!("{}", n.header.number())), n.hash, | ||
Colour::White.bold().paint(format!("{}", ancestor.number)), ancestor.hash, | ||
), | ||
Ok(_) => {}, | ||
Err(e) => warn!("Error computing tree route: {}", e), | ||
} | ||
} | ||
} | ||
|
||
if n.is_new_best { | ||
last_best = Some((n.header.number().clone(), n.hash.clone())); | ||
} | ||
future::join( | ||
display_notifications, | ||
display_block_import | ||
).map(|_| ()) | ||
}} | ||
} | ||
|
||
info!(target: "substrate", "✨ Imported #{} ({})", Colour::White.bold().paint(format!("{}", n.header.number())), n.hash); | ||
future::ready(()) | ||
}); | ||
/// Creates an informant in the form of a `Future` that must be polled regularly. | ||
cecton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[cfg(not(target_os = "unknown"))] | ||
pub fn build<B: BlockT, C>( | ||
client: Arc<C>, | ||
network_status_stream_builder: impl FnOnce( | ||
Duration, | ||
) -> TracingUnboundedReceiver<( | ||
NetworkStatus<B>, | ||
NetworkState, | ||
)>, | ||
pool: Arc<impl TransactionPool + MallocSizeOf>, | ||
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. Just because of this Please create a trait that based on the target os has different super trait requirements. 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. Ohhh 💡 nice!! Thanks 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. Done in 9728c25 |
||
format: OutputFormat, | ||
) -> impl futures::Future<Output = ()> | ||
where | ||
C: UsageProvider<B> + HeaderMetadata<B> + BlockchainEvents<B>, | ||
<C as HeaderMetadata<B>>::Error: Display, | ||
{ | ||
build!(client, network_status_stream_builder, pool, format) | ||
} | ||
|
||
future::join( | ||
display_notifications, | ||
display_block_import | ||
).map(|_| ()) | ||
/// Creates an informant in the form of a `Future` that must be polled regularly. | ||
#[cfg(target_os = "unknown")] | ||
pub fn build<B: BlockT, C>( | ||
client: Arc<C>, | ||
network_status_stream_builder: impl FnOnce( | ||
Duration, | ||
) -> TracingUnboundedReceiver<( | ||
NetworkStatus<B>, | ||
NetworkState, | ||
)>, | ||
pool: Arc<impl TransactionPool>, | ||
format: OutputFormat, | ||
) -> impl futures::Future<Output = ()> | ||
where | ||
C: UsageProvider<B> + HeaderMetadata<B> + BlockchainEvents<B>, | ||
<C as HeaderMetadata<B>>::Error: Display, | ||
{ | ||
build!(client, network_status_stream_builder, pool, format) | ||
} |
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.
if self.format.colors
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.
Done in e32866a