Skip to content

Commit

Permalink
Avoid excessive logging of BN online status (sigp#4315)
Browse files Browse the repository at this point in the history
## Issue Addressed

sigp#4309 (comment)

## Proposed Changes

Log the `Connected to beacon node` message only if the node was previously offline. This avoids a regression in logging after sigp#4295, whereby the `Connected to beacon node` message would be logged every slot.

The new reduced logging is _slightly different_ from what we had prior to my changes in sigp#4295. The main difference is that we used to log the `Connected` message whenever a node was online and subject to a health check (for being unhealthy in some other way). I think the new behaviour is reasonable, as the `Connected` message isn't particularly helpful if the BN is unhealthy, and the specific reason for unhealthiness will be logged by the warnings for `is_compatible`/`is_synced`.
  • Loading branch information
michaelsproul authored and Woodpile37 committed Jan 6, 2024
1 parent 559744a commit 66a8045
Showing 1 changed file with 13 additions and 8 deletions.
21 changes: 13 additions & 8 deletions validator_client/src/beacon_node_fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,10 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
spec: &ChainSpec,
log: &Logger,
) -> Result<(), CandidateError> {
let new_status = if let Err(e) = self.is_online(log).await {
let previous_status = self.status(RequireSynced::Yes).await;
let was_offline = matches!(previous_status, Err(CandidateError::Offline));

let new_status = if let Err(e) = self.is_online(was_offline, log).await {
Err(e)
} else if let Err(e) = self.is_compatible(spec, log).await {
Err(e)
Expand All @@ -202,7 +205,7 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
}

/// Checks if the node is reachable.
async fn is_online(&self, log: &Logger) -> Result<(), CandidateError> {
async fn is_online(&self, was_offline: bool, log: &Logger) -> Result<(), CandidateError> {
let result = self
.beacon_node
.get_node_version()
Expand All @@ -211,12 +214,14 @@ impl<E: EthSpec> CandidateBeaconNode<E> {

match result {
Ok(version) => {
info!(
log,
"Connected to beacon node";
"version" => version,
"endpoint" => %self.beacon_node,
);
if was_offline {
info!(
log,
"Connected to beacon node";
"version" => version,
"endpoint" => %self.beacon_node,
);
}
Ok(())
}
Err(e) => {
Expand Down

0 comments on commit 66a8045

Please sign in to comment.