From 043e5ca880a9a16e8461ab12f00bcafba98263c6 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 22 Sep 2023 15:43:21 +0530 Subject: [PATCH] fix: skip first record only for rate metrics (#3609) --- pkg/query-service/model/v3/v3.go | 18 ++++++++++++++++++ pkg/query-service/rules/thresholdRule.go | 12 +++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/pkg/query-service/model/v3/v3.go b/pkg/query-service/model/v3/v3.go index 6dc7b1d24b..23819f81f7 100644 --- a/pkg/query-service/model/v3/v3.go +++ b/pkg/query-service/model/v3/v3.go @@ -138,6 +138,24 @@ func (a AggregateOperator) RequireAttribute(dataSource DataSource) bool { } } +func (a AggregateOperator) IsRateOperator() bool { + switch a { + case AggregateOperatorRate, + AggregateOperatorSumRate, + AggregateOperatorAvgRate, + AggregateOperatorMinRate, + AggregateOperatorMaxRate, + AggregateOperatorRateSum, + AggregateOperatorRateAvg, + AggregateOperatorRateMin, + AggregateOperatorRateMax: + return true + + default: + return false + } +} + type ReduceToOperator string const ( diff --git a/pkg/query-service/rules/thresholdRule.go b/pkg/query-service/rules/thresholdRule.go index ced9577b79..f5bbef8fad 100644 --- a/pkg/query-service/rules/thresholdRule.go +++ b/pkg/query-service/rules/thresholdRule.go @@ -406,6 +406,16 @@ func (r *ThresholdRule) prepareQueryRange(ts time.Time) *v3.QueryRangeParamsV3 { } } +func (r *ThresholdRule) shouldSkipFirstRecord() bool { + shouldSkip := false + for _, q := range r.ruleCondition.CompositeQuery.BuilderQueries { + if q.DataSource == v3.DataSourceMetrics && q.AggregateOperator.IsRateOperator() { + shouldSkip = true + } + } + return shouldSkip +} + // queryClickhouse runs actual query against clickhouse func (r *ThresholdRule) runChQuery(ctx context.Context, db clickhouse.Conn, query string) (Vector, error) { rows, err := db.Query(ctx, query) @@ -553,7 +563,7 @@ func (r *ThresholdRule) runChQuery(ctx context.Context, db clickhouse.Conn, quer // we skip the first record to support rate cases correctly // improvement(amol): explore approaches to limit this only for // rate uses cases - if exists := skipFirstRecord[labelHash]; exists { + if exists := skipFirstRecord[labelHash]; exists || !r.shouldSkipFirstRecord() { resultMap[labelHash] = sample } else { // looks like the first record for this label combo, skip it