Add simple telemetry feed #284
Conversation
|
Here are all messages defined there https://github.com/paritytech/substrate-telemetry/blob/b25f3fbf2270c009aafebefa10bdcc034b8d1cc0/backend/telemetry_core/src/feed_message.rs#L105-L125 Then we can check which telemetry events are not being processed and add additional support. |
AndreiEres
force-pushed
the
AndreiEres/telemetry-feed
branch
from
bbeb431 to
45ec085
Compare
| pub fn from_bytes(bytes: &[u8]) -> color_eyre::Result<Vec<TelemetryFeed>> { | ||
| let v: Vec<&RawValue> = serde_json::from_slice(bytes)?; | ||
|
|
||
| let mut feed_messages = vec![]; |
There was a problem hiding this comment.
Nit: we might probably set a size hint here for feed_messages vector.
| async fn on_shutdown(shutdown_tx: Sender<()>) { | ||
| signal::ctrl_c().await.unwrap(); | ||
| let _ = shutdown_tx.send(()); | ||
| } |
introspector/src/whois/mod.rs
Outdated
|
|
||
| async fn watch(mut update: Receiver<TelemetryEvent>) { | ||
| loop { | ||
| match update.try_recv() { |
There was a problem hiding this comment.
I would consider it as a bad pattern: instead of polling the channel with update.recv we are querying it manually with added timer. It introduces additional latency, extra timer, and gives nothing compared to the simple recv.await call. I would also suggest to use priority-channel instead of mpsc::channel to unify the code.
AndreiEres
force-pushed
the
AndreiEres/telemetry-feed
branch
from
45ec085 to
596dd33
Compare
| } | ||
|
|
||
| for message in feed.unwrap() { | ||
| info!("[telemetry] {:?}", message); |
There was a problem hiding this comment.
I think this will spam a lot should be debug.
| Self { consumers } | ||
| } | ||
|
|
||
| // Sets up per websocket tasks to handle updates and reconnects on errors. |
There was a problem hiding this comment.
This comment seems to not reflect what the function does. This is actually the task.
| pub url: String, | ||
| // Chain's genesis hash | ||
| #[clap(name = "chain", long)] | ||
| pub chain_hash: H256, |
There was a problem hiding this comment.
Telemetry in the beginning sends events only about added chains. We need to subscribe to one of them to start getting its events.
There was a problem hiding this comment.
Ah, this looks like usability trouble. In practice do we have multiple chains on the same backend ?
|
|
||
| #[derive(Clone, Debug, Parser)] | ||
| #[clap(rename_all = "kebab-case")] | ||
| pub(crate) struct TelemetryOptions { |
There was a problem hiding this comment.
So this is actually a very simple CLI telemetry frontend. I'd add at least some filter by node id, otherwise the output would be too much for a large net.
introspector/src/telemetry/mod.rs
Outdated
| let mut node_id: Option<FeedNodeId> = None; | ||
|
|
||
| loop { | ||
| match update.try_recv() { |
There was a problem hiding this comment.
Why not just do update.recv().await and block until a message arrives or an error happens.
AndreiEres
force-pushed
the
AndreiEres/telemetry-feed
branch
from
85479b7 to
1788771
Compare
Related to #283
Adds a subscription to telemetry feed. Could be used as a scaffolding for further work.