fix: order by selection decides the result series (#3138)

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 {
+			orderByList := builderQueries[result.QueryName].OrderBy
-				if item.ColumnName == constants.SigNozOrderByValue {
-					orderAsc = strings.ToLower(item.Order) == "asc"
-					break
-				}
-			}
 			if limit != 0 {
-				sort.Slice(result.Series, func(i, j int) bool {
+				if len(orderByList) == 0 {
-					if orderAsc {
+					// If no orderBy is specified, sort by value in descending order
-						return result.Series[i].Points[0].Value < result.Series[j].Points[0].Value
+					orderByList = []v3.OrderBy{{ColumnName: constants.SigNozOrderByValue, Order: "desc"}}
 				}
-					return result.Series[i].Points[0].Value > result.Series[j].Points[0].Value
+				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
+							}
+						}
+					}
+					// Preserve original order if no matching orderBy is found
+					return i < j
 				})
+
 				if len(result.Series) > int(limit) {
 					result.Series = result.Series[:limit]
 				}

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 {