Skip to content

Commit

Permalink
feat(starknet_batcher): storage height metric
Browse files Browse the repository at this point in the history
  • Loading branch information
yair-starkware committed Jan 13, 2025
1 parent cee26cb commit 778c278
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 5 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/starknet_batcher/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ async-trait.workspace = true
blockifier.workspace = true
chrono.workspace = true
indexmap.workspace = true
metrics.workspace = true
papyrus_config.workspace = true
papyrus_state_reader.workspace = true
papyrus_storage.workspace = true
Expand All @@ -32,7 +33,9 @@ validator.workspace = true
assert_matches.workspace = true
chrono = { workspace = true }
futures.workspace = true
starknet_infra_utils.workspace = true
mempool_test_utils.workspace = true
metrics-exporter-prometheus.workspace = true
mockall.workspace = true
rstest.workspace = true
starknet-types-core.workspace = true
Expand Down
12 changes: 10 additions & 2 deletions crates/starknet_batcher/src/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use crate::block_builder::{
BlockMetadata,
};
use crate::config::BatcherConfig;
use crate::metrics::RegisteredMetrics;
use crate::transaction_provider::{ProposeTransactionProvider, ValidateTransactionProvider};
use crate::utils::{
deadline_as_instant,
Expand Down Expand Up @@ -91,6 +92,8 @@ pub struct Batcher {
// Each stream is kept until SendProposalContent::Finish/Abort is received, or a new height is
// started.
validate_tx_streams: HashMap<ProposalId, InputStreamSender>,

metrics: RegisteredMetrics,
}

impl Batcher {
Expand All @@ -102,6 +105,9 @@ impl Batcher {
mempool_client: SharedMempoolClient,
block_builder_factory: Box<dyn BlockBuilderFactoryTrait>,
) -> Self {
let storage_height = storage_reader
.height()
.expect("Failed to get height from storage during batcher creation.");
Self {
config: config.clone(),
storage_reader,
Expand All @@ -115,6 +121,7 @@ impl Batcher {
executed_proposals: Arc::new(Mutex::new(HashMap::new())),
propose_tx_streams: HashMap::new(),
validate_tx_streams: HashMap::new(),
metrics: RegisteredMetrics::register_metrics(storage_height),
}
}

Expand Down Expand Up @@ -331,15 +338,15 @@ impl Batcher {
Ok(SendProposalContentResponse { response: ProposalStatus::Aborted })
}

fn get_height_from_storage(&mut self) -> BatcherResult<BlockNumber> {
fn get_height_from_storage(&self) -> BatcherResult<BlockNumber> {
self.storage_reader.height().map_err(|err| {
error!("Failed to get height from storage: {}", err);
BatcherError::InternalError
})
}

#[instrument(skip(self), err)]
pub async fn get_height(&mut self) -> BatcherResult<GetHeightResponse> {
pub async fn get_height(&self) -> BatcherResult<GetHeightResponse> {
let height = self.get_height_from_storage()?;
Ok(GetHeightResponse { height })
}
Expand Down Expand Up @@ -445,6 +452,7 @@ impl Batcher {
error!("Failed to commit proposal to storage: {}", err);
BatcherError::InternalError
})?;
self.metrics.increment_storage_height();
let mempool_result = self
.mempool_client
.commit_block(CommitBlockArgs { address_to_nonce, rejected_tx_hashes })
Expand Down
39 changes: 36 additions & 3 deletions crates/starknet_batcher/src/batcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;
use assert_matches::assert_matches;
use blockifier::abi::constants;
use indexmap::indexmap;
use metrics_exporter_prometheus::PrometheusBuilder;
use mockall::predicate::eq;
use rstest::rstest;
use starknet_api::block::{BlockHeaderWithoutHash, BlockInfo, BlockNumber};
Expand All @@ -29,6 +30,7 @@ use starknet_batcher_types::batcher_types::{
ValidateBlockInput,
};
use starknet_batcher_types::errors::BatcherError;
use starknet_infra_utils::metrics::parse_numeric_metric;
use starknet_l1_provider_types::MockL1ProviderClient;
use starknet_mempool_types::communication::MockMempoolClient;
use starknet_mempool_types::mempool_types::CommitBlockArgs;
Expand Down Expand Up @@ -160,6 +162,19 @@ async fn batcher_with_active_validate_block(
batcher
}

#[tokio::test]
async fn metrics_registered() {
let recorder = PrometheusBuilder::new().build_recorder();
let _g = metrics::set_default_local_recorder(&recorder);
let metrics_handle = recorder.handle();
let _batcher = create_batcher(MockDependencies::default());
let metrics = metrics_handle.render();
assert_eq!(
parse_numeric_metric::<u64>(&metrics, crate::metrics::STORAGE_HEIGHT.name),
Some(INITIAL_HEIGHT.0)
);
}

#[rstest]
#[tokio::test]
async fn start_height_success() {
Expand Down Expand Up @@ -216,8 +231,9 @@ async fn no_active_height() {
#[tokio::test]
async fn consecutive_heights_success() {
let mut storage_reader = MockBatcherStorageReaderTrait::new();
storage_reader.expect_height().times(1).returning(|| Ok(INITIAL_HEIGHT));
storage_reader.expect_height().times(1).returning(|| Ok(INITIAL_HEIGHT.unchecked_next()));
storage_reader.expect_height().times(1).returning(|| Ok(INITIAL_HEIGHT)); // metrics registration
storage_reader.expect_height().times(1).returning(|| Ok(INITIAL_HEIGHT)); // first start_height
storage_reader.expect_height().times(1).returning(|| Ok(INITIAL_HEIGHT.unchecked_next())); // second start_height

let mut block_builder_factory = MockBlockBuilderFactoryTrait::new();
for _ in 0..2 {
Expand Down Expand Up @@ -401,7 +417,7 @@ async fn get_height() {
let mut storage_reader = MockBatcherStorageReaderTrait::new();
storage_reader.expect_height().returning(|| Ok(INITIAL_HEIGHT));

let mut batcher = create_batcher(MockDependencies { storage_reader, ..Default::default() });
let batcher = create_batcher(MockDependencies { storage_reader, ..Default::default() });

let result = batcher.get_height().await.unwrap();
assert_eq!(result, GetHeightResponse { height: INITIAL_HEIGHT });
Expand Down Expand Up @@ -486,6 +502,9 @@ async fn concurrent_proposals_generation_fail() {
#[rstest]
#[tokio::test]
async fn add_sync_block() {
let recorder = PrometheusBuilder::new().build_recorder();
let _g = metrics::set_default_local_recorder(&recorder);
let metrics_handle = recorder.handle();
let mut mock_dependencies = MockDependencies::default();

mock_dependencies
Expand Down Expand Up @@ -516,6 +535,11 @@ async fn add_sync_block() {
transaction_hashes: test_tx_hashes().into_iter().collect(),
};
batcher.add_sync_block(sync_block).await.unwrap();
let metrics = metrics_handle.render();
assert_eq!(
parse_numeric_metric::<u64>(&metrics, crate::metrics::STORAGE_HEIGHT.name),
Some(INITIAL_HEIGHT.unchecked_next().0)
);
}

#[rstest]
Expand All @@ -537,6 +561,9 @@ async fn add_sync_block_mismatch_block_number() {
#[rstest]
#[tokio::test]
async fn decision_reached() {
let recorder = PrometheusBuilder::new().build_recorder();
let _g = metrics::set_default_local_recorder(&recorder);
let metrics_handle = recorder.handle();
let mut mock_dependencies = MockDependencies::default();
let expected_artifacts = BlockExecutionArtifacts::create_for_testing();

Expand Down Expand Up @@ -572,6 +599,12 @@ async fn decision_reached() {
batcher.decision_reached(DecisionReachedInput { proposal_id: PROPOSAL_ID }).await.unwrap();
assert_eq!(response.state_diff, expected_artifacts.state_diff());
assert_eq!(response.l2_gas_used, expected_artifacts.l2_gas_used);

let metrics = metrics_handle.render();
assert_eq!(
parse_numeric_metric::<u64>(&metrics, crate::metrics::STORAGE_HEIGHT.name),
Some(INITIAL_HEIGHT.unchecked_next().0)
);
}

#[rstest]
Expand Down
1 change: 1 addition & 0 deletions crates/starknet_batcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod block_builder;
mod block_builder_test;
pub mod communication;
pub mod config;
mod metrics;
#[cfg(test)]
mod test_utils;
mod transaction_executor;
Expand Down
31 changes: 31 additions & 0 deletions crates/starknet_batcher/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use metrics::{counter, describe_counter, Counter};
use starknet_api::block::BlockNumber;

pub const STORAGE_HEIGHT: Metric =
Metric { name: "batcher_storage_height", description: "The height of the batcher storage" };

pub struct Metric {
pub name: &'static str,
pub description: &'static str,
}

pub struct RegisteredMetrics {
// Ideally, we would have a `Gauge` here because of reverts, but we can't because
// the value will need to implement `Into<f64>` and `BlockNumber` doesn't.
// In case of reverts, consider calling `absolute`.
storage_height: Counter,
}

impl RegisteredMetrics {
pub fn register_metrics(storage_height: BlockNumber) -> Self {
let storage_height_metric = counter!(STORAGE_HEIGHT.name);
describe_counter!(STORAGE_HEIGHT.name, STORAGE_HEIGHT.description);
storage_height_metric.absolute(storage_height.0);

Self { storage_height: storage_height_metric }
}

pub fn increment_storage_height(&self) {
self.storage_height.increment(1);
}
}

0 comments on commit 778c278

Please sign in to comment.