From a4381608a03b329026b2d3cd299a32ec32affff6 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Mon, 13 May 2024 10:43:06 +1000 Subject: [PATCH 1/4] Add failing test case Signed-off-by: Charles Korn --- promql/engine_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/promql/engine_test.go b/promql/engine_test.go index cc918554689..7b8deed935b 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3209,6 +3209,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 TestNativeHistogramRate(t *testing.T) { // TODO(beorn7): Integrate histograms into the PromQL testing framework // and write more tests there. From 036c87223c521af0f26e3963ba11e591332ffe26 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Mon, 13 May 2024 10:45:13 +1000 Subject: [PATCH 2/4] Ensure series in matrix values returned for instant queries are always sorted Signed-off-by: Charles Korn --- promql/engine.go | 1 + 1 file changed, 1 insertion(+) diff --git a/promql/engine.go b/promql/engine.go index ea4bc1af85e..ed6b7ca2c3b 100644 --- a/promql/engine.go +++ b/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: + sort.Sort(mat) return mat, warnings, nil default: panic(fmt.Errorf("promql.Engine.exec: unexpected expression type %q", s.Expr.Type())) From 0e934dba8e7238372c89c0bec6308b494d45d85e Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Mon, 13 May 2024 19:47:18 +1000 Subject: [PATCH 3/4] Capture timing information while sorting Signed-off-by: Charles Korn --- promql/engine.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index ed6b7ca2c3b..f9d6f16fc75 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -752,7 +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: - sort.Sort(mat) + ng.sortMatrixResult(ctx, query, mat) return mat, warnings, nil default: panic(fmt.Errorf("promql.Engine.exec: unexpected expression type %q", s.Expr.Type())) @@ -791,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. From 76b123721555e7d57ec374ba3980a34ce4ce821f Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Fri, 17 May 2024 13:54:08 +1000 Subject: [PATCH 4/4] Document sorting behaviour Signed-off-by: Charles Korn --- docs/querying/api.md | 7 +++++++ docs/querying/functions.md | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/docs/querying/api.md b/docs/querying/api.md index 46e79181ed0..71e01b3b952 100644 --- a/docs/querying/api.md +++ b/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 diff --git a/docs/querying/functions.md b/docs/querying/functions.md index c9e65fe6cca..c8fda286551 100644 --- a/docs/querying/functions.md +++ b/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`.**