Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use better value for threshold value in alert description #5844

Merged
merged 3 commits into from
Sep 4, 2024
Merged

fix: use better value for threshold value in alert description #5844

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e919f78
fix: use better value for threshold value in alert description
srikanthccv Sep 3, 2024
d2bfb48
chore: fix the choice of value in prom rule and add tests
srikanthccv Sep 4, 2024
1efc35f
Merge branch 'develop' into fix-value-in-alert
srikanthccv Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions pkg/query-service/rules/prom_rule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,13 +593,31 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) {
break
}
}
if shouldAlert {
srikanthccv marked this conversation as resolved.
Show resolved Hide resolved
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() {
shouldAlert = false
break
}
}
if shouldAlert {
var minValue float64 = math.Inf(1)
srikanthccv marked this conversation as resolved.
Show resolved Hide resolved
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() {
Expand All @@ -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.
Expand Down
191 changes: 106 additions & 85 deletions pkg/query-service/rules/promrule_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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
{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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
{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down
Loading
Loading