Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Add rejected blocks metric #6006

Closed
wants to merge 1 commit into from
Closed

Add rejected blocks metric #6006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions client/service/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ use sc_executor::{NativeExecutor, NativeExecutionDispatch, RuntimeInfo};
use std::{
collections::HashMap,
io::{Read, Write, Seek},
marker::PhantomData, sync::Arc, pin::Pin
marker::PhantomData,
pin::Pin,
sync::Arc,
sync::atomic::{AtomicU64, Ordering},
};
use wasm_timer::SystemTime;
use sc_telemetry::{telemetry, SUBSTRATE_INFO};
Expand Down Expand Up @@ -96,8 +99,9 @@ pub struct ServiceBuilder<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp,
transaction_pool: Arc<TExPool>,
rpc_extensions: TRpc,
remote_backend: Option<Arc<dyn RemoteBlockchain<TBl>>>,
marker: PhantomData<(TBl, TRtApi)>,
block_announce_validator_builder: Option<Box<dyn FnOnce(Arc<TCl>) -> Box<dyn BlockAnnounceValidator<TBl> + Send> + Send>>,
pub (crate) rejected_blocks_number: Arc<AtomicU64>,
marker: PhantomData<(TBl, TRtApi)>,
}

/// Full client type.
Expand Down Expand Up @@ -312,6 +316,7 @@ impl ServiceBuilder<(), (), (), (), (), (), (), (), (), (), ()> {
rpc_extensions: Default::default(),
remote_backend: None,
block_announce_validator_builder: None,
rejected_blocks_number: Arc::new(AtomicU64::new(0)),
marker: PhantomData,
})
}
Expand Down Expand Up @@ -395,6 +400,7 @@ impl ServiceBuilder<(), (), (), (), (), (), (), (), (), (), ()> {
rpc_extensions: Default::default(),
remote_backend: Some(remote_blockchain),
block_announce_validator_builder: None,
rejected_blocks_number: Arc::new(AtomicU64::new(0)),
marker: PhantomData,
})
}
Expand Down Expand Up @@ -468,6 +474,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -512,6 +519,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -550,6 +558,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -614,6 +623,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -677,6 +687,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -705,6 +716,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: self.block_announce_validator_builder,
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -733,6 +745,7 @@ impl<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
rpc_extensions: self.rpc_extensions,
remote_backend: self.remote_backend,
block_announce_validator_builder: Some(Box::new(block_announce_validator_builder)),
rejected_blocks_number: self.rejected_blocks_number,
marker: self.marker,
})
}
Expand Down Expand Up @@ -851,6 +864,7 @@ ServiceBuilder<
rpc_extensions,
remote_backend,
block_announce_validator_builder,
rejected_blocks_number,
} = self;

sp_session::generate_initial_session_keys(
Expand Down Expand Up @@ -1065,6 +1079,7 @@ ServiceBuilder<
&info,
&transaction_pool_.status(),
&net_status,
rejected_blocks_number.load(Ordering::Relaxed)
);
ready(())
});
Expand Down
9 changes: 8 additions & 1 deletion client/service/src/chain_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ use sc_executor::{NativeExecutor, NativeExecutionDispatch};
use sp_core::storage::{StorageKey, well_known_keys, ChildInfo, Storage, StorageChild, StorageMap};
use sc_client_api::{StorageProvider, BlockBackend, UsageProvider};

use std::{io::{Read, Write, Seek}, pin::Pin, collections::HashMap};
use std::{
collections::HashMap,
io::{Read, Write, Seek},
pin::Pin,
sync::atomic::Ordering,
};

/// Build a chain spec json
pub fn build_spec(spec: &dyn ChainSpec, raw: bool) -> error::Result<String> {
Expand Down Expand Up @@ -104,6 +109,7 @@ impl<
let mut count = None::<u64>;
let mut read_block_count = 0;
let mut link = WaitLink::new();
let rejected_blocks_number = self.rejected_blocks_number.clone();

// Importing blocks is implemented as a future, because we want the operation to be
// interruptible.
Expand Down Expand Up @@ -175,6 +181,7 @@ impl<
"Stopping after #{} blocks because of an error",
link.imported_blocks,
);
rejected_blocks_number.fetch_add(1, Ordering::Relaxed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you've confused the location here. This is the import-cli command. In this context we a) don't have metrics activated, nor are we sending them anywhere and b) fail immediately on the error, thus the number would always be 1 and never actually increase.

return std::task::Poll::Ready(Ok(()));
}

Expand Down
8 changes: 8 additions & 0 deletions client/service/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ struct PrometheusMetrics {
block_height: GaugeVec<U64>,
number_leaves: Gauge<U64>,
ready_transactions_number: Gauge<U64>,
rejected_blocks_number: Gauge<U64>,

// I/O
network_per_sec_bytes: GaugeVec<U64>,
Expand Down Expand Up @@ -125,6 +126,10 @@ impl PrometheusMetrics {
"ready_transactions_number", "Number of transactions in the ready queue",
)?, registry)?,

rejected_blocks_number: register(Gauge::new(
"rejected_blocks_number", "Number of blocks rejected as invalid",
)?, registry)?,

// I/ O
network_per_sec_bytes: register(GaugeVec::new(
Opts::new("network_per_sec_bytes", "Networking bytes per second"),
Expand Down Expand Up @@ -313,6 +318,7 @@ impl MetricsService {
info: &ClientInfo<T>,
txpool_status: &PoolStatus,
net_status: &NetworkStatus<T>,
rejected_blocks_number: u64,
) {

let best_number = info.chain.best_number.saturated_into::<u64>();
Expand Down Expand Up @@ -387,6 +393,8 @@ impl MetricsService {

metrics.ready_transactions_number.set(txpool_status.ready as u64);

metrics.rejected_blocks_number.set(rejected_blocks_number);

if let Some(best_seen_block) = best_seen_block {
metrics.block_height.with_label_values(&["sync_target"]).set(best_seen_block);
}
Expand Down