Skip to content

Commit

Permalink
Deduplicate metrics dependencies (#1561)
Browse files Browse the repository at this point in the history
## Motivation

This PR is motivated by the regression identified in #1349. That PR notes that the metrics stopped working for most of the crates other than `zebrad`.

## Solution

This PR resolves the regression by deduplicating the `metrics` crate dependency. During a recent change we upgraded the metrics version in `zebrad` and a couple other of our crates, but we never updated the dependencies in `zebra-state`, `zebra-consensus`, or `zebra-network`. This caused the metrics macros to attempt to retrieve the current metrics exporter through the wrong function. We would install the metrics exporter in `0.13`, but then attempt to look it up through the `0.12` crate, which contains a different instance of the metrics exporter static variable which is unset. Doing this causes the metrics macros to return `None` for the current exporter after which they just silently give up.

## Related Issues

closes #1349

## Follow Up Work

I noticed we have quite a few duplicate dependencies in our tree. We might be able to save some compilation time by auditing those and deduplicating them as much as possible.

- #1582
Co-authored-by: teor <teor@riseup.net>
  • Loading branch information
yaahc authored Jan 12, 2021
1 parent caca450 commit 1569824
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 51 deletions.
27 changes: 6 additions & 21 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion zebra-consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ serde = { version = "1", features = ["serde_derive"] }

futures = "0.3.7"
futures-util = "0.3.6"
metrics = "0.12"
metrics = "0.13.0-alpha.8"
thiserror = "1.0.23"
tokio = { version = "0.3.4", features = ["time", "sync", "stream", "tracing"] }
tower = { version = "0.4", features = ["timeout", "util", "buffer"] }
Expand Down
26 changes: 13 additions & 13 deletions zebra-consensus/src/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ where
if height >= checkpoint_list.max_height() {
(None, Progress::FinalCheckpoint)
} else {
metrics::gauge!("checkpoint.verified.height", height.0 as i64);
metrics::gauge!("checkpoint.processing.next.height", height.0 as i64);
metrics::gauge!("checkpoint.verified.height", height.0 as f64);
metrics::gauge!("checkpoint.processing.next.height", height.0 as f64);
(Some(hash), Progress::InitialTip(height))
}
}
Expand Down Expand Up @@ -298,7 +298,7 @@ where
}
metrics::gauge!(
"checkpoint.queued.continuous.height",
pending_height.0 as i64
pending_height.0 as f64
);

// Now find the start of the checkpoint range
Expand All @@ -320,11 +320,11 @@ where
if let Some(block::Height(target_checkpoint)) = target_checkpoint {
metrics::gauge!(
"checkpoint.processing.next.height",
target_checkpoint as i64
target_checkpoint as f64
);
} else {
// Use the start height if there is no potential next checkpoint
metrics::gauge!("checkpoint.processing.next.height", start_height.0 as i64);
metrics::gauge!("checkpoint.processing.next.height", start_height.0 as f64);
metrics::counter!("checkpoint.waiting.count", 1);
}

Expand Down Expand Up @@ -393,12 +393,12 @@ where
/// Increase the current checkpoint height to `verified_height`,
fn update_progress(&mut self, verified_height: block::Height) {
if let Some(max_height) = self.queued.keys().next_back() {
metrics::gauge!("checkpoint.queued.max.height", max_height.0 as i64);
metrics::gauge!("checkpoint.queued.max.height", max_height.0 as f64);
} else {
// use -1 as a sentinel value for "None", because 0 is a valid height
metrics::gauge!("checkpoint.queued.max.height", -1);
// use f64::NAN as a sentinel value for "None", because 0 is a valid height
metrics::gauge!("checkpoint.queued.max.height", f64::NAN);
}
metrics::gauge!("checkpoint.queued_slots", self.queued.len() as i64);
metrics::gauge!("checkpoint.queued_slots", self.queued.len() as f64);

// Ignore blocks that are below the previous checkpoint, or otherwise
// have invalid heights.
Expand All @@ -413,10 +413,10 @@ where

// Ignore heights that aren't checkpoint heights
if verified_height == self.checkpoint_list.max_height() {
metrics::gauge!("checkpoint.verified.height", verified_height.0 as i64);
metrics::gauge!("checkpoint.verified.height", verified_height.0 as f64);
self.verifier_progress = FinalCheckpoint;
} else if self.checkpoint_list.contains(verified_height) {
metrics::gauge!("checkpoint.verified.height", verified_height.0 as i64);
metrics::gauge!("checkpoint.verified.height", verified_height.0 as f64);
self.verifier_progress = PreviousCheckpoint(verified_height);
// We're done with the initial tip hash now
self.initial_tip_hash = None;
Expand Down Expand Up @@ -525,7 +525,7 @@ where
.keys()
.next_back()
.expect("queued has at least one entry")
.0 as i64
.0 as f64
);

let is_checkpoint = self.checkpoint_list.contains(height);
Expand Down Expand Up @@ -853,7 +853,7 @@ where
let rx = self.queue_block(block.clone());
self.process_checkpoint_range();

metrics::gauge!("checkpoint.queued_slots", self.queued.len() as i64);
metrics::gauge!("checkpoint.queued_slots", self.queued.len() as f64);

// Because the checkpoint verifier duplicates state from the state
// service (it tracks which checkpoints have been verified), we must
Expand Down
2 changes: 1 addition & 1 deletion zebra-network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ tokio = { version = "0.3.4", features = ["net", "time", "stream", "tracing", "ma
tokio-util = { version = "0.5", features = ["codec"] }
tower = { version = "0.4", features = ["retry", "discover", "load", "load-shed", "timeout", "util", "buffer"] }

metrics = "0.12"
metrics = "0.13.0-alpha.8"
tracing = "0.1"
tracing-futures = "0.2"
tracing-error = { version = "0.1.2", features = ["traced-error"] }
Expand Down
6 changes: 3 additions & 3 deletions zebra-network/src/peer_set/candidate_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ where
}

pub fn next(&mut self) -> Option<MetaAddr> {
metrics::gauge!("candidate_set.disconnected", self.disconnected.len() as i64);
metrics::gauge!("candidate_set.gossiped", self.gossiped.len() as i64);
metrics::gauge!("candidate_set.failed", self.failed.len() as i64);
metrics::gauge!("candidate_set.disconnected", self.disconnected.len() as f64);
metrics::gauge!("candidate_set.gossiped", self.gossiped.len() as f64);
metrics::gauge!("candidate_set.failed", self.failed.len() as f64);
debug!(
disconnected_peers = self.disconnected.len(),
gossiped_peers = self.gossiped.len(),
Expand Down
8 changes: 7 additions & 1 deletion zebra-network/src/peer_set/initialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,13 @@ where
let mut crawl_timer = tokio::time::interval(new_peer_interval);

loop {
metrics::gauge!("crawler.in_flight_handshakes", handshakes.len() as i64 - 1);
metrics::gauge!(
"crawler.in_flight_handshakes",
handshakes
.len()
.checked_sub(1)
.expect("the pool always contains an unresolved future") as f64
);
// This is a little awkward because there's no select3.
match select(
select(demand_rx.next(), crawl_timer.next()),
Expand Down
7 changes: 3 additions & 4 deletions zebra-network/src/peer_set/set.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::net::SocketAddr;
use std::{
collections::HashMap,
convert::TryInto,
fmt::Debug,
future::Future,
marker::PhantomData,
Expand Down Expand Up @@ -385,9 +384,9 @@ where
let num_ready = self.ready_services.len();
let num_unready = self.unready_services.len();
let num_peers = num_ready + num_unready;
metrics::gauge!("pool.num_ready", num_ready.try_into().unwrap());
metrics::gauge!("pool.num_unready", num_unready.try_into().unwrap());
metrics::gauge!("pool.num_peers", num_peers.try_into().unwrap());
metrics::gauge!("pool.num_ready", num_ready as f64);
metrics::gauge!("pool.num_unready", num_unready as f64);
metrics::gauge!("pool.num_peers", num_peers as f64);
}
}

Expand Down
2 changes: 1 addition & 1 deletion zebra-state/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ lazy_static = "1.4.0"
serde = { version = "1", features = ["serde_derive"] }

futures = "0.3.7"
metrics = "0.12"
metrics = "0.13.0-alpha.8"
tower = { version = "0.4", features = ["buffer", "util"] }
tracing = "0.1"
thiserror = "1.0.23"
Expand Down
7 changes: 5 additions & 2 deletions zebra-state/src/service/finalized_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,13 @@ impl FinalizedState {
self.max_queued_height = std::cmp::max(self.max_queued_height, height.0 as _);
}

metrics::gauge!("state.finalized.queued.max.height", self.max_queued_height);
metrics::gauge!(
"state.finalized.queued.max.height",
self.max_queued_height as f64
);
metrics::gauge!(
"state.finalized.queued.block.count",
self.queued_by_prev_hash.len() as _
self.queued_by_prev_hash.len() as f64
);
}

Expand Down
8 changes: 4 additions & 4 deletions zebra-state/src/service/non_finalized_state/queued_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,12 @@ impl QueuedBlocks {
/// Update metrics after the queue is modified
fn update_metrics(&self) {
if let Some(max_height) = self.by_height.keys().next_back() {
metrics::gauge!("state.memory.queued.max.height", max_height.0 as i64);
metrics::gauge!("state.memory.queued.max.height", max_height.0 as f64);
} else {
// use -1 as a sentinel value for "None", because 0 is a valid height
metrics::gauge!("state.memory.queued.max.height", -1);
// use f64::NAN as a sentinel value for "None", because 0 is a valid height
metrics::gauge!("state.memory.queued.max.height", f64::NAN);
}
metrics::gauge!("state.memory.queued.block.count", self.blocks.len() as _);
metrics::gauge!("state.memory.queued.block.count", self.blocks.len() as f64);
}

/// Try to look up this UTXO in any queued block.
Expand Down

0 comments on commit 1569824

Please sign in to comment.