fix: anomaly with below operator negates the target (#9288)

2025-10-08 12:11:31 +05:30
parent a22ef64bb0
commit 9cebd49a2c
4 changed files with 367 additions and 11 deletions
--- a/ee/anomaly/seasonal.go
+++ b/ee/anomaly/seasonal.go
@@ -232,7 +232,7 @@ func (p *BaseSeasonalProvider) getPredictedSeries(
 // moving avg of the previous period series + z score threshold * std dev of the series
 // moving avg of the previous period series - z score threshold * std dev of the series
 func (p *BaseSeasonalProvider) getBounds(
-	series, predictedSeries *qbtypes.TimeSeries,
+	series, predictedSeries, weekSeries *qbtypes.TimeSeries,
 	zScoreThreshold float64,
 ) (*qbtypes.TimeSeries, *qbtypes.TimeSeries) {
 	upperBoundSeries := &qbtypes.TimeSeries{
@@ -246,8 +246,8 @@ func (p *BaseSeasonalProvider) getBounds(
 	}

 	for idx, curr := range series.Values {
-		upperBound := p.getMovingAvg(predictedSeries, movingAvgWindowSize, idx) + zScoreThreshold*p.getStdDev(series)
-		lowerBound := p.getMovingAvg(predictedSeries, movingAvgWindowSize, idx) - zScoreThreshold*p.getStdDev(series)
+		upperBound := p.getMovingAvg(predictedSeries, movingAvgWindowSize, idx) + zScoreThreshold*p.getStdDev(weekSeries)
+		lowerBound := p.getMovingAvg(predictedSeries, movingAvgWindowSize, idx) - zScoreThreshold*p.getStdDev(weekSeries)
 		upperBoundSeries.Values = append(upperBoundSeries.Values, &qbtypes.TimeSeriesValue{
 			Timestamp: curr.Timestamp,
 			Value:     upperBound,
@@ -398,8 +398,6 @@ func (p *BaseSeasonalProvider) getAnomalies(ctx context.Context, orgID valuer.UU
 		aggOfInterest := result.Aggregations[0]

 		for _, series := range aggOfInterest.Series {
-			stdDev := p.getStdDev(series)
-			p.logger.InfoContext(ctx, "calculated standard deviation for series", "anomaly_std_dev", stdDev, "anomaly_labels", series.Labels)

 			pastPeriodSeries := p.getMatchingSeries(ctx, pastPeriodResult, series)
 			currentSeasonSeries := p.getMatchingSeries(ctx, currentSeasonResult, series)
@@ -407,6 +405,9 @@ func (p *BaseSeasonalProvider) getAnomalies(ctx context.Context, orgID valuer.UU
 			past2SeasonSeries := p.getMatchingSeries(ctx, past2SeasonResult, series)
 			past3SeasonSeries := p.getMatchingSeries(ctx, past3SeasonResult, series)

+			stdDev := p.getStdDev(currentSeasonSeries)
+			p.logger.InfoContext(ctx, "calculated standard deviation for series", "anomaly_std_dev", stdDev, "anomaly_labels", series.Labels)
+
 			prevSeriesAvg := p.getAvg(pastPeriodSeries)
 			currentSeasonSeriesAvg := p.getAvg(currentSeasonSeries)
 			pastSeasonSeriesAvg := p.getAvg(pastSeasonSeries)
@@ -435,6 +436,7 @@ func (p *BaseSeasonalProvider) getAnomalies(ctx context.Context, orgID valuer.UU
 			upperBoundSeries, lowerBoundSeries := p.getBounds(
 				series,
 				predictedSeries,
+				currentSeasonSeries,
 				zScoreThreshold,
 			)
 			aggOfInterest.UpperBoundSeries = append(aggOfInterest.UpperBoundSeries, upperBoundSeries)
--- a/ee/query-service/rules/anomaly.go
+++ b/ee/query-service/rules/anomaly.go
@@ -78,11 +78,6 @@ func NewAnomalyRule(

 	opts = append(opts, baserules.WithLogger(logger))

-	if p.RuleCondition.CompareOp == ruletypes.ValueIsBelow {
-		target := -1 * *p.RuleCondition.Target
-		p.RuleCondition.Target = &target
-	}
-
 	baseRule, err := baserules.NewBaseRule(id, orgID, p, reader, opts...)
 	if err != nil {
 		return nil, err
--- a/pkg/types/ruletypes/api_params.go
+++ b/pkg/types/ruletypes/api_params.go
@@ -207,6 +207,7 @@ func (r *PostableRule) processRuleDefaults() error {
 				q.Expression = qLabel
 			}
 		}
+
 		//added alerts v2 fields
 		if r.SchemaVersion == DefaultSchemaVersion {
 			thresholdName := CriticalThresholdName
@@ -215,12 +216,20 @@ func (r *PostableRule) processRuleDefaults() error {
 					thresholdName = severity
 				}
 			}
+
+			// For anomaly detection with ValueIsBelow, negate the target
+			targetValue := r.RuleCondition.Target
+			if r.RuleType == RuleTypeAnomaly && r.RuleCondition.CompareOp == ValueIsBelow && targetValue != nil {
+				negated := -1 * *targetValue
+				targetValue = &negated
+			}
+
 			thresholdData := RuleThresholdData{
 				Kind: BasicThresholdKind,
 				Spec: BasicRuleThresholds{{
 					Name:        thresholdName,
 					TargetUnit:  r.RuleCondition.TargetUnit,
-					TargetValue: r.RuleCondition.Target,
+					TargetValue: targetValue,
 					MatchType:   r.RuleCondition.MatchType,
 					CompareOp:   r.RuleCondition.CompareOp,
 					Channels:    r.PreferredChannels,
--- a/pkg/types/ruletypes/api_params_test.go
+++ b/pkg/types/ruletypes/api_params_test.go
@@ -718,3 +718,353 @@ func TestParseIntoRuleMultipleThresholds(t *testing.T) {

 	assert.Equal(t, 1, len(vector))
 }
+
+func TestAnomalyNegationShouldAlert(t *testing.T) {
+	tests := []struct {
+		name          string
+		ruleJSON      []byte
+		series        v3.Series
+		shouldAlert   bool
+		expectedValue float64
+	}{
+		{
+			name: "anomaly rule with ValueIsBelow - should alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyBelowTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "1",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: -2.1}, // below & at least once, should alert
+					{Timestamp: 2000, Value: -2.3},
+				},
+			},
+			shouldAlert:   true,
+			expectedValue: -2.1,
+		},
+		{
+			name: "anomaly rule with ValueIsBelow; should not alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyBelowTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "1",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`), // below & at least once, no value below -2.0
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: -1.9},
+					{Timestamp: 2000, Value: -1.8},
+				},
+			},
+			shouldAlert: false,
+		},
+		{
+			name: "anomaly rule with ValueIsAbove; should alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyAboveTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "1",
+					"op": "1",
+					"selectedQuery": "A"
+				}
+			}`), // above & at least once, should alert
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: 2.1}, // above 2.0, should alert
+					{Timestamp: 2000, Value: 2.2},
+				},
+			},
+			shouldAlert:   true,
+			expectedValue: 2.1,
+		},
+		{
+			name: "anomaly rule with ValueIsAbove; should not alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyAboveTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "1",
+					"op": "1",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: 1.1},
+					{Timestamp: 2000, Value: 1.2},
+				},
+			},
+			shouldAlert: false,
+		},
+		{
+			name: "anomaly rule with ValueIsBelow and AllTheTimes; should alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyBelowAllTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "2",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`), // below and all the times
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: -2.1}, // all below -2
+					{Timestamp: 2000, Value: -2.2},
+					{Timestamp: 3000, Value: -2.5},
+				},
+			},
+			shouldAlert:   true,
+			expectedValue: -2.1, // max value when all are below threshold
+		},
+		{
+			name: "anomaly rule with ValueIsBelow and AllTheTimes; should not alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyBelowAllTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 2.0,
+					"matchType": "2",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: -3.0},
+					{Timestamp: 2000, Value: -1.0}, // above -2, breaks condition
+					{Timestamp: 3000, Value: -2.5},
+				},
+			},
+			shouldAlert: false,
+		},
+		{
+			name: "anomaly rule with ValueOutsideBounds; should alert",
+			ruleJSON: []byte(`{
+				"alert": "AnomalyOutOfBoundsTest",
+				"ruleType": "anomaly_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 7.0,
+					"matchType": "1",
+					"op": "7",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: -8.0}, // abs(−8) >= 7, alert
+					{Timestamp: 2000, Value: 5.0},
+				},
+			},
+			shouldAlert:   true,
+			expectedValue: -8.0,
+		},
+		{
+			name: "non-anomaly threshold rule with ValueIsBelow; should alert",
+			ruleJSON: []byte(`{
+				"alert": "ThresholdTest",
+				"ruleType": "threshold_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 90.0,
+					"matchType": "1",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: 80.0}, // below 90, should alert
+					{Timestamp: 2000, Value: 85.0},
+				},
+			},
+			shouldAlert:   true,
+			expectedValue: 80.0,
+		},
+		{
+			name: "non-anomaly rule with ValueIsBelow - should not alert",
+			ruleJSON: []byte(`{
+				"alert": "ThresholdTest",
+				"ruleType": "threshold_rule",
+				"condition": {
+					"compositeQuery": {
+						"queryType": "builder",
+						"queries": [{
+							"type": "builder_query",
+							"spec": {
+								"name": "A",
+								"signal": "metrics",
+								"aggregations": [{"metricName": "test", "spaceAggregation": "p50"}],
+								"stepInterval": "5m"
+							}
+						}]
+					},
+					"target": 50.0,
+					"matchType": "1",
+					"op": "2",
+					"selectedQuery": "A"
+				}
+			}`),
+			series: v3.Series{
+				Labels: map[string]string{"host": "server1"},
+				Points: []v3.Point{
+					{Timestamp: 1000, Value: 60.0}, // below, should alert
+					{Timestamp: 2000, Value: 90.0},
+				},
+			},
+			shouldAlert: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rule := PostableRule{}
+			err := json.Unmarshal(tt.ruleJSON, &rule)
+			if err != nil {
+				t.Fatalf("Failed to unmarshal rule: %v", err)
+			}
+
+			ruleThreshold, err := rule.RuleCondition.Thresholds.GetRuleThreshold()
+			if err != nil {
+				t.Fatalf("unexpected error from GetRuleThreshold: %v", err)
+			}
+
+			resultVector, err := ruleThreshold.ShouldAlert(tt.series, "")
+			if err != nil {
+				t.Fatalf("unexpected error from ShouldAlert: %v", err)
+			}
+
+			shouldAlert := len(resultVector) > 0
+
+			if shouldAlert != tt.shouldAlert {
+				t.Errorf("Expected shouldAlert=%v, got %v. %s",
+					tt.shouldAlert, shouldAlert, tt.name)
+			}
+
+			if tt.shouldAlert && len(resultVector) > 0 {
+				sample := resultVector[0]
+				if sample.V != tt.expectedValue {
+					t.Errorf("Expected alert value=%.2f, got %.2f. %s",
+						tt.expectedValue, sample.V, tt.name)
+				}
+			}
+		})
+	}
+}