From e919f7819ec56abd98b057e56d0e2ed971002c13 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 4 Sep 2024 04:36:39 +0530 Subject: [PATCH 1/2] fix: use better value for threshold value in alert description --- pkg/query-service/rules/prom_rule.go | 26 +++ pkg/query-service/rules/promrule_test.go | 191 +++++++++------- pkg/query-service/rules/threshold_rule.go | 28 +++ .../rules/threshold_rule_test.go | 212 +++++++++++------- 4 files changed, 286 insertions(+), 171 deletions(-) diff --git a/pkg/query-service/rules/prom_rule.go b/pkg/query-service/rules/prom_rule.go index 06f9ae311d..405b608459 100644 --- a/pkg/query-service/rules/prom_rule.go +++ b/pkg/query-service/rules/prom_rule.go @@ -593,6 +593,15 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { break } } + if shouldAlert { + var maxValue float64 = math.Inf(-1) + for _, smpl := range series.Floats { + if smpl.F > maxValue { + maxValue = smpl.F + } + } + alertSmpl = pql.Sample{F: maxValue, Metric: series.Metric} + } } else if r.compareOp() == ValueIsBelow { for _, smpl := range series.Floats { if smpl.F >= r.targetVal() { @@ -600,6 +609,15 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { break } } + if shouldAlert { + var minValue float64 = math.Inf(1) + for _, smpl := range series.Floats { + if smpl.F < minValue { + minValue = smpl.F + } + } + alertSmpl = pql.Sample{F: minValue, Metric: series.Metric} + } } else if r.compareOp() == ValueIsEq { for _, smpl := range series.Floats { if smpl.F != r.targetVal() { @@ -614,6 +632,14 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { break } } + if shouldAlert { + for _, smpl := range series.Floats { + if !math.IsInf(smpl.F, 0) && !math.IsNaN(smpl.F) { + alertSmpl = pql.Sample{F: smpl.F, Metric: series.Metric} + break + } + } + } } case OnAverage: // If the average of all samples matches the condition, the rule is firing. diff --git a/pkg/query-service/rules/promrule_test.go b/pkg/query-service/rules/promrule_test.go index a06b510f2e..7995af858a 100644 --- a/pkg/query-service/rules/promrule_test.go +++ b/pkg/query-service/rules/promrule_test.go @@ -38,11 +38,12 @@ func TestPromRuleShouldAlert(t *testing.T) { } cases := []struct { - values pql.Series - expectAlert bool - compareOp string - matchType string - target float64 + values pql.Series + expectAlert bool + compareOp string + matchType string + target float64 + expectedAlertSample v3.Point }{ // Test cases for Equals Always { @@ -55,10 +56,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 0.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "2", // Always - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "2", // Always + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: pql.Series{ @@ -116,10 +118,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 0.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: pql.Series{ @@ -131,10 +134,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: pql.Series{ @@ -146,10 +150,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: pql.Series{ @@ -177,10 +182,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "2", // Always - target: 1.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "2", // Always + target: 1.5, + expectedAlertSample: v3.Point{Value: 2.0}, }, { values: pql.Series{ @@ -208,10 +214,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "1", // Once - target: 4.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "1", // Once + target: 4.5, + expectedAlertSample: v3.Point{Value: 10.0}, }, { values: pql.Series{ @@ -269,10 +276,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "2", // Always - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "2", // Always + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: pql.Series{ @@ -300,10 +308,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 0.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: pql.Series{ @@ -330,10 +339,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: pql.Series{ @@ -345,10 +355,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, // Test cases for Less Than Always { @@ -361,10 +372,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 1.5}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "2", // Always - target: 4, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "2", // Always + target: 4, + expectedAlertSample: v3.Point{Value: 1.5}, }, { values: pql.Series{ @@ -392,10 +404,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.5}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "1", // Once - target: 4, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "1", // Once + target: 4, + expectedAlertSample: v3.Point{Value: 2.5}, }, { values: pql.Series{ @@ -423,10 +436,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "3", // OnAverage - target: 6.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "3", // OnAverage + target: 6.0, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: pql.Series{ @@ -453,10 +467,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "3", // OnAverage - target: 4.5, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "3", // OnAverage + target: 4.5, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: pql.Series{ @@ -483,10 +498,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "3", // OnAverage - target: 4.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "3", // OnAverage + target: 4.5, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: pql.Series{ @@ -498,10 +514,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "3", // OnAverage - target: 12.0, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "3", // OnAverage + target: 12.0, + expectedAlertSample: v3.Point{Value: 6.0}, }, // Test cases for InTotal { @@ -514,10 +531,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 2.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "4", // InTotal - target: 30.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "4", // InTotal + target: 30.0, + expectedAlertSample: v3.Point{Value: 30.0}, }, { values: pql.Series{ @@ -540,10 +558,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 10.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "4", // InTotal - target: 9.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "4", // InTotal + target: 9.0, + expectedAlertSample: v3.Point{Value: 10.0}, }, { values: pql.Series{ @@ -563,10 +582,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 10.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "4", // InTotal - target: 10.0, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "4", // InTotal + target: 10.0, + expectedAlertSample: v3.Point{Value: 20.0}, }, { values: pql.Series{ @@ -587,10 +607,11 @@ func TestPromRuleShouldAlert(t *testing.T) { {F: 10.0}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "4", // InTotal - target: 30.0, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "4", // InTotal + target: 30.0, + expectedAlertSample: v3.Point{Value: 20.0}, }, { values: pql.Series{ diff --git a/pkg/query-service/rules/threshold_rule.go b/pkg/query-service/rules/threshold_rule.go index 9bdecbc63d..e657af9288 100644 --- a/pkg/query-service/rules/threshold_rule.go +++ b/pkg/query-service/rules/threshold_rule.go @@ -1205,6 +1205,16 @@ func (r *ThresholdRule) shouldAlert(series v3.Series) (Sample, bool) { break } } + // use min value from the series + if shouldAlert { + var minValue float64 = math.Inf(1) + for _, smpl := range series.Points { + if smpl.Value < minValue { + minValue = smpl.Value + } + } + alertSmpl = Sample{Point: Point{V: minValue}, Metric: lblsNormalized, MetricOrig: lbls} + } } else if r.compareOp() == ValueIsBelow { for _, smpl := range series.Points { if smpl.Value >= r.targetVal() { @@ -1212,6 +1222,15 @@ func (r *ThresholdRule) shouldAlert(series v3.Series) (Sample, bool) { break } } + if shouldAlert { + var maxValue float64 = math.Inf(-1) + for _, smpl := range series.Points { + if smpl.Value > maxValue { + maxValue = smpl.Value + } + } + alertSmpl = Sample{Point: Point{V: maxValue}, Metric: lblsNormalized, MetricOrig: lbls} + } } else if r.compareOp() == ValueIsEq { for _, smpl := range series.Points { if smpl.Value != r.targetVal() { @@ -1226,6 +1245,15 @@ func (r *ThresholdRule) shouldAlert(series v3.Series) (Sample, bool) { break } } + // use any non-inf or nan value from the series + if shouldAlert { + for _, smpl := range series.Points { + if !math.IsInf(smpl.Value, 0) && !math.IsNaN(smpl.Value) { + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + break + } + } + } } case OnAverage: // If the average of all samples matches the condition, the rule is firing. diff --git a/pkg/query-service/rules/threshold_rule_test.go b/pkg/query-service/rules/threshold_rule_test.go index 05bd613900..29782e4f23 100644 --- a/pkg/query-service/rules/threshold_rule_test.go +++ b/pkg/query-service/rules/threshold_rule_test.go @@ -42,11 +42,12 @@ func TestThresholdRuleShouldAlert(t *testing.T) { } cases := []struct { - values v3.Series - expectAlert bool - compareOp string - matchType string - target float64 + values v3.Series + expectAlert bool + compareOp string + matchType string + target float64 + expectedAlertSample v3.Point }{ // Test cases for Equals Always { @@ -59,10 +60,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 0.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "2", // Always - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "2", // Always + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: v3.Series{ @@ -120,10 +122,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 0.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: v3.Series{ @@ -135,10 +138,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: v3.Series{ @@ -150,10 +154,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 0.0}, }, { values: v3.Series{ @@ -181,10 +186,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "2", // Always - target: 1.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "2", // Always + target: 1.5, + expectedAlertSample: v3.Point{Value: 2.0}, }, { values: v3.Series{ @@ -212,10 +218,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "1", // Once - target: 4.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "1", // Once + target: 4.5, + expectedAlertSample: v3.Point{Value: 10.0}, }, { values: v3.Series{ @@ -273,10 +280,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "2", // Always - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "2", // Always + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: v3.Series{ @@ -304,10 +312,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 0.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: v3.Series{ @@ -334,10 +343,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, { values: v3.Series{ @@ -349,10 +359,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "1", // Once - target: 0.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "1", // Once + target: 0.0, + expectedAlertSample: v3.Point{Value: 1.0}, }, // Test cases for Less Than Always { @@ -365,10 +376,27 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 1.5}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "2", // Always - target: 4, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "2", // Always + target: 4, + expectedAlertSample: v3.Point{Value: 1.5}, + }, + { + values: v3.Series{ + Points: []v3.Point{ + {Value: 1.5}, + {Value: 2.5}, + {Value: 1.5}, + {Value: 3.5}, + {Value: 1.5}, + }, + }, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "2", // Always + target: 4, + expectedAlertSample: v3.Point{Value: 3.5}, }, { values: v3.Series{ @@ -396,10 +424,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.5}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "1", // Once - target: 4, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "1", // Once + target: 4, + expectedAlertSample: v3.Point{Value: 2.5}, }, { values: v3.Series{ @@ -427,10 +456,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "3", // OnAverage - target: 6.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "3", // OnAverage + target: 6.0, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: v3.Series{ @@ -457,10 +487,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "3", // OnAverage - target: 4.5, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "3", // OnAverage + target: 4.5, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: v3.Series{ @@ -487,10 +518,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "3", // OnAverage - target: 4.5, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "3", // OnAverage + target: 4.5, + expectedAlertSample: v3.Point{Value: 6.0}, }, { values: v3.Series{ @@ -502,10 +534,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "3", // OnAverage - target: 12.0, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "3", // OnAverage + target: 12.0, + expectedAlertSample: v3.Point{Value: 6.0}, }, // Test cases for InTotal { @@ -518,10 +551,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 2.0}, }, }, - expectAlert: true, - compareOp: "3", // Equals - matchType: "4", // InTotal - target: 30.0, + expectAlert: true, + compareOp: "3", // Equals + matchType: "4", // InTotal + target: 30.0, + expectedAlertSample: v3.Point{Value: 30.0}, }, { values: v3.Series{ @@ -544,10 +578,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 10.0}, }, }, - expectAlert: true, - compareOp: "4", // Not Equals - matchType: "4", // InTotal - target: 9.0, + expectAlert: true, + compareOp: "4", // Not Equals + matchType: "4", // InTotal + target: 9.0, + expectedAlertSample: v3.Point{Value: 10.0}, }, { values: v3.Series{ @@ -567,10 +602,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 10.0}, }, }, - expectAlert: true, - compareOp: "1", // Greater Than - matchType: "4", // InTotal - target: 10.0, + expectAlert: true, + compareOp: "1", // Greater Than + matchType: "4", // InTotal + target: 10.0, + expectedAlertSample: v3.Point{Value: 20.0}, }, { values: v3.Series{ @@ -591,10 +627,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { {Value: 10.0}, }, }, - expectAlert: true, - compareOp: "2", // Less Than - matchType: "4", // InTotal - target: 30.0, + expectAlert: true, + compareOp: "2", // Less Than + matchType: "4", // InTotal + target: 30.0, + expectedAlertSample: v3.Point{Value: 20.0}, }, { values: v3.Series{ @@ -626,8 +663,11 @@ func TestThresholdRuleShouldAlert(t *testing.T) { values.Points[i].Timestamp = time.Now().UnixMilli() } - _, shoulAlert := rule.shouldAlert(c.values) + smpl, shoulAlert := rule.shouldAlert(c.values) assert.Equal(t, c.expectAlert, shoulAlert, "Test case %d", idx) + if shoulAlert { + assert.Equal(t, c.expectedAlertSample.Value, smpl.V, "Test case %d", idx) + } } } From d2bfb4879ee236ddbfb4e7ec9c3ae689aa266193 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 4 Sep 2024 18:07:38 +0530 Subject: [PATCH 2/2] chore: fix the choice of value in prom rule and add tests --- pkg/query-service/rules/prom_rule.go | 17 +++++----- pkg/query-service/rules/promrule_test.go | 32 +++++++++++++++++++ .../rules/threshold_rule_test.go | 32 +++++++++++++++++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/pkg/query-service/rules/prom_rule.go b/pkg/query-service/rules/prom_rule.go index 405b608459..7623553810 100644 --- a/pkg/query-service/rules/prom_rule.go +++ b/pkg/query-service/rules/prom_rule.go @@ -593,14 +593,15 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { break } } + // use min value from the series if shouldAlert { - var maxValue float64 = math.Inf(-1) + var minValue float64 = math.Inf(1) for _, smpl := range series.Floats { - if smpl.F > maxValue { - maxValue = smpl.F + if smpl.F < minValue { + minValue = smpl.F } } - alertSmpl = pql.Sample{F: maxValue, Metric: series.Metric} + alertSmpl = pql.Sample{F: minValue, Metric: series.Metric} } } else if r.compareOp() == ValueIsBelow { for _, smpl := range series.Floats { @@ -610,13 +611,13 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { } } if shouldAlert { - var minValue float64 = math.Inf(1) + var maxValue float64 = math.Inf(-1) for _, smpl := range series.Floats { - if smpl.F < minValue { - minValue = smpl.F + if smpl.F > maxValue { + maxValue = smpl.F } } - alertSmpl = pql.Sample{F: minValue, Metric: series.Metric} + alertSmpl = pql.Sample{F: maxValue, Metric: series.Metric} } } else if r.compareOp() == ValueIsEq { for _, smpl := range series.Floats { diff --git a/pkg/query-service/rules/promrule_test.go b/pkg/query-service/rules/promrule_test.go index 7995af858a..672a86cc59 100644 --- a/pkg/query-service/rules/promrule_test.go +++ b/pkg/query-service/rules/promrule_test.go @@ -188,6 +188,38 @@ func TestPromRuleShouldAlert(t *testing.T) { target: 1.5, expectedAlertSample: v3.Point{Value: 2.0}, }, + { + values: pql.Series{ + Floats: []pql.FPoint{ + {F: 11.0}, + {F: 4.0}, + {F: 3.0}, + {F: 7.0}, + {F: 12.0}, + }, + }, + expectAlert: true, + compareOp: "1", // Above + matchType: "2", // Always + target: 2.0, + expectedAlertSample: v3.Point{Value: 3.0}, + }, + { + values: pql.Series{ + Floats: []pql.FPoint{ + {F: 11.0}, + {F: 4.0}, + {F: 3.0}, + {F: 7.0}, + {F: 12.0}, + }, + }, + expectAlert: true, + compareOp: "2", // Below + matchType: "2", // Always + target: 13.0, + expectedAlertSample: v3.Point{Value: 12.0}, + }, { values: pql.Series{ Floats: []pql.FPoint{ diff --git a/pkg/query-service/rules/threshold_rule_test.go b/pkg/query-service/rules/threshold_rule_test.go index 29782e4f23..6cfeac83d9 100644 --- a/pkg/query-service/rules/threshold_rule_test.go +++ b/pkg/query-service/rules/threshold_rule_test.go @@ -524,6 +524,38 @@ func TestThresholdRuleShouldAlert(t *testing.T) { target: 4.5, expectedAlertSample: v3.Point{Value: 6.0}, }, + { + values: v3.Series{ + Points: []v3.Point{ + {Value: 11.0}, + {Value: 4.0}, + {Value: 3.0}, + {Value: 7.0}, + {Value: 12.0}, + }, + }, + expectAlert: true, + compareOp: "1", // Above + matchType: "2", // Always + target: 2.0, + expectedAlertSample: v3.Point{Value: 3.0}, + }, + { + values: v3.Series{ + Points: []v3.Point{ + {Value: 11.0}, + {Value: 4.0}, + {Value: 3.0}, + {Value: 7.0}, + {Value: 12.0}, + }, + }, + expectAlert: true, + compareOp: "2", // Below + matchType: "2", // Always + target: 13.0, + expectedAlertSample: v3.Point{Value: 12.0}, + }, { values: v3.Series{ Points: []v3.Point{