Repository: NetApp/harvest
Pull request #1594 (2022-12-06 10:28:07)

feat: handle record values in metric calculation


Patch:
diff --git a/grafana/dashboards/cmode/svm.json b/grafana/dashboards/cmode/svm.json
index abe6f6c0..e545b1d1 100644
--- a/grafana/dashboards/cmode/svm.json
+++ b/grafana/dashboards/cmode/svm.json
@@ -3040,8 +3040,7 @@
                 "lineWidth": 1,
                 "pointSize": 5,
                 "scaleDistribution": {
-                  "log": 2,
-                  "type": "log"
+                  "type": "linear"
                 },
                 "showPoints": "auto",
                 "spanNulls": true,
diff --git a/pkg/matrix/metric.go b/pkg/matrix/metric.go
index 80d9db70..c0d267c4 100644
--- a/pkg/matrix/metric.go
+++ b/pkg/matrix/metric.go
@@ -288,8 +288,7 @@ func (m *Metric) GetValueBytes(i *Instance) ([]byte, bool) {
 	return []byte(s), ok
 }
 
-// vector arithmetics
-
+// Delta vector arithmetics
 func (m *Metric) Delta(prevMetric *Metric, prevMat *Matrix, curMat *Matrix, logger *logging.Logger) (int, error) {
 	var skips int
 	prevRaw := prevMetric.values
@@ -317,6 +316,15 @@ func (m *Metric) Delta(prevMetric *Metric, prevMat *Matrix, curMat *Matrix, logg
 						Str("instKey", key).
 						Msg("Negative cooked value")
 				}
+			} else {
+				m.record[currIndex] = false
+				skips++
+				logger.Trace().
+					Str("metric", m.GetName()).
+					Float64("currentRaw", curRaw).
+					Float64("previousRaw", prevRaw[prevIndex]).
+					Str("instKey", key).
+					Msg("Delta calculation skipped")
 			}
 		} else {
 			m.record[currIndex] = false
@@ -350,12 +358,20 @@ func (m *Metric) Divide(s *Metric, logger *logging.Logger) (int, error) {
 						Str("metric", m.GetName()).
 						Float64("numerator", m.values[i]).
 						Float64("denominator", sValues[i]).
-						Msg("No pass values")
+						Msg("Divide calculation skipped")
 				}
 				m.values[i] /= sValues[i]
 			} else {
 				m.values[i] = 0
 			}
+		} else {
+			m.record[i] = false
+			skips++
+			logger.Trace().
+				Str("metric", m.GetName()).
+				Float64("numerator", m.values[i]).
+				Float64("denominator", sValues[i]).
+				Msg("Divide calculation skipped")
 		}
 	}
 	return skips, nil
@@ -381,11 +397,22 @@ func (m *Metric) DivideWithThreshold(s *Metric, t int, logger *logging.Logger) (
 				Float64("numerator", v).
 				Float64("denominator", sValues[i]).
 				Msg("Negative values")
+			return skips, nil
 		}
-		if m.record[i] && sRecord[i] && sValues[i] >= x {
-			m.values[i] /= sValues[i]
+		if m.record[i] && sRecord[i] {
+			if sValues[i] >= x {
+				m.values[i] /= sValues[i]
+			} else {
+				m.values[i] = 0
+			}
 		} else {
-			m.values[i] = 0
+			m.record[i] = false
+			skips++
+			logger.Trace().
+				Str("metric", m.GetName()).
+				Float64("numerator", m.values[i]).
+				Float64("denominator", sValues[i]).
+				Msg("Divide threshold calculation skipped")
 		}
 	}
 	return skips, nil
@@ -407,6 +434,14 @@ func (m *Metric) MultiplyByScalar(s uint, logger *logging.Logger) (int, error) {
 					Msg("Negative value")
 			}
 			m.values[i] *= x
+		} else {
+			m.record[i] = false
+			skips++
+			logger.Trace().
+				Str("metric", m.GetName()).
+				Float64("currentRaw", m.values[i]).
+				Uint("scalar", s).
+				Msg("Scalar multiplication skipped")
 		}
 	}
 	return skips, nil


Test patch:
diff --git a/pkg/matrix/metric_float64_test.go b/pkg/matrix/metric_test.go
similarity index 64%
rename from pkg/matrix/metric_float64_test.go
rename to pkg/matrix/metric_test.go
index 9f4ba314..4042ec85 100644
--- a/pkg/matrix/metric_float64_test.go
+++ b/pkg/matrix/metric_test.go
@@ -58,6 +58,28 @@ func setupMatrix(previousRaw float64, currentRaw float64, mop matrixOp) (*Matrix
 	return m, m1
 }
 
+func setupMatrixAdv(previousRaw []float64, currentRaw []float64) (*Matrix, *Matrix) {
+	prevMat := New("Test", "test", "test")
+	averageLatency, _ := prevMat.NewMetricFloat64("average_latency")
+	totalOps, _ := prevMat.NewMetricFloat64("total_ops")
+	names := instanceNames[sameInstance]
+	for _, instanceName := range names.prev {
+		instance, _ := prevMat.NewInstance(instanceName)
+		_ = averageLatency.SetValueFloat64(instance, previousRaw[0])
+		_ = totalOps.SetValueFloat64(instance, previousRaw[1])
+	}
+
+	currentMat := New("Test", "test", "test")
+	averageLatency1, _ := currentMat.NewMetricFloat64("average_latency")
+	totalOps1, _ := currentMat.NewMetricFloat64("total_ops")
+	for _, instanceName := range names.cur {
+		instance, _ := currentMat.NewInstance(instanceName)
+		_ = averageLatency1.SetValueFloat64(instance, currentRaw[0])
+		_ = totalOps1.SetValueFloat64(instance, currentRaw[1])
+	}
+	return prevMat, currentMat
+}
+
 type test struct {
 	name      string
 	curRaw    float64
@@ -69,6 +91,16 @@ type test struct {
 	record    []bool
 }
 
+type testAdv struct {
+	name      string
+	curRaw    []float64
+	prevRaw   []float64
+	cooked    []float64
+	skips     int
+	threshold int
+	record    []bool
+}
+
 func TestMetricFloat64_Delta(t *testing.T) {
 	testDelta(t, sameInstance)
 	testDelta(t, deleteInstance)
@@ -141,14 +173,36 @@ func TestMetricFloat64_Divide(t *testing.T) {
 			matrixTest(t, tt, numerator, skips, err)
 		})
 	}
+
+	testsAdv := []testAdv{
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 10}, cooked: []float64{2}, skips: 0, record: []bool{true}, name: "normal"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 0}, cooked: []float64{10}, skips: 1, record: []bool{false}, name: "denominator record false"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{5, 10}, cooked: []float64{-5}, skips: 1, record: []bool{false}, name: "numerator record false"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 0}, cooked: []float64{10}, skips: 1, record: []bool{false}, name: "numerator denominator record false"},
+	}
+
+	for _, tt := range testsAdv {
+		t.Run(tt.name, func(t *testing.T) {
+			prevMat, curMat := setupMatrixAdv(tt.prevRaw, tt.curRaw)
+			for k := range curMat.GetMetrics() {
+				_, err := curMat.GetMetric(k).Delta(prevMat.GetMetric(k), prevMat, curMat, logging.Get())
+				if err != nil {
+					t.Error("unexpected error", err)
+					return
+				}
+			}
+			skips, err := curMat.GetMetric("average_latency").Divide(curMat.GetMetric("total_ops"), logging.Get())
+			matrixTestAdv(t, tt, curMat, skips, err)
+		})
+	}
 }
 
 func TestMetricFloat64_DivideWithThreshold(t *testing.T) {
 	tests := []test{
 		{prevRaw: 20, curRaw: 10, threshold: 5, cooked: []float64{2}, skips: 0, record: []bool{true}, name: "normal"},
 		{prevRaw: 20, curRaw: 10, threshold: 15, cooked: []float64{0}, skips: 0, record: []bool{true}, name: "normal < threshold"},
-		{prevRaw: 20, curRaw: -10, threshold: 5, cooked: []float64{0}, skips: 1, record: []bool{false}, name: "bug negative den"},
-		{prevRaw: -20, curRaw: -10, threshold: 5, cooked: []float64{0}, skips: 1, record: []bool{false}, name: "bug negative both"},
+		{prevRaw: 20, curRaw: -10, threshold: 5, cooked: []float64{20}, skips: 1, record: []bool{false}, name: "bug negative den"},
+		{prevRaw: -20, curRaw: -10, threshold: 5, cooked: []float64{-20}, skips: 1, record: []bool{false}, name: "bug negative both"},
 		{prevRaw: 20, curRaw: 0, threshold: 0, cooked: []float64{math.Inf(1)}, skips: 0, record: []bool{true}, name: "allow no threshold"},
 		{prevRaw: 0, curRaw: 0, threshold: 5, cooked: []float64{0}, skips: 0, record: []bool{true}, name: "allow zero both"},
 	}
@@ -161,6 +215,28 @@ func TestMetricFloat64_DivideWithThreshold(t *testing.T) {
 			matrixTest(t, tt, numerator, skips, err)
 		})
 	}
+
+	testsAdv := []testAdv{
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 10}, threshold: 1, cooked: []float64{2}, skips: 0, record: []bool{true}, name: "normal"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 0}, threshold: 1, cooked: []float64{10}, skips: 1, record: []bool{false}, name: "denominator record false"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{5, 10}, threshold: 1, cooked: []float64{-5}, skips: 1, record: []bool{false}, name: "numerator record false"},
+		{prevRaw: []float64{10, 5}, curRaw: []float64{20, 0}, threshold: 1, cooked: []float64{10}, skips: 1, record: []bool{false}, name: "numerator denominator record false"},
+	}
+
+	for _, tt := range testsAdv {
+		t.Run(tt.name, func(t *testing.T) {
+			prevMat, curMat := setupMatrixAdv(tt.prevRaw, tt.curRaw)
+			for k := range curMat.GetMetrics() {
+				_, err := curMat.GetMetric(k).Delta(prevMat.GetMetric(k), prevMat, curMat, logging.Get())
+				if err != nil {
+					t.Error("unexpected error", err)
+					return
+				}
+			}
+			skips, err := curMat.GetMetric("average_latency").DivideWithThreshold(curMat.GetMetric("total_ops"), tt.threshold, logging.Get())
+			matrixTestAdv(t, tt, curMat, skips, err)
+		})
+	}
 }
 
 func TestMetricFloat64_MultiplyByScalar(t *testing.T) {
@@ -178,6 +254,30 @@ func TestMetricFloat64_MultiplyByScalar(t *testing.T) {
 	}
 }
 
+func matrixTestAdv(t *testing.T, tt testAdv, cur *Matrix, skips int, err error) {
+	if err != nil {
+		t.Error("unexpected error", err)
+		return
+	}
+	if skips != tt.skips {
+		t.Errorf("skips expected = %d, got %d", tt.skips, skips)
+	}
+
+	cooked := cur.GetMetric("average_latency").values
+	for i := range cooked {
+		if cooked[i] != tt.cooked[i] {
+			t.Errorf("cooked expected = %v, got %v", tt.cooked, cooked)
+		}
+	}
+
+	record := cur.GetMetric("average_latency").GetRecords()
+	for i := range record {
+		if record[i] != tt.record[i] {
+			t.Errorf("record expected = %t, got %t", tt.record, record)
+		}
+	}
+}
+
 func matrixTest(t *testing.T, tt test, cur *Matrix, skips int, err error) {
 	if err != nil {
 		t.Error("unexpected error", err)