app/health: fix peer and error checks

app/health/checks.go

@@ -40,22 +40,23 @@ type query func(name string, selector labelSelector, reducer seriesReducer) (flo
 // checks is a list of health checks.
 var checks = []check{
 	{
-		Name:        "error_logs",
-		Description: "Error logs detected that require human intervention.",
-		Severity:    severityCritical,
-		Func: func(q query, _ Metadata) (bool, error) {
+		// TODO(corver): Change this to critical on any error once we aligned with only logging errors when human intervention is required.
+		Name:        "high_error_log_rate",
+		Description: "High rate of error logs. Please check the logs for more details.",
+		Severity:    severityWarning,
+		Func: func(q query, m Metadata) (bool, error) {
 			increase, err := q("app_log_error_total", noLabels, increase)
 			if err != nil {
 				return false, err
 			}
 
-			return increase > 0, nil
+			return increase > 2*float64(m.NumValidators), nil // Allow 2 errors per validator.
 		},
 	},
 	{
 		Name:        "high_warning_log_rate",
 		Description: "High rate of warning logs. Please check the logs for more details.",
-		Severity:    severityCritical,
+		Severity:    severityWarning,
 		Func: func(q query, m Metadata) (bool, error) {
 			increase, err := q("app_log_warning_total", noLabels, increase)
 			if err != nil {
@@ -83,12 +84,14 @@ var checks = []check{
 		Description: "Not connected to at least quorum peers. Check logs for networking issue or coordinate with peers.",
 		Severity:    severityCritical,
 		Func: func(q query, m Metadata) (bool, error) {
-			max, err := q("ping_success", countNonZeroLabels, gaugeMax)
+			max, err := q("p2p_ping_success", countNonZeroLabels, gaugeMax)
 			if err != nil {
 				return false, err
 			}
 
-			return max < float64(m.QuorumPeers), nil
+			required := float64(m.QuorumPeers) - 1 // Exclude self
+
+			return max < required, nil
 		},
 	},
 	{

app/health/checks_internal_test.go

@@ -136,7 +136,7 @@ func TestInsufficientPeerCheck(t *testing.T) {
 		QuorumPeers: 2,
 	}
 	checkName := "insufficient_connected_peers"
-	metricName := "ping_success"
+	metricName := "p2p_ping_success"
 
 	peer1 := genLabels("peer", "1")
 	peer2 := genLabels("peer", "2")
@@ -216,8 +216,10 @@ func TestBNSyncingCheck(t *testing.T) {
 }
 
 func TestErrorLogsCheck(t *testing.T) {
-	m := Metadata{}
-	checkName := "error_logs"
+	m := Metadata{
+		NumValidators: 10,
+	}
+	checkName := "high_error_log_rate"
 	metricName := "app_log_error_total"
 
 	t.Run("no data", func(t *testing.T) {
@@ -236,15 +238,15 @@ func TestErrorLogsCheck(t *testing.T) {
 		)
 	})
 
-	t.Run("single error", func(t *testing.T) {
-		testCheck(t, m, checkName, true,
-			genFam(metricName, genCounter(nil, 0, 0, 1)),
+	t.Run("too few", func(t *testing.T) {
+		testCheck(t, m, checkName, false,
+			genFam(metricName, genCounter(nil, 0, 0, 10)),
 		)
 	})
 
-	t.Run("multiple errors", func(t *testing.T) {
+	t.Run("sufficient", func(t *testing.T) {
 		testCheck(t, m, checkName, true,
-			genFam(metricName, genCounter(nil, 10, 20, 30, 40, 50)),
+			genFam(metricName, genCounter(nil, 10, 20, 30, 40, 500)),
 		)
 	})
 }