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

Implement this error for MonitoringError and fix clippy warnings #4773

Merged
merged 2 commits into from
May 10, 2024
Merged
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
4 changes: 2 additions & 2 deletions stacks-signer/src/monitoring/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub fn update_signer_nonce(nonce: u64) {
/// The `origin` parameter is the base path of the RPC call, e.g. `http://node.com`.
/// The `origin` parameter is removed from `full_path` when storing in prometheus.
#[cfg(feature = "monitoring_prom")]
pub fn new_rpc_call_timer(full_path: &str, origin: &String) -> HistogramTimer {
pub fn new_rpc_call_timer(full_path: &str, origin: &str) -> HistogramTimer {
let path = &full_path[origin.len()..];
let histogram = prometheus::SIGNER_RPC_CALL_LATENCIES_HISTOGRAM.with_label_values(&[path]);
histogram.start_timer()
Expand All @@ -157,7 +157,7 @@ impl NoOpTimer {

/// Stop and record the no-op timer.
#[cfg(not(feature = "monitoring_prom"))]
pub fn new_rpc_call_timer(_full_path: &str, _origin: &String) -> NoOpTimer {
pub fn new_rpc_call_timer(_full_path: &str, _origin: &str) -> NoOpTimer {
NoOpTimer
}

Expand Down
31 changes: 11 additions & 20 deletions stacks-signer/src/monitoring/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,21 @@ use crate::config::{GlobalConfig, Network};
use crate::monitoring::prometheus::gather_metrics_string;
use crate::monitoring::{update_signer_nonce, update_stacks_tip_height};

#[derive(Debug)]
#[derive(thiserror::Error, Debug)]
/// Monitoring server errors
pub enum MonitoringError {
/// Already bound to an address
#[error("Already bound to an address")]
AlreadyBound,
/// Server terminated
#[error("Server terminated")]
Terminated,
/// No endpoint configured
#[error("Prometheus endpoint not configured.")]
EndpointNotConfigured,
/// Error fetching metrics from stacks node
FetchError(ClientError),
#[error("Error fetching data from stacks node: {0}")]
FetchError(#[from] ClientError),
}

/// Metrics and monitoring server
Expand Down Expand Up @@ -173,10 +177,7 @@ impl MonitoringServer {
/// Update metrics by making RPC calls to the Stacks node
fn update_metrics(&self) -> Result<(), MonitoringError> {
debug!("{}: Updating metrics", self);
let peer_info = self
.stacks_client
.get_peer_info()
.map_err(|e| MonitoringError::FetchError(e))?;
let peer_info = self.stacks_client.get_peer_info()?;
if let Ok(height) = i64::try_from(peer_info.stacks_tip_height) {
update_stacks_tip_height(height);
} else {
Expand All @@ -185,29 +186,19 @@ impl MonitoringServer {
peer_info.stacks_tip_height
);
}
let pox_info = self
.stacks_client
.get_pox_data()
.map_err(|e| MonitoringError::FetchError(e))?;
let pox_info = self.stacks_client.get_pox_data()?;
if let Ok(reward_cycle) = i64::try_from(pox_info.reward_cycle_id) {
update_reward_cycle(reward_cycle);
}
let signer_stx_addr = self.stacks_client.get_signer_address();
let account_entry = self
.stacks_client
.get_account_entry(&signer_stx_addr)
.map_err(|e| MonitoringError::FetchError(e))?;
let account_entry = self.stacks_client.get_account_entry(signer_stx_addr)?;
let balance = i64::from_str_radix(&account_entry.balance[2..], 16).map_err(|e| {
MonitoringError::FetchError(ClientError::MalformedClarityValue(format!(
"Failed to parse balance: {} with err: {}",
&account_entry.balance, e,
)))
})?;
if let Ok(nonce) = u64::try_from(account_entry.nonce) {
update_signer_nonce(nonce);
} else {
warn!("Failed to parse nonce: {}", account_entry.nonce);
}
update_signer_nonce(account_entry.nonce);
update_signer_stx_balance(balance);
Ok(())
}
Expand All @@ -226,7 +217,7 @@ impl MonitoringServer {
/// Poll the Stacks node's `v2/info` endpoint to validate the connection
fn heartbeat(&self) -> bool {
let url = format!("{}/v2/info", self.stacks_node_origin);
let response = self.stacks_node_client.get(&url).send();
let response = self.stacks_node_client.get(url).send();
match response {
Ok(response) => {
if response.status().is_success() {
Expand Down
2 changes: 1 addition & 1 deletion stacks-signer/src/runloop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl RewardCycleInfo {
let effective_height = burnchain_block_height - self.first_burnchain_block_height;
let reward_index = effective_height % self.reward_cycle_length;

reward_index >= u64::from(self.reward_cycle_length - self.prepare_phase_block_length)
reward_index >= (self.reward_cycle_length - self.prepare_phase_block_length)
&& self.get_reward_cycle(burnchain_block_height) == self.reward_cycle
}
}
Expand Down
16 changes: 8 additions & 8 deletions stacks-signer/src/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl Signer {
fn execute_command(&mut self, stacks_client: &StacksClient, command: &Command) {
match command {
Command::Dkg => {
crate::monitoring::increment_commands_processed(&"dkg");
crate::monitoring::increment_commands_processed("dkg");
if self.approved_aggregate_public_key.is_some() {
debug!("Reward cycle #{} Signer #{}: Already have an aggregate key. Ignoring DKG command.", self.reward_cycle, self.signer_id);
return;
Expand Down Expand Up @@ -450,7 +450,7 @@ impl Signer {
is_taproot,
merkle_root,
} => {
crate::monitoring::increment_commands_processed(&"sign");
crate::monitoring::increment_commands_processed("sign");
if self.approved_aggregate_public_key.is_none() {
debug!("{self}: Cannot sign a block without an approved aggregate public key. Ignore it.");
return;
Expand Down Expand Up @@ -1043,25 +1043,25 @@ impl Signer {
// Signers only every trigger non-taproot signing rounds over blocks. Ignore SignTaproot results
match operation_result {
OperationResult::Sign(signature) => {
crate::monitoring::increment_operation_results(&"sign");
crate::monitoring::increment_operation_results("sign");
debug!("{self}: Received signature result");
self.process_signature(signature);
}
OperationResult::SignTaproot(_) => {
crate::monitoring::increment_operation_results(&"sign_taproot");
crate::monitoring::increment_operation_results("sign_taproot");
debug!("{self}: Received a signature result for a taproot signature. Nothing to broadcast as we currently sign blocks with a FROST signature.");
}
OperationResult::Dkg(aggregate_key) => {
crate::monitoring::increment_operation_results(&"dkg");
crate::monitoring::increment_operation_results("dkg");
self.process_dkg(stacks_client, aggregate_key);
}
OperationResult::SignError(e) => {
crate::monitoring::increment_operation_results(&"sign_error");
crate::monitoring::increment_operation_results("sign_error");
warn!("{self}: Received a Sign error: {e:?}");
self.process_sign_error(e);
}
OperationResult::DkgError(e) => {
crate::monitoring::increment_operation_results(&"dkg_error");
crate::monitoring::increment_operation_results("dkg_error");
warn!("{self}: Received a DKG error: {e:?}");
// TODO: process these errors and track malicious signers to report
}
Expand Down Expand Up @@ -1389,7 +1389,7 @@ impl Signer {
return Ok(());
}
// Check stackerdb for any missed DKG messages to catch up our state.
self.read_dkg_stackerdb_messages(&stacks_client, res, current_reward_cycle)?;
self.read_dkg_stackerdb_messages(stacks_client, res, current_reward_cycle)?;
// Check if we should still queue DKG
if !self.should_queue_dkg(stacks_client)? {
return Ok(());
Expand Down
1 change: 1 addition & 0 deletions stackslib/src/monitoring/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub fn increment_rpc_calls_counter() {
prometheus::RPC_CALL_COUNTER.inc();
}

#[allow(unused_mut)]
pub fn instrument_http_request_handler<F, R>(
conv_http: &mut ConversationHttp,
mut req: StacksHttpRequest,
Expand Down