From 9c4f3b057c3078bf673148f1d9b0d8393212ef00 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Sun, 4 Oct 2026 11:31:07 -0400 Subject: [PATCH 1/3] test(query-engine): cover temporal anchors and topk ordering --- promql-compliance/HOW_TO.md | 27 ++ promql-compliance/datasets/topk-ordering.yaml | 20 ++ promql-compliance/runner/Makefile | 1 + .../checked_in_fixture_validation_test.go | 1 + promql-compliance/runner/compare.go | 121 +++++++- promql-compliance/runner/compare_test.go | 274 ++++++++++++++++++ promql-compliance/runner/config.go | 84 +++++- promql-compliance/runner/config_test.go | 59 +++- promql-compliance/suites/aggregations.yaml | 116 ++++++++ promql-compliance/suites/topk-ordering.yaml | 36 +++ 10 files changed, 715 insertions(+), 24 deletions(-) create mode 100644 promql-compliance/datasets/topk-ordering.yaml create mode 100644 promql-compliance/suites/topk-ordering.yaml diff --git a/promql-compliance/HOW_TO.md b/promql-compliance/HOW_TO.md index 086bd2bf..71bacc33 100644 --- a/promql-compliance/HOW_TO.md +++ b/promql-compliance/HOW_TO.md @@ -154,6 +154,33 @@ queries: If tolerance is omitted, values are compared exactly. A tolerance should be small and justified; a broad tolerance can hide a correctness bug. +### Require PromQL instant-vector ordering + +Vector and matrix comparisons are order-insensitive by default. Opt into the +PromQL ordering rule for an instant `topk` or `bottomk` response with: + +```yaml +comparison: + instant_vector_order: + direction: descending +``` + +Use `ascending` for `bottomk`. Grouped TopK/BottomK keeps ordering inside a +bucket but does not impose an order on buckets, so declare the grouping used +by the query: + +```yaml +comparison: + instant_vector_order: + direction: descending + grouping: + mode: by + labels: [job] +``` + +`mode: without` is also supported. This policy applies only to `/query` +responses; `/query_range` and range-at-t parity remain order-insensitive. + ## Run with DEBUG logging The default Compose stack uses `INFO`. Temporarily change the query engine diff --git a/promql-compliance/datasets/topk-ordering.yaml b/promql-compliance/datasets/topk-ordering.yaml new file mode 100644 index 00000000..e04b0e61 --- /dev/null +++ b/promql-compliance/datasets/topk-ordering.yaml @@ -0,0 +1,20 @@ +name: topk-ordering +series: + - metric: ordered_data + labels: {job: backend, instance: a} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] + - metric: ordered_data + labels: {job: backend, instance: b} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] + - metric: ordered_data + labels: {job: backend, instance: c} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] + - metric: ordered_data + labels: {job: frontend, instance: a} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] + - metric: ordered_data + labels: {job: frontend, instance: b} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] + - metric: ordered_data + labels: {job: frontend, instance: c} + samples: [{offset_seconds: 0, value: 1}, {offset_seconds: 300, value: 1}, {offset_seconds: 600, value: 1}] diff --git a/promql-compliance/runner/Makefile b/promql-compliance/runner/Makefile index a844a202..0461ac2e 100644 --- a/promql-compliance/runner/Makefile +++ b/promql-compliance/runner/Makefile @@ -12,6 +12,7 @@ CI_CASES := \ single-rate-off-grid-rate:../datasets/single-rate.yaml:../suites/off-grid-rate.yaml \ aggregations-native-dag:../datasets/aggregations.yaml:../suites/native-dag-aggregations.yaml \ aggregations:../datasets/aggregations.yaml:../suites/aggregations.yaml \ + topk-ordering:../datasets/topk-ordering.yaml:../suites/topk-ordering.yaml \ quantiles:../datasets/quantiles.yaml:../suites/quantiles.yaml # Tracks planner coverage (#738); most queries are expected to fail. diff --git a/promql-compliance/runner/checked_in_fixture_validation_test.go b/promql-compliance/runner/checked_in_fixture_validation_test.go index e4a08898..45561544 100644 --- a/promql-compliance/runner/checked_in_fixture_validation_test.go +++ b/promql-compliance/runner/checked_in_fixture_validation_test.go @@ -10,6 +10,7 @@ import ( func TestCheckedInFixturesAndSuites(t *testing.T) { pairs := []struct{ dataset, suite string }{ {"../datasets/aggregations.yaml", "../suites/aggregations.yaml"}, + {"../datasets/topk-ordering.yaml", "../suites/topk-ordering.yaml"}, {"../datasets/quantiles.yaml", "../suites/quantiles.yaml"}, {"../datasets/olly-bench.yaml", "../suites/olly-bench.yaml"}, } diff --git a/promql-compliance/runner/compare.go b/promql-compliance/runner/compare.go index 7a9c6245..c4afcac0 100644 --- a/promql-compliance/runner/compare.go +++ b/promql-compliance/runner/compare.go @@ -24,7 +24,7 @@ type QueryAPI interface { type QueryReport struct { Name string `json:"name"` Expr string `json:"expr"` - Tolerance ComparisonPolicy `json:"tolerance"` + Comparison ComparisonPolicy `json:"comparison"` Range *ComparisonOutcome `json:"range,omitempty"` Instant []InstantComparison `json:"instant,omitempty"` ReferenceParity []ParityComparison `json:"referenceParity,omitempty"` @@ -55,12 +55,12 @@ type ParityComparison struct { // It compares the two targets and also checks range-at-t against // instant-at-t within each target. func CompareQuery(ctx context.Context, reference, test QueryAPI, query QueryCase, base time.Time, defaults ComparisonPolicy) (QueryReport, error) { - effective := query.EffectiveTolerance(defaults) + effective := query.EffectiveComparison(defaults) report := QueryReport{ - Name: query.Name, - Expr: query.Expr, - Tolerance: effective, - Passed: true, + Name: query.Name, + Expr: query.Expr, + Comparison: effective, + Passed: true, } var referenceRange, testRange model.Value @@ -81,7 +81,7 @@ func CompareQuery(ctx context.Context, reference, test QueryAPI, query QueryCase for index, instantTime := range instantTimes { referenceInstant, _, referenceErr := reference.Query(ctx, query.Expr, instantTime) testInstant, _, testErr := test.Query(ctx, query.Expr, instantTime) - outcome := responseComparison(referenceInstant, testInstant, referenceErr, testErr, effective) + outcome := instantResponseComparison(referenceInstant, testInstant, referenceErr, testErr, effective) report.Instant = append(report.Instant, InstantComparison{ OffsetSeconds: query.InstantOffsetsSeconds[index], Time: instantTime, @@ -125,6 +125,18 @@ func (q QueryCase) RangeAt(base time.Time) (clientv1.Range, error) { } func responseComparison(reference, test model.Value, referenceErr, testErr error, tolerance ComparisonPolicy) ComparisonOutcome { + return responseComparisonWith(reference, test, referenceErr, testErr, func(reference, test model.Value) string { + return compareValues(reference, test, tolerance) + }) +} + +func instantResponseComparison(reference, test model.Value, referenceErr, testErr error, policy ComparisonPolicy) ComparisonOutcome { + return responseComparisonWith(reference, test, referenceErr, testErr, func(reference, test model.Value) string { + return compareInstantValues(reference, test, policy) + }) +} + +func responseComparisonWith(reference, test model.Value, referenceErr, testErr error, compare func(model.Value, model.Value) string) ComparisonOutcome { outcome := ComparisonOutcome{} if referenceErr != nil { outcome.ReferenceError = referenceErr.Error() @@ -136,7 +148,7 @@ func responseComparison(reference, test model.Value, referenceErr, testErr error outcome.Passed = false return outcome } - outcome.Diff = compareValues(reference, test, tolerance) + outcome.Diff = compare(reference, test) outcome.Passed = outcome.Diff == "" return outcome } @@ -166,6 +178,99 @@ func compareValues(reference, test model.Value, tolerance ComparisonPolicy) stri return compareNormalized(referenceNormalized, testNormalized, tolerance) } +func compareInstantValues(reference, test model.Value, policy ComparisonPolicy) string { + if policy.InstantVectorOrder == nil { + return compareValues(reference, test, policy) + } + referenceVector, referenceIsVector := reference.(model.Vector) + testVector, testIsVector := test.(model.Vector) + if !referenceIsVector || !testIsVector { + return fmt.Sprintf( + "instant vector order policy requires vector results, got reference %T and test %T", + reference, + test, + ) + } + + referenceGroups, err := orderedVectorGroups(referenceVector, policy.InstantVectorOrder.Grouping) + if err != nil { + return orderedVectorDiff("reference instant vector grouping is invalid: "+err.Error(), referenceVector, testVector) + } + testGroups, err := orderedVectorGroups(testVector, policy.InstantVectorOrder.Grouping) + if err != nil { + return orderedVectorDiff("test instant vector grouping is invalid: "+err.Error(), referenceVector, testVector) + } + if len(referenceGroups) != len(testGroups) { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group count differs: reference %d, test %d", len(referenceGroups), len(testGroups)), referenceVector, testVector) + } + for key, referenceGroup := range referenceGroups { + testGroup, found := testGroups[key] + if !found { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q is absent from test result", key), referenceVector, testVector) + } + if len(referenceGroup) != len(testGroup) { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q sample count differs: reference %d, test %d", key, len(referenceGroup), len(testGroup)), referenceVector, testVector) + } + for index := range referenceGroup { + left, right := referenceGroup[index], testGroup[index] + if left.Metric != right.Metric || left.Timestamp != right.Timestamp { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q differs at sample %d", key, index), referenceVector, testVector) + } + if !equalFloat(left.Value, right.Value, policy.ValueTolerance) { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q value differs at sample %d", key, index), referenceVector, testVector) + } + } + } + return "" +} + +func orderedVectorDiff(reason string, reference, test model.Vector) string { + return fmt.Sprintf("%s\nreference: %v\ntest: %v", reason, reference, test) +} + +func orderedVectorGroups(vector model.Vector, grouping *OrderGrouping) (map[string][]normalizedSample, error) { + groups := make(map[string][]normalizedSample) + var currentKey string + hasCurrentGroup := false + for _, sample := range vector { + key := orderGroupKey(sample.Metric, grouping) + if !hasCurrentGroup || key != currentKey { + if _, seen := groups[key]; seen { + return nil, fmt.Errorf("group %q is not contiguous", key) + } + groups[key] = []normalizedSample{} + currentKey = key + hasCurrentGroup = true + } + groups[key] = append(groups[key], normalizedSample{Metric: metricString(sample.Metric), Timestamp: int64(sample.Timestamp), Value: float64(sample.Value)}) + } + return groups, nil +} + +func orderGroupKey(metric model.Metric, grouping *OrderGrouping) string { + if grouping == nil { + return "" + } + labels := make(model.Metric) + if grouping.Mode == orderGroupingBy { + for _, label := range grouping.Labels { + labelName := model.LabelName(label) + labels[labelName] = metric[labelName] + } + return metricString(labels) + } + excluded := make(map[model.LabelName]struct{}, len(grouping.Labels)) + for _, label := range grouping.Labels { + excluded[model.LabelName(label)] = struct{}{} + } + for label, value := range metric { + if _, skip := excluded[label]; !skip { + labels[label] = value + } + } + return metricString(labels) +} + type normalizedValue struct { Type string `json:"type"` Samples []normalizedSample `json:"samples,omitempty"` diff --git a/promql-compliance/runner/compare_test.go b/promql-compliance/runner/compare_test.go index 42f1dc85..4403b2c4 100644 --- a/promql-compliance/runner/compare_test.go +++ b/promql-compliance/runner/compare_test.go @@ -144,3 +144,277 @@ func TestCompareValuesHonorsExplicitToleranceOnlyForValues(t *testing.T) { t.Fatal("comparison accepted different labels because values were within tolerance") } } + +func TestCompareQueryRejectsReorderedInstantVectorWhenOrderIsRequired(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + reference := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 2, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + } + test := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 2, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "ordered-topk", + Expr: "topk(2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "descending", + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): reference}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): test}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if report.Instant[0].Comparison.Passed { + t.Fatal("ordered instant comparison accepted reordered samples") + } +} + +func TestCompareQueryKeepsDefaultInstantVectorComparisonOrderInsensitive(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + reference := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 2, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + } + test := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 2, Timestamp: timestamp}, + } + query := QueryCase{Name: "unordered-vector", Expr: "up", InstantOffsetsSeconds: []float64{0}} + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): reference}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): test}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if !report.Instant[0].Comparison.Passed { + t.Fatalf("default instant comparison rejected reordered samples: %#v", report.Instant[0].Comparison) + } +} + +func TestCompareQueryRequiresFullLabelOrderForEqualInstantValues(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + reference := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + } + test := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 1, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "tied-topk", + Expr: "topk(2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "descending", + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): reference}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): test}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if report.Instant[0].Comparison.Passed { + t.Fatal("ordered instant comparison accepted reversed equal-value labels") + } +} + +func TestCompareQueryAcceptsPrometheusTieOrderAsTheReference(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + prometheusOrder := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 1, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "tied-topk", + Expr: "topk(2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: instantOrderDescending, + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): prometheusOrder}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): prometheusOrder}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if !report.Instant[0].Comparison.Passed { + t.Fatalf("ordered instant comparison rejected Prometheus's tied sequence: %#v", report.Instant[0].Comparison) + } +} + +func TestCompareQueryAllowsGroupedInstantBucketsInAnyOrder(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + frontend := []*model.Sample{ + {Metric: model.Metric{"job": "frontend", "instance": "a"}, Value: 3, Timestamp: timestamp}, + {Metric: model.Metric{"job": "frontend", "instance": "b"}, Value: 2, Timestamp: timestamp}, + } + backend := []*model.Sample{ + {Metric: model.Metric{"job": "backend", "instance": "a"}, Value: 4, Timestamp: timestamp}, + {Metric: model.Metric{"job": "backend", "instance": "b"}, Value: 1, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "grouped-topk", + Expr: "topk by (job) (2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "descending", + Grouping: &OrderGrouping{Mode: "by", Labels: []string{"job"}}, + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): model.Vector(append(frontend, backend...))}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): model.Vector(append(backend, frontend...))}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if !report.Instant[0].Comparison.Passed { + t.Fatalf("grouped instant comparison rejected reordered buckets: %#v", report.Instant[0].Comparison) + } +} + +func TestCompareQueryAllowsWithoutGroupedInstantBucketsInAnyOrder(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + frontend := []*model.Sample{ + {Metric: model.Metric{"job": "frontend", "instance": "a"}, Value: 3, Timestamp: timestamp}, + {Metric: model.Metric{"job": "frontend", "instance": "b"}, Value: 2, Timestamp: timestamp}, + } + backend := []*model.Sample{ + {Metric: model.Metric{"job": "backend", "instance": "a"}, Value: 4, Timestamp: timestamp}, + {Metric: model.Metric{"job": "backend", "instance": "b"}, Value: 1, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "without-grouped-topk", + Expr: "topk without (instance) (2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "descending", + Grouping: &OrderGrouping{Mode: "without", Labels: []string{"instance"}}, + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): model.Vector(append(frontend, backend...))}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): model.Vector(append(backend, frontend...))}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if !report.Instant[0].Comparison.Passed { + t.Fatalf("without-grouped instant comparison rejected reordered buckets: %#v", report.Instant[0].Comparison) + } +} + +func TestCompareQueryAcceptsAscendingInstantVectorOrder(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + ordered := model.Vector{ + &model.Sample{Metric: model.Metric{"instance": "a"}, Value: 1, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"instance": "b"}, Value: 2, Timestamp: timestamp}, + } + query := QueryCase{ + Name: "ordered-bottomk", + Expr: "bottomk(2, up)", + InstantOffsetsSeconds: []float64{0}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "ascending", + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): ordered}}, + fakeTarget{instantByMS: map[int64]model.Value{base.UnixMilli(): ordered}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if !report.Instant[0].Comparison.Passed { + t.Fatalf("ascending instant comparison failed: %#v", report.Instant[0].Comparison) + } +} + +func TestCompareQueryKeepsRangeComparisonsOrderInsensitive(t *testing.T) { + base := time.UnixMilli(1_700_000_000_000).UTC() + timestamp := model.Time(base.UnixMilli()) + first := &model.SampleStream{Metric: model.Metric{"instance": "a"}, Values: []model.SamplePair{{Timestamp: timestamp, Value: 2}}} + second := &model.SampleStream{Metric: model.Metric{"instance": "b"}, Values: []model.SamplePair{{Timestamp: timestamp, Value: 1}}} + query := QueryCase{ + Name: "range-topk", + Expr: "topk(2, up)", + Range: &RangeSpec{StartOffsetSeconds: 0, EndOffsetSeconds: 60, StepSeconds: 60}, + Comparison: &ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: "descending", + }}, + } + + report, err := CompareQuery( + context.Background(), + fakeTarget{rangeValue: model.Matrix{first, second}}, + fakeTarget{rangeValue: model.Matrix{second, first}}, + query, base, ComparisonPolicy{}, + ) + if err != nil { + t.Fatalf("CompareQuery: %v", err) + } + if report.Range == nil || !report.Range.Passed { + t.Fatalf("range comparison rejected reordered streams: %#v", report.Range) + } +} + +func TestInstantVectorOrderRejectsInvalidResponseShapesAndSequences(t *testing.T) { + order := ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{Direction: instantOrderDescending}} + if diff := compareInstantValues(&model.Scalar{Value: 1}, model.Vector{}, order); diff == "" { + t.Fatal("ordered comparison accepted a non-vector response") + } + + timestamp := model.Time(1) + grouped := ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: instantOrderDescending, + Grouping: &OrderGrouping{Mode: orderGroupingBy, Labels: []string{"job"}}, + }} + nonContiguous := model.Vector{ + &model.Sample{Metric: model.Metric{"job": "a"}, Value: 3, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"job": "b"}, Value: 2, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{"job": "a"}, Value: 1, Timestamp: timestamp}, + } + if diff := compareInstantValues(nonContiguous, nonContiguous, grouped); diff == "" { + t.Fatal("ordered comparison accepted non-contiguous grouping buckets") + } +} diff --git a/promql-compliance/runner/config.go b/promql-compliance/runner/config.go index 5bbb256f..a0857636 100644 --- a/promql-compliance/runner/config.go +++ b/promql-compliance/runner/config.go @@ -10,6 +10,13 @@ import ( "gopkg.in/yaml.v3" ) +const ( + instantOrderAscending = "ascending" + instantOrderDescending = "descending" + orderGroupingBy = "by" + orderGroupingWithout = "without" +) + // Suite is a data-independent collection of PromQL cases. All timestamps are // offsets from the dataset base time selected for a run. type Suite struct { @@ -35,7 +42,20 @@ type RangeSpec struct { // ComparisonPolicy is intentionally pointer-valued: omitted means exact // comparison, while zero is a valid explicit tolerance. type ComparisonPolicy struct { - ValueTolerance *Tolerance `yaml:"value_tolerance" json:"valueTolerance"` + ValueTolerance *Tolerance `yaml:"value_tolerance" json:"valueTolerance"` + InstantVectorOrder *InstantVectorOrder `yaml:"instant_vector_order" json:"instantVectorOrder"` +} + +// InstantVectorOrder opts an instant-vector comparison into PromQL ordering +// rules. Range responses deliberately remain order-insensitive. +type InstantVectorOrder struct { + Direction string `yaml:"direction" json:"direction"` + Grouping *OrderGrouping `yaml:"grouping" json:"grouping"` +} + +type OrderGrouping struct { + Mode string `yaml:"mode" json:"mode"` + Labels []string `yaml:"labels" json:"labels"` } type Tolerance struct { @@ -92,7 +112,11 @@ func LoadSuite(contents []byte) (Suite, error) { } } } - if err := validateTolerance(query.EffectiveTolerance(suite.ComparisonDefaults).ValueTolerance); err != nil { + effective := query.EffectiveComparison(suite.ComparisonDefaults) + if err := validateTolerance(effective.ValueTolerance); err != nil { + return Suite{}, fmt.Errorf("query %q: %w", query.Name, err) + } + if err := validateInstantVectorOrder(effective.InstantVectorOrder); err != nil { return Suite{}, fmt.Errorf("query %q: %w", query.Name, err) } } @@ -116,22 +140,27 @@ func (q QueryCase) InstantTimes(base time.Time) []time.Time { return result } -func (q QueryCase) EffectiveTolerance(defaults ComparisonPolicy) ComparisonPolicy { - if q.Comparison == nil || q.Comparison.ValueTolerance == nil { +func (q QueryCase) EffectiveComparison(defaults ComparisonPolicy) ComparisonPolicy { + if q.Comparison == nil { return defaults } result := defaults - if result.ValueTolerance == nil { - result.ValueTolerance = &Tolerance{} - } - merged := *result.ValueTolerance - if q.Comparison.ValueTolerance.Relative != nil { - merged.Relative = q.Comparison.ValueTolerance.Relative + if q.Comparison.ValueTolerance != nil { + if result.ValueTolerance == nil { + result.ValueTolerance = &Tolerance{} + } + merged := *result.ValueTolerance + if q.Comparison.ValueTolerance.Relative != nil { + merged.Relative = q.Comparison.ValueTolerance.Relative + } + if q.Comparison.ValueTolerance.Absolute != nil { + merged.Absolute = q.Comparison.ValueTolerance.Absolute + } + result.ValueTolerance = &merged } - if q.Comparison.ValueTolerance.Absolute != nil { - merged.Absolute = q.Comparison.ValueTolerance.Absolute + if q.Comparison.InstantVectorOrder != nil { + result.InstantVectorOrder = q.Comparison.InstantVectorOrder } - result.ValueTolerance = &merged return result } @@ -178,6 +207,35 @@ func validateTolerance(tolerance *Tolerance) error { return nil } +func validateInstantVectorOrder(order *InstantVectorOrder) error { + if order == nil { + return nil + } + if order.Direction != instantOrderAscending && order.Direction != instantOrderDescending { + return fmt.Errorf("instant vector order direction must be ascending or descending") + } + if order.Grouping == nil { + return nil + } + if order.Grouping.Mode != orderGroupingBy && order.Grouping.Mode != orderGroupingWithout { + return fmt.Errorf("instant vector order grouping mode must be by or without") + } + if len(order.Grouping.Labels) == 0 { + return fmt.Errorf("instant vector order grouping must include at least one label") + } + seen := make(map[string]struct{}, len(order.Grouping.Labels)) + for _, label := range order.Grouping.Labels { + if label == "" { + return fmt.Errorf("instant vector order grouping labels must be non-empty") + } + if _, duplicate := seen[label]; duplicate { + return fmt.Errorf("instant vector order grouping label %q is duplicated", label) + } + seen[label] = struct{}{} + } + return nil +} + func addSeconds(base time.Time, seconds float64) time.Time { return base.Add(time.Duration(seconds * float64(time.Second))) } diff --git a/promql-compliance/runner/config_test.go b/promql-compliance/runner/config_test.go index ee65e2d3..d38d8a2d 100644 --- a/promql-compliance/runner/config_test.go +++ b/promql-compliance/runner/config_test.go @@ -50,13 +50,13 @@ queries: t.Fatalf("range end = %s, want %s", got, want) } - if got := first.EffectiveTolerance(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Relative == nil || *got.ValueTolerance.Relative != 0.01 { + if got := first.EffectiveComparison(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Relative == nil || *got.ValueTolerance.Relative != 0.01 { t.Fatalf("global tolerance was not inherited: %#v", got) } - if got := suite.Queries[1].EffectiveTolerance(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Absolute == nil || *got.ValueTolerance.Absolute != 0.000001 { + if got := suite.Queries[1].EffectiveComparison(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Absolute == nil || *got.ValueTolerance.Absolute != 0.000001 { t.Fatalf("per-query tolerance was not applied: %#v", got) } - if got := suite.Queries[1].EffectiveTolerance(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Relative == nil || *got.ValueTolerance.Relative != 0.01 { + if got := suite.Queries[1].EffectiveComparison(suite.ComparisonDefaults); got.ValueTolerance == nil || got.ValueTolerance.Relative == nil || *got.ValueTolerance.Relative != 0.01 { t.Fatalf("per-query tolerance did not inherit global relative value: %#v", got) } } @@ -118,3 +118,56 @@ queries: t.Fatal("LoadSuite accepted a sub-millisecond range") } } + +func TestLoadSuiteAcceptsInstantVectorOrderPolicy(t *testing.T) { + suite, err := LoadSuite([]byte(`name: ordered +queries: + - name: grouped-topk + expr: topk by (job) (3, up) + instant_offsets_seconds: [60] + comparison: + instant_vector_order: + direction: descending + grouping: + mode: by + labels: [job] +`)) + if err != nil { + t.Fatalf("LoadSuite: %v", err) + } + + order := suite.Queries[0].EffectiveComparison(suite.ComparisonDefaults).InstantVectorOrder + if order == nil || order.Direction != "descending" || order.Grouping == nil || order.Grouping.Mode != "by" { + t.Fatalf("instant vector order = %#v, want descending by(job)", order) + } +} + +func TestLoadSuiteRejectsInvalidInstantVectorOrderPolicy(t *testing.T) { + for _, policy := range []string{ + "direction: sideways", + "direction: descending\n grouping:\n mode: sideways\n labels: [job]", + "direction: descending\n grouping:\n mode: by\n labels: []", + "direction: descending\n grouping:\n mode: by\n labels: ['', job]", + "direction: descending\n grouping:\n mode: by\n labels: [job, job]", + } { + _, err := LoadSuite([]byte("name: invalid-order\nqueries:\n - name: topk\n expr: topk(3, up)\n instant_offsets_seconds: [60]\n comparison:\n instant_vector_order:\n " + policy + "\n")) + if err == nil { + t.Fatalf("LoadSuite accepted invalid instant vector order policy: %s", policy) + } + } +} + +func TestLoadSuiteRejectsInvalidInstantVectorOrderDirection(t *testing.T) { + _, err := LoadSuite([]byte(`name: invalid-order +queries: + - name: topk + expr: topk(3, up) + instant_offsets_seconds: [60] + comparison: + instant_vector_order: + direction: sideways +`)) + if err == nil { + t.Fatal("LoadSuite accepted an invalid instant vector order direction") + } +} diff --git a/promql-compliance/suites/aggregations.yaml b/promql-compliance/suites/aggregations.yaml index 114fc074..68bb26d0 100644 --- a/promql-compliance/suites/aggregations.yaml +++ b/promql-compliance/suites/aggregations.yaml @@ -28,6 +28,18 @@ queries: expr: "count_over_time(data[5m])" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + - name: avg-over-time + expr: "avg_over_time(data[5m])" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: min-over-time + expr: "min_over_time(data[5m])" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: max-over-time + expr: "max_over_time(data[5m])" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range # Bare-selector topk (no wrapped temporal function): unlike every # topk-over-temporal case below, this keeps __name__ -- see #710. @@ -35,39 +47,60 @@ queries: expr: "topk(1, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: &instant_topk_descending + instant_vector_order: + direction: descending - name: topk-3-bare expr: "topk(3, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-5-bare expr: "topk(5, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-10-bare expr: "topk(10, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-3-bare-by-job expr: "topk by (job) (3, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: &instant_topk_descending_by_job + instant_vector_order: + direction: descending + grouping: + mode: by + labels: [job] - name: topk-3-bare-by-job-instance expr: "topk by (job, instance) (3, data)" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: &instant_topk_descending_by_job_instance + instant_vector_order: + direction: descending + grouping: + mode: by + labels: [job, instance] - name: topk-3-rate expr: "topk(3, rate(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-3-rate-by-job expr: "topk by (job) (3, rate(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-3-rate-by-job-instance expr: "topk by (job, instance) (3, rate(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance # Zero-label grouping is the ordinary aggregate form. One- and two-label # forms retain job, then job+instance, respectively. @@ -75,99 +108,123 @@ queries: expr: "topk(1, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-3-sum-over-time expr: "topk(3, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-5-sum-over-time expr: "topk(5, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-10-sum-over-time expr: "topk(10, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-1-sum-over-time-by-job expr: "topk by (job) (1, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-3-sum-over-time-by-job expr: "topk by (job) (3, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-5-sum-over-time-by-job expr: "topk by (job) (5, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-10-sum-over-time-by-job expr: "topk by (job) (10, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-1-sum-over-time-by-job-instance expr: "topk by (job, instance) (1, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-3-sum-over-time-by-job-instance expr: "topk by (job, instance) (3, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-5-sum-over-time-by-job-instance expr: "topk by (job, instance) (5, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-10-sum-over-time-by-job-instance expr: "topk by (job, instance) (10, sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-1-count-over-time expr: "topk(1, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-3-count-over-time expr: "topk(3, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-5-count-over-time expr: "topk(5, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-10-count-over-time expr: "topk(10, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending - name: topk-1-count-over-time-by-job expr: "topk by (job) (1, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-3-count-over-time-by-job expr: "topk by (job) (3, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-5-count-over-time-by-job expr: "topk by (job) (5, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-10-count-over-time-by-job expr: "topk by (job) (10, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job - name: topk-1-count-over-time-by-job-instance expr: "topk by (job, instance) (1, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-3-count-over-time-by-job-instance expr: "topk by (job, instance) (3, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-5-count-over-time-by-job-instance expr: "topk by (job, instance) (5, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: topk-10-count-over-time-by-job-instance expr: "topk by (job, instance) (10, count_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + comparison: *instant_topk_descending_by_job_instance - name: sum expr: "sum(data)" @@ -210,3 +267,62 @@ queries: expr: "sum by (job, instance) (sum_over_time(data[5m]))" instant_offsets_seconds: *evaluation_offsets range: *evaluation_range + - name: sum-sum-over-time + expr: "sum(sum_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: sum-without-job-sum-over-time + expr: "sum without (job) (sum_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + + - name: sum-count-over-time + expr: "sum(count_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: sum-by-job-count-over-time + expr: "sum by (job) (count_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: sum-by-job-instance-count-over-time + expr: "sum by (job, instance) (count_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: sum-without-job-count-over-time + expr: "sum without (job) (count_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + + - name: min-min-over-time + expr: "min(min_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: min-by-job-min-over-time + expr: "min by (job) (min_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: min-by-job-instance-min-over-time + expr: "min by (job, instance) (min_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: min-without-job-min-over-time + expr: "min without (job) (min_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + + - name: max-max-over-time + expr: "max(max_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: max-by-job-max-over-time + expr: "max by (job) (max_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: max-by-job-instance-max-over-time + expr: "max by (job, instance) (max_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: max-without-job-max-over-time + expr: "max without (job) (max_over_time(data[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range diff --git a/promql-compliance/suites/topk-ordering.yaml b/promql-compliance/suites/topk-ordering.yaml new file mode 100644 index 00000000..37057fee --- /dev/null +++ b/promql-compliance/suites/topk-ordering.yaml @@ -0,0 +1,36 @@ +name: topk-ordering + +queries: + # Equal values make the full label set the TopK tie-breaker. + - name: topk-tied + expr: "topk(6, ordered_data)" + instant_offsets_seconds: &evaluation_offsets [300, 600] + range: &evaluation_range + start_offset_seconds: 300 + end_offset_seconds: 600 + step_seconds: 300 + comparison: + instant_vector_order: + direction: descending + + - name: topk-tied-by-job + expr: "topk by (job) (3, ordered_data)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: + instant_vector_order: + direction: descending + grouping: + mode: by + labels: [job] + + - name: topk-tied-without-instance + expr: "topk without (instance) (3, ordered_data)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: + instant_vector_order: + direction: descending + grouping: + mode: without + labels: [instance] From dd294e8eb0c9b8129dd44b1b75c6b52761fc1634 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Sun, 4 Oct 2026 15:56:50 -0400 Subject: [PATCH 2/3] fix(query-engine): align topk tie ordering semantics --- promql-compliance/runner/compare.go | 38 +++++++++++++++++++----- promql-compliance/runner/compare_test.go | 6 ++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/promql-compliance/runner/compare.go b/promql-compliance/runner/compare.go index c4afcac0..4d66af54 100644 --- a/promql-compliance/runner/compare.go +++ b/promql-compliance/runner/compare.go @@ -208,22 +208,31 @@ func compareInstantValues(reference, test model.Value, policy ComparisonPolicy) if !found { return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q is absent from test result", key), referenceVector, testVector) } + if err := validateOrderedValues(testGroup, policy.InstantVectorOrder.Direction); err != nil { + return orderedVectorDiff(fmt.Sprintf("test instant vector ordered group %q: %v", key, err), referenceVector, testVector) + } if len(referenceGroup) != len(testGroup) { return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q sample count differs: reference %d, test %d", key, len(referenceGroup), len(testGroup)), referenceVector, testVector) } - for index := range referenceGroup { - left, right := referenceGroup[index], testGroup[index] - if left.Metric != right.Metric || left.Timestamp != right.Timestamp { - return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q differs at sample %d", key, index), referenceVector, testVector) - } - if !equalFloat(left.Value, right.Value, policy.ValueTolerance) { - return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q value differs at sample %d", key, index), referenceVector, testVector) - } + if diff := compareSampleMembership(referenceGroup, testGroup, policy.ValueTolerance); diff != "" { + return orderedVectorDiff(fmt.Sprintf("instant vector ordered group %q %s", key, diff), referenceVector, testVector) } } return "" } +func compareSampleMembership(reference, test []normalizedSample, tolerance *Tolerance) string { + referenceSamples := append([]normalizedSample(nil), reference...) + testSamples := append([]normalizedSample(nil), test...) + sortNormalizedSamples(referenceSamples) + sortNormalizedSamples(testSamples) + return compareNormalized( + normalizedValue{Type: "vector", Samples: referenceSamples}, + normalizedValue{Type: "vector", Samples: testSamples}, + ComparisonPolicy{ValueTolerance: tolerance}, + ) +} + func orderedVectorDiff(reason string, reference, test model.Vector) string { return fmt.Sprintf("%s\nreference: %v\ntest: %v", reason, reference, test) } @@ -247,6 +256,19 @@ func orderedVectorGroups(vector model.Vector, grouping *OrderGrouping) (map[stri return groups, nil } +func validateOrderedValues(samples []normalizedSample, direction string) error { + for index := 1; index < len(samples); index++ { + previous, current := samples[index-1], samples[index] + if direction == instantOrderDescending && previous.Value < current.Value { + return fmt.Errorf("values are not descending at sample %d", index) + } + if direction == instantOrderAscending && previous.Value > current.Value { + return fmt.Errorf("values are not ascending at sample %d", index) + } + } + return nil +} + func orderGroupKey(metric model.Metric, grouping *OrderGrouping) string { if grouping == nil { return "" diff --git a/promql-compliance/runner/compare_test.go b/promql-compliance/runner/compare_test.go index 4403b2c4..944d0ae4 100644 --- a/promql-compliance/runner/compare_test.go +++ b/promql-compliance/runner/compare_test.go @@ -206,7 +206,7 @@ func TestCompareQueryKeepsDefaultInstantVectorComparisonOrderInsensitive(t *test } } -func TestCompareQueryRequiresFullLabelOrderForEqualInstantValues(t *testing.T) { +func TestCompareQueryIgnoresEqualValueInstantTieOrder(t *testing.T) { base := time.UnixMilli(1_700_000_000_000).UTC() timestamp := model.Time(base.UnixMilli()) reference := model.Vector{ @@ -235,8 +235,8 @@ func TestCompareQueryRequiresFullLabelOrderForEqualInstantValues(t *testing.T) { if err != nil { t.Fatalf("CompareQuery: %v", err) } - if report.Instant[0].Comparison.Passed { - t.Fatal("ordered instant comparison accepted reversed equal-value labels") + if !report.Instant[0].Comparison.Passed { + t.Fatalf("ordered instant comparison rejected equal-value tie ordering: %#v", report.Instant[0].Comparison) } } From 50a58c73a7bc007e3a7ce8201fa7411172c81659 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Sun, 4 Oct 2026 17:06:55 -0400 Subject: [PATCH 3/3] fix(query-engine): group without topk across metric names --- promql-compliance/runner/compare.go | 1 + promql-compliance/runner/compare_test.go | 17 +++++++++++++++++ promql-compliance/suites/topk-ordering.yaml | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/promql-compliance/runner/compare.go b/promql-compliance/runner/compare.go index 4d66af54..6305c42c 100644 --- a/promql-compliance/runner/compare.go +++ b/promql-compliance/runner/compare.go @@ -282,6 +282,7 @@ func orderGroupKey(metric model.Metric, grouping *OrderGrouping) string { return metricString(labels) } excluded := make(map[model.LabelName]struct{}, len(grouping.Labels)) + excluded[model.MetricNameLabel] = struct{}{} for _, label := range grouping.Labels { excluded[model.LabelName(label)] = struct{}{} } diff --git a/promql-compliance/runner/compare_test.go b/promql-compliance/runner/compare_test.go index 944d0ae4..d3710294 100644 --- a/promql-compliance/runner/compare_test.go +++ b/promql-compliance/runner/compare_test.go @@ -340,6 +340,23 @@ func TestCompareQueryAllowsWithoutGroupedInstantBucketsInAnyOrder(t *testing.T) } } +func TestCompareQueryWithoutGroupingIgnoresMetricName(t *testing.T) { + timestamp := model.Time(1) + policy := ComparisonPolicy{InstantVectorOrder: &InstantVectorOrder{ + Direction: instantOrderDescending, + Grouping: &OrderGrouping{Mode: orderGroupingWithout, Labels: []string{"instance"}}, + }} + reference := model.Vector{ + &model.Sample{Metric: model.Metric{model.MetricNameLabel: "a", "job": "api", "instance": "one"}, Value: 2, Timestamp: timestamp}, + &model.Sample{Metric: model.Metric{model.MetricNameLabel: "b", "job": "api", "instance": "two"}, Value: 1, Timestamp: timestamp}, + } + test := model.Vector{reference[1], reference[0]} + + if diff := compareInstantValues(reference, test, policy); diff == "" { + t.Fatal("without grouping treated metric names as separate buckets") + } +} + func TestCompareQueryAcceptsAscendingInstantVectorOrder(t *testing.T) { base := time.UnixMilli(1_700_000_000_000).UTC() timestamp := model.Time(base.UnixMilli()) diff --git a/promql-compliance/suites/topk-ordering.yaml b/promql-compliance/suites/topk-ordering.yaml index 37057fee..4d979312 100644 --- a/promql-compliance/suites/topk-ordering.yaml +++ b/promql-compliance/suites/topk-ordering.yaml @@ -1,7 +1,7 @@ name: topk-ordering queries: - # Equal values make the full label set the TopK tie-breaker. + # Equal values make bucket contiguity observable independently of ranking. - name: topk-tied expr: "topk(6, ordered_data)" instant_offsets_seconds: &evaluation_offsets [300, 600]