Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add contract sync iterations metrics #5181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions rust/main/hyperlane-base/src/contract_sync/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ pub struct ContractSyncMetrics {
/// See `last_known_message_nonce` in CoreMetrics.
pub message_nonce: IntGaugeVec,

/// Contract sync iteration metric
pub iterations: IntCounterVec,

/// Metrics for SequenceAware and RateLimited cursors.
pub cursor_metrics: Arc<CursorMetrics>,
}
Expand All @@ -49,13 +52,22 @@ impl ContractSyncMetrics {
)
.expect("failed to register stored_events metric");

let iterations = metrics
.new_int_counter(
"contract_sync_iterations",
"Number of iterations made by contract sync",
&["data_type", "chain"],
)
.expect("failed to register iterations metric");

let message_nonce = metrics.last_known_message_nonce();
let cursor_metrics = Arc::new(CursorMetrics::new(metrics));

ContractSyncMetrics {
indexed_height,
stored_events,
message_nonce,
iterations,
cursor_metrics,
}
}
Expand Down
5 changes: 5 additions & 0 deletions rust/main/hyperlane-base/src/contract_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,13 @@ where
.metrics
.stored_events
.with_label_values(&[label, chain_name]);
let iteration_metric = self
.metrics
.iterations
.with_label_values(&[label, chain_name]);

loop {
iteration_metric.inc();
Copy link
Contributor

@daniel-savu daniel-savu Jan 15, 2025

Choose a reason for hiding this comment

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

over a long enough time window, this metric will overflow right? Since there are way more loops than new blocks or messages. If we just want to make sure the loop is alive we can just alternate between two values (like true and false) and liveness would be lost when no more state alternations are happening

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose this metric is incremented every time when there are messages in internal channel (receiver) and there are new blocks to index (cursor). Otherwise, there is a 5 seconds sleep period which should prevent this metric to grow very quickly.

If the frequency of this metric to overflow, we should be fine.

Regarding the alternating metric, presumably, we can end up in a situation when this metrics will seemly stay constant if Prometheus is not scraping metrics frequently enough. Effectively, this metric will alternate between two values, but Prometheus will scrape only rare entries of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this metric is incremented every time when there are messages in internal channel (receiver) and there are new blocks to index (cursor). Otherwise, there is a 5 seconds sleep period which should prevent this metric to grow very quickly.
If the frequency of this metric to overflow, we should be fine.

fair, could be useful to mention some of this in a comment

Regarding the alternating metric, presumably, we can end up in a situation when this metrics will seemly stay constant if Prometheus is not scraping metrics frequently enough. Effectively, this metric will alternate between two values, but Prometheus will scrape only rare entries of it.

that's also true, I guess another solution would be to set the unix timestamp as the metric to follow (last_tick_timestamp). Happy to go with whatever you think is easier to understand

if let Some(rx) = opts.tx_id_receiver.as_mut() {
self.fetch_logs_from_receiver(rx, &stored_logs_metric).await;
}
Expand Down
Loading