From 06c075466b910bbcdc5611b9bf965b54e420e878 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 9 Aug 2024 12:34:40 +0530 Subject: [PATCH] chore: add eval tests for threshold rule (#5398) --- go.sum | 8 - pkg/query-service/app/querier/querier.go | 2 +- pkg/query-service/app/querier/v2/querier.go | 2 +- pkg/query-service/rules/thresholdRule.go | 11 +- pkg/query-service/rules/thresholdRule_test.go | 168 ++++++++++++++++++ 5 files changed, 178 insertions(+), 13 deletions(-) diff --git a/go.sum b/go.sum index e3df44618e..b1eeff2873 100644 --- a/go.sum +++ b/go.sum @@ -48,12 +48,8 @@ github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mx github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/ClickHouse/ch-go v0.61.3 h1:MmBwUhXrAOBZK7n/sWBzq6FdIQ01cuF2SaaO8KlDRzI= -github.com/ClickHouse/ch-go v0.61.3/go.mod h1:1PqXjMz/7S1ZUaKvwPA3i35W2bz2mAMFeCi6DIXgGwQ= github.com/ClickHouse/ch-go v0.61.5 h1:zwR8QbYI0tsMiEcze/uIMK+Tz1D3XZXLdNrlaOpeEI4= github.com/ClickHouse/ch-go v0.61.5/go.mod h1:s1LJW/F/LcFs5HJnuogFMta50kKDO0lf9zzfrbl0RQg= -github.com/ClickHouse/clickhouse-go/v2 v2.20.0 h1:bvlLQ31XJfl7MxIqAq2l1G6JhHYzqEXdvfpMeU6bkKc= -github.com/ClickHouse/clickhouse-go/v2 v2.20.0/go.mod h1:VQfyA+tCwCRw2G7ogfY8V0fq/r0yJWzy8UDrjiP/Lbs= github.com/ClickHouse/clickhouse-go/v2 v2.23.2 h1:+DAKPMnxLS7pduQZsrJc8OhdLS2L9MfDEJ2TS+hpYDM= github.com/ClickHouse/clickhouse-go/v2 v2.23.2/go.mod h1:aNap51J1OM3yxQJRgM+AlP/MPkGBCL8A74uQThoQhR0= github.com/Code-Hex/go-generics-cache v1.5.1 h1:6vhZGc5M7Y/YD8cIUcY8kcuQLB4cHR7U+0KMqAA0KcU= @@ -695,8 +691,6 @@ github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFt github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= github.com/shoenig/test v0.6.4 h1:kVTaSd7WLz5WZ2IaoM0RSzRsUD+m8wRR+5qvntpn4LU= github.com/shoenig/test v0.6.4/go.mod h1:byHiCGXqrVaflBLAMq/srcZIHynQPQgeyvkvXnjqq0k= -github.com/shopspring/decimal v1.3.1 h1:2Usl1nmF/WZucqkFZhnfFYxxxu8LG21F6nPQBE5gKV8= -github.com/shopspring/decimal v1.3.1/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k= github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= @@ -722,8 +716,6 @@ github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/srikanthccv/ClickHouse-go-mock v0.7.0 h1:XhRMX2663xkDGq3DYavw8m75O94s9u76hOIjo9QBl8c= -github.com/srikanthccv/ClickHouse-go-mock v0.7.0/go.mod h1:IJZ/eL1h4cOy/Jo3PzNKXSPmqRus15BC2MbduYPpA/g= github.com/srikanthccv/ClickHouse-go-mock v0.8.0 h1:DeeM8XLbTFl6sjYPPwazPEXx7kmRV8TgPFVkt1SqT0Y= github.com/srikanthccv/ClickHouse-go-mock v0.8.0/go.mod h1:pgJm+apjvi7FHxEdgw1Bt4MRbUYpVxyhKQ/59Wkig24= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/pkg/query-service/app/querier/querier.go b/pkg/query-service/app/querier/querier.go index 95cfc7cc73..64e4a33ed2 100644 --- a/pkg/query-service/app/querier/querier.go +++ b/pkg/query-service/app/querier/querier.go @@ -251,7 +251,7 @@ func filterCachedPoints(cachedSeries []*v3.Series, start, end int64) { for _, c := range cachedSeries { points := []v3.Point{} for _, p := range c.Points { - if p.Timestamp < start || p.Timestamp > end { + if (p.Timestamp < start || p.Timestamp > end) && p.Timestamp != 0 { continue } points = append(points, p) diff --git a/pkg/query-service/app/querier/v2/querier.go b/pkg/query-service/app/querier/v2/querier.go index 9c1f458adf..5e0c18afb5 100644 --- a/pkg/query-service/app/querier/v2/querier.go +++ b/pkg/query-service/app/querier/v2/querier.go @@ -260,7 +260,7 @@ func filterCachedPoints(cachedSeries []*v3.Series, start, end int64) { for _, c := range cachedSeries { points := []v3.Point{} for _, p := range c.Points { - if p.Timestamp < start || p.Timestamp > end { + if (p.Timestamp < start || p.Timestamp > end) && p.Timestamp != 0 { continue } points = append(points, p) diff --git a/pkg/query-service/rules/thresholdRule.go b/pkg/query-service/rules/thresholdRule.go index ef65f7966f..5d4670c62c 100644 --- a/pkg/query-service/rules/thresholdRule.go +++ b/pkg/query-service/rules/thresholdRule.go @@ -376,9 +376,14 @@ func (r *ThresholdRule) populateTemporality(ctx context.Context, qp *v3.QueryRan } } - nameToTemporality, err := r.FetchTemporality(ctx, missingTemporality, ch) - if err != nil { - return err + var nameToTemporality map[string]map[v3.Temporality]bool + var err error + + if len(missingTemporality) > 0 { + nameToTemporality, err = r.FetchTemporality(ctx, missingTemporality, ch) + if err != nil { + return err + } } if qp.CompositeQuery != nil && len(qp.CompositeQuery.BuilderQueries) > 0 { diff --git a/pkg/query-service/rules/thresholdRule_test.go b/pkg/query-service/rules/thresholdRule_test.go index 65f4da088a..1363bfcc9c 100644 --- a/pkg/query-service/rules/thresholdRule_test.go +++ b/pkg/query-service/rules/thresholdRule_test.go @@ -1,13 +1,18 @@ package rules import ( + "context" + "strings" "testing" "time" "github.com/stretchr/testify/assert" + "go.signoz.io/signoz/pkg/query-service/app/clickhouseReader" "go.signoz.io/signoz/pkg/query-service/featureManager" v3 "go.signoz.io/signoz/pkg/query-service/model/v3" "go.signoz.io/signoz/pkg/query-service/utils/labels" + + cmock "github.com/srikanthccv/ClickHouse-go-mock" ) func TestThresholdRuleShouldAlert(t *testing.T) { @@ -931,3 +936,166 @@ func TestThresholdRuleClickHouseTmpl(t *testing.T) { assert.Equal(t, c.expectedQuery, secondTimeParams.CompositeQuery.ClickHouseQueries["A"].Query, "Test case %d", idx) } } + +type queryMatcherAny struct { +} + +func (m *queryMatcherAny) Match(string, string) error { + return nil +} + +func TestThresholdRuleUnitCombinations(t *testing.T) { + postableRule := PostableRule{ + AlertName: "Units test", + AlertType: "METRIC_BASED_ALERT", + RuleType: RuleTypeThreshold, + EvalWindow: Duration(5 * time.Minute), + Frequency: Duration(1 * time.Minute), + RuleCondition: &RuleCondition{ + CompositeQuery: &v3.CompositeQuery{ + QueryType: v3.QueryTypeBuilder, + BuilderQueries: map[string]*v3.BuilderQuery{ + "A": { + QueryName: "A", + StepInterval: 60, + AggregateAttribute: v3.AttributeKey{ + Key: "signoz_calls_total", + }, + AggregateOperator: v3.AggregateOperatorSumRate, + DataSource: v3.DataSourceMetrics, + Expression: "A", + }, + }, + }, + }, + } + fm := featureManager.StartManager() + mock, err := cmock.NewClickHouseWithQueryMatcher(nil, &queryMatcherAny{}) + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + + cols := make([]cmock.ColumnType, 0) + cols = append(cols, cmock.ColumnType{Name: "value", Type: "Float64"}) + cols = append(cols, cmock.ColumnType{Name: "attr", Type: "String"}) + cols = append(cols, cmock.ColumnType{Name: "timestamp", Type: "String"}) + + cases := []struct { + targetUnit string + yAxisUnit string + values [][]interface{} + expectAlerts int + compareOp string + matchType string + target float64 + summaryAny []string + }{ + { + targetUnit: "s", + yAxisUnit: "ns", + values: [][]interface{}{ + {float64(572588400), "attr", time.Now()}, // 0.57 seconds + {float64(572386400), "attr", time.Now().Add(1 * time.Second)}, // 0.57 seconds + {float64(300947400), "attr", time.Now().Add(2 * time.Second)}, // 0.3 seconds + {float64(299316000), "attr", time.Now().Add(3 * time.Second)}, // 0.3 seconds + {float64(66640400.00000001), "attr", time.Now().Add(4 * time.Second)}, // 0.06 seconds + }, + expectAlerts: 0, + compareOp: "1", // Above + matchType: "1", // Once + target: 1, // 1 second + }, + { + targetUnit: "ms", + yAxisUnit: "ns", + values: [][]interface{}{ + {float64(572588400), "attr", time.Now()}, // 572.58 ms + {float64(572386400), "attr", time.Now().Add(1 * time.Second)}, // 572.38 ms + {float64(300947400), "attr", time.Now().Add(2 * time.Second)}, // 300.94 ms + {float64(299316000), "attr", time.Now().Add(3 * time.Second)}, // 299.31 ms + {float64(66640400.00000001), "attr", time.Now().Add(4 * time.Second)}, // 66.64 ms + }, + expectAlerts: 4, + compareOp: "1", // Above + matchType: "1", // Once + target: 200, // 200 ms + summaryAny: []string{ + "observed metric value is 299 ms", + "the observed metric value is 573 ms", + "the observed metric value is 572 ms", + "the observed metric value is 301 ms", + }, + }, + { + targetUnit: "decgbytes", + yAxisUnit: "bytes", + values: [][]interface{}{ + {float64(2863284053), "attr", time.Now()}, // 2.86 GB + {float64(2863388842), "attr", time.Now().Add(1 * time.Second)}, // 2.86 GB + {float64(300947400), "attr", time.Now().Add(2 * time.Second)}, // 0.3 GB + {float64(299316000), "attr", time.Now().Add(3 * time.Second)}, // 0.3 GB + {float64(66640400.00000001), "attr", time.Now().Add(4 * time.Second)}, // 66.64 MB + }, + expectAlerts: 0, + compareOp: "1", // Above + matchType: "1", // Once + target: 200, // 200 GB + }, + } + + for idx, c := range cases { + rows := cmock.NewRows(cols, c.values) + + // We are testing the eval logic after the query is run + // so we don't care about the query string here + queryString := "SELECT any" + mock. + ExpectQuery(queryString). + WillReturnRows(rows) + postableRule.RuleCondition.CompareOp = CompareOp(c.compareOp) + postableRule.RuleCondition.MatchType = MatchType(c.matchType) + postableRule.RuleCondition.Target = &c.target + postableRule.RuleCondition.CompositeQuery.Unit = c.yAxisUnit + postableRule.RuleCondition.TargetUnit = c.targetUnit + postableRule.Annotations = map[string]string{ + "description": "This alert is fired when the defined metric (current value: {{$value}}) crosses the threshold ({{$threshold}})", + "summary": "The rule threshold is set to {{$threshold}}, and the observed metric value is {{$value}}", + } + + options := clickhouseReader.NewOptions("", 0, 0, 0, "", "archiveNamespace") + reader := clickhouseReader.NewReaderFromClickhouseConnection(mock, options, nil, "", fm, "") + + rule, err := NewThresholdRule("69", &postableRule, ThresholdRuleOpts{}, fm, reader) + rule.temporalityMap = map[string]map[v3.Temporality]bool{ + "signoz_calls_total": { + v3.Delta: true, + }, + } + if err != nil { + assert.NoError(t, err) + } + + queriers := Queriers{ + Ch: mock, + } + + retVal, err := rule.Eval(context.Background(), time.Now(), &queriers) + if err != nil { + assert.NoError(t, err) + } + + assert.Equal(t, c.expectAlerts, retVal.(int), "case %d", idx) + if c.expectAlerts != 0 { + foundCount := 0 + for _, item := range rule.active { + for _, summary := range c.summaryAny { + if strings.Contains(item.Annotations.Get("summary"), summary) { + foundCount++ + break + } + } + } + assert.Equal(t, c.expectAlerts, foundCount, "case %d", idx) + } + } +}