Merge pull request #14083 from charleskorn/sort-matrix-series

promql: ensure series in matrix values returned for instant queries are sorted

docs/querying/api.md

@@ -473,6 +473,9 @@ Range vectors are returned as result type `matrix`. The corresponding
 Each series could have the `"values"` key, or the `"histograms"` key, or both. 
 For a given timestamp, there will only be one sample of either float or histogram type.
 
+Series are returned sorted by `metric`. Functions such as [`sort`](functions.md#sort)
+and [`sort_by_label`](functions.md#sort_by_label) have no effect for range vectors.
+
 ### Instant vectors
 
 Instant vectors are returned as result type `vector`. The corresponding
@@ -491,6 +494,10 @@ Instant vectors are returned as result type `vector`. The corresponding
 
 Each series could have the `"value"` key, or the `"histogram"` key, but not both.
 
+Series are not guaranteed to be returned in any particular order unless a function
+such as [`sort`](functions.md#sort) or [`sort_by_label`](functions.md#sort_by_label)`
+is used.
+
 ### Scalars
 
 Scalar results are returned as result type `scalar`. The corresponding

docs/querying/functions.md

@@ -596,10 +596,14 @@ have exactly one element, `scalar` will return `NaN`.
 `sort(v instant-vector)` returns vector elements sorted by their sample values,
 in ascending order. Native histograms are sorted by their sum of observations.
 
+Please note that `sort` only affects the results of instant queries, as range query results always have a fixed output ordering.
+
 ## `sort_desc()`
 
 Same as `sort`, but sorts in descending order.
 
+Like `sort`, `sort_desc` only affects the results of instant queries, as range query results always have a fixed output ordering.
+
 ## `sort_by_label()`
 
 **This function has to be enabled via the [feature flag](../feature_flags/) `--enable-feature=promql-experimental-functions`.**

promql/engine.go

@@ -752,6 +752,7 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval
 		case parser.ValueTypeScalar:
 			return Scalar{V: mat[0].Floats[0].F, T: start}, warnings, nil
 		case parser.ValueTypeMatrix:
+			ng.sortMatrixResult(ctx, query, mat)
 			return mat, warnings, nil
 		default:
 			panic(fmt.Errorf("promql.Engine.exec: unexpected expression type %q", s.Expr.Type()))
@@ -790,11 +791,15 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval
 	}
 
 	// TODO(fabxc): where to ensure metric labels are a copy from the storage internals.
+	ng.sortMatrixResult(ctx, query, mat)
+
+	return mat, warnings, nil
+}
+
+func (ng *Engine) sortMatrixResult(ctx context.Context, query *query, mat Matrix) {
 	sortSpanTimer, _ := query.stats.GetSpanTimer(ctx, stats.ResultSortTime, ng.metrics.queryResultSort)
 	sort.Sort(mat)
 	sortSpanTimer.Finish()
-
-	return mat, warnings, nil
 }
 
 // subqueryTimes returns the sum of offsets and ranges of all subqueries in the path.

promql/engine_test.go

@@ -3222,6 +3222,82 @@ func TestRangeQuery(t *testing.T) {
 	}
 }
 
+func TestInstantQueryWithRangeVectorSelector(t *testing.T) {
+	engine := newTestEngine()
+
+	baseT := timestamp.Time(0)
+	storage := promqltest.LoadedStorage(t, `
+		load 1m
+			some_metric{env="1"} 0+1x4
+			some_metric{env="2"} 0+2x4
+			some_metric_with_stale_marker 0 1 stale 3
+	`)
+	t.Cleanup(func() { require.NoError(t, storage.Close()) })
+
+	testCases := map[string]struct {
+		expr     string
+		expected promql.Matrix
+		ts       time.Time
+	}{
+		"matches series with points in range": {
+			expr: "some_metric[1m]",
+			ts:   baseT.Add(2 * time.Minute),
+			expected: promql.Matrix{
+				{
+					Metric: labels.FromStrings("__name__", "some_metric", "env", "1"),
+					Floats: []promql.FPoint{
+						{T: timestamp.FromTime(baseT.Add(time.Minute)), F: 1},
+						{T: timestamp.FromTime(baseT.Add(2 * time.Minute)), F: 2},
+					},
+				},
+				{
+					Metric: labels.FromStrings("__name__", "some_metric", "env", "2"),
+					Floats: []promql.FPoint{
+						{T: timestamp.FromTime(baseT.Add(time.Minute)), F: 2},
+						{T: timestamp.FromTime(baseT.Add(2 * time.Minute)), F: 4},
+					},
+				},
+			},
+		},
+		"matches no series": {
+			expr:     "some_nonexistent_metric[1m]",
+			ts:       baseT,
+			expected: promql.Matrix{},
+		},
+		"no samples in range": {
+			expr:     "some_metric[1m]",
+			ts:       baseT.Add(20 * time.Minute),
+			expected: promql.Matrix{},
+		},
+		"metric with stale marker": {
+			expr: "some_metric_with_stale_marker[3m]",
+			ts:   baseT.Add(3 * time.Minute),
+			expected: promql.Matrix{
+				{
+					Metric: labels.FromStrings("__name__", "some_metric_with_stale_marker"),
+					Floats: []promql.FPoint{
+						{T: timestamp.FromTime(baseT), F: 0},
+						{T: timestamp.FromTime(baseT.Add(time.Minute)), F: 1},
+						{T: timestamp.FromTime(baseT.Add(3 * time.Minute)), F: 3},
+					},
+				},
+			},
+		},
+	}
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			q, err := engine.NewInstantQuery(context.Background(), storage, nil, testCase.expr, testCase.ts)
+			require.NoError(t, err)
+			defer q.Close()
+
+			res := q.Exec(context.Background())
+			require.NoError(t, res.Err)
+			testutil.RequireEqual(t, testCase.expected, res.Value)
+		})
+	}
+}
+
 func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {
 	// TODO(codesome): Integrate histograms into the PromQL testing framework
 	// and write more tests there.