From 7d6644a10fd64a0d03bcbab88d2e3406496b2e48 Mon Sep 17 00:00:00 2001
From: Donia Chaiehloudj <donia.cld@isovalent.com>
Date: Tue, 25 Jul 2023 10:08:06 +0200
Subject: [PATCH] metrics: Set first retrieved metrics to 0 if absent

When a cluster is freshly deployed and no traffic was observed, certain metrics
does not appear yet. This commit intents to assume the default value of the
counters is 0 the counters if the metric is absent.

Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
---
 connectivity/check/action.go      |  2 +-
 connectivity/check/metrics.go     | 19 +++++++++++++++++--
 connectivity/check/result.go      | 14 +++++++++-----
 connectivity/check/result_test.go |  4 ++--
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/connectivity/check/action.go b/connectivity/check/action.go
index 3ad8db7a5b..09e51be310 100644
--- a/connectivity/check/action.go
+++ b/connectivity/check/action.go
@@ -1046,7 +1046,7 @@ func (a *Action) validateMetric(ctx context.Context, node string, result Metrics
 			a.Debugf("failed to check metrics on node %s: %s\n", node, err)
 		} else {
 			// Metrics check succeed, let's exit.
-			a.Debugf("checked metrics properly on node %s: %s\n", node)
+			a.Debugf("checked metrics properly on node %s\n", node)
 			return
 
 		}
diff --git a/connectivity/check/metrics.go b/connectivity/check/metrics.go
index 9caf60e4a4..0bb92e5e04 100644
--- a/connectivity/check/metrics.go
+++ b/connectivity/check/metrics.go
@@ -113,9 +113,24 @@ func parseMetrics(reader io.Reader) (promMetricsFamily, error) {
 
 // metricsIncrease verifies for all the metrics that the values increased.
 func metricsIncrease(mf1, mf2 *dto.MetricFamily) error {
-	metrics1 := mf1.GetMetric()
 	metrics2 := mf2.GetMetric()
 
+	// Absent initial metric means it's zero, check that after metric is non-zero.
+	if mf1 == nil && mf2 != nil {
+		for _, m2 := range metrics2 {
+			if m2.GetCounter() == nil {
+				return fmt.Errorf("metric %s is not a Counter: %v", mf2.GetName(), mf2)
+			}
+
+			if val := m2.GetCounter().GetValue(); val == 0.0 {
+				return fmt.Errorf("metric %s did not increase as expected, value 1: %f and value 2: %f", mf2.GetName(), 0.0, val)
+			}
+		}
+		return nil
+	}
+
+	metrics1 := mf1.GetMetric()
+
 	if len(metrics1) != len(metrics2) {
 		return fmt.Errorf("metric %s has different length metrics 1: %d and metrics 2: %d", mf1.GetName(), len(metrics1), len(metrics2))
 	}
@@ -128,7 +143,7 @@ func metricsIncrease(mf1, mf2 *dto.MetricFamily) error {
 			return fmt.Errorf("metric %s is not a Counter: %v", mf1.GetName(), metrics2[i])
 		}
 
-		value1 := metrics1[i].GetCounter().GetValue() // Here we assume that metrics are of Counter type.
+		value1 := metrics1[i].GetCounter().GetValue()
 		value2 := metrics2[i].GetCounter().GetValue()
 		if value1 >= value2 {
 			return fmt.Errorf("metric %s did not increase as expected, value 1: %f and value 2: %f", mf1.GetName(), value1, value2)
diff --git a/connectivity/check/result.go b/connectivity/check/result.go
index 84bde76f38..3ef7556d1a 100644
--- a/connectivity/check/result.go
+++ b/connectivity/check/result.go
@@ -6,6 +6,7 @@ package check
 import (
 	"errors"
 	"fmt"
+	"log"
 	"strconv"
 	"strings"
 
@@ -167,19 +168,22 @@ func assertMetricsIncrease(metrics ...string) assertMetricsFunc {
 	return func(before, after map[string]*dto.MetricFamily) error {
 		var err error
 		for _, metricName := range metrics {
-			bValue, ok := before[metricName]
+			beforeValue, ok := before[metricName]
 			if !ok {
-				err = errors.Join(err, fmt.Errorf("metric %s has not been retrieved before action", metricName))
+				// Set the metric to nil, meaning the default values will be 0.
+				beforeValue = nil
+
+				log.Printf("beforeValue is absent, we set it to nil")
 			}
 
-			aValue, ok := after[metricName]
+			afterValue, ok := after[metricName]
 			if !ok {
 				err = errors.Join(err, fmt.Errorf("metric %s has not been retrieved after action", metricName))
 			}
 
 			// Additional check needed because previously we do not return in case of error, otherwise we will panic!
-			if bValue != nil && aValue != nil {
-				errM := metricsIncrease(bValue, aValue)
+			if afterValue != nil {
+				errM := metricsIncrease(beforeValue, afterValue)
 				if errM != nil {
 					err = errors.Join(err, errM)
 				}
diff --git a/connectivity/check/result_test.go b/connectivity/check/result_test.go
index b67aeb55ab..6d29a7d686 100644
--- a/connectivity/check/result_test.go
+++ b/connectivity/check/result_test.go
@@ -115,7 +115,7 @@ func TestExpectMetricsToIncrease(t *testing.T) {
 			metricsAfter:  metricsBefore,
 			wantErr:       true,
 		},
-		"metric name not present in the metrics before": {
+		"metric name not present in the metrics before, counters should be set 0": {
 			metrics: "cilium_forward_count_total",
 			source: MetricsSource{
 				Name: components.CiliumAgentName,
@@ -124,7 +124,7 @@ func TestExpectMetricsToIncrease(t *testing.T) {
 			},
 			metricsBefore: otherMetric,
 			metricsAfter:  metricsAfter,
-			wantErr:       true,
+			wantErr:       false,
 		},
 		"metric name not present in the metrics after": {
 			metrics:       "cilium_forward_count_total",