Skip to content

Commit

Permalink
metrics: Set first retrieved metrics to 0 if absent
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
doniacld committed Jul 27, 2023
1 parent fdff063 commit 7d6644a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
2 changes: 1 addition & 1 deletion connectivity/check/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

}
Expand Down
19 changes: 17 additions & 2 deletions connectivity/check/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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)
Expand Down
14 changes: 9 additions & 5 deletions connectivity/check/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package check
import (
"errors"
"fmt"
"log"
"strconv"
"strings"

Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions connectivity/check/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand Down

0 comments on commit 7d6644a

Please sign in to comment.