Skip to content

Commit

Permalink
fix: skip first record only for rate metrics (#3609)
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanthccv authored Sep 22, 2023
1 parent 5c437dd commit 043e5ca
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
18 changes: 18 additions & 0 deletions pkg/query-service/model/v3/v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
12 changes: 11 additions & 1 deletion pkg/query-service/rules/thresholdRule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 043e5ca

Please sign in to comment.