[Metrics] Track synced blocks. #3231
Conversation
| @@ -461,6 +461,9 @@ impl<N: Network> BlockSync<N> { | |||
| let is_synced = num_blocks_behind <= max_blocks_behind; | |||
| // Update the sync status. | |||
| self.is_block_synced.store(is_synced, Ordering::SeqCst); | |||
| // Update the `IS_SYNCED` metric. | |||
| #[cfg(feature = "metrics")] | |||
| metrics::gauge(metrics::bft::IS_SYNCED, is_synced); | |||
There was a problem hiding this comment.
This looks good! One thing that might make a difference in recording is that metrics are "registered" before they are seen, which basically initializes all metrics in the names folder at 0. Its probably a trivial difference, as for this metric we initialize it and then update sync status pretty quickly, however just wanted to point out that it will be initialized at 0. If we only want to start recording the metric the first time this line of code is hit, the metric name should be removed from the names.rs file
There was a problem hiding this comment.
I think it should be fine since is_block_synced is initialized with Default::default() == false. So we'll have the property that is_block_synced matches IS_SYNCED.
There was a problem hiding this comment.
Doesnt it update when its initializing the sync module? I think it should reflect the current state 🤔
There was a problem hiding this comment.
@miazn - reading further, this does get triggered with the sync is initialized. You're right.
d0cd
requested review from
iamalwaysuncomfortable,
miazn,
howardwu and
raychu86
This PR introduces a metric to track the number of blocks added through syncing.