From d95148359789c5403de97a6e71b5ba2c577dff9d Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Tue, 21 Mar 2023 11:47:04 +0530 Subject: [PATCH] fix: substitute nan negative rate from couter resets (#2449) --- .../app/metrics/query_builder.go | 14 ++++++--- .../app/metrics/query_builder_test.go | 31 ++++++++++++++++--- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pkg/query-service/app/metrics/query_builder.go b/pkg/query-service/app/metrics/query_builder.go index 784b727514..c57cdf49ca 100644 --- a/pkg/query-service/app/metrics/query_builder.go +++ b/pkg/query-service/app/metrics/query_builder.go @@ -44,6 +44,9 @@ var AggregateOperatorToSQLFunc = map[model.AggregateOperator]string{ model.RATE_MIN: "min", } +// See https://github.com/SigNoz/signoz/issues/2151#issuecomment-1467249056 +var rateWithoutNegative = `if (runningDifference(value) < 0 OR runningDifference(ts) < 0, nan, runningDifference(value)/runningDifference(ts))` + var SupportedFunctions = []string{"exp", "log", "ln", "exp2", "log2", "exp10", "log10", "sqrt", "cbrt", "erf", "erfc", "lgamma", "tgamma", "sin", "cos", "tan", "asin", "acos", "atan", "degrees", "radians"} func GoValuateFuncs() map[string]govaluate.ExpressionFunction { @@ -200,7 +203,7 @@ func BuildMetricQuery(qp *model.QueryRangeParamsV2, mq *model.MetricQuery, table subQuery := fmt.Sprintf( queryTmpl, "any(labels) as labels, "+groupTags, qp.Step, op, filterSubQuery, groupBy, groupTags, ) // labels will be same so any should be fine - query := `SELECT %s ts, runningDifference(value)/runningDifference(ts) as value FROM(%s)` + query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s)` query = fmt.Sprintf(query, "labels as fullLabels,", subQuery) return query, nil @@ -211,14 +214,14 @@ func BuildMetricQuery(qp *model.QueryRangeParamsV2, mq *model.MetricQuery, table subQuery := fmt.Sprintf( queryTmpl, rateGroupTags, qp.Step, op, filterSubQuery, rateGroupBy, rateGroupTags, ) // labels will be same so any should be fine - query := `SELECT %s ts, runningDifference(value)/runningDifference(ts) as value FROM(%s) OFFSET 1` + query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s)` query = fmt.Sprintf(query, groupTags, subQuery) query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTags, query, groupBy, groupTags) return query, nil case model.RATE_SUM, model.RATE_MAX, model.RATE_AVG, model.RATE_MIN: op := fmt.Sprintf("%s(value)", AggregateOperatorToSQLFunc[mq.AggregateOperator]) subQuery := fmt.Sprintf(queryTmpl, groupTags, qp.Step, op, filterSubQuery, groupBy, groupTags) - query := `SELECT %s ts, runningDifference(value)/runningDifference(ts) as value FROM(%s) OFFSET 1` + query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s)` query = fmt.Sprintf(query, groupTags, subQuery) return query, nil case model.P05, model.P10, model.P20, model.P25, model.P50, model.P75, model.P90, model.P95, model.P99: @@ -232,9 +235,10 @@ func BuildMetricQuery(qp *model.QueryRangeParamsV2, mq *model.MetricQuery, table subQuery := fmt.Sprintf( queryTmpl, rateGroupTags, qp.Step, op, filterSubQuery, rateGroupBy, rateGroupTags, ) // labels will be same so any should be fine - query := `SELECT %s ts, runningDifference(value)/runningDifference(ts) as value FROM(%s) OFFSET 1` + query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s)` query = fmt.Sprintf(query, groupTags, subQuery) - query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTags, query, groupBy, groupTags) + // filter out NaN values from the rate query as histogramQuantile doesn't support NaN values + query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s HAVING isNaN(value) = 0 ORDER BY %s ts`, groupTags, query, groupBy, groupTags) value := AggregateOperatorToPercentile[mq.AggregateOperator] query = fmt.Sprintf(`SELECT %s ts, histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(value), %.3f) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTagsWithoutLe, value, query, groupByWithoutLe, groupTagsWithoutLe) diff --git a/pkg/query-service/app/metrics/query_builder_test.go b/pkg/query-service/app/metrics/query_builder_test.go index 92bc60c5b0..c749224689 100644 --- a/pkg/query-service/app/metrics/query_builder_test.go +++ b/pkg/query-service/app/metrics/query_builder_test.go @@ -28,7 +28,30 @@ func TestBuildQuery(t *testing.T) { queries := PrepareBuilderMetricQueries(q, "table").Queries So(len(queries), ShouldEqual, 1) So(queries["A"], ShouldContainSubstring, "WHERE metric_name = 'name'") - So(queries["A"], ShouldContainSubstring, "runningDifference(value)/runningDifference(ts)") + So(queries["A"], ShouldContainSubstring, rateWithoutNegative) + }) + + Convey("TestSimpleQueryWithHistQuantile", t, func() { + q := &model.QueryRangeParamsV2{ + Start: 1650991982000, + End: 1651078382000, + Step: 60, + CompositeMetricQuery: &model.CompositeMetricQuery{ + BuilderQueries: map[string]*model.MetricQuery{ + "A": { + QueryName: "A", + MetricName: "name", + AggregateOperator: model.HIST_QUANTILE_99, + Expression: "A", + }, + }, + }, + } + queries := PrepareBuilderMetricQueries(q, "table").Queries + So(len(queries), ShouldEqual, 1) + So(queries["A"], ShouldContainSubstring, "WHERE metric_name = 'name'") + So(queries["A"], ShouldContainSubstring, rateWithoutNegative) + So(queries["A"], ShouldContainSubstring, "HAVING isNaN(value) = 0") }) } @@ -57,7 +80,7 @@ func TestBuildQueryWithFilters(t *testing.T) { So(len(queries), ShouldEqual, 1) So(queries["A"], ShouldContainSubstring, "WHERE metric_name = 'name' AND JSONExtractString(labels, 'a') != 'b'") - So(queries["A"], ShouldContainSubstring, "runningDifference(value)/runningDifference(ts)") + So(queries["A"], ShouldContainSubstring, rateWithoutNegative) So(queries["A"], ShouldContainSubstring, "not match(JSONExtractString(labels, 'code'), 'ERROR_*')") }) } @@ -91,7 +114,7 @@ func TestBuildQueryWithMultipleQueries(t *testing.T) { queries := PrepareBuilderMetricQueries(q, "table").Queries So(len(queries), ShouldEqual, 2) So(queries["A"], ShouldContainSubstring, "WHERE metric_name = 'name' AND JSONExtractString(labels, 'in') IN ['a','b','c']") - So(queries["A"], ShouldContainSubstring, "runningDifference(value)/runningDifference(ts)") + So(queries["A"], ShouldContainSubstring, rateWithoutNegative) }) } @@ -128,7 +151,7 @@ func TestBuildQueryWithMultipleQueriesAndFormula(t *testing.T) { So(len(queries), ShouldEqual, 3) So(queries["C"], ShouldContainSubstring, "SELECT A.ts as ts, A.value / B.value") So(queries["C"], ShouldContainSubstring, "WHERE metric_name = 'name' AND JSONExtractString(labels, 'in') IN ['a','b','c']") - So(queries["C"], ShouldContainSubstring, "runningDifference(value)/runningDifference(ts)") + So(queries["C"], ShouldContainSubstring, rateWithoutNegative) }) }