From c27f2bf9c6a33d3b7ac630ca52e884c0e1737e99 Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Mon, 22 May 2023 02:36:43 +0000
Subject: [PATCH] Avoid excessive logging of BN online status (#4315)

## Issue Addressed

https://github.com/sigp/lighthouse/pull/4309#issuecomment-1556052261

## Proposed Changes

Log the `Connected to beacon node` message only if the node was previously offline. This avoids a regression in logging after #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 #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`.
---
 validator_client/src/beacon_node_fallback.rs | 21 ++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/validator_client/src/beacon_node_fallback.rs b/validator_client/src/beacon_node_fallback.rs
index 2cbab3b218..531cec08ac 100644
--- a/validator_client/src/beacon_node_fallback.rs
+++ b/validator_client/src/beacon_node_fallback.rs
@@ -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)
@@ -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()
@@ -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) => {