diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index f5671f4d0e..1f0769bb08 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -3056,6 +3056,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que return } + postprocess.ApplyHavingClause(result, queryRangeParams) postprocess.ApplyMetricLimit(result, queryRangeParams) sendQueryResultEvents(r, result, queryRangeParams) @@ -3065,6 +3066,10 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que postprocess.ApplyFunctions(result, queryRangeParams) } + if queryRangeParams.CompositeQuery.FillGaps { + postprocess.FillGaps(result, queryRangeParams) + } + resp := v3.QueryRangeResponse{ Result: result, } diff --git a/pkg/query-service/postprocess/having.go b/pkg/query-service/postprocess/having.go index a37ba70379..5c2be7ce8b 100644 --- a/pkg/query-service/postprocess/having.go +++ b/pkg/query-service/postprocess/having.go @@ -6,14 +6,17 @@ import ( v3 "go.signoz.io/signoz/pkg/query-service/model/v3" ) -// applyHavingClause applies the having clause to the result +// ApplyHavingClause applies the having clause to the result // each query has its own having clause // there can be multiple having clauses for each query -func applyHavingClause(result []*v3.Result, queryRangeParams *v3.QueryRangeParamsV3) { +func ApplyHavingClause(result []*v3.Result, queryRangeParams *v3.QueryRangeParamsV3) { for _, result := range result { builderQueries := queryRangeParams.CompositeQuery.BuilderQueries - if builderQueries != nil && (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics) { + // apply having clause for metrics and formula + if builderQueries != nil && + (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics || + builderQueries[result.QueryName].QueryName != builderQueries[result.QueryName].Expression) { havingClause := builderQueries[result.QueryName].Having for i := 0; i < len(result.Series); i++ { @@ -37,46 +40,38 @@ func evaluateHavingClause(having []v3.Having, value float64) bool { return true } + satisfied := true + for _, h := range having { switch h.Operator { case v3.HavingOperatorEqual: - if value == h.Value.(float64) { - return true + if value != h.Value.(float64) { + satisfied = false } case v3.HavingOperatorNotEqual: - if value != h.Value.(float64) { - return true + if value == h.Value.(float64) { + satisfied = false } case v3.HavingOperatorGreaterThan: - if value > h.Value.(float64) { - return true + if value <= h.Value.(float64) { + satisfied = false } case v3.HavingOperatorGreaterThanOrEq: - if value >= h.Value.(float64) { - return true + if value < h.Value.(float64) { + satisfied = false } case v3.HavingOperatorLessThan: - if value < h.Value.(float64) { - return true + if value >= h.Value.(float64) { + satisfied = false } case v3.HavingOperatorLessThanOrEq: - if value <= h.Value.(float64) { - return true + if value > h.Value.(float64) { + satisfied = false } case v3.HavingOperatorIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorIn))): values, ok := h.Value.([]interface{}) if !ok { - return false - } - for _, v := range values { - if value == v.(float64) { - return true - } - } - case v3.HavingOperatorNotIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorNotIn))): - values, ok := h.Value.([]interface{}) - if !ok { - return true + satisfied = false } found := false for _, v := range values { @@ -86,9 +81,24 @@ func evaluateHavingClause(having []v3.Having, value float64) bool { } } if !found { - return true + satisfied = false + } + case v3.HavingOperatorNotIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorNotIn))): + values, ok := h.Value.([]interface{}) + if !ok { + satisfied = false + } + found := false + for _, v := range values { + if value == v.(float64) { + found = true + break + } + } + if found { + satisfied = false } } } - return false + return satisfied } diff --git a/pkg/query-service/postprocess/having_test.go b/pkg/query-service/postprocess/having_test.go index e7e37095fb..90986d14a9 100644 --- a/pkg/query-service/postprocess/having_test.go +++ b/pkg/query-service/postprocess/having_test.go @@ -248,22 +248,86 @@ func TestApplyHavingCaluse(t *testing.T) { }, }, }, + { + name: "test multiple having clause", + results: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Points: []v3.Point{ + { + Value: 0.5, + }, + { + Value: 0.4, + }, + { + Value: 0.3, + }, + { + Value: 0.2, + }, + { + Value: 0.1, + }, + }, + }, + }, + }, + }, + params: &v3.QueryRangeParamsV3{ + CompositeQuery: &v3.CompositeQuery{ + BuilderQueries: map[string]*v3.BuilderQuery{ + "A": { + DataSource: v3.DataSourceMetrics, + Having: []v3.Having{ + { + Operator: v3.HavingOperatorGreaterThanOrEq, + Value: 0.3, + }, + { + Operator: v3.HavingOperatorLessThanOrEq, + Value: 0.4, + }, + }, + }, + }, + }, + }, + want: []*v3.Result{ + { + Series: []*v3.Series{ + { + Points: []v3.Point{ + { + Value: 0.4, + }, + { + Value: 0.3, + }, + }, + }, + }, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - applyHavingClause(tc.results, tc.params) + ApplyHavingClause(tc.results, tc.params) got := tc.results if len(got) != len(tc.want) { - t.Errorf("got %v, want %v", got, tc.want) + t.Errorf("got %v, want %v", len(got), len(tc.want)) } for i := range got { if len(got[i].Series) != len(tc.want[i].Series) { - t.Errorf("got %v, want %v", got, tc.want) + t.Errorf("got %v, want %v", len(got[i].Series), len(tc.want[i].Series)) } for j := range got[i].Series { @@ -273,7 +337,7 @@ func TestApplyHavingCaluse(t *testing.T) { for k := range got[i].Series[j].Points { if got[i].Series[j].Points[k].Value != tc.want[i].Series[j].Points[k].Value { - t.Errorf("got %v, want %v", got, tc.want) + t.Errorf("got %v, want %v", got[i].Series[j].Points[k].Value, tc.want[i].Series[j].Points[k].Value) } } } diff --git a/pkg/query-service/postprocess/limit.go b/pkg/query-service/postprocess/limit.go index 491c91da54..49cd5d04b9 100644 --- a/pkg/query-service/postprocess/limit.go +++ b/pkg/query-service/postprocess/limit.go @@ -17,7 +17,10 @@ func ApplyMetricLimit(results []*v3.Result, queryRangeParams *v3.QueryRangeParam for _, result := range results { builderQueries := queryRangeParams.CompositeQuery.BuilderQueries - if builderQueries != nil && (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics) { + // apply limit for metrics and formula + if builderQueries != nil && + (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics || + builderQueries[result.QueryName].QueryName != builderQueries[result.QueryName].Expression) { limit := builderQueries[result.QueryName].Limit orderByList := builderQueries[result.QueryName].OrderBy diff --git a/pkg/query-service/postprocess/query_range.go b/pkg/query-service/postprocess/process.go similarity index 96% rename from pkg/query-service/postprocess/query_range.go rename to pkg/query-service/postprocess/process.go index 890c37148e..fc35b404de 100644 --- a/pkg/query-service/postprocess/query_range.go +++ b/pkg/query-service/postprocess/process.go @@ -18,7 +18,7 @@ func PostProcessResult(result []*v3.Result, queryRangeParams *v3.QueryRangeParam // It's not included in the query because it doesn't work nicely with caching // With this change, if you have a query with a having clause, and then you change the having clause // to something else, the query will still be cached. - applyHavingClause(result, queryRangeParams) + ApplyHavingClause(result, queryRangeParams) // We apply the metric limit here because it's not part of the clickhouse query // The limit in the context of the time series query is the number of time series // So for the limit to work, we need to know what series to keep and what to discard @@ -64,6 +64,8 @@ func PostProcessResult(result []*v3.Result, queryRangeParams *v3.QueryRangeParam return nil, err } formulaResult.QueryName = query.QueryName + ApplyHavingClause([]*v3.Result{formulaResult}, queryRangeParams) + ApplyMetricLimit([]*v3.Result{formulaResult}, queryRangeParams) result = append(result, formulaResult) } } diff --git a/pkg/query-service/postprocess/process_test.go b/pkg/query-service/postprocess/process_test.go new file mode 100644 index 0000000000..5f599f7be0 --- /dev/null +++ b/pkg/query-service/postprocess/process_test.go @@ -0,0 +1,222 @@ +package postprocess + +import ( + "reflect" + "testing" + + v3 "go.signoz.io/signoz/pkg/query-service/model/v3" +) + +func TestPostProcessResult(t *testing.T) { + testCases := []struct { + name string + results []*v3.Result + queryRangeParams *v3.QueryRangeParamsV3 + want []*v3.Result + }{ + { + name: "test1", + results: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "frontend", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 10, + }, + { + Timestamp: 2, + Value: 20, + }, + }, + }, + { + Labels: map[string]string{ + "service_name": "redis", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 12, + }, + { + Timestamp: 2, + Value: 45, + }, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 2, + }, + { + Timestamp: 2, + Value: 45, + }, + }, + }, + }, + }, + { + QueryName: "B", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "redis", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 6, + }, + { + Timestamp: 2, + Value: 9, + }, + }, + }, + }, + }, + }, + want: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "redis", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 12, + }, + { + Timestamp: 2, + Value: 45, + }, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 2, + }, + { + Timestamp: 2, + Value: 45, + }, + }, + }, + { + Labels: map[string]string{ + "service_name": "frontend", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 10, + }, + { + Timestamp: 2, + Value: 20, + }, + }, + }, + }, + }, + { + QueryName: "B", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "redis", + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 6, + }, + { + Timestamp: 2, + Value: 9, + }, + }, + }, + }, + }, + { + QueryName: "F1", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "redis", + }, + LabelsArray: []map[string]string{ + { + "service_name": "redis", + }, + }, + Points: []v3.Point{ + { + Timestamp: 1, + Value: 0.5, + }, + { + Timestamp: 2, + Value: 0.2, + }, + }, + }, + }, + }, + }, + queryRangeParams: &v3.QueryRangeParamsV3{ + CompositeQuery: &v3.CompositeQuery{ + BuilderQueries: map[string]*v3.BuilderQuery{ + "A": { + DataSource: v3.DataSourceMetrics, + QueryName: "A", + Expression: "A", + }, + "B": { + DataSource: v3.DataSourceMetrics, + QueryName: "B", + Expression: "B", + }, + "F1": { + Expression: "B/A", + QueryName: "F1", + Limit: 1, + }, + }, + }, + }, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + got, err := PostProcessResult(tt.results, tt.queryRangeParams) + if err != nil { + t.Errorf("PostProcessResult() error = %v", err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("PostProcessResult() = %v, want %v", got, tt.want) + } + }) + } +}