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 set to 0 the counters of the
MetricFamily if the metric was not present.

Signed-off-by: Donia Chaiehloudj <[email protected]>
  • Loading branch information
doniacld committed Jul 25, 2023
1 parent fdff063 commit 924c162
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 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
54 changes: 48 additions & 6 deletions connectivity/check/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
package check

import (
"encoding/json"
"errors"
"fmt"
"reflect"
"strconv"
"strings"

flowpb "github.com/cilium/cilium/api/v1/flow"
"github.com/cloudflare/cfssl/log"
dto "github.com/prometheus/client_model/go"
)

Expand Down Expand Up @@ -167,19 +170,19 @@ 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]
afterValue, ok := after[metricName]
if !ok {
err = errors.Join(err, fmt.Errorf("metric %s has not been retrieved before action", metricName))
err = errors.Join(err, fmt.Errorf("metric %s has not been retrieved after action", metricName))
}

aValue, ok := after[metricName]
beforeValue, ok := before[metricName]
if !ok {
err = errors.Join(err, fmt.Errorf("metric %s has not been retrieved after action", metricName))
beforeValue = setMetricFamilyCountersToZero(afterValue, 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 beforeValue != nil && afterValue != nil {
errM := metricsIncrease(beforeValue, afterValue)
if errM != nil {
err = errors.Join(err, errM)
}
Expand All @@ -188,3 +191,42 @@ func assertMetricsIncrease(metrics ...string) assertMetricsFunc {
return err
}
}

// setMetricFamilyCountersToZero copies the content of the input MetricFamily to retrieve all the metadata,
// replace the counters' values to 0 and returns the created MetricFamily.
func setMetricFamilyCountersToZero(in *dto.MetricFamily, metricName string) *dto.MetricFamily {
log.Warningf("metric %s was not found before the action was run", metricName)

b, err := deepCopy(in)
if err != nil {
log.Warningf("failed to copy content from after metric %s into before one", metricName)
}

beforeValue := b.(*dto.MetricFamily)
for _, m := range beforeValue.GetMetric() {
value := float64(0)
m.Counter.Value = &value
}

log.Warningf("metric %s counters was set to 0", metricName)

return beforeValue
}

// deepCopy is a helper to copy the content of an interface into a new one by de-referencing possible pointers.
// Since it uses json.Unmarshal function, unexported fields will not be copied.
// Note that this solution is not particularly efficient its purpose in the assertion of the metrics
// should happen only occasionally.
func deepCopy(v interface{}) (interface{}, error) {
data, err := json.Marshal(v)
if err != nil {
return nil, err
}

vptr := reflect.New(reflect.TypeOf(v))
err = json.Unmarshal(data, vptr.Interface())
if err != nil {
return nil, err
}
return vptr.Elem().Interface(), err
}
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 924c162

Please sign in to comment.