diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index 51d6ed226d..c32ae0d426 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -2854,20 +2854,48 @@ func applyMetricLimit(results []*v3.Result, queryRangeParams *v3.QueryRangeParam builderQueries := queryRangeParams.CompositeQuery.BuilderQueries if builderQueries != nil && builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics { limit := builderQueries[result.QueryName].Limit - var orderAsc bool - for _, item := range builderQueries[result.QueryName].OrderBy { - if item.ColumnName == constants.SigNozOrderByValue { - orderAsc = strings.ToLower(item.Order) == "asc" - break - } - } + + orderByList := builderQueries[result.QueryName].OrderBy if limit != 0 { - sort.Slice(result.Series, func(i, j int) bool { - if orderAsc { - return result.Series[i].Points[0].Value < result.Series[j].Points[0].Value + if len(orderByList) == 0 { + // If no orderBy is specified, sort by value in descending order + orderByList = []v3.OrderBy{{ColumnName: constants.SigNozOrderByValue, Order: "desc"}} + } + sort.SliceStable(result.Series, func(i, j int) bool { + for _, orderBy := range orderByList { + if orderBy.ColumnName == constants.SigNozOrderByValue { + if result.Series[i].GroupingSetsPoint == nil || result.Series[j].GroupingSetsPoint == nil { + // Handle nil GroupingSetsPoint, if needed + // Here, we assume non-nil values are always less than nil values + return result.Series[i].GroupingSetsPoint != nil + } + if orderBy.Order == "asc" { + return result.Series[i].GroupingSetsPoint.Value < result.Series[j].GroupingSetsPoint.Value + } else if orderBy.Order == "desc" { + return result.Series[i].GroupingSetsPoint.Value > result.Series[j].GroupingSetsPoint.Value + } + } else { + // Sort based on Labels map + labelI, existsI := result.Series[i].Labels[orderBy.ColumnName] + labelJ, existsJ := result.Series[j].Labels[orderBy.ColumnName] + + if !existsI || !existsJ { + // Handle missing labels, if needed + // Here, we assume non-existent labels are always less than existing ones + return existsI + } + + if orderBy.Order == "asc" { + return strings.Compare(labelI, labelJ) < 0 + } else if orderBy.Order == "desc" { + return strings.Compare(labelI, labelJ) > 0 + } + } } - return result.Series[i].Points[0].Value > result.Series[j].Points[0].Value + // Preserve original order if no matching orderBy is found + return i < j }) + if len(result.Series) > int(limit) { result.Series = result.Series[:limit] } diff --git a/pkg/query-service/app/http_handler_test.go b/pkg/query-service/app/http_handler_test.go index 958bcdeee2..014ec900e1 100644 --- a/pkg/query-service/app/http_handler_test.go +++ b/pkg/query-service/app/http_handler_test.go @@ -443,6 +443,278 @@ func TestApplyLimitOnMetricResult(t *testing.T) { }, }, }, + { + // ["GET /api/v1/health", "DELETE /api/v1/health"] so result should be ["DELETE /api/v1/health"] although it has lower value + name: "test limit with operation asc", + inputResult: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "frontend", + "operation": "GET /api/v1/health", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 19.2, + }, + { + Timestamp: 1689220096000, + Value: 19.5, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 19.3, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + }, + }, + }, + params: &v3.QueryRangeParamsV3{ + Start: 1689220036000, + End: 1689220096000, + Step: 60, + CompositeQuery: &v3.CompositeQuery{ + BuilderQueries: map[string]*v3.BuilderQuery{ + "A": { + QueryName: "A", + AggregateAttribute: v3.AttributeKey{Key: "signo_calls_total"}, + DataSource: v3.DataSourceMetrics, + AggregateOperator: v3.AggregateOperatorSumRate, + Expression: "A", + GroupBy: []v3.AttributeKey{{Key: "service_name"}}, + Limit: 1, + OrderBy: []v3.OrderBy{{ColumnName: "operation", Order: "asc"}}, + }, + }, + QueryType: v3.QueryTypeBuilder, + PanelType: v3.PanelTypeGraph, + }, + }, + expectedResult: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + }, + }, + }, + }, + { + name: "test limit with multiple order by labels", + inputResult: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "frontend", + "operation": "GET /api/v1/health", + "status_code": "200", + "priority": "P0", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 19.2, + }, + { + Timestamp: 1689220096000, + Value: 19.5, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 19.3, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + "status_code": "301", + "priority": "P1", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + "status_code": "400", + "priority": "P0", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + "status_code": "200", + "priority": "P1", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + }, + }, + }, + params: &v3.QueryRangeParamsV3{ + Start: 1689220036000, + End: 1689220096000, + Step: 60, + CompositeQuery: &v3.CompositeQuery{ + BuilderQueries: map[string]*v3.BuilderQuery{ + "A": { + QueryName: "A", + AggregateAttribute: v3.AttributeKey{Key: "signo_calls_total"}, + DataSource: v3.DataSourceMetrics, + AggregateOperator: v3.AggregateOperatorSumRate, + Expression: "A", + GroupBy: []v3.AttributeKey{{Key: "service_name"}, {Key: "operation"}, {Key: "status_code"}, {Key: "priority"}}, + Limit: 2, + OrderBy: []v3.OrderBy{ + {ColumnName: "priority", Order: "asc"}, + {ColumnName: "status_code", Order: "desc"}, + }, + }, + }, + QueryType: v3.QueryTypeBuilder, + PanelType: v3.PanelTypeGraph, + }, + }, + expectedResult: []*v3.Result{ + { + QueryName: "A", + Series: []*v3.Series{ + { + Labels: map[string]string{ + "service_name": "frontend", + "operation": "GET /api/v1/health", + "status_code": "200", + "priority": "P0", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 19.2, + }, + { + Timestamp: 1689220096000, + Value: 19.5, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 19.3, + }, + }, + { + Labels: map[string]string{ + "service_name": "route", + "operation": "DELETE /api/v1/health", + "status_code": "400", + "priority": "P0", + }, + Points: []v3.Point{ + { + Timestamp: 1689220036000, + Value: 8.83, + }, + { + Timestamp: 1689220096000, + Value: 8.83, + }, + }, + GroupingSetsPoint: &v3.Point{ + Timestamp: 0, + Value: 8.83, + }, + }, + }, + }, + }, + }, } for _, c := range cases {