From a29ac3360cfd1666a094f8188d03e07b7c994fdd Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 26 Aug 2026 00:22:54 -0400 Subject: [PATCH 1/5] feat(promql-compliance): add remote-write data seeder (#594) Adds promql-compliance/seeder, a standalone Go module that builds a prompb.WriteRequest for a fixed, hand-authored dataset (counters, a gauge, and a label-churn series designed to exercise instant-vs-range divergence), snappy-compresses it, and pushes the same bytes via remote write to both a reference Prometheus and ASAPQuery's own remote-write ingest endpoint. This is the data-seeding half of the differential PromQL compliance harness proposed in #594; the comparison-harness half is a separate workstream. Co-Authored-By: Claude Sonnet 5 --- promql-compliance/seeder/README.md | 105 +++++++++ promql-compliance/seeder/cmd/seed/main.go | 48 ++++ promql-compliance/seeder/dataset.go | 150 ++++++++++++ promql-compliance/seeder/go.mod | 18 ++ promql-compliance/seeder/go.sum | 59 +++++ promql-compliance/seeder/push.go | 142 ++++++++++++ promql-compliance/seeder/push_test.go | 264 ++++++++++++++++++++++ 7 files changed, 786 insertions(+) create mode 100644 promql-compliance/seeder/README.md create mode 100644 promql-compliance/seeder/cmd/seed/main.go create mode 100644 promql-compliance/seeder/dataset.go create mode 100644 promql-compliance/seeder/go.mod create mode 100644 promql-compliance/seeder/go.sum create mode 100644 promql-compliance/seeder/push.go create mode 100644 promql-compliance/seeder/push_test.go diff --git a/promql-compliance/seeder/README.md b/promql-compliance/seeder/README.md new file mode 100644 index 0000000..e33392e --- /dev/null +++ b/promql-compliance/seeder/README.md @@ -0,0 +1,105 @@ +# promql-compliance seeder + +Part of [#594](https://github.com/ProjectASAP/ASAPQuery/issues/594): pushes a +fixed, hand-authored dataset via Prometheus remote-write to two targets — a +real Prometheus (started with `--web.enable-remote-write-receiver`) and +ASAPQuery's own remote-write ingest endpoint (`asap-query-engine/src/drivers/ingest/prometheus_remote_write.rs`, +served at `POST /api/v1/write`, snappy + protobuf, same wire format). Using +the exact same code path against both targets means there's no risk of two +different ingestion mechanisms producing false diffs in the comparison +harness that consumes this. + +This is a standalone Go module (`go.mod` in this directory) — there is no +other Go code in the ASAPQuery repo. + +## Build / test + +``` +go build ./... +go test ./... +``` + +No live Prometheus/ASAPQuery instance is required to build or test: the unit +tests build a `WriteRequest`, snappy-encode it, decode it back, and assert +round-trip equality; the HTTP path is tested against an `httptest.Server` +instead of a real endpoint. + +## Usage + +``` +go run ./cmd/seed --reference-url=http://localhost:9090 --test-url=http://localhost:9091 +``` + +Each URL is the target's base URL; the seeder POSTs to `/api/v1/write`. +Prints the base timestamp (Unix ms) it anchored the dataset to — the +comparison harness needs this to compute absolute query timestamps (see +below). + +Optionally pass `--base-time-ms` to pin the anchor instead of using +"now minus 30 minutes" (the default). Real Prometheus rejects samples that +are too old or too far in the future relative to wall-clock time, so the +dataset's timestamps are expressed as **offsets in seconds from a base time +chosen at seed time**, not fixed absolute timestamps. The *values* are fully +deterministic; only the absolute wall-clock placement moves on each run. + +## Dataset shape + +Defined in `dataset.go`. 20-minute window, sampled every 60s: offsets +0, 60, 120, ..., 1200 (seconds) from the run's base time. Six series across +three metrics: + +| Metric | Labels | Kind | Range | +|---|---|---|---| +| `http_requests_total` | `host="a"` | counter, +1/s | offsets 0..1200 (21 samples) | +| `http_requests_total` | `host="b"` | counter, +2/s | offsets 0..1200 (21 samples) | +| `node_memory_used_bytes` | `host="a"` | gauge, triangle 500->1000->500 | offsets 0..1200 (21 samples) | +| `node_memory_used_bytes` | `host="b"` | gauge, flat 2000 | offsets 0..1200 (21 samples) | +| `checkout_up` | `service="checkout",region="us-east"` | gauge, value 1 | offsets 0..300 only (6 samples) | +| `checkout_up` | `service="checkout",region="us-west"` | gauge, value 1 | offsets 900..1200 only (6 samples) | + +The `checkout_up` pair is deliberate: the us-east series stops at offset 300 +and us-west doesn't start until offset 900, a 600s gap — comfortably past +PromQL's default 5m (300s) staleness/lookback window. This is built to +exercise the same class of instant-vs-range divergence bugs as #589/#583/#584. + +## Hand-computed expected values + +Let `base` = the printed base-time-ms. All timestamps below are +`base + offset_seconds * 1000`. + +- `rate(http_requests_total{host="a"}[5m])` at any `t` in `[base+300s, base+1200s]` = **1.0** exactly. +- `rate(http_requests_total{host="b"}[5m])` at the same `t` = **2.0** exactly. +- `sum(rate(http_requests_total[5m]))` at `t = base+600s` = **3.0**. +- `increase(http_requests_total{host="a"}[5m])` at `t = base+600s` = **300**. +- `http_requests_total{host="a"}` instant value at `t = base+1200s` = **1200**. +- `max_over_time(node_memory_used_bytes{host="a"}[20m])` = **1000**; `min_over_time(...)` = **500**. +- `node_memory_used_bytes{host="a"}` instant value at `t = base+600s` (the peak) = **1000**. +- `avg_over_time(node_memory_used_bytes{host="b"}[20m])` = **2000** exactly (flat series). +- `sum(node_memory_used_bytes)` at `t = base+600s` = **3000** (1000 + 2000). +- Instant query `checkout_up{service="checkout"}` at `t = base+660s` = **empty result vector** + (us-east's last sample at offset 300 is 360s stale, > 5m lookback; us-west's + first sample isn't until offset 900). This is the instant/range divergence probe. +- Range query `checkout_up{service="checkout"}[20m]` evaluated at `t = base+1200s` + returns a matrix with **two series**: us-east (6 samples, offsets 0..300) and + us-west (6 samples, offsets 900..1200) — i.e. the range query surfaces both + series even though no single instant in the gap does. + +See the doc comment at the top of `dataset.go` for the full derivation. + +## Known limitations + +- `go mod init`/`go get` pulled the real `github.com/prometheus/prometheus/prompb` + and `github.com/golang/snappy` packages from the public Go module proxy — + network access was available in this environment, so no vendoring fallback + was needed. `prompb` messages use `github.com/gogo/protobuf/proto` for + marshal/unmarshal (matches upstream `prometheus/prometheus`), which is an + explicit dependency here (`push.go`). +- Building `prometheus/prometheus` bumped the module's Go toolchain + requirement to 1.25.8 (from the system's 1.21.13); `go` auto-downloaded and + used the newer toolchain per `go.mod`'s `go 1.25.8` directive. This only + affects `promql-compliance/seeder`'s own module — it does not touch the + Rust workspace or any other part of the repo. +- Not run against a live Prometheus or ASAPQuery instance — none was + available in this environment. `go build ./...` and `go test ./...` both + pass; the HTTP POST path (headers, path, body encoding) is covered via + `httptest.Server` in `push_test.go`. diff --git a/promql-compliance/seeder/cmd/seed/main.go b/promql-compliance/seeder/cmd/seed/main.go new file mode 100644 index 0000000..0c3614f --- /dev/null +++ b/promql-compliance/seeder/cmd/seed/main.go @@ -0,0 +1,48 @@ +// Command seed pushes the fixed dataset defined in package seeder to two +// Prometheus-remote-write-compatible endpoints: a real Prometheus (started +// with --web.enable-remote-write-receiver) and ASAPQuery's own remote-write +// ingest endpoint. Using the same WriteRequest bytes against both means +// there's no risk of the two ingestion mechanisms disagreeing and producing +// false diffs in the differential PromQL compliance harness (see issue +// #594). +package main + +import ( + "context" + "flag" + "fmt" + "log" + "os" + "time" + + "github.com/ProjectASAP/ASAPQuery/promql-compliance/seeder" +) + +func main() { + referenceURL := flag.String("reference-url", "", "base URL of the reference target (real Prometheus), e.g. http://localhost:9090") + testURL := flag.String("test-url", "", "base URL of the test target (ASAPQuery), e.g. http://localhost:9091") + baseTimeFlag := flag.Int64("base-time-ms", 0, "base Unix time in ms to anchor the dataset's offsets to (default: now, floored to the minute, minus 30 minutes so the whole 20-minute window is safely in the past)") + flag.Parse() + + if *referenceURL == "" || *testURL == "" { + fmt.Fprintln(os.Stderr, "usage: seed --reference-url= --test-url= [--base-time-ms=]") + os.Exit(2) + } + + baseTimeMs := *baseTimeFlag + if baseTimeMs == 0 { + now := time.Now().UTC().Truncate(time.Minute) + baseTimeMs = now.Add(-30 * time.Minute).UnixMilli() + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + if err := seeder.PushDataset(ctx, baseTimeMs, *referenceURL, *testURL); err != nil { + log.Fatalf("seeding failed: %v", err) + } + + fmt.Printf("seeded dataset to %s and %s\n", *referenceURL, *testURL) + fmt.Printf("base-time-ms=%d (dataset offsets 0..1200s map to this base)\n", baseTimeMs) + fmt.Printf("dataset window: [%d, %d] ms unix\n", baseTimeMs, baseTimeMs+1200*1000) +} diff --git a/promql-compliance/seeder/dataset.go b/promql-compliance/seeder/dataset.go new file mode 100644 index 0000000..ccc53d8 --- /dev/null +++ b/promql-compliance/seeder/dataset.go @@ -0,0 +1,150 @@ +package seeder + +// This file defines the seeder's one fixed, hand-authored dataset. +// +// # Shape +// +// The dataset spans a 20-minute window sampled every 60s: 21 timestamps at +// offsets (in seconds from a base time chosen at seed time, see BaseTimeMs +// in push.go) of 0, 60, 120, ..., 1200. +// +// It contains three metrics, six series total: +// +// 1. http_requests_total{host="a"|"b"} — a counter-like metric (strictly +// increasing), present for the whole window, to exercise rate()/increase(). +// 2. node_memory_used_bytes{host="a"|"b"} — a gauge-like metric, present for +// the whole window, to exercise min/max/avg_over_time(). +// 3. checkout_up{service="checkout", region="us-east"|"us-west"} — a series +// whose label set changes partway through the window: the us-east series +// only has samples in the first 5 minutes, the us-west series only has +// samples in the last 5 minutes, with a >5m silent gap in between. This +// is deliberately built to exercise instant-vs-range divergence bugs +// (see #589/#583/#584): an instant query evaluated inside the gap must +// see an empty result (both series are stale/not-yet-started under the +// default 5m lookback), while a range query covering the same window +// returns raw matrix samples for both series. +// +// # Hand-computed expected values +// +// Let base = the base time (ms) the seeder used for this run (printed by +// cmd/seed on push). All timestamps below are "base + offset seconds". +// +// - http_requests_total{host="a"}: value(offset) = offset (seconds). +// So value(0)=0, value(600)=600, value(1200)=1200. +// rate(http_requests_total{host="a"}[5m]) at any t in [base+300, base+1200] +// = 1.0 exactly (1 unit/second). +// increase(http_requests_total{host="a"}[5m]) at those same t = 300. +// +// - http_requests_total{host="b"}: value(offset) = 1000 + 2*offset. +// value(0)=1000, value(600)=2200, value(1200)=3400. +// rate(http_requests_total{host="b"}[5m]) at any t in [base+300, base+1200] +// = 2.0 exactly. +// +// - sum(rate(http_requests_total[5m])) at t=base+600 = 1.0 + 2.0 = 3.0. +// +// - node_memory_used_bytes{host="a"}: a triangle wave. Rises from 500 to +// 1000 in steps of 50 over offsets 0..600 (11 points), then falls back +// from 950 to 500 in steps of 50 over offsets 660..1200 (10 points). +// max_over_time(node_memory_used_bytes{host="a"}[20m]) at t=base+1200 = 1000. +// min_over_time(node_memory_used_bytes{host="a"}[20m]) at t=base+1200 = 500. +// Instant value at t=base+600 = 1000 (the peak). +// +// - node_memory_used_bytes{host="b"}: flat 2000 for every sample. +// avg_over_time(node_memory_used_bytes{host="b"}[20m]) = 2000 exactly. +// +// - sum(node_memory_used_bytes) at t=base+600 (instant) = 1000 + 2000 = 3000. +// +// - checkout_up{service="checkout",region="us-east"}: value=1 at offsets +// 0,60,120,180,240,300, then no more samples. +// checkout_up{service="checkout",region="us-west"}: value=1 at offsets +// 900,960,1020,1080,1140,1200, no samples before that. +// At t=base+660 (360s after the last us-east sample, i.e. > the 5m +// default lookback, and 240s before the first us-west sample): an +// instant query for checkout_up{service="checkout"} must return an +// EMPTY result vector (both series are absent/stale). A range query +// for checkout_up{service="checkout"}[20m] evaluated at t=base+1200 +// must return a matrix with two series: us-east with 6 samples +// (offsets 0..300) and us-west with 6 samples (offsets 900..1200). +// This is the instant-vs-range divergence case #594 is meant to catch. +// +// Point counts: http_requests_total and node_memory_used_bytes each have 21 +// samples per series (offsets 0,60,...,1200). checkout_up has 6 samples per +// series (12 total), deliberately sparse and non-overlapping in time. + +// Dataset returns the fixed set of series pushed by the seeder. It is a +// plain Go literal (built with small loops below for the repetitive parts) +// rather than data read from a file, so the values above are exactly what +// gets pushed — no external format to keep in sync. +func Dataset() []SeriesDef { + offsets := make([]int64, 0, 21) + for o := int64(0); o <= 1200; o += 60 { + offsets = append(offsets, o) + } + + httpRequestsA := SeriesDef{ + Name: "http_requests_total", + Labels: map[string]string{"host": "a"}, + } + httpRequestsB := SeriesDef{ + Name: "http_requests_total", + Labels: map[string]string{"host": "b"}, + } + for _, o := range offsets { + httpRequestsA.Samples = append(httpRequestsA.Samples, Sample{ + OffsetSeconds: o, + Value: float64(o), // 1 unit/sec + }) + httpRequestsB.Samples = append(httpRequestsB.Samples, Sample{ + OffsetSeconds: o, + Value: 1000 + 2*float64(o), // 2 units/sec + }) + } + + memA := SeriesDef{ + Name: "node_memory_used_bytes", + Labels: map[string]string{"host": "a"}, + } + memB := SeriesDef{ + Name: "node_memory_used_bytes", + Labels: map[string]string{"host": "b"}, + } + for _, o := range offsets { + var v float64 + switch { + case o <= 600: + // Rising leg: 500 at o=0 up to 1000 at o=600, step 50 per 60s. + v = 500 + 50*float64(o/60) + default: + // Falling leg: 950 at o=660 down to 500 at o=1200, step 50 per 60s. + stepsPastPeak := (o - 600) / 60 + v = 1000 - 50*float64(stepsPastPeak) + } + memA.Samples = append(memA.Samples, Sample{OffsetSeconds: o, Value: v}) + memB.Samples = append(memB.Samples, Sample{OffsetSeconds: o, Value: 2000}) + } + + checkoutUpEast := SeriesDef{ + Name: "checkout_up", + Labels: map[string]string{"service": "checkout", "region": "us-east"}, + } + for o := int64(0); o <= 300; o += 60 { + checkoutUpEast.Samples = append(checkoutUpEast.Samples, Sample{OffsetSeconds: o, Value: 1}) + } + + checkoutUpWest := SeriesDef{ + Name: "checkout_up", + Labels: map[string]string{"service": "checkout", "region": "us-west"}, + } + for o := int64(900); o <= 1200; o += 60 { + checkoutUpWest.Samples = append(checkoutUpWest.Samples, Sample{OffsetSeconds: o, Value: 1}) + } + + return []SeriesDef{ + httpRequestsA, + httpRequestsB, + memA, + memB, + checkoutUpEast, + checkoutUpWest, + } +} diff --git a/promql-compliance/seeder/go.mod b/promql-compliance/seeder/go.mod new file mode 100644 index 0000000..1c94b48 --- /dev/null +++ b/promql-compliance/seeder/go.mod @@ -0,0 +1,18 @@ +module github.com/ProjectASAP/ASAPQuery/promql-compliance/seeder + +go 1.25.8 + +require ( + github.com/gogo/protobuf v1.3.2 + github.com/golang/snappy v1.0.0 + github.com/prometheus/prometheus v0.314.0 +) + +require ( + github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/grafana/regexp v0.0.0-20250905093917-f7b3be9d1853 // indirect + github.com/prometheus/client_model v0.6.2 // indirect + github.com/prometheus/common v0.70.1 // indirect + golang.org/x/text v0.40.0 // indirect + google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect +) diff --git a/promql-compliance/seeder/go.sum b/promql-compliance/seeder/go.sum new file mode 100644 index 0000000..ef4738c --- /dev/null +++ b/promql-compliance/seeder/go.sum @@ -0,0 +1,59 @@ +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= +github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang/snappy v1.0.0 h1:Oy607GVXHs7RtbggtPBnr2RmDArIsAefDwvrdWvRhGs= +github.com/golang/snappy v1.0.0/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/grafana/regexp v0.0.0-20250905093917-f7b3be9d1853 h1:cLN4IBkmkYZNnk7EAJ0BHIethd+J6LqxFNw5mSiI2bM= +github.com/grafana/regexp v0.0.0-20250905093917-f7b3be9d1853/go.mod h1:+JKpmjMGhpgPL+rXZ5nsZieVzvarn86asRlBg4uNGnk= +github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= +github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= +github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= +github.com/prometheus/common v0.70.1 h1:1HvjP4D5oL3t8RsPlwxA9onvvStjtIHYE5XuuwOi/PY= +github.com/prometheus/common v0.70.1/go.mod h1:VdFUQDMZK3VLkurFUVhia6uys/0suUp86TJz5qbJRhc= +github.com/prometheus/prometheus v0.314.0 h1:YjsimqsIi6/mOtzZcrPEYUALO6zpfaht9O5sXqDz2vg= +github.com/prometheus/prometheus v0.314.0/go.mod h1:zjg3pMTAkY0/JG8jy/h8/YgSQUVB+aCXMhUqN6l64jg= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ= +go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af h1:+5/Sw3GsDNlEmu7TfklWKPdQ0Ykja5VEmq2i817+jbI= +google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/promql-compliance/seeder/push.go b/promql-compliance/seeder/push.go new file mode 100644 index 0000000..01f9373 --- /dev/null +++ b/promql-compliance/seeder/push.go @@ -0,0 +1,142 @@ +// Package seeder builds Prometheus remote-write WriteRequests from a fixed, +// hand-authored dataset and pushes them to one or more remote-write +// endpoints. It exists to seed a real Prometheus and ASAPQuery's own +// remote-write ingest endpoint with the exact same bytes, so a differential +// PromQL compliance test (see GitHub issue #594) has no risk of the two +// ingestion mechanisms disagreeing and producing false diffs. +package seeder + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "sort" + + "github.com/gogo/protobuf/proto" + "github.com/golang/snappy" + "github.com/prometheus/prometheus/prompb" +) + +// Sample is one (offset, value) point in a SeriesDef. OffsetSeconds is +// relative to a base time supplied at push time (see BuildWriteRequest), +// not an absolute Unix timestamp — this keeps the dataset's values fully +// deterministic while letting the actual wall-clock timestamps be chosen +// fresh on every run, which real Prometheus requires (it rejects samples +// that are too far in the past or future relative to "now"). +type Sample struct { + OffsetSeconds int64 + Value float64 +} + +// SeriesDef is one time series: a metric name, a label set (NOT including +// __name__), and its samples. +type SeriesDef struct { + Name string + Labels map[string]string + Samples []Sample +} + +// BuildWriteRequest converts a set of SeriesDef into a prompb.WriteRequest, +// resolving each sample's absolute timestamp as baseTimeMs + +// sample.OffsetSeconds*1000. +func BuildWriteRequest(baseTimeMs int64, series []SeriesDef) *prompb.WriteRequest { + wr := &prompb.WriteRequest{ + Timeseries: make([]prompb.TimeSeries, 0, len(series)), + } + + for _, s := range series { + labels := make([]prompb.Label, 0, len(s.Labels)+1) + labels = append(labels, prompb.Label{Name: "__name__", Value: s.Name}) + for k, v := range s.Labels { + labels = append(labels, prompb.Label{Name: k, Value: v}) + } + // Prometheus's remote-write receiver requires labels to be sorted + // by name (excluding this, some implementations reject the write). + sort.Slice(labels, func(i, j int) bool { return labels[i].Name < labels[j].Name }) + + samples := make([]prompb.Sample, 0, len(s.Samples)) + for _, sm := range s.Samples { + samples = append(samples, prompb.Sample{ + Value: sm.Value, + Timestamp: baseTimeMs + sm.OffsetSeconds*1000, + }) + } + + wr.Timeseries = append(wr.Timeseries, prompb.TimeSeries{ + Labels: labels, + Samples: samples, + }) + } + + return wr +} + +// EncodeSnappy protobuf-marshals a WriteRequest and snappy-compresses the +// result, i.e. produces exactly the body Prometheus remote-write expects. +func EncodeSnappy(wr *prompb.WriteRequest) ([]byte, error) { + data, err := proto.Marshal(wr) + if err != nil { + return nil, fmt.Errorf("marshal WriteRequest: %w", err) + } + return snappy.Encode(nil, data), nil +} + +// DecodeSnappy reverses EncodeSnappy: snappy-decompresses and +// protobuf-unmarshals a remote-write body back into a WriteRequest. It is +// primarily useful for tests that want to assert on what was actually sent. +func DecodeSnappy(body []byte) (*prompb.WriteRequest, error) { + decompressed, err := snappy.Decode(nil, body) + if err != nil { + return nil, fmt.Errorf("snappy decode: %w", err) + } + wr := &prompb.WriteRequest{} + if err := proto.Unmarshal(decompressed, wr); err != nil { + return nil, fmt.Errorf("unmarshal WriteRequest: %w", err) + } + return wr, nil +} + +// Push POSTs a WriteRequest to url + "/api/v1/write" using the standard +// Prometheus remote-write wire format: snappy-compressed protobuf, with the +// headers a remote-write receiver expects. +func Push(ctx context.Context, url string, wr *prompb.WriteRequest) error { + body, err := EncodeSnappy(wr) + if err != nil { + return err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url+"/api/v1/write", bytes.NewReader(body)) + if err != nil { + return fmt.Errorf("build request for %s: %w", url, err) + } + req.Header.Set("Content-Type", "application/x-protobuf") + req.Header.Set("Content-Encoding", "snappy") + req.Header.Set("X-Prometheus-Remote-Write-Version", "0.1.0") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return fmt.Errorf("POST %s: %w", url, err) + } + defer resp.Body.Close() + + if resp.StatusCode/100 != 2 { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + return fmt.Errorf("POST %s: unexpected status %s: %s", url, resp.Status, string(respBody)) + } + return nil +} + +// PushDataset builds the write request for the fixed Dataset() at the given +// base time and pushes it to every URL in urls, stopping at the first +// error. +func PushDataset(ctx context.Context, baseTimeMs int64, urls ...string) error { + wr := BuildWriteRequest(baseTimeMs, Dataset()) + for _, u := range urls { + if err := Push(ctx, u, wr); err != nil { + return err + } + } + return nil +} diff --git a/promql-compliance/seeder/push_test.go b/promql-compliance/seeder/push_test.go new file mode 100644 index 0000000..a9782bf --- /dev/null +++ b/promql-compliance/seeder/push_test.go @@ -0,0 +1,264 @@ +package seeder + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "reflect" + "sort" + "testing" + + "github.com/prometheus/prometheus/prompb" +) + +func TestBuildWriteRequest_TimestampsAndLabels(t *testing.T) { + series := []SeriesDef{ + { + Name: "test_metric", + Labels: map[string]string{"region": "us-east-1", "host": "a"}, + Samples: []Sample{ + {OffsetSeconds: 0, Value: 1.5}, + {OffsetSeconds: 60, Value: 2.5}, + }, + }, + } + + const base int64 = 1_700_000_000_000 + wr := BuildWriteRequest(base, series) + + if len(wr.Timeseries) != 1 { + t.Fatalf("expected 1 timeseries, got %d", len(wr.Timeseries)) + } + ts := wr.Timeseries[0] + + if len(ts.Samples) != 2 { + t.Fatalf("expected 2 samples, got %d", len(ts.Samples)) + } + if ts.Samples[0].Timestamp != base { + t.Errorf("sample 0 timestamp = %d, want %d", ts.Samples[0].Timestamp, base) + } + if ts.Samples[1].Timestamp != base+60_000 { + t.Errorf("sample 1 timestamp = %d, want %d", ts.Samples[1].Timestamp, base+60_000) + } + if ts.Samples[0].Value != 1.5 || ts.Samples[1].Value != 2.5 { + t.Errorf("unexpected sample values: %+v", ts.Samples) + } + + // Labels must include __name__ and be sorted by name. + names := make([]string, len(ts.Labels)) + for i, l := range ts.Labels { + names[i] = l.Name + } + if !sort.StringsAreSorted(names) { + t.Errorf("labels not sorted by name: %v", names) + } + + got := map[string]string{} + for _, l := range ts.Labels { + got[l.Name] = l.Value + } + want := map[string]string{"__name__": "test_metric", "region": "us-east-1", "host": "a"} + for k, v := range want { + if got[k] != v { + t.Errorf("label %q = %q, want %q", k, got[k], v) + } + } +} + +func TestEncodeDecodeSnappyRoundTrip(t *testing.T) { + wr := BuildWriteRequest(1000, []SeriesDef{ + { + Name: "roundtrip_metric", + Labels: map[string]string{"env": "prod"}, + Samples: []Sample{ + {OffsetSeconds: 0, Value: 42}, + {OffsetSeconds: 5, Value: 43.5}, + }, + }, + }) + + body, err := EncodeSnappy(wr) + if err != nil { + t.Fatalf("EncodeSnappy: %v", err) + } + + decoded, err := DecodeSnappy(body) + if err != nil { + t.Fatalf("DecodeSnappy: %v", err) + } + + if len(decoded.Timeseries) != 1 { + t.Fatalf("expected 1 timeseries after roundtrip, got %d", len(decoded.Timeseries)) + } + if !reflect.DeepEqual(decoded.Timeseries[0].Labels, wr.Timeseries[0].Labels) || + !reflect.DeepEqual(decoded.Timeseries[0].Samples, wr.Timeseries[0].Samples) { + t.Errorf("roundtripped timeseries mismatch:\ngot: %+v\nwant: %+v", decoded.Timeseries[0], wr.Timeseries[0]) + } +} + +func TestDatasetBuildsAndEncodesCleanly(t *testing.T) { + wr := BuildWriteRequest(1_700_000_000_000, Dataset()) + + if len(wr.Timeseries) != 6 { + t.Fatalf("expected 6 series in Dataset(), got %d", len(wr.Timeseries)) + } + + totalSamples := 0 + for _, ts := range wr.Timeseries { + totalSamples += len(ts.Samples) + } + // 4 series x 21 samples + 2 series x 6 samples = 96. + if want := 4*21 + 2*6; totalSamples != want { + t.Errorf("total samples = %d, want %d", totalSamples, want) + } + + if _, err := EncodeSnappy(wr); err != nil { + t.Fatalf("EncodeSnappy(Dataset): %v", err) + } +} + +func TestDatasetHandComputedValues(t *testing.T) { + series := Dataset() + + find := func(name string, labels map[string]string) SeriesDef { + for _, s := range series { + if s.Name != name || len(s.Labels) != len(labels) { + continue + } + match := true + for k, v := range labels { + if s.Labels[k] != v { + match = false + break + } + } + if match { + return s + } + } + t.Fatalf("series %s%v not found in Dataset()", name, labels) + return SeriesDef{} + } + + sampleAt := func(s SeriesDef, offset int64) (float64, bool) { + for _, sm := range s.Samples { + if sm.OffsetSeconds == offset { + return sm.Value, true + } + } + return 0, false + } + + httpA := find("http_requests_total", map[string]string{"host": "a"}) + if v, ok := sampleAt(httpA, 1200); !ok || v != 1200 { + t.Errorf("http_requests_total{host=a} at offset 1200 = %v (ok=%v), want 1200", v, ok) + } + + httpB := find("http_requests_total", map[string]string{"host": "b"}) + if v, ok := sampleAt(httpB, 600); !ok || v != 2200 { + t.Errorf("http_requests_total{host=b} at offset 600 = %v (ok=%v), want 2200", v, ok) + } + + memA := find("node_memory_used_bytes", map[string]string{"host": "a"}) + if v, ok := sampleAt(memA, 600); !ok || v != 1000 { + t.Errorf("node_memory_used_bytes{host=a} peak at offset 600 = %v (ok=%v), want 1000", v, ok) + } + if v, ok := sampleAt(memA, 1200); !ok || v != 500 { + t.Errorf("node_memory_used_bytes{host=a} at offset 1200 = %v (ok=%v), want 500", v, ok) + } + + memB := find("node_memory_used_bytes", map[string]string{"host": "b"}) + for _, sm := range memB.Samples { + if sm.Value != 2000 { + t.Errorf("node_memory_used_bytes{host=b} at offset %d = %v, want flat 2000", sm.OffsetSeconds, sm.Value) + } + } + + east := find("checkout_up", map[string]string{"service": "checkout", "region": "us-east"}) + if len(east.Samples) != 6 { + t.Errorf("checkout_up{region=us-east} has %d samples, want 6", len(east.Samples)) + } + for _, sm := range east.Samples { + if sm.OffsetSeconds > 300 { + t.Errorf("checkout_up{region=us-east} has sample at offset %d, want all <= 300", sm.OffsetSeconds) + } + } + + west := find("checkout_up", map[string]string{"service": "checkout", "region": "us-west"}) + if len(west.Samples) != 6 { + t.Errorf("checkout_up{region=us-west} has %d samples, want 6", len(west.Samples)) + } + for _, sm := range west.Samples { + if sm.OffsetSeconds < 900 { + t.Errorf("checkout_up{region=us-west} has sample at offset %d, want all >= 900", sm.OffsetSeconds) + } + } +} + +func TestPushPostsSnappyProtobufWithExpectedHeaders(t *testing.T) { + var gotContentType, gotContentEncoding, gotVersion, gotPath string + var gotBody []byte + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotContentType = r.Header.Get("Content-Type") + gotContentEncoding = r.Header.Get("Content-Encoding") + gotVersion = r.Header.Get("X-Prometheus-Remote-Write-Version") + b, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("reading request body: %v", err) + } + gotBody = b + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + wr := BuildWriteRequest(1000, []SeriesDef{ + { + Name: "push_test_metric", + Labels: map[string]string{"k": "v"}, + Samples: []Sample{{OffsetSeconds: 0, Value: 7}}, + }, + }) + + if err := Push(context.Background(), srv.URL, wr); err != nil { + t.Fatalf("Push: %v", err) + } + + if gotPath != "/api/v1/write" { + t.Errorf("path = %q, want /api/v1/write", gotPath) + } + if gotContentType != "application/x-protobuf" { + t.Errorf("Content-Type = %q, want application/x-protobuf", gotContentType) + } + if gotContentEncoding != "snappy" { + t.Errorf("Content-Encoding = %q, want snappy", gotContentEncoding) + } + if gotVersion != "0.1.0" { + t.Errorf("X-Prometheus-Remote-Write-Version = %q, want 0.1.0", gotVersion) + } + + decoded, err := DecodeSnappy(gotBody) + if err != nil { + t.Fatalf("DecodeSnappy(received body): %v", err) + } + if len(decoded.Timeseries) != 1 || + !reflect.DeepEqual(decoded.Timeseries[0].Labels, wr.Timeseries[0].Labels) || + !reflect.DeepEqual(decoded.Timeseries[0].Samples, wr.Timeseries[0].Samples) { + t.Errorf("received WriteRequest does not match what was built:\ngot: %+v\nwant: %+v", decoded, wr) + } +} + +func TestPushReturnsErrorOnNon2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte("boom")) + })) + defer srv.Close() + + wr := &prompb.WriteRequest{} + if err := Push(context.Background(), srv.URL, wr); err == nil { + t.Fatal("expected error on 400 response, got nil") + } +} From 18f5a37fb1e05b916a8fa27d14751da8e9d33839 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 26 Aug 2026 00:25:57 -0400 Subject: [PATCH 2/5] test(query-engine): add generic multi-step range/instant equivalence oracle (#590) Adds the test #590 calls out as missing: for a stable key set, one range(start,end,step) query's per-timestamp series must exactly equal N separate instant(t) queries (one per step), not just checked at a single timestamp. Covers {Tumbling, Sliding} x {single-population Sum, dual-population Count} x >=4 steps. All four pass against current code. Co-Authored-By: Claude Sonnet 5 --- asap-query-engine/src/tests/mod.rs | 1 + ...nge_multistep_instant_equivalence_tests.rs | 565 ++++++++++++++++++ 2 files changed, 566 insertions(+) create mode 100644 asap-query-engine/src/tests/range_multistep_instant_equivalence_tests.rs diff --git a/asap-query-engine/src/tests/mod.rs b/asap-query-engine/src/tests/mod.rs index b4a5bbe..9ab9f4c 100644 --- a/asap-query-engine/src/tests/mod.rs +++ b/asap-query-engine/src/tests/mod.rs @@ -10,6 +10,7 @@ pub mod native_pipeline_merge_tests; pub mod native_range_query_tests; pub mod prometheus_forwarding_tests; pub mod query_equivalence_tests; +pub mod range_multistep_instant_equivalence_tests; pub mod range_query_arithmetic_tests; pub mod sql_pattern_matching_tests; pub mod store_correctness_tests; diff --git a/asap-query-engine/src/tests/range_multistep_instant_equivalence_tests.rs b/asap-query-engine/src/tests/range_multistep_instant_equivalence_tests.rs new file mode 100644 index 0000000..208571d --- /dev/null +++ b/asap-query-engine/src/tests/range_multistep_instant_equivalence_tests.rs @@ -0,0 +1,565 @@ +//! Generic multi-step range/instant equivalence oracle (issue #590). +//! +//! Every hand-crafted regression test elsewhere in this directory +//! (`native_range_query_tests.rs`, `exact_window_grid_adversarial_tests.rs`, +//! ...) pins one specific bug shape at one specific pair of timestamps. What +//! none of them do -- and what #590 explicitly calls out as the test that +//! would have caught #584 "without needing a one-off regression case per +//! bug" -- is the generic property: +//! +//! > For a stable key set over `[start, end]`, `range(start, end, step)` +//! > equals `{instant(t) for t in steps}`. +//! +//! i.e. run ONE range query covering N steps, run N separate instant +//! queries (one per step timestamp), and assert the range query's +//! per-timestamp series exactly match the corresponding instant query's +//! result set -- for EVERY step, not just the first/last. +//! +//! This is deliberately the easy/happy-path case: the key set is stable +//! (never appears/disappears mid-range). Changing key sets are already +//! covered by the hand-crafted tests referenced above -- duplicating that +//! here would just be more of the same, not a new kind of coverage. +//! +//! Covers {Tumbling, Sliding} x {single-population, dual-population} x +//! {Sum, Count}, at least 4 steps each: +//! - `range_multistep_tumbling_single_population_sum` +//! - `range_multistep_sliding_single_population_sum` +//! - `range_multistep_tumbling_dual_population_count` +//! - `range_multistep_sliding_dual_population_count` +//! +//! If one of these ever fails, that's a real divergence between +//! `execute_range_query_pipeline` and the instant path +//! (`execute_and_merge_store_queries`) -- per the task's ground rules, do +//! NOT weaken the assertion and do NOT patch the engine here; mark the test +//! `#[ignore]` with the failure documented and flag it for separate fix-up. + +#[cfg(test)] +mod tests { + use crate::data_model::{ + AggregationConfig, AggregationReference, AggregationType, CleanupPolicy, InferenceConfig, + KeyByLabelValues, PrecomputedOutput, PromQLSchema, QueryConfig, QueryLanguage, + SchemaConfig, StreamingConfig, WindowType, + }; + use crate::engines::query_result::QueryResult; + use crate::engines::simple_engine::SimpleEngine; + use crate::precompute_operators::sum_accumulator::SumAccumulator; + use crate::precompute_operators::{ + CountMinSketchAccumulator, DeltaSetAggregatorAccumulator, SetAggregatorAccumulator, + }; + use crate::stores::simple_map_store::SimpleMapStore; + use crate::stores::Store; + use crate::tests::test_utilities::engine_factories::create_engine_multi_timestamp_with_window; + use crate::AggregateCore; + use promql_utilities::data_model::KeyByLabelNames; + use std::collections::HashMap; + use std::sync::Arc; + + /// One slide-interval-wide pane: (pane end timestamp ms, grouping label + /// values, accumulator). Mirrors the `TimeSeriesData` shape used + /// throughout `native_range_query_tests.rs` / `engine_factories.rs`. + type TimeSeriesData = Vec<(u64, Option>, Box)>; + + /// A single output step's result, as an order-independent, sorted-by- + /// label snapshot of (label values, value) pairs. + type StepSnapshot = Vec<(Vec, f64)>; + + fn snapshot_vector(qr: QueryResult) -> StepSnapshot { + match qr { + QueryResult::Vector(iv) => { + let mut v: StepSnapshot = iv + .values + .into_iter() + .map(|e| (e.labels.labels, e.value)) + .collect(); + v.sort_by(|a, b| a.0.cmp(&b.0)); + v + } + QueryResult::Matrix(_) => panic!("expected instant vector result, got a matrix"), + } + } + + fn snapshot_matrix_at(qr: &QueryResult, ts: u64) -> StepSnapshot { + match qr { + QueryResult::Matrix(m) => { + let mut v: StepSnapshot = m + .values + .iter() + .filter_map(|e| { + e.samples + .iter() + .find(|s| s.timestamp == ts) + .map(|s| (e.labels.labels.clone(), s.value)) + }) + .collect(); + v.sort_by(|a, b| a.0.cmp(&b.0)); + v + } + QueryResult::Vector(_) => panic!("expected matrix (range vector) result, got a vector"), + } + } + + fn labels_only(snapshot: &StepSnapshot) -> Vec<&Vec> { + snapshot.iter().map(|(labels, _)| labels).collect() + } + + /// The core oracle (#590): run `range(start, end, step)` once, run + /// `instant(t)` once per `step_timestamps_ms`, and assert the range + /// query's per-timestamp slice exactly matches (same series, same + /// values within float tolerance) the corresponding instant query's + /// result -- at EVERY step, collecting every mismatch before panicking + /// (house style: see `assert_all_at` in `native_range_query_tests.rs`) + /// rather than stopping at the first divergence. + fn assert_range_equals_instants( + engine: &SimpleEngine, + query: &str, + start: f64, + end: f64, + step: f64, + step_timestamps_ms: &[u64], + ) { + let (_, range_qr) = engine + .handle_range_query_promql(query.to_string(), start, end, step) + .unwrap_or_else(|| { + panic!("range query `{query}` ({start}..{end} step {step}) returned None") + }); + + let mut mismatches: Vec = Vec::new(); + for &ts in step_timestamps_ms { + let query_time_s = ts as f64 / 1000.0; + let (_, instant_qr) = engine + .handle_query_promql(query.to_string(), query_time_s) + .unwrap_or_else(|| { + panic!("instant query `{query}` at t={query_time_s} returned None") + }); + + let instant_snapshot = snapshot_vector(instant_qr); + let range_snapshot = snapshot_matrix_at(&range_qr, ts); + + if instant_snapshot.len() != range_snapshot.len() { + mismatches.push(format!( + "t={ts}: series count differs -- range has {} series {:?}, instant has {} series {:?}", + range_snapshot.len(), + labels_only(&range_snapshot), + instant_snapshot.len(), + labels_only(&instant_snapshot), + )); + continue; + } + + for ((r_labels, r_val), (i_labels, i_val)) in + range_snapshot.iter().zip(instant_snapshot.iter()) + { + if r_labels != i_labels { + mismatches.push(format!( + "t={ts}: label mismatch -- range={r_labels:?}, instant={i_labels:?}" + )); + } else if (r_val - i_val).abs() >= 1e-6 { + mismatches.push(format!( + "t={ts}: {r_labels:?} value mismatch -- range={r_val}, instant={i_val}" + )); + } + } + } + + assert!( + mismatches.is_empty(), + "range({start},{end},{step}) query `{query}` diverged from per-step instant query \ + at:\n{}", + mismatches.join("\n") + ); + } + + /// Inserts `data` (slide_interval_ms-wide panes, keyed by pane end + /// timestamp) into `store` as pre-merged, window_size_ms-wide buckets + /// for aggregation `agg_id` -- mirroring + /// `worker.rs::merge_panes_for_window`, the same technique + /// `create_engine_multi_timestamp_with_window` (`engine_factories.rs`) + /// uses for a single aggregation. Generalized here (parameterized over + /// `agg_id`) so it can be called once per aggregation -- value AND keys + /// -- to build genuinely multi-step, multi-window dual-population + /// engines, which none of the `pub` factories in `engine_factories.rs` + /// support (`create_engine_dual_input` is Tumbling-only and inserts at + /// a single fixed timestamp). + #[allow(clippy::type_complexity)] + fn insert_windowed_panes( + store: &SimpleMapStore, + agg_id: u64, + window_size_ms: u64, + slide_interval_ms: u64, + data: TimeSeriesData, + ) { + let num_panes = window_size_ms / slide_interval_ms; + + let mut per_key: HashMap>, Vec<(u64, Box)>> = + HashMap::new(); + for (timestamp, label_values_opt, acc) in data { + per_key + .entry(label_values_opt) + .or_default() + .push((timestamp, acc)); + } + + for (label_values_opt, mut panes) in per_key { + let key = label_values_opt.map(|labels| KeyByLabelValues { labels }); + panes.sort_by_key(|(ts, _)| *ts); + + if num_panes <= 1 { + for (ts, acc) in panes { + let output = + PrecomputedOutput::new(ts - window_size_ms, ts, key.clone(), agg_id); + store.insert_precomputed_output(output, acc).unwrap(); + } + continue; + } + + let pane_map: HashMap> = + panes.iter().map(|(ts, acc)| (*ts, acc)).collect(); + let (min_ts, max_ts) = (panes[0].0, panes[panes.len() - 1].0); + + let mut window_start = min_ts.saturating_sub(window_size_ms); + while window_start + window_size_ms <= max_ts { + let pane_ends: Vec = (1..=num_panes) + .map(|i| window_start + i * slide_interval_ms) + .collect(); + if pane_ends.iter().all(|t| pane_map.contains_key(t)) { + let mut merged = pane_map[&pane_ends[0]].clone_boxed_core(); + for t in &pane_ends[1..] { + merged = merged.merge_with(pane_map[t].as_ref()).unwrap(); + } + let output = PrecomputedOutput::new( + window_start, + window_start + window_size_ms, + key.clone(), + agg_id, + ); + store.insert_precomputed_output(output, merged).unwrap(); + } + window_start += slide_interval_ms; + } + } + } + + /// Dual-population engine builder (separate value/keys aggregations, + /// `count(metric) by (...)`-shaped queries) with a configurable window + /// (Tumbling or Sliding) and genuinely multiple output steps -- what + /// `create_engine_dual_input` (single fixed timestamp, Tumbling only) + /// doesn't support. Both the value and keys aggregations share the same + /// window shape here: this test is deliberately the happy-path/stable- + /// key-set case (#590), not the #600/#583-style mismatched-window-width + /// edge cases already covered in `native_range_query_tests.rs`. + #[allow(clippy::too_many_arguments)] + fn create_dual_pop_engine_with_window( + metric: &str, + value_agg_type: AggregationType, + key_agg_type: AggregationType, + grouping_labels: Vec<&str>, + aggregated_labels: Vec<&str>, + value_data: TimeSeriesData, + keys_data: TimeSeriesData, + promql_query: &str, + window_size_ms: u64, + slide_interval_ms: u64, + window_type: WindowType, + ) -> SimpleEngine { + let grouping_label_strings: Vec = + grouping_labels.iter().map(|s| s.to_string()).collect(); + let aggregated_label_strings: Vec = + aggregated_labels.iter().map(|s| s.to_string()).collect(); + let all_labels: Vec = grouping_label_strings + .iter() + .chain(aggregated_label_strings.iter()) + .cloned() + .collect(); + + let mut aggregation_configs = HashMap::new(); + aggregation_configs.insert( + 1u64, + AggregationConfig { + aggregation_id: 1, + aggregation_type: value_agg_type, + aggregation_sub_type: String::new(), + parameters: HashMap::new(), + grouping_labels: KeyByLabelNames::new(grouping_label_strings.clone()), + aggregated_labels: KeyByLabelNames::empty(), + rollup_labels: KeyByLabelNames::empty(), + original_yaml: String::new(), + window_size_ms, + slide_interval_ms, + window_type, + spatial_filter: String::new(), + spatial_filter_normalized: String::new(), + metric: metric.to_string(), + num_aggregates_to_retain: None, + read_count_threshold: None, + table_name: None, + value_column: None, + }, + ); + aggregation_configs.insert( + 2u64, + AggregationConfig { + aggregation_id: 2, + aggregation_type: key_agg_type, + aggregation_sub_type: String::new(), + parameters: HashMap::new(), + grouping_labels: KeyByLabelNames::new(grouping_label_strings), + aggregated_labels: KeyByLabelNames::new(aggregated_label_strings), + rollup_labels: KeyByLabelNames::empty(), + original_yaml: String::new(), + window_size_ms, + slide_interval_ms, + window_type, + spatial_filter: String::new(), + spatial_filter_normalized: String::new(), + metric: metric.to_string(), + num_aggregates_to_retain: None, + read_count_threshold: None, + table_name: None, + value_column: None, + }, + ); + + let streaming_config = Arc::new(StreamingConfig { + aggregation_configs, + }); + + let store = Arc::new(SimpleMapStore::new( + streaming_config.clone(), + CleanupPolicy::NoCleanup, + )); + + insert_windowed_panes(&store, 1, window_size_ms, slide_interval_ms, value_data); + insert_windowed_panes(&store, 2, window_size_ms, slide_interval_ms, keys_data); + + let promql_schema = + PromQLSchema::new().add_metric(metric.to_string(), KeyByLabelNames::new(all_labels)); + + let query_config = QueryConfig::new(promql_query.to_string()) + .add_aggregation(AggregationReference::new(1, None)) + .add_aggregation(AggregationReference::new(2, None)); + + let inference_config = InferenceConfig { + schema: SchemaConfig::PromQL(promql_schema), + query_configs: vec![query_config], + cleanup_policy: CleanupPolicy::NoCleanup, + }; + + SimpleEngine::new( + store, + inference_config, + streaming_config, + slide_interval_ms, + QueryLanguage::promql, + ) + } + + // ════════════════════════════════════════════════════════════════════ + // ── Tumbling x single-population (Sum) ────────────────────────────── + // ════════════════════════════════════════════════════════════════════ + + /// Stable 2-key set (host-a, host-b), 4 consecutive 1s Tumbling buckets. + /// `sum(cpu_usage) by (host)` over range(1.0, 4.0, 1.0) must equal the + /// 4 separate instant queries at t=1.0, 2.0, 3.0, 4.0. + #[tokio::test(flavor = "multi_thread")] + async fn range_multistep_tumbling_single_population_sum() { + let data: TimeSeriesData = vec![ + ( + 1000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(10.0)), + ), + ( + 1000, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(100.0)), + ), + ( + 2000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(20.0)), + ), + ( + 2000, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(200.0)), + ), + ( + 3000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(30.0)), + ), + ( + 3000, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(300.0)), + ), + ( + 4000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(40.0)), + ), + ( + 4000, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(400.0)), + ), + ] + .into_iter() + .map(|(ts, labels, acc)| (ts, labels, acc as Box)) + .collect(); + + let query = "sum(cpu_usage) by (host)"; + let engine = create_engine_multi_timestamp_with_window( + "cpu_usage", + AggregationType::Sum, + vec!["host"], + data, + query, + 1000, + 1000, + WindowType::Tumbling, + ); + + assert_range_equals_instants(&engine, query, 1.0, 4.0, 1.0, &[1000, 2000, 3000, 4000]); + } + + // ════════════════════════════════════════════════════════════════════ + // ── Sliding x single-population (Sum) ─────────────────────────────── + // ════════════════════════════════════════════════════════════════════ + + /// Stable 2-key set, window_size=2000ms/slide=1000ms Sliding. Panes at + /// t=1000..5000 (5 panes, 2 per window) produce 4 fully-formed windows + /// ending at 2000, 3000, 4000, 5000. `sum(cpu_load) by (host)` over + /// range(2.0, 5.0, 1.0) must equal the 4 separate instant queries. + #[tokio::test(flavor = "multi_thread")] + async fn range_multistep_sliding_single_population_sum() { + let host_a_panes = [1.0, 2.0, 3.0, 4.0, 5.0]; + let host_b_panes = [10.0, 20.0, 30.0, 40.0, 50.0]; + let mut data: TimeSeriesData = Vec::new(); + for (i, ts) in (1000..=5000).step_by(1000).enumerate() { + data.push(( + ts, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(host_a_panes[i])) as Box, + )); + data.push(( + ts, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(host_b_panes[i])) as Box, + )); + } + + let query = "sum(cpu_load) by (host)"; + let engine = create_engine_multi_timestamp_with_window( + "cpu_load", + AggregationType::Sum, + vec!["host"], + data, + query, + 2000, + 1000, + WindowType::Sliding, + ); + + assert_range_equals_instants(&engine, query, 2.0, 5.0, 1.0, &[2000, 3000, 4000, 5000]); + } + + // ════════════════════════════════════════════════════════════════════ + // ── Tumbling x dual-population (Count via DeltaSetAggregator keys) ── + // ════════════════════════════════════════════════════════════════════ + + /// Stable 2-key set ((host-a, evt-1), (host-b, evt-2)), 4 consecutive 1s + /// Tumbling buckets, dual-population `count(event_frequency) by (host, + /// event)` (value=CountMinSketch, keys=DeltaSetAggregator -- the same + /// pairing `native_range_query_tests.rs`'s Tumbling dual-population + /// tests use). range(1.0, 4.0, 1.0) must equal the 4 separate instant + /// queries. + #[tokio::test(flavor = "multi_thread")] + async fn range_multistep_tumbling_dual_population_count() { + let mut value_data: TimeSeriesData = Vec::new(); + let mut keys_data: TimeSeriesData = Vec::new(); + for ts in (1000..=4000).step_by(1000) { + value_data.push(( + ts, + None, + Box::new(CountMinSketchAccumulator::new(2, 3)) as Box, + )); + + let mut keys = DeltaSetAggregatorAccumulator::new(); + keys.add_key(KeyByLabelValues { + labels: vec!["host-a".to_string(), "evt-1".to_string()], + }); + keys.add_key(KeyByLabelValues { + labels: vec!["host-b".to_string(), "evt-2".to_string()], + }); + keys_data.push((ts, None, Box::new(keys) as Box)); + } + + let query = "count(event_frequency) by (host, event)"; + let engine = create_dual_pop_engine_with_window( + "event_frequency", + AggregationType::CountMinSketch, + AggregationType::DeltaSetAggregator, + vec![], + vec!["host", "event"], + value_data, + keys_data, + query, + 1000, + 1000, + WindowType::Tumbling, + ); + + assert_range_equals_instants(&engine, query, 1.0, 4.0, 1.0, &[1000, 2000, 3000, 4000]); + } + + // ════════════════════════════════════════════════════════════════════ + // ── Sliding x dual-population (Count via SetAggregator keys) ──────── + // ════════════════════════════════════════════════════════════════════ + + /// Stable 2-key set, window_size=2000ms/slide=1000ms Sliding for BOTH + /// the value and keys aggregations. Keys aggregation uses SetAggregator + /// (not DeltaSetAggregator, which #606 restricts to Tumbling -- + /// `native_range_query_tests.rs`'s Sliding-keys tests use the same + /// substitution). Panes at t=1000..5000 produce 4 fully-formed windows + /// ending at 2000, 3000, 4000, 5000, each containing both stable keys. + /// range(2.0, 5.0, 1.0) must equal the 4 separate instant queries. + #[tokio::test(flavor = "multi_thread")] + async fn range_multistep_sliding_dual_population_count() { + let mut value_data: TimeSeriesData = Vec::new(); + let mut keys_data: TimeSeriesData = Vec::new(); + for ts in (1000..=5000).step_by(1000) { + value_data.push(( + ts, + None, + Box::new(CountMinSketchAccumulator::new(2, 3)) as Box, + )); + + let mut keys = SetAggregatorAccumulator::new(); + keys.add_key(KeyByLabelValues { + labels: vec!["host-a".to_string(), "evt-1".to_string()], + }); + keys.add_key(KeyByLabelValues { + labels: vec!["host-b".to_string(), "evt-2".to_string()], + }); + keys_data.push((ts, None, Box::new(keys) as Box)); + } + + let query = "count(event_frequency) by (host, event)"; + let engine = create_dual_pop_engine_with_window( + "event_frequency", + AggregationType::CountMinSketch, + AggregationType::SetAggregator, + vec![], + vec!["host", "event"], + value_data, + keys_data, + query, + 2000, + 1000, + WindowType::Sliding, + ); + + assert_range_equals_instants(&engine, query, 2.0, 5.0, 1.0, &[2000, 3000, 4000, 5000]); + } +} From fdc8bca38fc22db0c3f0a96ba2e118e1c5c569be Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 26 Aug 2026 00:26:35 -0400 Subject: [PATCH 3/5] feat(promql-compliance): add instant-query diffing to promql-compliance-tester Vendor prometheus/compliance/promql (commit 67b8327, Apache 2.0) into promql-compliance/harness/ and patch Comparer.Compare to also run and diff an instant query (PromAPI.Query) alongside the existing range query, since upstream only ever exercised QueryRange. Range and instant outcomes are tracked and reported independently (Result.RangeSuccess/InstantSuccess), so "PASS: range, FAIL: instant" is a representable outcome instead of one aggregate pass/fail -- this is the exact bug class cataloged in #589. Adds config.yaml with placeholder reference/test target URLs and four seed regression test cases ported from the query patterns described in #589, #583, and #584 (top-k over range, per-step key snapshot churn, and two sliding-window rate queries). Adds comparer_test.go with fake in-process PromAPI doubles covering both diverging directions plus error/should-fail handling, since no unit tests existed upstream. Part of #594. Co-Authored-By: Claude Sonnet 5 --- promql-compliance/harness/Dockerfile | 14 + promql-compliance/harness/LICENSE | 201 ++++++++++++++ promql-compliance/harness/Makefile | 3 + promql-compliance/harness/README.md | 114 ++++++++ .../cmd/promql-compliance-tester/main.go | 184 +++++++++++++ .../harness/comparer/comparer.go | 259 ++++++++++++++++++ .../harness/comparer/comparer_test.go | 172 ++++++++++++ promql-compliance/harness/config.yaml | 81 ++++++ promql-compliance/harness/config/config.go | 89 ++++++ promql-compliance/harness/go.mod | 30 ++ promql-compliance/harness/go.sum | 81 ++++++ .../harness/output/example-output.html | 60 ++++ promql-compliance/harness/output/html.go | 63 +++++ promql-compliance/harness/output/json.go | 23 ++ promql-compliance/harness/output/outputter.go | 9 + promql-compliance/harness/output/text.go | 93 +++++++ promql-compliance/harness/output/tsv.go | 40 +++ .../harness/prometheus-test-data.yml | 10 + .../harness/promql-test-queries.yml | 243 ++++++++++++++++ promql-compliance/harness/testcases/expand.go | 154 +++++++++++ 20 files changed, 1923 insertions(+) create mode 100644 promql-compliance/harness/Dockerfile create mode 100644 promql-compliance/harness/LICENSE create mode 100644 promql-compliance/harness/Makefile create mode 100644 promql-compliance/harness/README.md create mode 100644 promql-compliance/harness/cmd/promql-compliance-tester/main.go create mode 100644 promql-compliance/harness/comparer/comparer.go create mode 100644 promql-compliance/harness/comparer/comparer_test.go create mode 100644 promql-compliance/harness/config.yaml create mode 100644 promql-compliance/harness/config/config.go create mode 100644 promql-compliance/harness/go.mod create mode 100644 promql-compliance/harness/go.sum create mode 100644 promql-compliance/harness/output/example-output.html create mode 100644 promql-compliance/harness/output/html.go create mode 100644 promql-compliance/harness/output/json.go create mode 100644 promql-compliance/harness/output/outputter.go create mode 100644 promql-compliance/harness/output/text.go create mode 100644 promql-compliance/harness/output/tsv.go create mode 100644 promql-compliance/harness/prometheus-test-data.yml create mode 100644 promql-compliance/harness/promql-test-queries.yml create mode 100644 promql-compliance/harness/testcases/expand.go diff --git a/promql-compliance/harness/Dockerfile b/promql-compliance/harness/Dockerfile new file mode 100644 index 0000000..f354309 --- /dev/null +++ b/promql-compliance/harness/Dockerfile @@ -0,0 +1,14 @@ +FROM golang:1.25 as build-env + +WORKDIR /go/src/promql +COPY . /go/src/promql + +ENV CGO_ENABLED 0 + +RUN go build ./cmd/promql-compliance-tester + +FROM quay.io/prometheus/busybox:latest +COPY --from=build-env /go/src/promql/promql-compliance-tester / +COPY --from=build-env /go/src/promql/promql-test-queries.yml / + +ENTRYPOINT ["/promql-compliance-tester"] diff --git a/promql-compliance/harness/LICENSE b/promql-compliance/harness/LICENSE new file mode 100644 index 0000000..261eeb9 --- /dev/null +++ b/promql-compliance/harness/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/promql-compliance/harness/Makefile b/promql-compliance/harness/Makefile new file mode 100644 index 0000000..a7b3db8 --- /dev/null +++ b/promql-compliance/harness/Makefile @@ -0,0 +1,3 @@ +.PHONY: docker +docker: + docker build . -t promql-compliance-tester:latest diff --git a/promql-compliance/harness/README.md b/promql-compliance/harness/README.md new file mode 100644 index 0000000..6162933 --- /dev/null +++ b/promql-compliance/harness/README.md @@ -0,0 +1,114 @@ +# PromQL Compliance Tester (ASAPQuery fork) + +This is a vendored copy of [`prometheus/compliance/promql`](https://github.com/prometheus/compliance/tree/main/promql) +(commit `67b8327a2e93dc28f64d4b21bbce00b362f565d5`), kept under its original +Apache 2.0 license (see `LICENSE`) and import path, with one local patch on +top: `comparer.Compare` (in `comparer/comparer.go`) now also runs an instant +query (`PromAPI.Query`) at each test case's end timestamp, alongside the +existing range query, and diffs both independently (`Result.RangeSuccess()` / +`Result.InstantSuccess()`). Upstream only ever exercised `QueryRange` and +never called `Query`, which meant it could not catch instant-vs-range +divergence bugs -- exactly the class of bug cataloged in ASAPQuery issue #589. +See `config.yaml` for ASAPQuery's target configuration and seed regression +test cases (ported from issues #589/#583/#584). + +--- + +The PromQL Compliance Tester is a tool for running comparison tests between native Prometheus and vendor PromQL API implementations. + +The tool was [first published and described](https://promlabs.com/blog/2020/08/06/comparing-promql-correctness-across-vendors) in August 2020. [Test results have been published](https://promlabs.com/promql-compliance-tests) on 2020-08-06 and 2020-12-01. + +## Building via Docker + +If you have docker installed, you can build the tool using docker. + +```bash +make docker +``` + +## Building from source + +### Requirements + +This tool is written in Go and requires a working Go setup to build. Library dependencies are handled via [Go Modules](https://blog.golang.org/using-go-modules). + +### Building + +To build the tool: + +```bash +go build ./cmd/promql-compliance-tester +``` + +## Executing + +The tool allows setting the following flags: + +``` +$ ./promql-compliance-tester -h +Usage of ./promql-compliance-tester: + -config-file value + The path to the configuration file. If repeated, the specified files will be concatenated before YAML parsing. + -output-format string + The comparison output format. Valid values: [text, html, json] (default "text") + -output-html-template string + The HTML template to use when using HTML as the output format. (default "./output/example-output.html") + -output-passing + Whether to also include passing test cases in the output. + -query-parallelism int + Maximum number of comparison queries to run in parallel. (default 20) +``` + +Running the tool will execute all test cases in `-config-file` and compare results between reference and target provided in the same file. + +At the end of the run, the output is provided in the form of the number of executed tests and errors if any. Example output can be seen here: + +```bash +./promql-compliance-tester -config-file config.yaml -config-file ./promql-test-queries.yml +529 / 529 [-----------------------------------------------------------------------------------------------------------] 100.00% 278 p/s +Total: 529 / 529 (100.00%) passed, 0 unsupported +``` + +If all tests were executed correctly and passing the tool returns a 0 exit code, otherwise it returns 1. + +## Configuration + +A standard suite of test cases is defined in the [`promql-test-queries.yml`](./promql-test-queries.yml) file, while separate `test-.yml` config files specify test target configurations and query tweaks for a number of individual projects and vendors. To run the tester tool, you need to specify both the test suite config file as well as a config file for a single vendor. + +For example, to run the tester against Cortex: + +```bash +./promql-compliance-tester -config-file=promql-test-queries.yml -config-file=test-cortex.yml +``` + +Note that some of the vendor-specific configuration files require you to replace certain placeholder values for endpoints and credentials before using them. + +## Testing your implementation for compliance + +We encourage projects and vendors to test their implementations for PromQL compliance. To do this, follow these steps: + +1. Check out this repository: `git clone git@github.com:prometheus/compliance`. +2. Change into the repo's `promql` directory: `cd compliance/promql`. +3. Either edit the appropriate `test-.yml` file for your project or service or create a new test target configuration file to be able to query from both your reference Prometheus server and your PromQL-compatible datasource. +4. Edit `prometheus-test-data.yml` to either add a `remote_write` section for your system or make any other adjustments that are necessary to enable propagation of the scraped data to your system (e.g. adding external labels for Thanos). +5. Run a reference Prometheus server that ingests the expected test data (we assume that you have Prometheus installed): `prometheus --config.file=prometheus-test-data.yml`. +6. Wait for at least one hour for sufficient test data to be ingested into both the reference Prometheus server and the system to be tested. +7. Build the tester tool: `go build ./cmd/promql-compliance-tester`. +8. Run the tester tool (replacing `` as appropriate): `./promql-compliance-tester -config-file=promql-test-queries.yml -config-file=test-.yml`. + +If the tool reports a test score of 100% without any cross-cutting query tweaks, your implementation is PromQL-compliant. + +## Contributing + +Help is wanted to improve the PromQL Compliance Tester. In particular, we would love to add and improve the following points: + +* ~~Test instant queries in addition to range queries.~~ Done in this fork -- see the note at the top of this file. +* Add more variation and configurability to input timestamps. +* Flesh out a more comprehensive (and less overlapping) set of input test queries. +* Automate and integrate data loading into different systems. +* Test more vendor implementations of PromQL. +* Version test results and make pretty output presentations easier. + +**Note:** Many people will be interested in benchmarking performance differences between PromQL implementations. While this is important as well, the PromQL Compliance Tester focuses solely on correctness testing. Please contact the [maintainers](../MAINTAINERS.md) if you want to work on performance testing. + +If you would like to help flesh out the tester, please [file issues](https://github.com/prometheus/compliance/issues) or [pull requests](https://github.com/prometheus/compliance/pulls). diff --git a/promql-compliance/harness/cmd/promql-compliance-tester/main.go b/promql-compliance/harness/cmd/promql-compliance-tester/main.go new file mode 100644 index 0000000..1598865 --- /dev/null +++ b/promql-compliance/harness/cmd/promql-compliance-tester/main.go @@ -0,0 +1,184 @@ +package main + +import ( + "flag" + "fmt" + "log" + "math" + "net/http" + "os" + "strconv" + "strings" + "sync" + "time" + + "github.com/cheggaaa/pb/v3" + "github.com/prometheus/client_golang/api" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" + "github.com/prometheus/compliance/promql/output" + "github.com/prometheus/compliance/promql/testcases" + "go.uber.org/atomic" +) + +func newPromAPI(targetConfig config.TargetConfig) (v1.API, error) { + apiConfig := api.Config{Address: targetConfig.QueryURL} + if len(targetConfig.Headers) > 0 || targetConfig.BasicAuthUser != "" { + apiConfig.RoundTripper = roundTripperWithSettings{headers: targetConfig.Headers, basicAuthUser: targetConfig.BasicAuthUser, basicAuthPass: targetConfig.BasicAuthPass} + } + client, err := api.NewClient(apiConfig) + if err != nil { + return nil, fmt.Errorf("error creating Prometheus API client for %q: %w", targetConfig.QueryURL, err) + } + + return v1.NewAPI(client), nil +} + +type roundTripperWithSettings struct { + headers map[string]string + basicAuthUser string + basicAuthPass string +} + +func (rt roundTripperWithSettings) RoundTrip(req *http.Request) (*http.Response, error) { + // Per RoundTrip's documentation, RoundTrip should not modify the request, + // except for consuming and closing the Request's Body. + // TODO: Update the Go Prometheus client code to support adding headers to request. + + if rt.basicAuthUser != "" { + req.SetBasicAuth(rt.basicAuthUser, rt.basicAuthPass) + } + + for key, value := range rt.headers { + if strings.ToLower(key) == "host" { + req.Host = value + } else { + req.Header.Add(key, value) + } + } + return http.DefaultTransport.RoundTrip(req) +} + +type arrayFlags []string + +func (i *arrayFlags) String() string { + return "my string representation" +} + +func (i *arrayFlags) Set(value string) error { + *i = append(*i, value) + return nil +} + +func main() { + var configFiles arrayFlags + flag.Var(&configFiles, "config-file", "The path to the configuration file. If repeated, the specified files will be concatenated before YAML parsing.") + outputFormat := flag.String("output-format", "text", "The comparison output format. Valid values: [text, html, json]") + outputHTMLTemplate := flag.String("output-html-template", "./output/example-output.html", "The HTML template to use when using HTML as the output format.") + outputPassing := flag.Bool("output-passing", false, "Whether to also include passing test cases in the output.") + queryParallelism := flag.Int("query-parallelism", 20, "Maximum number of comparison queries to run in parallel.") + flag.Parse() + + var outp output.Outputter + switch *outputFormat { + case "text": + outp = output.Text + case "html": + var err error + outp, err = output.HTML(*outputHTMLTemplate) + if err != nil { + log.Fatalf("Error reading output HTML template: %v", err) + } + case "json": + outp = output.JSON + case "tsv": + outp = output.TSV + default: + log.Fatalf("Invalid output format %q", *outputFormat) + } + + cfg, err := config.LoadFromFiles(configFiles) + if err != nil { + log.Fatalf("Error loading configuration file: %v", err) + } + refAPI, err := newPromAPI(cfg.ReferenceTargetConfig) + if err != nil { + log.Fatalf("Error creating reference API: %v", err) + } + testAPI, err := newPromAPI(cfg.TestTargetConfig) + if err != nil { + log.Fatalf("Error creating test API: %v", err) + } + + comp := comparer.New(refAPI, testAPI, cfg.QueryTweaks) + + end := getTime(cfg.QueryTimeParameters.EndTime, time.Now().UTC().Add(-12*time.Minute)) + start := end.Add( + -getNonZeroDuration(cfg.QueryTimeParameters.RangeInSeconds, 10*time.Minute)) + resolution := getNonZeroDuration( + cfg.QueryTimeParameters.ResolutionInSeconds, 10*time.Second) + expandedTestCases := testcases.ExpandTestCases(cfg.TestCases, cfg.QueryTweaks, start, end, resolution) + + var wg sync.WaitGroup + results := make([]*comparer.Result, len(expandedTestCases)) + progressBar := pb.StartNew(len(results)) + wg.Add(len(results)) + + workCh := make(chan struct{}, *queryParallelism) + + allSuccess := atomic.NewBool(true) + for i, tc := range expandedTestCases { + workCh <- struct{}{} + + go func(i int, tc *comparer.TestCase) { + res, err := comp.Compare(tc) + if err != nil { + log.Fatalf("Error running comparison: %v", err) + } + results[i] = res + if !res.Success() { + allSuccess.Store(false) + } + progressBar.Increment() + <-workCh + wg.Done() + }(i, tc) + } + + wg.Wait() + progressBar.Finish() + + outp(results, *outputPassing, cfg.QueryTweaks) + + if !allSuccess.Load() { + os.Exit(1) + } +} + +func getTime(timeStr string, defaultTime time.Time) time.Time { + result, err := parseTime(timeStr) + if err != nil { + return defaultTime + } + return result +} + +func getNonZeroDuration( + seconds float64, defaultDuration time.Duration) time.Duration { + if seconds == 0.0 { + return defaultDuration + } + return time.Duration(seconds * float64(time.Second)) +} + +func parseTime(s string) (time.Time, error) { + if t, err := strconv.ParseFloat(s, 64); err == nil { + s, ns := math.Modf(t) + return time.Unix(int64(s), int64(ns*float64(time.Second))).UTC(), nil + } + if t, err := time.Parse(time.RFC3339Nano, s); err == nil { + return t, nil + } + return time.Time{}, fmt.Errorf("cannot parse %q to a valid timestamp", s) +} diff --git a/promql-compliance/harness/comparer/comparer.go b/promql-compliance/harness/comparer/comparer.go new file mode 100644 index 0000000..b832dc5 --- /dev/null +++ b/promql-compliance/harness/comparer/comparer.go @@ -0,0 +1,259 @@ +package comparer + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" + "github.com/prometheus/common/model" + "github.com/prometheus/compliance/promql/config" +) + +const ( + defaultFraction = 0.00001 + defaultMargin = 0.0 +) + +// PromAPI allows running instant and range queries against a Prometheus-compatible API. +type PromAPI interface { + // Query performs a query for the given time. + Query(ctx context.Context, query string, ts time.Time, opts ...v1.Option) (model.Value, v1.Warnings, error) + // QueryRange performs a query for the given range. + QueryRange(ctx context.Context, query string, r v1.Range, opts ...v1.Option) (model.Value, v1.Warnings, error) +} + +// TestCase represents a fully expanded query to be tested. +type TestCase struct { + Query string `json:"query"` + SkipComparison bool `json:"skipComparison"` + ShouldFail bool `json:"shouldFail"` + Start time.Time `json:"start"` + End time.Time `json:"end"` + Resolution time.Duration `json:"resolution"` +} + +// A Comparer allows comparing query results for test cases between a reference API and a test API. +type Comparer struct { + refAPI PromAPI + testAPI PromAPI + queryTweaks []*config.QueryTweak + compareOptions cmp.Options +} + +// New returns a new Comparer. +func New(refAPI, testAPI PromAPI, queryTweaks []*config.QueryTweak) *Comparer { + var options cmp.Options + addFloatCompareOptions(queryTweaks, &options) + addDropResultLabelsOptions(queryTweaks, &options) + addCaseInsensitiveCompareOptions(queryTweaks, &options) + return &Comparer{ + refAPI: refAPI, + testAPI: testAPI, + queryTweaks: queryTweaks, + compareOptions: options, + } +} + +// Result tracks a single test case's query comparison result. +// +// The range-query outcome (Diff/UnexpectedFailure/UnexpectedSuccess/Unsupported) +// and the instant-query outcome (the Instant-prefixed fields) are tracked and +// reported independently, rather than folded into one aggregate pass/fail: a +// query can pass as a range query but fail as an instant query (or vice versa) +// -- see RangeSuccess/InstantSuccess. +type Result struct { + TestCase *TestCase `json:"testCase"` + Diff string `json:"diff"` + UnexpectedFailure string `json:"unexpectedFailure"` + UnexpectedSuccess bool `json:"unexpectedSuccess"` + Unsupported bool `json:"unsupported"` + + InstantDiff string `json:"instantDiff"` + InstantUnexpectedFailure string `json:"instantUnexpectedFailure"` + InstantUnexpectedSuccess bool `json:"instantUnexpectedSuccess"` + InstantUnsupported bool `json:"instantUnsupported"` +} + +// RangeSuccess returns true if the range-query comparison was successful. +func (r *Result) RangeSuccess() bool { + return r.Diff == "" && !r.UnexpectedSuccess && r.UnexpectedFailure == "" +} + +// InstantSuccess returns true if the instant-query comparison was successful. +func (r *Result) InstantSuccess() bool { + return r.InstantDiff == "" && !r.InstantUnexpectedSuccess && r.InstantUnexpectedFailure == "" +} + +// Success returns true if both the range-query and instant-query comparison results were successful. +func (r *Result) Success() bool { + return r.RangeSuccess() && r.InstantSuccess() +} + +// sortInstantValue sorts vector and matrix instant-query results into a +// deterministic order so that cmp.Diff doesn't report spurious differences +// due to ordering alone. Mirrors the sort.Sort(testResult.(model.Matrix)) call +// done for range-query results below. Scalars and strings need no sorting. +func sortInstantValue(v model.Value) { + switch val := v.(type) { + case model.Vector: + sort.Sort(val) + case model.Matrix: + sort.Sort(val) + } +} + +// Compare runs a test case query against the reference API and the test API and compares the results. +// +// It runs both a range query (over [tc.Start, tc.End] at tc.Resolution) and an +// instant query (evaluated at tc.End, the same timestamp the range query ends +// on) against both APIs, and diffs each independently using the same +// tolerance/label-drop config. Prometheus's instant-query and range-query +// evaluation paths are not just two views of the same code -- ASAPQuery in +// particular has had multiple bugs where the two diverge -- so exercising only +// one (as upstream does) misses an entire class of bugs. +func (c *Comparer) Compare(tc *TestCase) (*Result, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + r := v1.Range{ + Start: tc.Start, + End: tc.End, + Step: tc.Resolution, + } + + // TODO: Handle warnings (second, ignored return value). + refResult, _, refErr := c.refAPI.QueryRange(ctx, tc.Query, r) + testResult, _, testErr := c.testAPI.QueryRange(ctx, tc.Query, r) + refInstantResult, _, refInstantErr := c.refAPI.Query(ctx, tc.Query, tc.End) + testInstantResult, _, testInstantErr := c.testAPI.Query(ctx, tc.Query, tc.End) + + if (refErr != nil) != tc.ShouldFail { + if refErr != nil { + return nil, fmt.Errorf("error querying reference API for %q: %w", tc.Query, refErr) + } + return nil, fmt.Errorf("expected reference API query %q to fail, but succeeded", tc.Query) + } + if (refInstantErr != nil) != tc.ShouldFail { + if refInstantErr != nil { + return nil, fmt.Errorf("error querying reference API (instant) for %q: %w", tc.Query, refInstantErr) + } + return nil, fmt.Errorf("expected reference API instant query %q to fail, but succeeded", tc.Query) + } + + res := &Result{TestCase: tc} + rangeErrMismatch := (testErr != nil) != tc.ShouldFail + instantErrMismatch := (testInstantErr != nil) != tc.ShouldFail + if rangeErrMismatch { + if testErr != nil { + res.UnexpectedFailure = testErr.Error() + res.Unsupported = strings.Contains(testErr.Error(), "501") + } else { + res.UnexpectedSuccess = true + } + } + if instantErrMismatch { + if testInstantErr != nil { + res.InstantUnexpectedFailure = testInstantErr.Error() + res.InstantUnsupported = strings.Contains(testInstantErr.Error(), "501") + } else { + res.InstantUnexpectedSuccess = true + } + } + if rangeErrMismatch || instantErrMismatch { + return res, nil + } + + if tc.SkipComparison || tc.ShouldFail { + return res, nil + } + + sort.Sort(testResult.(model.Matrix)) + sortInstantValue(refInstantResult) + sortInstantValue(testInstantResult) + + for _, qt := range c.queryTweaks { + if qt.IgnoreFirstStep { + for _, r := range refResult.(model.Matrix) { + if len(r.Values) > 0 && r.Values[0].Timestamp.Time().Sub(tc.Start) <= 2*time.Millisecond { + r.Values = r.Values[1:] + } + } + } + } + + res.Diff = cmp.Diff(refResult, testResult, c.compareOptions) + res.InstantDiff = cmp.Diff(refInstantResult, testInstantResult, c.compareOptions) + + return res, nil +} + +func addFloatCompareOptions(queryTweaks []*config.QueryTweak, options *cmp.Options) { + fraction := defaultFraction + margin := defaultMargin + for _, rt := range queryTweaks { + if rt.AdjustValueTolerance != nil { + if rt.AdjustValueTolerance.Fraction != nil { + fraction = *rt.AdjustValueTolerance.Fraction + } + if rt.AdjustValueTolerance.Margin != nil { + margin = *rt.AdjustValueTolerance.Margin + } + } + } + *options = append( + *options, + // Translate sample values into float64 so that cmpopts.EquateApprox() works. + cmp.Transformer("TranslateFloat64", func(in model.SampleValue) float64 { + return float64(in) + }), + cmpopts.EquateApprox(fraction, margin), + // A NaN is usually not treated as equal to another NaN, but we want to treat it as such here. + cmpopts.EquateNaNs(), + ) +} + +func addDropResultLabelsOptions(queryTweaks []*config.QueryTweak, options *cmp.Options) { + for _, rt := range queryTweaks { + if len(rt.DropResultLabels) != 0 { + localRt := rt + *options = append( + *options, + cmp.Transformer( + "DropResultLabels", + func(in model.Metric) model.Metric { + m := in.Clone() + for _, ln := range localRt.DropResultLabels { + delete(m, ln) + } + return m + }, + ), + ) + } + } +} + +func addCaseInsensitiveCompareOptions(queryTweaks []*config.QueryTweak, options *cmp.Options) { + for _, rt := range queryTweaks { + if rt.IgnoreCase { + *options = append( + *options, + // Translate metric names and labels into lowercase. + cmp.Transformer("TranslateToLowerCase", + func(in model.Metric) model.Metric { + m := map[model.LabelName]model.LabelValue{} + for key, val := range in { + m[model.LabelName(strings.ToLower(string(key)))] = model.LabelValue(strings.ToLower(string(val))) + } + return m + }, + ), + ) + } + } +} diff --git a/promql-compliance/harness/comparer/comparer_test.go b/promql-compliance/harness/comparer/comparer_test.go new file mode 100644 index 0000000..4782784 --- /dev/null +++ b/promql-compliance/harness/comparer/comparer_test.go @@ -0,0 +1,172 @@ +package comparer + +import ( + "context" + "errors" + "testing" + "time" + + v1 "github.com/prometheus/client_golang/api/prometheus/v1" + "github.com/prometheus/common/model" +) + +// fakeAPI is an in-process PromAPI double that lets tests script distinct +// instant (Query) and range (QueryRange) responses, without needing a real +// HTTP server. It exists specifically to exercise the instant-query diffing +// path added to Comparer.Compare, which upstream promql-compliance-tester +// never invoked (it only ever called QueryRange). +type fakeAPI struct { + queryValue model.Value + queryErr error + + rangeValue model.Value + rangeErr error +} + +func (f *fakeAPI) Query(_ context.Context, _ string, _ time.Time, _ ...v1.Option) (model.Value, v1.Warnings, error) { + return f.queryValue, nil, f.queryErr +} + +func (f *fakeAPI) QueryRange(_ context.Context, _ string, _ v1.Range, _ ...v1.Option) (model.Value, v1.Warnings, error) { + return f.rangeValue, nil, f.rangeErr +} + +func vectorOf(value float64, labels model.Metric) model.Vector { + return model.Vector{ + &model.Sample{ + Metric: labels, + Value: model.SampleValue(value), + Timestamp: model.Time(0), + }, + } +} + +func matrixOf(value float64, labels model.Metric) model.Matrix { + return model.Matrix{ + &model.SampleStream{ + Metric: labels, + Values: []model.SamplePair{{Timestamp: model.Time(0), Value: model.SampleValue(value)}}, + }, + } +} + +func newTestCase(query string) *TestCase { + end := time.Unix(1000, 0).UTC() + return &TestCase{ + Query: query, + Start: end.Add(-time.Minute), + End: end, + Resolution: 15 * time.Second, + } +} + +func TestCompare_BothMatch_Succeeds(t *testing.T) { + labels := model.Metric{"__name__": "up"} + ref := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(1, labels)} + test := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(1, labels)} + + c := New(ref, test, nil) + res, err := c.Compare(newTestCase("up")) + if err != nil { + t.Fatalf("Compare returned error: %v", err) + } + if !res.RangeSuccess() { + t.Errorf("expected RangeSuccess() to be true, diff: %q", res.Diff) + } + if !res.InstantSuccess() { + t.Errorf("expected InstantSuccess() to be true, diff: %q", res.InstantDiff) + } + if !res.Success() { + t.Errorf("expected Success() to be true") + } +} + +// TestCompare_InstantDivergesRangeMatches reproduces the exact shape of +// ASAPQuery's known bug class (see issue #589): a query whose range-query +// evaluation matches the reference target but whose instant-query evaluation +// does not. Comparer.Compare must surface this as "PASS: range, FAIL: +// instant" rather than as one aggregate pass, which is only possible because +// range and instant are diffed independently. +func TestCompare_InstantDivergesRangeMatches(t *testing.T) { + labels := model.Metric{"__name__": "up"} + ref := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(1, labels)} + test := &fakeAPI{queryValue: vectorOf(2, labels), rangeValue: matrixOf(1, labels)} + + c := New(ref, test, nil) + res, err := c.Compare(newTestCase("up")) + if err != nil { + t.Fatalf("Compare returned error: %v", err) + } + if !res.RangeSuccess() { + t.Errorf("expected RangeSuccess() to be true, diff: %q", res.Diff) + } + if res.InstantSuccess() { + t.Errorf("expected InstantSuccess() to be false") + } + if res.InstantDiff == "" { + t.Errorf("expected a non-empty InstantDiff") + } + if res.Success() { + t.Errorf("expected Success() to be false when only the instant result diverges") + } +} + +// TestCompare_RangeDivergesInstantMatches is the mirror image of the above: +// range diverges but instant matches. +func TestCompare_RangeDivergesInstantMatches(t *testing.T) { + labels := model.Metric{"__name__": "up"} + ref := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(1, labels)} + test := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(2, labels)} + + c := New(ref, test, nil) + res, err := c.Compare(newTestCase("up")) + if err != nil { + t.Fatalf("Compare returned error: %v", err) + } + if res.RangeSuccess() { + t.Errorf("expected RangeSuccess() to be false") + } + if !res.InstantSuccess() { + t.Errorf("expected InstantSuccess() to be true, diff: %q", res.InstantDiff) + } + if res.Success() { + t.Errorf("expected Success() to be false when only the range result diverges") + } +} + +func TestCompare_InstantUnexpectedFailure(t *testing.T) { + labels := model.Metric{"__name__": "up"} + ref := &fakeAPI{queryValue: vectorOf(1, labels), rangeValue: matrixOf(1, labels)} + test := &fakeAPI{queryErr: errors.New("501 Not Implemented"), rangeValue: matrixOf(1, labels)} + + c := New(ref, test, nil) + res, err := c.Compare(newTestCase("up")) + if err != nil { + t.Fatalf("Compare returned error: %v", err) + } + if res.InstantUnexpectedFailure == "" { + t.Errorf("expected InstantUnexpectedFailure to be set") + } + if !res.InstantUnsupported { + t.Errorf("expected InstantUnsupported to be true for a 501 error") + } + if res.Success() { + t.Errorf("expected Success() to be false") + } +} + +func TestCompare_ShouldFail_BothAPIsFail_Skips(t *testing.T) { + ref := &fakeAPI{queryErr: errors.New("boom"), rangeErr: errors.New("boom")} + test := &fakeAPI{queryErr: errors.New("boom"), rangeErr: errors.New("boom")} + + c := New(ref, test, nil) + tc := newTestCase("invalid_query(") + tc.ShouldFail = true + res, err := c.Compare(tc) + if err != nil { + t.Fatalf("Compare returned error: %v", err) + } + if !res.Success() { + t.Errorf("expected Success() to be true when both APIs failed as expected") + } +} diff --git a/promql-compliance/harness/config.yaml b/promql-compliance/harness/config.yaml new file mode 100644 index 0000000..5d13179 --- /dev/null +++ b/promql-compliance/harness/config.yaml @@ -0,0 +1,81 @@ +# Differential test config for ASAPQuery, run with: +# +# go run ./cmd/promql-compliance-tester -config-file=config.yaml +# +# reference_target_config points at a real Prometheus instance and +# test_target_config at ASAPQuery's PromQL query endpoint. Both are +# placeholders below: no live instance is running yet. The sibling +# remote-write seeder (promql-compliance/seeder/) is expected to push a fixed, +# hand-authored dataset to both of these query_urls before this config is +# actually run, so that reference/test results are directly comparable and +# expected values are computable by hand for the regression cases below. +reference_target_config: + # A real Prometheus, started with --web.enable-remote-write-receiver so the + # seeder can push the same fixed dataset here as to the test target. + query_url: 'http://localhost:9090' + +test_target_config: + # ASAPQuery's query endpoint (its own /api/v1/query and /api/v1/query_range + # implementation). + query_url: 'http://localhost:9091' + +query_time_parameters: + # Placeholder evaluation window. Once seeded data exists, this should be + # narrowed to cover exactly the timestamps the seeder wrote, so results are + # deterministic and independent of wall-clock time. + end_time: '2026-01-01T00:00:00Z' + range_in_seconds: 300 + resolution_in_seconds: 15 + +query_tweaks: + - note: >- + Sketch-backed aggregations (e.g. count-min sketch, HyperLogLog, quantile + sketches) are approximate by design, so results won't bit-for-bit match + real Prometheus. Allow a generous relative tolerance rather than exact + equality for all comparisons in this suite. + adjust_value_tolerance: + fraction: 0.02 + +# Regression test cases seeded from the instant-vs-range divergence catalog in +# issue #589 (and the two issues it was scoped out of, #583 and #584). Each of +# these is a query *pattern* known to have previously produced different +# results between ASAPQuery's instant-query and range-query pipelines, so they +# are exactly the class of bug this harness's instant+range dual-comparison +# (see comparer/comparer.go) exists to catch automatically instead of by hand. +# +# The concrete metric/label names below are placeholders to be replaced with +# whatever names the remote-write seeder actually writes; the query *shapes* +# are what matter and are preserved from the issues. +test_cases: + # From #584 ("Range queries drop self-keyed accumulator expansion (top-k) + # for single-population metrics"): a topk() query grouped by nothing at the + # store level relies on the value accumulator's own get_keys() to expand + # into multiple output series. The instant path + # (collect_results_same_aggregation) does this; the range path's + # single-population branch historically did not, and returned empty instead + # of the top-k series over a range. + - query: 'topk(3, requests_total)' + + # From #583 ("Range query key expansion uses one snapshot instead of + # per-step keys"): grouping by a label whose value set changes partway + # through the queried interval (e.g. an instance/job that starts or stops + # appearing). The range path historically fetched the key set once at the + # range's end and reused that single snapshot for every step, producing + # phantom or missing series relative to per-step ground truth. + - query: 'sum by (job) (up)' + + # From #589 item 4 ("Sliding-window fetch strategy diverges between instant + # and range"): a sliding-window rate/counter query. Instant queries use the + # exact-match store call for Sliding aggregations; range queries force a + # wide fetch plus manual bucket/window reconstruction -- two independent + # implementations of "which buckets fall in this window" for the same + # aggregation type, which can silently disagree. + - query: 'rate(requests_total[5m])' + + # From #589 item 4/5, compounded through an outer aggregation (closer to how + # these sliding-window bugs actually surfaced in practice, per #587): a + # windowed rate summed across series. This also exercises the + # WindowMerger.slide() vs. fresh initialize()+get_merged() divergence noted + # in #589 item 5, since the range path recomputes the merge from scratch at + # every step instead of sliding it incrementally. + - query: 'sum(rate(requests_total[5m]))' diff --git a/promql-compliance/harness/config/config.go b/promql-compliance/harness/config/config.go new file mode 100644 index 0000000..2991d6e --- /dev/null +++ b/promql-compliance/harness/config/config.go @@ -0,0 +1,89 @@ +package config + +import ( + "bytes" + "fmt" + "os" + + "github.com/prometheus/common/model" + "gopkg.in/yaml.v2" +) + +// Config models the main configuration file. +type Config struct { + ReferenceTargetConfig TargetConfig `yaml:"reference_target_config"` + TestTargetConfig TargetConfig `yaml:"test_target_config"` + QueryTweaks []*QueryTweak `yaml:"query_tweaks"` + TestCases []*TestCase `yaml:"test_cases"` + QueryTimeParameters QueryTimeParameters `yaml:"query_time_parameters"` +} + +type QueryTimeParameters struct { + EndTime string `yaml:"end_time"` + RangeInSeconds float64 `yaml:"range_in_seconds"` + ResolutionInSeconds float64 `yaml:"resolution_in_seconds"` +} + +// TargetConfig represents the configuration of a single Prometheus API endpoint. +type TargetConfig struct { + QueryURL string `yaml:"query_url"` + BasicAuthUser string `yaml:"basic_auth_user"` + BasicAuthPass string `yaml:"basic_auth_pass"` + Headers map[string]string `yaml:"headers"` + TSDBPath string `yaml:"tsdb_path"` +} + +// A QueryTweak restricts or modifies a query in certain ways that avoids certain systematic errors and/or later comparison problems. +type QueryTweak struct { + Note string `yaml:"note" json:"note"` + NoBug bool `yaml:"no_bug,omitempty" json:"noBug,omitempty"` + TruncateTimestampsToMS int64 `yaml:"truncate_timestamps_to_ms" json:"truncateTimestampsToMS,omitempty"` + AlignTimestampsToStep bool `yaml:"align_timestamps_to_step" json:"alignTimestampsToStep,omitempty"` + OffsetTimestampsByMS int64 `yaml:"offset_timestamps_by_ms" json:"offsetTimestampsByMS,omitempty"` + DropResultLabels []model.LabelName `yaml:"drop_result_labels" json:"dropResultLabels,omitempty"` + IgnoreFirstStep bool `yaml:"ignore_first_step" json:"ignoreFirstStep,omitempty"` + IgnoreCase bool `yaml:"ignore_case" json:"ignoreCase,omitempty"` + AdjustValueTolerance *AdjustValueTolerance `yaml:"adjust_value_tolerance" json:"adjustValueTolerance,omitempty"` +} + +type AdjustValueTolerance struct { + Fraction *float64 `yaml:"fraction" json:"fraction,omitempty"` + Margin *float64 `yaml:"margin" json:"margin,omitempty"` +} + +// TestCase represents a given query (pattern) to be tested. +type TestCase struct { + Query string `yaml:"query"` + VariantArgs []string `yaml:"variant_args,omitempty"` + SkipComparison bool `yaml:"skip_comparison,omitempty"` + ShouldFail bool `yaml:"should_fail,omitempty"` +} + +// LoadFromFiles parses the given YAML files into a Config. +func LoadFromFiles(filenames []string) (*Config, error) { + var buf bytes.Buffer + for _, f := range filenames { + content, err := os.ReadFile(f) + if err != nil { + return nil, fmt.Errorf("error reading config file %s: %w", f, err) + } + if _, err := buf.Write(content); err != nil { + return nil, fmt.Errorf("error appending config file %s to buffer: %w", f, err) + } + } + cfg, err := Load(buf.Bytes()) + if err != nil { + return nil, fmt.Errorf("error parsing YAML files %s: %w", filenames, err) + } + return cfg, nil +} + +// Load parses the YAML input into a Config. +func Load(content []byte) (*Config, error) { + cfg := &Config{} + err := yaml.UnmarshalStrict(content, cfg) + if err != nil { + return nil, err + } + return cfg, nil +} diff --git a/promql-compliance/harness/go.mod b/promql-compliance/harness/go.mod new file mode 100644 index 0000000..db0d3a1 --- /dev/null +++ b/promql-compliance/harness/go.mod @@ -0,0 +1,30 @@ +module github.com/prometheus/compliance/promql + +go 1.25.0 + +require ( + github.com/cheggaaa/pb/v3 v3.1.7 + github.com/google/go-cmp v0.7.0 + github.com/prometheus/client_golang v1.23.2 + github.com/prometheus/common v0.66.1 + go.uber.org/atomic v1.11.0 + gopkg.in/yaml.v2 v2.4.0 +) + +require ( + github.com/VividCortex/ewma v1.2.0 // indirect + github.com/fatih/color v1.18.0 // indirect + github.com/json-iterator/go v1.1.12 // indirect + github.com/kr/text v0.2.0 // indirect + github.com/mattn/go-colorable v0.1.14 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-runewidth v0.0.16 // indirect + github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect + github.com/modern-go/reflect2 v1.0.2 // indirect + github.com/prometheus/client_model v0.6.2 // indirect + github.com/rivo/uniseg v0.4.7 // indirect + go.yaml.in/yaml/v2 v2.4.2 // indirect + golang.org/x/net v0.54.0 // indirect + golang.org/x/sys v0.44.0 // indirect + google.golang.org/protobuf v1.36.8 // indirect +) diff --git a/promql-compliance/harness/go.sum b/promql-compliance/harness/go.sum new file mode 100644 index 0000000..37ba8b7 --- /dev/null +++ b/promql-compliance/harness/go.sum @@ -0,0 +1,81 @@ +github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow= +github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4= +github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= +github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/cheggaaa/pb/v3 v3.1.7 h1:2FsIW307kt7A/rz/ZI2lvPO+v3wKazzE4K/0LtTWsOI= +github.com/cheggaaa/pb/v3 v3.1.7/go.mod h1:/Ji89zfVPeC/u5j8ukD0MBPHt2bzTYp74lQ7KlgFWTQ= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= +github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA= +github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= +github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= +github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= +github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= +github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= +github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= +github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= +github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= +github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU= +github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o= +github.com/prometheus/client_golang v1.23.2/go.mod h1:Tb1a6LWHB3/SPIzCoaDXI4I8UHKeFTEQ1YCr+0Gyqmg= +github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= +github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= +github.com/prometheus/common v0.66.1 h1:h5E0h5/Y8niHc5DlaLlWLArTQI7tMrsfQjHV+d9ZoGs= +github.com/prometheus/common v0.66.1/go.mod h1:gcaUsgf3KfRSwHY4dIMXLPV0K/Wg1oZ8+SbZk/HH/dA= +github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzMyRg= +github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is= +github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= +github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= +github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= +github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= +github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= +go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= +go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI= +go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= +golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= +golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= +golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= +golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc= +google.golang.org/protobuf v1.36.8/go.mod h1:fuxRtAxBytpl4zzqUh6/eyUujkJdNiuEkXntxiD/uRU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/promql-compliance/harness/output/example-output.html b/promql-compliance/harness/output/example-output.html new file mode 100644 index 0000000..2ed61cb --- /dev/null +++ b/promql-compliance/harness/output/example-output.html @@ -0,0 +1,60 @@ + + + + + +

Passed: {{ numPassed .Results }} / {{ numResults .Results }} ({{ printf "%.2f" (percent (numPassed .Results) (numResults .Results)) }}%)

+ + + + + + + {{ $includePassing := .IncludePassing }} + {{ range .Results }} + {{ if include $includePassing . }} + + + + + + {{ if .UnexpectedFailure }} + + {{ end }} + {{ if .UnexpectedSuccess }} + + {{ end }} + {{ if .Diff }} + + {{ end }} + {{ end }} + {{ end }} +
QueryOutcome
{{ .TestCase.Query }}
{{ if .Success }}PASS{{ else }}FAIL{{ end }}
The query failed to run against the test target: {{ .UnexpectedFailure }}
The query ran successfully against the test target, but should have failed.
{{ .Diff }}
+ + diff --git a/promql-compliance/harness/output/html.go b/promql-compliance/harness/output/html.go new file mode 100644 index 0000000..2d5b9da --- /dev/null +++ b/promql-compliance/harness/output/html.go @@ -0,0 +1,63 @@ +package output + +import ( + "fmt" + "html/template" + "log" + "os" + "path" + + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +var funcMap = map[string]interface{}{ + "include": func(includePassing bool, result *comparer.Result) bool { + return includePassing || !result.Success() + }, + "numResults": func(results []*comparer.Result) int { + return len(results) + }, + "numPassed": func(results []*comparer.Result) int { + num := 0 + for _, r := range results { + if r.Success() { + num++ + } + } + return num + }, + "numFailed": func(results []*comparer.Result) int { + num := 0 + for _, r := range results { + if !r.Success() { + num++ + } + } + return num + }, + "percent": func(part, total int) float64 { + return 100 * float64(part) / float64(total) + }, +} + +// HTML produces HTML output for a number of query results. +func HTML(tplFile string) (Outputter, error) { + t, err := template.New(path.Base(tplFile)).Funcs(funcMap).ParseFiles(tplFile) + if err != nil { + return nil, fmt.Errorf("error parsing template file %q: %w", tplFile, err) + } + + return func(results []*comparer.Result, includePassing bool, tweaks []*config.QueryTweak) { + err := t.Execute(os.Stdout, struct { + Results []*comparer.Result + IncludePassing bool + }{ + Results: results, + IncludePassing: includePassing, + }) + if err != nil { + log.Println("executing template:", err) + } + }, nil +} diff --git a/promql-compliance/harness/output/json.go b/promql-compliance/harness/output/json.go new file mode 100644 index 0000000..fca17b6 --- /dev/null +++ b/promql-compliance/harness/output/json.go @@ -0,0 +1,23 @@ +package output + +import ( + "encoding/json" + "fmt" + + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +// JSON produces JSON-based output for a number of query results. +func JSON(results []*comparer.Result, includePassing bool, tweaks []*config.QueryTweak) { + buf, err := json.Marshal(map[string]interface{}{ + "totalResults": len(results), // Needed because we may exclude passing results. + "results": results, + "includePassing": includePassing, + "queryTweaks": tweaks, + }) + if err != nil { + panic(err) + } + fmt.Print(string(buf)) +} diff --git a/promql-compliance/harness/output/outputter.go b/promql-compliance/harness/output/outputter.go new file mode 100644 index 0000000..aa81895 --- /dev/null +++ b/promql-compliance/harness/output/outputter.go @@ -0,0 +1,9 @@ +package output + +import ( + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +// An Outputter outputs a number of test results. +type Outputter func(results []*comparer.Result, includePassing bool, tweaks []*config.QueryTweak) diff --git a/promql-compliance/harness/output/text.go b/promql-compliance/harness/output/text.go new file mode 100644 index 0000000..8a0d09f --- /dev/null +++ b/promql-compliance/harness/output/text.go @@ -0,0 +1,93 @@ +package output + +import ( + "fmt" + "strings" + + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +// Text produces text-based output for a number of query results. +func Text(results []*comparer.Result, includePassing bool, tweaks []*config.QueryTweak) { + successes := 0 + unsupported := 0 + for _, res := range results { + if res.Success() { + successes++ + if !includePassing { + continue + } + } + if res.Unsupported || res.InstantUnsupported { + unsupported++ + } + + fmt.Println(strings.Repeat("-", 80)) + fmt.Printf("QUERY: %v\n", res.TestCase.Query) + fmt.Printf("START: %v, STOP: %v, STEP: %v\n", res.TestCase.Start, res.TestCase.End, res.TestCase.Resolution) + + // Range and instant results are reported separately, since a query can + // pass one and fail the other (e.g. "PASS: instant, FAIL: range"). + fmt.Printf("RESULT (range): %v\n", rangeResultLabel(res)) + if !res.RangeSuccess() { + if res.UnexpectedFailure != "" { + fmt.Printf("Query failed unexpectedly: %v\n", res.UnexpectedFailure) + } + if res.UnexpectedSuccess { + fmt.Println("Query succeeded, but should have failed.") + } + if res.Diff != "" { + fmt.Println("Query returned different results:") + fmt.Println(res.Diff) + } + } + + fmt.Printf("RESULT (instant): %v\n", instantResultLabel(res)) + if !res.InstantSuccess() { + if res.InstantUnexpectedFailure != "" { + fmt.Printf("Instant query failed unexpectedly: %v\n", res.InstantUnexpectedFailure) + } + if res.InstantUnexpectedSuccess { + fmt.Println("Instant query succeeded, but should have failed.") + } + if res.InstantDiff != "" { + fmt.Println("Instant query returned different results:") + fmt.Println(res.InstantDiff) + } + } + } + + fmt.Println(strings.Repeat("=", 80)) + fmt.Println("General query tweaks:") + if len(tweaks) == 0 { + fmt.Println("None.") + } + for _, t := range tweaks { + fmt.Println("* ", t.Note) + } + fmt.Println(strings.Repeat("=", 80)) + fmt.Printf("Total: %d / %d (%.2f%%) passed, %d unsupported\n", successes, len(results), 100*float64(successes)/float64(len(results)), unsupported) +} + +func rangeResultLabel(res *comparer.Result) string { + switch { + case res.RangeSuccess(): + return "PASSED" + case res.Unsupported: + return "UNSUPPORTED" + default: + return "FAILED" + } +} + +func instantResultLabel(res *comparer.Result) string { + switch { + case res.InstantSuccess(): + return "PASSED" + case res.InstantUnsupported: + return "UNSUPPORTED" + default: + return "FAILED" + } +} diff --git a/promql-compliance/harness/output/tsv.go b/promql-compliance/harness/output/tsv.go new file mode 100644 index 0000000..233da89 --- /dev/null +++ b/promql-compliance/harness/output/tsv.go @@ -0,0 +1,40 @@ +package output + +import ( + "fmt" + + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +// TSV produces tab separated values output for a number of query results. +func TSV(results []*comparer.Result, passing bool, tweaks []*config.QueryTweak) { + successes := 0 + unsupported := 0 + + fmt.Println("QUERY\tSTART\tSTOP\tSTEP\tRESULT") + + for _, res := range results { + if res.Success() { + successes++ + } + if res.Unsupported { + unsupported++ + } + + fmt.Printf("%v\t%v\t%v\t%v\t", res.TestCase.Query, res.TestCase.Start, res.TestCase.End, res.TestCase.Resolution) + if res.Success() { + fmt.Println("PASSED") + } else if res.Unsupported { + fmt.Println("UNSUPPORTED") + } else { + fmt.Println("FAILED") + } + } + totalTestCases := len(results) + totalFailed := totalTestCases - successes - unsupported + fmt.Printf("\n\t\tPASSED\t%v\t%.4f\n", successes, float64(successes)/float64(totalTestCases)) + fmt.Printf("\t\tFAILED\t%v\t%.4f\n", totalFailed, float64(totalFailed)/float64(totalTestCases)) + fmt.Printf("\t\tUNSUPPORTED\t%v\t%.4f\n", unsupported, float64(unsupported)/float64(totalTestCases)) + fmt.Printf("\t\tTOTAL\t%v\t%.4f\n", totalTestCases, float64(1)) +} diff --git a/promql-compliance/harness/prometheus-test-data.yml b/promql-compliance/harness/prometheus-test-data.yml new file mode 100644 index 0000000..2f19bff --- /dev/null +++ b/promql-compliance/harness/prometheus-test-data.yml @@ -0,0 +1,10 @@ +global: + scrape_interval: 5s + +scrape_configs: +- job_name: 'demo' + static_configs: + - targets: + - 'demo.promlabs.com:10000' + - 'demo.promlabs.com:10001' + - 'demo.promlabs.com:10002' diff --git a/promql-compliance/harness/promql-test-queries.yml b/promql-compliance/harness/promql-test-queries.yml new file mode 100644 index 0000000..be3053d --- /dev/null +++ b/promql-compliance/harness/promql-test-queries.yml @@ -0,0 +1,243 @@ +# This set of example queries expects data from the following Prometheus configuration file to have +# been ingested into both a vanilla Prometheus server and the third-party system for several hours, +# so that the tester can compare query results from both systems over a range of time: +# +# ----------- prometheus.yml ----------- +# global: +# scrape_interval: 5s +# +# scrape_configs: +# - job_name: 'demo' +# static_configs: +# - targets: +# - 'demo.promlabs.com:10000' +# - 'demo.promlabs.com:10001' +# - 'demo.promlabs.com:10002' +# -------------------------------------- +# +# You will have to add a "remote_write" section to this configuration to ingest data into the third-party +# system, or in the case of Thanos, add a Thanos sidecar to the Prometheus running with this configuration. +# See https://promlabs.com/blog/2020/08/06/comparing-promql-correctness-across-vendors#first-comparisons for +# more background information. +# +# The demo service instances expose a predictable set of synthetic metrics and are hosted on a best-effort +# basis by PromLabs. If you want to run your own demo service instances instead, you can do so via: +# +# docker run -p 10000:10000 julius/prometheus-demo-service:latest -listen-address=:10000 +# docker run -p 10001:10001 julius/prometheus-demo-service:latest -listen-address=:10001 +# docker run -p 10002:10002 julius/prometheus-demo-service:latest -listen-address=:10002 +# +# You will then also need to replace the host "demo.promlabs.com" in the test queries below with whatever +# host you are running the instances on. +test_cases: + # Scalar literals. + - query: '42' + - query: '1.234' + - query: '.123' + - query: '1.23e-3' + - query: '0x3d' + - query: 'Inf' + - query: '+Inf' + - query: '-Inf' + - query: 'NaN' + + # Vector selectors. + # TODO: Add tests for staleness support. + - query: 'demo_memory_usage_bytes' + - query: '{__name__="demo_memory_usage_bytes"}' + - query: 'demo_memory_usage_bytes{type="free"}' + - query: 'demo_memory_usage_bytes{type!="free"}' + - query: 'demo_memory_usage_bytes{instance=~"demo.promlabs.com:.*"}' + - query: 'demo_memory_usage_bytes{instance=~"host"}' + - query: 'demo_memory_usage_bytes{instance!~".*:10000"}' + - query: 'demo_memory_usage_bytes{type="free", instance!="demo.promlabs.com:10000"}' + - query: '{type="free", instance!="demo.promlabs.com:10000"}' + - query: '{__name__=~".*"}' + should_fail: true + - query: "nonexistent_metric_name" + - query: 'demo_memory_usage_bytes offset {{.offset}}' + variant_args: ['offset'] + - query: 'demo_memory_usage_bytes offset -{{.offset}}' + variant_args: ['offset'] + # Test staleness handling. + - query: demo_intermittent_metric + + # Aggregation operators. + - query: '{{.simpleAggrOp}}(demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}}(nonexistent_metric_name)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} by() (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} by(instance) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} by(instance, type) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} by(nonexistent) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} without() (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} without(instance) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} without(instance, type) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.simpleAggrOp}} without(nonexistent) (demo_memory_usage_bytes)' + variant_args: ['simpleAggrOp'] + - query: '{{.topBottomOp}} (3, demo_memory_usage_bytes)' + variant_args: ['topBottomOp'] + - query: '{{.topBottomOp}} by(instance) (2, demo_memory_usage_bytes)' + variant_args: ['topBottomOp'] + - query: '{{.topBottomOp}} without(instance) (2, demo_memory_usage_bytes)' + variant_args: ['topBottomOp'] + - query: '{{.topBottomOp}} without() (2, demo_memory_usage_bytes)' + variant_args: ['topBottomOp'] + - query: 'quantile({{.quantile}}, demo_memory_usage_bytes)' + variant_args: ['quantile'] + - query: 'avg(max by(type) (demo_memory_usage_bytes))' + + # Binary operators. + - query: '1 * 2 + 4 / 6 - 10 % 2 ^ 2' + - query: 'demo_num_cpus + (1 {{.compBinOp}} bool 2)' + variant_args: ['compBinOp'] + - query: 'demo_memory_usage_bytes {{.binOp}} 1.2345' + variant_args: ['binOp'] + - query: 'demo_memory_usage_bytes {{.compBinOp}} bool 1.2345' + variant_args: ['compBinOp'] + - query: '1.2345 {{.compBinOp}} bool demo_memory_usage_bytes' + variant_args: ['compBinOp'] + - query: '0.12345 {{.binOp}} demo_memory_usage_bytes' + variant_args: ['binOp'] + - query: '(1 * 2 + 4 / 6 - (10%7)^2) {{.binOp}} demo_memory_usage_bytes' + variant_args: ['binOp'] + - query: 'demo_memory_usage_bytes {{.binOp}} (1 * 2 + 4 / 6 - 10)' + variant_args: ['binOp'] + # Check that vector-scalar binops set output timestamps correctly. + - query: 'timestamp(demo_memory_usage_bytes * 1)' + # Check that unary minus sets timestamps correctly. + # TODO: Check this more systematically for every node type? + - query: 'timestamp(-demo_memory_usage_bytes)' + - query: 'demo_memory_usage_bytes {{.binOp}} on(instance, job, type) demo_memory_usage_bytes' + variant_args: ['binOp'] + - query: 'sum by(instance, type) (demo_memory_usage_bytes) {{.binOp}} on(instance, type) group_left(job) demo_memory_usage_bytes' + variant_args: ['binOp'] + - query: 'demo_memory_usage_bytes {{.compBinOp}} bool on(instance, job, type) demo_memory_usage_bytes' + variant_args: ['compBinOp'] + # Check that __name__ is always dropped, even if it's part of the matching labels. + - query: 'demo_memory_usage_bytes / on(instance, job, type, __name__) demo_memory_usage_bytes' + - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) demo_memory_usage_bytes' + - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) group_left demo_memory_usage_bytes' + - query: 'sum without(job) (demo_memory_usage_bytes) / on(instance, type) group_left(job) demo_memory_usage_bytes' + - query: 'demo_memory_usage_bytes / on(instance, job) group_left demo_num_cpus' + - query: 'demo_memory_usage_bytes / on(instance, type, job, non_existent) demo_memory_usage_bytes' + # TODO: Add non-explicit many-to-one / one-to-many that errors. + # TODO: Add many-to-many match that errors. + + # NaN/Inf/-Inf support. + - query: 'demo_num_cpus * Inf' + - query: 'demo_num_cpus * -Inf' + - query: 'demo_num_cpus * NaN' + + # Unary expressions. + - query: 'demo_memory_usage_bytes + -(1)' + - query: '-demo_memory_usage_bytes' + # Check precedence. + - query: -1 ^ 2 + + # Binops involving non-const scalars. + - query: '1 {{.arithBinOp}} time()' + variant_args: ['arithBinOp'] + - query: 'time() {{.arithBinOp}} 1' + variant_args: ['arithBinOp'] + - query: 'time() {{.compBinOp}} bool 1' + variant_args: ['compBinOp'] + - query: '1 {{.compBinOp}} bool time()' + variant_args: ['compBinOp'] + - query: 'time() {{.arithBinOp}} time()' + variant_args: ['arithBinOp'] + - query: 'time() {{.compBinOp}} bool time()' + variant_args: ['compBinOp'] + - query: 'time() {{.binOp}} demo_memory_usage_bytes' + variant_args: ['binOp'] + - query: 'demo_memory_usage_bytes {{.binOp}} time()' + variant_args: ['binOp'] + + # Functions. + - query: '{{.simpleTimeAggrOp}}_over_time(demo_memory_usage_bytes[{{.range}}])' + variant_args: ['simpleTimeAggrOp', 'range'] + - query: 'quantile_over_time({{.quantile}}, demo_memory_usage_bytes[{{.range}}])' + variant_args: ['quantile', 'range'] + - query: 'timestamp(demo_num_cpus)' + - query: 'timestamp(timestamp(demo_num_cpus))' + - query: '{{.simpleMathFunc}}(demo_memory_usage_bytes)' + variant_args: ['simpleMathFunc'] + - query: '{{.simpleMathFunc}}(-demo_memory_usage_bytes)' + variant_args: ['simpleMathFunc'] + - query: '{{.extrapolatedRateFunc}}(nonexistent_metric[5m])' + variant_args: ['extrapolatedRateFunc'] + - query: '{{.extrapolatedRateFunc}}(demo_cpu_usage_seconds_total[{{.range}}])' + variant_args: ['extrapolatedRateFunc', 'range'] + - query: 'deriv(demo_disk_usage_bytes[{{.range}}])' + variant_args: ['range'] + - query: 'predict_linear(demo_disk_usage_bytes[{{.range}}], 600)' + variant_args: ['range'] + - query: 'time()' + # label_replace does a full-string match and replace. + - query: 'label_replace(demo_num_cpus, "job", "destination-value-$1", "instance", "demo.promlabs.com:(.*)")' + # label_replace does not do a sub-string match. + - query: 'label_replace(demo_num_cpus, "job", "destination-value-$1", "instance", "host:(.*)")' + # label_replace works with multiple capture groups. + - query: 'label_replace(demo_num_cpus, "job", "$1-$2", "instance", "local(.*):(.*)")' + # label_replace does not overwrite the destination label if the source label does not exist. + - query: 'label_replace(demo_num_cpus, "job", "value-$1", "nonexistent-src", "source-value-(.*)")' + # label_replace overwrites the destination label if the source label is empty, but matched. + - query: 'label_replace(demo_num_cpus, "job", "value-$1", "nonexistent-src", "(.*)")' + # label_replace does not overwrite the destination label if the source label is not matched. + - query: 'label_replace(demo_num_cpus, "job", "value-$1", "instance", "non-matching-regex")' + # label_replace drops labels that are set to empty values. + - query: 'label_replace(demo_num_cpus, "job", "", "dst", ".*")' + # label_replace fails when the regex is invalid. + - query: 'label_replace(demo_num_cpus, "job", "value-$1", "src", "(.*")' + should_fail: true + # label_replace fails when the destination label name is not a valid Prometheus label name. + - query: 'label_replace(demo_num_cpus, "~invalid", "", "src", "(.*)")' + should_fail: true + # label_replace fails when there would be duplicated identical output label sets. + - query: 'label_replace(demo_num_cpus, "instance", "", "", "")' + should_fail: true + - query: 'label_join(demo_num_cpus, "new_label", "-", "instance", "job")' + - query: 'label_join(demo_num_cpus, "job", "-", "instance", "job")' + - query: 'label_join(demo_num_cpus, "job", "-", "instance")' + - query: 'label_join(demo_num_cpus, "~invalid", "-", "instance")' + should_fail: true + - query: '{{.dateFunc}}()' + variant_args: ['dateFunc'] + - query: '{{.dateFunc}}(demo_batch_last_success_timestamp_seconds offset {{.offset}})' + variant_args: ['dateFunc', 'offset'] + - query: '{{.instantRateFunc}}(demo_cpu_usage_seconds_total[{{.range}}])' + variant_args: ['instantRateFunc', 'range'] + - query: '{{.clampFunc}}(demo_memory_usage_bytes, 2)' + variant_args: ['clampFunc'] + - query: 'clamp(demo_memory_usage_bytes, 0, 1)' + - query: 'clamp(demo_memory_usage_bytes, 0, 1000000000000)' + - query: 'clamp(demo_memory_usage_bytes, 1000000000000, 0)' + - query: 'clamp(demo_memory_usage_bytes, 1000000000000, 1000000000000)' + - query: 'resets(demo_cpu_usage_seconds_total[{{.range}}])' + variant_args: ['range'] + - query: 'changes(demo_batch_last_success_timestamp_seconds[{{.range}}])' + variant_args: ['range'] + - query: 'vector(1.23)' + - query: 'vector(time())' + - query: 'histogram_quantile({{.quantile}}, rate(demo_api_request_duration_seconds_bucket[1m]))' + variant_args: ['quantile'] + - query: 'histogram_quantile(0.9, nonexistent_metric)' + - # Missing "le" label. + query: 'histogram_quantile(0.9, demo_memory_usage_bytes)' + - # Missing "le" label only in some series of the same grouping. + query: 'histogram_quantile(0.9, {__name__=~"demo_api_request_duration_seconds_.+"})' + - query: 'count_values("value", demo_api_request_duration_seconds_bucket)' + - query: 'absent(demo_memory_usage_bytes)' + - query: 'absent(nonexistent_metric_name)' + + # Subqueries. + - query: 'max_over_time((time() - max(demo_batch_last_success_timestamp_seconds) < 1000)[5m:10s] offset 5m)' + - query: 'avg_over_time(rate(demo_cpu_usage_seconds_total[1m])[2m:10s])' diff --git a/promql-compliance/harness/testcases/expand.go b/promql-compliance/harness/testcases/expand.go new file mode 100644 index 0000000..549d2d3 --- /dev/null +++ b/promql-compliance/harness/testcases/expand.go @@ -0,0 +1,154 @@ +// Some of this code has been taken and adapted from InfluxData: +// https://github.com/influxdata/influxdb/blob/26fdb792ffd74f773c253df5d9bebf64ef2b3214/query/promql/internal/promqltests/tests.go +// +// The original copyright notice and license of that code is reproduced here: +// +// ------------------------------------------------------------------------------- +// +// MIT License + +// Copyright (c) 2018 InfluxData + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. +// +// ------------------------------------------------------------------------------- + +package testcases + +import ( + "bytes" + "fmt" + "text/template" + "time" + + "github.com/prometheus/compliance/promql/comparer" + "github.com/prometheus/compliance/promql/config" +) + +var testVariantArgs = map[string][]string{ + "range": {"1s", "15s", "1m", "5m", "15m", "1h"}, + "offset": {"1m", "5m", "10m"}, + "simpleAggrOp": {"sum", "avg", "max", "min", "count", "stddev", "stdvar"}, + "simpleTimeAggrOp": {"sum", "avg", "max", "min", "count", "stddev", "stdvar", "absent", "last"}, + "topBottomOp": {"topk", "bottomk"}, + "quantile": { + "-0.5", + "0.1", + "0.5", + "0.75", + "0.95", + "0.90", + "0.99", + "1", + "1.5", + }, + "arithBinOp": {"+", "-", "*", "/", "%", "^"}, + "compBinOp": {"==", "!=", "<", ">", "<=", ">="}, + "binOp": {"+", "-", "*", "/", "%", "^", "==", "!=", "<", ">", "<=", ">="}, + "simpleMathFunc": {"abs", "ceil", "floor", "exp", "sqrt", "ln", "log2", "log10", "round"}, + "extrapolatedRateFunc": {"delta", "rate", "increase"}, + "clampFunc": {"clamp_min", "clamp_max"}, + "instantRateFunc": {"idelta", "irate"}, + "dateFunc": {"day_of_month", "day_of_week", "days_in_month", "hour", "minute", "month", "year"}, + "smoothingFactor": {"0.1", "0.5", "0.8"}, + "trendFactor": {"0.1", "0.5", "0.8"}, +} + +// tprintf replaces template arguments in a string with their instantiations from the provided map. +func tprintf(tmpl string, data map[string]string) string { + t := template.Must(template.New("Query").Parse(tmpl)) + buf := &bytes.Buffer{} + if err := t.Execute(buf, data); err != nil { + panic(err) + } + return buf.String() +} + +// getVariants returns every possible combinations (variants) of a template query. +func getVariants(query string, remainingVariantArgs []string, args map[string]string) []string { + // Either this Query had no variants defined to begin with or they have + // been fully filled out in "args" from recursive parent calls. + if len(remainingVariantArgs) == 0 { + return []string{tprintf(query, args)} + } + + // Recursively iterate through the values for each variant arg dimension, + // selecting one dimension (arg) to vary per recursion level and let the + // other recursion levels iterate through the remaining dimensions until + // all args are defined. + var queries []string + vArg := remainingVariantArgs[0] + filteredVArgs := make([]string, 0, len(remainingVariantArgs)-1) + for _, va := range remainingVariantArgs { + if va != vArg { + filteredVArgs = append(filteredVArgs, va) + } + } + + vals := testVariantArgs[vArg] + if len(vals) == 0 { + panic(fmt.Errorf("unknown variant arg %q", vArg)) + } + for _, variantVal := range vals { + args[vArg] = variantVal + qs := getVariants(query, filteredVArgs, args) + queries = append(queries, qs...) + } + return queries +} + +func applyQueryTweaks(tc *comparer.TestCase, tweaks []*config.QueryTweak) *comparer.TestCase { + resTC := *tc + for _, t := range tweaks { + if d := time.Duration(t.TruncateTimestampsToMS) * time.Millisecond; d != 0 { + resTC.Start = resTC.Start.Truncate(d) + resTC.End = resTC.End.Truncate(d) + } + if t.AlignTimestampsToStep { + resTC.Start = resTC.Start.Truncate(resTC.Resolution) + resTC.End = resTC.End.Truncate(resTC.Resolution) + } + if d := time.Duration(t.OffsetTimestampsByMS) * time.Millisecond; d != 0 { + resTC.Start = resTC.Start.Add(d) + resTC.End = resTC.End.Add(d) + } + } + return &resTC +} + +// ExpandTestCases returns the fully expanded test cases for a given set of templates test cases. +func ExpandTestCases(cases []*config.TestCase, tweaks []*config.QueryTweak, start, end time.Time, resolution time.Duration) []*comparer.TestCase { + tcs := make([]*comparer.TestCase, 0) + for _, q := range cases { + vs := getVariants(q.Query, q.VariantArgs, make(map[string]string)) + for _, v := range vs { + tc := &comparer.TestCase{ + Query: v, + SkipComparison: q.SkipComparison, + ShouldFail: q.ShouldFail, + Start: start, + End: end, + Resolution: resolution, + } + + tcs = append(tcs, applyQueryTweaks(tc, tweaks)) + } + } + return tcs +} From 549180ed8b1460a0f65f8d6bc1a4cb45f2a6cc83 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 26 Aug 2026 00:27:02 -0400 Subject: [PATCH 4/5] test(query-engine): add boundary/validation tests for range queries (#590) validate_range_query_params had no dedicated tests. Adds direct unit tests for its three error branches (start>=end, step==0, step not a multiple of the tumbling window) plus the happy path, in an inline test module next to the function (it's private and validate_range_query_params's error string is discarded before reaching any public caller, so the exact-string assertions can't be made from crate::tests). Also adds end-to-end coverage via handle_range_query_promql confirming each bad-param case is actually rejected in practice, including the start == end boundary specifically, and a new test that runs the same keys-but-no-value orphan-group scenario through both the instant and range entry points and diffs their skip/error behavior explicitly, so a future regression that splits their behavior fails here instead of only in one of the two existing per-path tests. --- .../src/engines/simple_engine/mod.rs | 155 ++++++++++ asap-query-engine/src/tests/mod.rs | 1 + .../src/tests/range_query_validation_tests.rs | 270 ++++++++++++++++++ 3 files changed, 426 insertions(+) create mode 100644 asap-query-engine/src/tests/range_query_validation_tests.rs diff --git a/asap-query-engine/src/engines/simple_engine/mod.rs b/asap-query-engine/src/engines/simple_engine/mod.rs index 6dee86a..616535e 100644 --- a/asap-query-engine/src/engines/simple_engine/mod.rs +++ b/asap-query-engine/src/engines/simple_engine/mod.rs @@ -3421,3 +3421,158 @@ mod sketch_query_tests { // assert!(result.is_none()); // } } + +/// Direct unit tests for `validate_range_query_params`'s three error +/// branches plus its happy path (issue #590, "Boundary/validation tests"). +/// +/// This function is private and has no public wrapper, so it can only be +/// called directly from a test module that is a descendant of this one in +/// the module tree (Rust privacy is module-tree scoped) -- hence this module +/// lives here rather than in `crate::tests`, alongside `range_query_tests` +/// and `merge_accumulators_regression_tests_596` above, which do the same +/// for their own private targets. +/// +/// `crate::tests::range_query_validation_tests` covers the complementary +/// end-to-end angle: proving through the public `handle_range_query_promql` +/// entry point that each of these rejections is actually enforced in +/// practice (that file can't assert the exact error *string*, since +/// `finish_range_context` in `promql.rs` discards it -- `Result<(), String>` +/// is folded into `Option` via `.ok()?` -- so the string-level assertions +/// belong here instead). +#[cfg(test)] +mod validate_range_query_params_tests { + use crate::data_model::{CleanupPolicy, InferenceConfig, QueryLanguage, StreamingConfig}; + use crate::engines::simple_engine::SimpleEngine; + use crate::stores::{Store, TimestampedBucketsMap}; + use crate::AggregateCore; + use std::collections::HashMap; + use std::sync::Arc; + + /// `validate_range_query_params` doesn't touch `self` or the store at + /// all -- it's a pure function of its four arguments -- so this store is + /// never actually called; it only exists because the method is `&self`. + struct NoOpStore; + + impl Store for NoOpStore { + fn insert_precomputed_output( + &self, + _: crate::data_model::PrecomputedOutput, + _: Box, + ) -> Result<(), Box> { + panic!("NoOpStore should not be called by validate_range_query_params tests"); + } + fn insert_precomputed_output_batch( + &self, + _: Vec<(crate::data_model::PrecomputedOutput, Box)>, + ) -> Result<(), Box> { + panic!("NoOpStore should not be called by validate_range_query_params tests"); + } + fn query_precomputed_output( + &self, + _: &str, + _: u64, + _: u64, + _: u64, + ) -> Result> { + panic!("NoOpStore should not be called by validate_range_query_params tests"); + } + fn query_precomputed_output_exact( + &self, + _: &str, + _: u64, + _: u64, + _: u64, + ) -> Result> { + panic!("NoOpStore should not be called by validate_range_query_params tests"); + } + fn query_precomputed_output_exact_batch( + &self, + _: &str, + _: u64, + _: &[crate::stores::TimestampRange], + ) -> Result> { + panic!("NoOpStore should not be called by validate_range_query_params tests"); + } + fn get_earliest_timestamp_per_aggregation_id( + &self, + ) -> Result, Box> { + Ok(HashMap::new()) + } + fn close(&self) -> Result<(), Box> { + Ok(()) + } + } + + fn test_engine() -> SimpleEngine { + let inference_config = + InferenceConfig::new(QueryLanguage::promql, CleanupPolicy::NoCleanup); + let streaming_config = Arc::new(StreamingConfig::default()); + SimpleEngine::new( + Arc::new(NoOpStore), + inference_config, + streaming_config, + 15, + QueryLanguage::promql, + ) + } + + #[test] + fn accepts_valid_params() { + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(0, 1000, 1000, 1000), + Ok(()) + ); + } + + #[test] + fn rejects_start_after_end() { + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(1000, 0, 1000, 1000), + Err("start must be before end".to_string()) + ); + } + + #[test] + fn rejects_start_equal_to_end() { + // The boundary case specifically called out by #590: `start >= end` + // must reject `start == end` too, not just `start > end`. + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(1000, 1000, 1000, 1000), + Err("start must be before end".to_string()) + ); + } + + #[test] + fn rejects_zero_step() { + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(0, 1000, 0, 1000), + Err("step must be positive".to_string()) + ); + } + + #[test] + fn rejects_step_not_a_multiple_of_tumbling_window() { + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(0, 1000, 1500, 1000), + Err("step (1500 ms) must be a multiple of tumbling window size (1000 ms)".to_string()) + ); + } + + #[test] + fn start_after_end_is_checked_before_step_checks() { + // Documents the actual branch order (start>=end first, then + // step==0, then step-multiple) so a future reordering that changes + // which error wins on a doubly-invalid call is a deliberate, + // reviewed choice rather than an accident. + let engine = test_engine(); + assert_eq!( + engine.validate_range_query_params(1000, 0, 0, 1000), + Err("start must be before end".to_string()) + ); + } +} diff --git a/asap-query-engine/src/tests/mod.rs b/asap-query-engine/src/tests/mod.rs index b4a5bbe..3ed0826 100644 --- a/asap-query-engine/src/tests/mod.rs +++ b/asap-query-engine/src/tests/mod.rs @@ -11,6 +11,7 @@ pub mod native_range_query_tests; pub mod prometheus_forwarding_tests; pub mod query_equivalence_tests; pub mod range_query_arithmetic_tests; +pub mod range_query_validation_tests; pub mod sql_pattern_matching_tests; pub mod store_correctness_tests; pub mod structural_matching_tests; diff --git a/asap-query-engine/src/tests/range_query_validation_tests.rs b/asap-query-engine/src/tests/range_query_validation_tests.rs new file mode 100644 index 0000000..bb2dc8c --- /dev/null +++ b/asap-query-engine/src/tests/range_query_validation_tests.rs @@ -0,0 +1,270 @@ +//! Boundary/validation tests for the range-query entry point (issue #590, +//! "Boundary/validation tests" section). +//! +//! `validate_range_query_params` (asap-query-engine/src/engines/simple_engine/mod.rs) +//! had NO dedicated unit tests before this file -- confirmed via +//! `grep -rn "validate_range_query_params" asap-query-engine/src/`, which +//! turns up only the function's own definition and its single call site in +//! `finish_range_context` (`engines/simple_engine/promql.rs`). +//! +//! That call site is also why these tests can't assert the exact error +//! *string* through the public API: `finish_range_context` folds +//! `validate_range_query_params`'s `Result<(), String>` into an `Option` via +//! `.map_err(|e| { warn!(...); e }).ok()?` -- the message only ever reaches a +//! `warn!` log, never the caller. And `handle_range_query_promql` itself +//! returns `Option<(KeyByLabelNames, QueryResult)>`, so `None` is *all* a +//! caller (including this test file) can ever observe for a validation +//! failure, whether it's "start >= end", "step == 0", or anything else. +//! `validate_range_query_params` is also a private method with no public +//! callers outside its own module, so it can't be invoked directly from +//! `crate::tests` either (Rust's privacy is module-tree scoped, and +//! `crate::tests` isn't a descendant of `engines::simple_engine`). +//! +//! So the exact-error-string assertions live directly next to the function, +//! in `validate_range_query_params_tests` at the bottom of +//! `engines/simple_engine/mod.rs` (a `#[cfg(test)]` module in the same file, +//! which *can* see private items). This file covers the other half: proving +//! end-to-end, through the real public entry point, that each bad-param +//! case is actually rejected (returns `None`) rather than silently +//! misbehaving -- including the `start == end` boundary specifically, which +//! is easy to get wrong (e.g. by treating it as a degenerate single-instant +//! range instead of rejecting it). + +#[cfg(test)] +mod tests { + use crate::data_model::{AggregationType, KeyByLabelValues}; + use crate::precompute_operators::sum_accumulator::SumAccumulator; + use crate::precompute_operators::{CountMinSketchAccumulator, DeltaSetAggregatorAccumulator}; + use crate::tests::test_utilities::engine_factories::{ + create_engine_dual_input, create_engine_single_pop, + }; + use crate::AggregateCore; + + /// Matches `engine_factories`' fixed insert timestamp (1_000_000 ms) and + /// `native_binary_instant_tests.rs`'s `QUERY_TIME`, so the same engine + /// can be queried both as an instant query (at this time) and as a + /// single-bucket range query ending at this time. + const QUERY_TIME_S: f64 = 1000.0; + + /// `create_engine_single_pop`/`create_engine_dual_input` configure a + /// Tumbling aggregation with `window_size_ms == slide_interval_ms == + /// 1000`, so `bucket_step_ms` (the `tumbling_window_ms` fed into + /// `validate_range_query_params`) is 1000ms for every engine built here. + const TUMBLING_WINDOW_MS: u64 = 1000; + + fn single_pop_engine() -> crate::engines::simple_engine::SimpleEngine { + create_engine_single_pop( + "requests_total", + AggregationType::Sum, + vec!["host"], + vec![( + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(42.0)) as Box, + )], + "sum(requests_total) by (host)", + ) + } + + // ---- Happy path: sanity baseline showing the recipe below IS capable + // ---- of succeeding, so the None results in the tests after it are + // ---- actually attributable to the validation branch under test, not to + // ---- some unrelated setup mistake. + #[tokio::test(flavor = "multi_thread")] + async fn valid_params_are_accepted() { + let engine = single_pop_engine(); + let query = "sum(requests_total) by (host)"; + let result = engine.handle_range_query_promql(query.to_string(), 999.0, QUERY_TIME_S, 1.0); + assert!( + result.is_some(), + "start=999.0 < end=1000.0, step=1000ms is a multiple of the \ + 1000ms tumbling window -- this must succeed" + ); + } + + // ---- start >= end ---- + + #[tokio::test(flavor = "multi_thread")] + async fn start_after_end_is_rejected() { + let engine = single_pop_engine(); + let query = "sum(requests_total) by (host)"; + let result = engine.handle_range_query_promql(query.to_string(), QUERY_TIME_S, 999.0, 1.0); + assert!( + result.is_none(), + "start > end must be rejected by validate_range_query_params \ + ('start must be before end'), surfaced as None from \ + handle_range_query_promql" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn start_equal_to_end_is_rejected() { + // Issue #590 boundary case: start == end must be rejected outright + // (validate_range_query_params's `start >= end` check), NOT silently + // treated as a degenerate single-instant/single-bucket range. This + // pins that the end-to-end behavior actually matches the validator's + // stated contract. + let engine = single_pop_engine(); + let query = "sum(requests_total) by (host)"; + let result = + engine.handle_range_query_promql(query.to_string(), QUERY_TIME_S, QUERY_TIME_S, 1.0); + assert!( + result.is_none(), + "start == end must be rejected, not treated as a valid \ + 1-bucket range -- if this fails, validate_range_query_params's \ + `start >= end` check is not actually being enforced end-to-end \ + (see #590)" + ); + } + + // ---- step == 0 ---- + + #[tokio::test(flavor = "multi_thread")] + async fn zero_step_is_rejected() { + let engine = single_pop_engine(); + let query = "sum(requests_total) by (host)"; + let result = engine.handle_range_query_promql(query.to_string(), 999.0, QUERY_TIME_S, 0.0); + assert!( + result.is_none(), + "step == 0 must be rejected by validate_range_query_params \ + ('step must be positive')" + ); + } + + // ---- step not a multiple of the tumbling window size ---- + + #[tokio::test(flavor = "multi_thread")] + async fn step_not_a_multiple_of_tumbling_window_is_rejected() { + let engine = single_pop_engine(); + let query = "sum(requests_total) by (host)"; + // 1500ms is not a multiple of the engine's 1000ms tumbling window. + let result = engine.handle_range_query_promql(query.to_string(), 999.0, QUERY_TIME_S, 1.5); + assert!( + result.is_none(), + "step (1500ms) not a multiple of tumbling_window_ms (1000ms) \ + must be rejected" + ); + } + + #[test] + fn tumbling_window_constant_matches_engine_factories_assumption() { + // Guards the premise the tests above rely on: if engine_factories + // ever changes its window/slide configuration, TUMBLING_WINDOW_MS + // here (and the 1.5s "not a multiple" step above) would silently + // stop testing what they claim to. + assert_eq!(TUMBLING_WINDOW_MS, 1000); + } + + // ---- #590/#582: instant vs. range parity on a missing-value-mid-merge + // ---- group (keys resolve a group, but that group has no value/CMS data + // ---- anywhere). native_range_query_tests.rs's + // ---- `range_query_dual_population_group_with_no_value_data_is_skipped_not_fatal` + // ---- and native_binary_instant_tests.rs's + // ---- `instant_query_dual_population_group_with_no_value_data_is_skipped_not_fatal` + // ---- already independently pin "skipped, not fatal" for range and + // ---- instant respectively (range fixed under #583, instant brought in + // ---- line under #597 per that test's own comment). Both already pass + // ---- as of this writing. This test doesn't re-derive that from + // ---- scratch -- it runs the *same* orphan-group scenario through both + // ---- entry points on data shaped so both are queryable, and diffs + // ---- their skip/error behavior explicitly, so a future regression that + // ---- reintroduces divergence between the two paths (one skips, the + // ---- other goes fatal) fails loudly right here instead of only in + // ---- whichever of the two dedicated tests happens to catch it. + #[tokio::test(flavor = "multi_thread")] + async fn instant_and_range_agree_on_skipping_value_less_orphan_group() { + let build_orphan_engine = || { + let cms_normal = CountMinSketchAccumulator::new(2, 3); + + let mut keys_normal = DeltaSetAggregatorAccumulator::new(); + keys_normal.add_key(KeyByLabelValues { + labels: vec![ + "normal".to_string(), + "host-a".to_string(), + "evt-1".to_string(), + ], + }); + let mut keys_orphan = DeltaSetAggregatorAccumulator::new(); + keys_orphan.add_key(KeyByLabelValues { + labels: vec![ + "orphan".to_string(), + "host-z".to_string(), + "evt-1".to_string(), + ], + }); + + create_engine_dual_input( + "event_frequency", + AggregationType::CountMinSketch, + AggregationType::DeltaSetAggregator, + vec!["region"], + vec!["host", "event"], + vec![( + Some(vec!["normal".to_string()]), + Box::new(cms_normal) as Box, + )], + // Deliberately NO value data for region=orphan, at any timestamp. + vec![ + ( + Some(vec!["normal".to_string()]), + Box::new(keys_normal) as Box, + ), + ( + Some(vec!["orphan".to_string()]), + Box::new(keys_orphan) as Box, + ), + ], + "count(event_frequency) by (region, host, event)", + ) + }; + + let query = "count(event_frequency) by (region, host, event)"; + + let instant_engine = build_orphan_engine(); + let instant_result = instant_engine.handle_query_promql(query.to_string(), QUERY_TIME_S); + + let range_engine = build_orphan_engine(); + // Single-bucket range ending at the same instant, so both paths hit + // the exact same underlying (Tumbling) window. + let range_result = + range_engine.handle_range_query_promql(query.to_string(), 999.0, QUERY_TIME_S, 1.0); + + let instant_skipped_not_fatal = instant_result.is_some(); + let range_skipped_not_fatal = range_result.is_some(); + + assert_eq!( + instant_skipped_not_fatal, + range_skipped_not_fatal, + "instant and range paths diverge on the value-less orphan-group \ + case: instant {} (is_some={}), range {} (is_some={}). Per #590 \ + (and the #583/#597 fixes it references), both paths must treat \ + a keys-resolved-but-value-less group identically -- either both \ + skip it and return the rest of the query's results, or both \ + fail the whole query. See native_range_query_tests.rs's \ + range_query_dual_population_group_with_no_value_data_is_skipped_not_fatal \ + and native_binary_instant_tests.rs's \ + instant_query_dual_population_group_with_no_value_data_is_skipped_not_fatal.", + if instant_skipped_not_fatal { + "succeeded" + } else { + "failed (None)" + }, + instant_skipped_not_fatal, + if range_skipped_not_fatal { + "succeeded" + } else { + "failed (None)" + }, + range_skipped_not_fatal, + ); + + // Both dedicated tests currently pin "skipped, not fatal" as the + // expected behavior for their own path; this asserts that's also + // what actually happened here, on top of the equality check above. + assert!( + instant_skipped_not_fatal && range_skipped_not_fatal, + "expected the value-less orphan group to be skipped (not fatal) \ + on BOTH paths -- instant is_some={instant_skipped_not_fatal}, \ + range is_some={range_skipped_not_fatal}" + ); + } +} From 996fadd6db3f3e30392dc79a21a566587d70f89a Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 26 Aug 2026 00:32:31 -0400 Subject: [PATCH 5/5] test(query-engine): keyed sliding-window property oracle (#590) Adds simulate_sliding_window_keyed, a pure-function sliding-window oracle extended to keyed/grouped data (a sibling to the existing simulate_sliding_window/simulate_sliding_window_with_alignment, kept separate since those two are exercised by several existing index-slicing call sites that don't have a notion of a key). Drives it against a small deterministic sweep of window/slide configs and per-key presence patterns (appearing, disappearing, oscillating, gap mid-range) through a real SimpleEngine via create_engine_multi_timestamp_with_window + handle_range_query_promql. The sweep surfaced a real bug: execute_range_query_pipeline computes each Sliding-window step's window_start as current_time.saturating_sub(lookback_ms), so every output step before window_size_ms worth of history exists aliases onto the store's start=0 window instead of correctly having no sample. Captured as a minimal, #[ignore]'d regression (sliding_window_range_query_start_before_window_size_ms_returns_wrong_value) rather than patched, per #590's ground rules; the main property sweep starts each scenario at its own window_size_ms to avoid that known-buggy region while still exercising the key-expansion/merge behavior it's meant to check. Co-Authored-By: Claude Sonnet 5 --- .../src/engines/simple_engine/mod.rs | 89 +++++ asap-query-engine/src/tests/mod.rs | 1 + .../sliding_window_keyed_oracle_tests.rs | 358 ++++++++++++++++++ 3 files changed, 448 insertions(+) create mode 100644 asap-query-engine/src/tests/sliding_window_keyed_oracle_tests.rs diff --git a/asap-query-engine/src/engines/simple_engine/mod.rs b/asap-query-engine/src/engines/simple_engine/mod.rs index 6dee86a..39d46d5 100644 --- a/asap-query-engine/src/engines/simple_engine/mod.rs +++ b/asap-query-engine/src/engines/simple_engine/mod.rs @@ -1834,6 +1834,95 @@ impl SimpleEngine { } } +/// #590: keyed sliding-window oracle used as the reference model by the +/// property-style test in `crate::tests::sliding_window_keyed_oracle_tests`. +/// Pure function -- no engine/store involved. +/// +/// A sibling to `range_query_tests::simulate_sliding_window` / +/// `simulate_sliding_window_with_alignment` (same file, below) rather than an +/// extension of either: those two operate on a single flat `Vec>` and are exercised by ~8 existing call sites via +/// bucket-index slicing (`buckets[bucket_index..]`) that has no notion of a +/// key at all. Bolting a key dimension onto that signature would mean either +/// changing it (breaking every existing call site) or threading an +/// `Option` through index-based slicing that was designed +/// for one series. A new function keeps those call sites untouched and lets +/// the keyed model use the simpler representation its own callers actually +/// have: raw per-key pane values, not `AggregateCore` trait objects (the +/// property test only exercises `SumAccumulator`, so plain `f64` sums are +/// sufficient and keep the oracle trivially auditable by hand). +/// +/// Mirrors the real lookup path exactly: `execute_range_query_pipeline`'s +/// `single_window` (used for `WindowType::Sliding`, see `mod.rs` above) does +/// one lookup at `bucket_map[current_time - window_size_ms]`, trusting that +/// position to already be the fully pre-merged window -- +/// `create_engine_multi_timestamp_with_window` (the test factory used to +/// drive the real engine) only inserts that pre-merged window into the store +/// when *every one* of its `window_size_ms / slide_interval_ms` panes is +/// present; a window missing even one pane is never inserted at all. This +/// oracle reproduces that same all-or-nothing rule directly over raw panes, +/// independently of the factory's insertion code, so the property test is +/// actually checking the engine against a second, independently-written +/// model rather than against the factory's own logic reflected back at it. +/// +/// `panes_by_key`: for each key, the list of `(pane_end_timestamp_ms, +/// sum_value)` pairs that exist for that key. Every pane is +/// `slide_interval_ms` wide (the real store's Sliding-window layout has no +/// narrower unit); a key with no entry at a given pane timestamp is treated +/// as absent there, not zero -- this is how "key disappears mid-range" and +/// "key appears mid-range" are expressed. +/// +/// Returns, per key, the `(output_timestamp_ms, merged_value)` pairs a real +/// `sum_over_time` Sliding-window range query should produce for that key. +/// A key with zero qualifying output steps is omitted from the map entirely +/// (never present with an empty `Vec`), matching how a real range query +/// never emits a zero-sample series for a key. +#[cfg(test)] +pub(crate) fn simulate_sliding_window_keyed( + panes_by_key: &HashMap>, Vec<(u64, f64)>>, + window_size_ms: u64, + slide_interval_ms: u64, + start_ms: u64, + end_ms: u64, + step_ms: u64, +) -> HashMap>, Vec<(u64, f64)>> { + assert!(slide_interval_ms > 0, "slide_interval_ms must be nonzero"); + assert!( + step_ms > 0, + "step_ms must be nonzero, or the output loop never terminates" + ); + assert!( + window_size_ms.is_multiple_of(slide_interval_ms), + "window_size_ms ({window_size_ms}) must be a whole number of \ + slide_interval_ms ({slide_interval_ms}) panes" + ); + let num_panes = window_size_ms / slide_interval_ms; + + let mut result = HashMap::new(); + for (key, panes) in panes_by_key { + let pane_map: HashMap = panes.iter().cloned().collect(); + let mut samples = Vec::new(); + let mut current_time = start_ms; + while current_time <= end_ms { + if current_time >= window_size_ms { + let window_start = current_time - window_size_ms; + let pane_ends: Vec = (1..=num_panes) + .map(|i| window_start + i * slide_interval_ms) + .collect(); + if pane_ends.iter().all(|t| pane_map.contains_key(t)) { + let merged: f64 = pane_ends.iter().map(|t| pane_map[t]).sum(); + samples.push((current_time, merged)); + } + } + current_time += step_ms; + } + if !samples.is_empty() { + result.insert(key.clone(), samples); + } + } + result +} + #[cfg(test)] mod range_query_tests { use crate::data_model::{AggregateCore, AggregationType, KeyByLabelValues, SerializableToSink}; diff --git a/asap-query-engine/src/tests/mod.rs b/asap-query-engine/src/tests/mod.rs index b4a5bbe..1dce835 100644 --- a/asap-query-engine/src/tests/mod.rs +++ b/asap-query-engine/src/tests/mod.rs @@ -11,6 +11,7 @@ pub mod native_range_query_tests; pub mod prometheus_forwarding_tests; pub mod query_equivalence_tests; pub mod range_query_arithmetic_tests; +pub mod sliding_window_keyed_oracle_tests; pub mod sql_pattern_matching_tests; pub mod store_correctness_tests; pub mod structural_matching_tests; diff --git a/asap-query-engine/src/tests/sliding_window_keyed_oracle_tests.rs b/asap-query-engine/src/tests/sliding_window_keyed_oracle_tests.rs new file mode 100644 index 0000000..70d3f18 --- /dev/null +++ b/asap-query-engine/src/tests/sliding_window_keyed_oracle_tests.rs @@ -0,0 +1,358 @@ +//! Issue #590, "Test suite ideas for instant/range PromQL query paths": a +//! property-style test for `execute_range_query_pipeline`'s Sliding-window +//! and key-expansion behavior, using `simulate_sliding_window_keyed` +//! (`engines::simple_engine::mod.rs`) as an independent, pure-function +//! oracle rather than more hand-picked engine-level regression cases. +//! +//! Those hand-picked cases already exist -- +//! `native_range_query_tests::range_query_sliding_window_merges_both_buckets`, +//! `range_query_sliding_window_merges_three_buckets_same_timestamp`, and +//! `range_query_delta_set_aggregator_oscillating_add_remove_across_five_windows` +//! -- and are deliberately not duplicated here. This test instead sweeps a +//! small deterministic grid of (window_size, slide_interval) configs crossed +//! with key-presence patterns (a key present throughout, appearing midway, +//! disappearing midway, oscillating, with a mid-range gap, or present for +//! only a single pane), builds a real `SimpleEngine` for each combination via +//! `create_engine_multi_timestamp_with_window` (the same factory the +//! hand-picked tests use), and checks the engine's actual range-query output +//! against `simulate_sliding_window_keyed`'s independently-computed +//! expectation. +//! +//! No `proptest`/`quickcheck` dependency is used or added (`asap-query-engine` +//! has neither as a dev-dependency) -- the sweep below is a small, fully +//! deterministic set of hand-enumerated configs (3 window configs x 7 +//! presence-pattern pairs = 21 scenarios x 2 keys each), not a fuzzer. + +#[cfg(test)] +mod tests { + use crate::data_model::{AggregationType, WindowType}; + use crate::engines::query_result::QueryResult; + use crate::engines::simple_engine::simulate_sliding_window_keyed; + use crate::precompute_operators::sum_accumulator::SumAccumulator; + use crate::tests::test_utilities::engine_factories::create_engine_multi_timestamp_with_window; + use crate::AggregateCore; + use std::collections::HashMap; + + /// Presence of a single key across the 6 fixed pane timestamps below -- + /// `true` means the key has a value at that pane, `false` means it's + /// absent there (simulating appearing/disappearing mid-range). + type Presence = [bool; 6]; + + /// One tumbling-window pane: (pane end timestamp ms, label values, + /// accumulator) -- same shape as `native_range_query_tests::TimeSeriesData`. + type PaneData = Vec<(u64, Option>, Box)>; + + const ALWAYS: Presence = [true, true, true, true, true, true]; + const APPEARS_MIDWAY: Presence = [false, false, false, true, true, true]; + const DISAPPEARS_MIDWAY: Presence = [true, true, true, false, false, false]; + const OSCILLATING: Presence = [true, false, true, false, true, false]; + const GAP_IN_MIDDLE: Presence = [true, true, false, true, true, true]; + const SINGLE_PANE: Presence = [false, false, true, false, false, false]; + + /// Pane end-timestamps shared by every scenario: 1s-wide panes from + /// 1000ms to 6000ms. + const PANE_TIMESTAMPS_MS: [u64; 6] = [1000, 2000, 3000, 4000, 5000, 6000]; + + /// Query range end/step shared by every scenario: covers every pane plus + /// one extra step past the last pane (7000ms), so the sweep also + /// exercises "no data left to slide into" at the tail end for every + /// config. + /// + /// The query START is deliberately NOT a shared constant -- it is + /// `scenario.window_size_ms` (set in `check_scenario` below), i.e. the + /// first output step for which a full window's worth of history could + /// possibly exist. Starting any earlier hits a real, separately-tracked + /// engine bug (see `sliding_window_range_query_start_before_window_size_ms_returns_wrong_value` + /// below, `#[ignore]`d): `execute_range_query_pipeline` computes each + /// step's window_start as `current_time.saturating_sub(lookback_ms)`, + /// so *every* current_time < window_size_ms saturates to the same + /// window_start=0 and aliases onto whatever legitimate window happens to + /// be stored at start=0 (built for the correct, later, current_time == + /// window_size_ms step) instead of correctly finding no data. That bug + /// is orthogonal to what this sweep is testing (key expansion timing + + /// multi-pane merge correctness), so the sweep avoids the affected + /// region rather than let it drown out the signal this test exists to + /// check. + const QUERY_END_MS: u64 = 7000; + const QUERY_STEP_MS: u64 = 1000; + + /// Builds `(pane_end_ms, value)` pairs for a key with the given presence + /// pattern. Per-pane value is `value_offset + pane_index` (1-indexed) so + /// every pane has a distinct, hand-verifiable value and the two keys in + /// a scenario (different `value_offset`s) can never accidentally collide + /// on a merged sum. + fn panes_for(presence: Presence, value_offset: f64) -> Vec<(u64, f64)> { + PANE_TIMESTAMPS_MS + .iter() + .zip(presence.iter()) + .enumerate() + .filter(|(_, (_, present))| **present) + .map(|(i, (ts, _))| (*ts, value_offset + (i as f64 + 1.0))) + .collect() + } + + /// One (window config) x (presence pattern pair) combination to check. + struct Scenario { + window_size_ms: u64, + slide_interval_ms: u64, + host_a_presence: Presence, + host_b_presence: Presence, + label: String, + } + + fn window_configs() -> Vec<(u64, u64, &'static str)> { + vec![ + (1000, 1000, "window=1000/slide=1000 (num_panes=1)"), + (2000, 1000, "window=2000/slide=1000 (num_panes=2)"), + (3000, 1000, "window=3000/slide=1000 (num_panes=3)"), + ] + } + + fn presence_pairs() -> Vec<(Presence, Presence, &'static str)> { + vec![ + (ALWAYS, APPEARS_MIDWAY, "always vs appears_midway"), + (ALWAYS, DISAPPEARS_MIDWAY, "always vs disappears_midway"), + (ALWAYS, OSCILLATING, "always vs oscillating"), + ( + APPEARS_MIDWAY, + DISAPPEARS_MIDWAY, + "appears_midway vs disappears_midway", + ), + (OSCILLATING, GAP_IN_MIDDLE, "oscillating vs gap_in_middle"), + (GAP_IN_MIDDLE, SINGLE_PANE, "gap_in_middle vs single_pane"), + ( + DISAPPEARS_MIDWAY, + SINGLE_PANE, + "disappears_midway vs single_pane", + ), + ] + } + + /// The full sweep: every window config crossed with every presence pair + /// (3 x 7 = 21 scenarios). + fn scenarios() -> Vec { + let mut out = Vec::new(); + for (window_size_ms, slide_interval_ms, window_label) in window_configs() { + for (host_a_presence, host_b_presence, pair_label) in presence_pairs() { + out.push(Scenario { + window_size_ms, + slide_interval_ms, + host_a_presence, + host_b_presence, + label: format!("{window_label} / {pair_label}"), + }); + } + } + out + } + + fn matrix_values(qr: QueryResult) -> Vec { + match qr { + QueryResult::Matrix(m) => m.values, + QueryResult::Vector(_) => panic!("expected matrix (range vector) result"), + } + } + + /// Runs one scenario end-to-end: builds the oracle's expectation, + /// builds a matching real engine, runs the same range query, and + /// returns a human-readable mismatch description per (key) pair that + /// diverged, or an empty Vec if the scenario passed cleanly. + fn check_scenario(scenario: &Scenario) -> Vec { + let a_panes = panes_for(scenario.host_a_presence, 0.0); + let b_panes = panes_for(scenario.host_b_presence, 1000.0); + + let mut panes_by_key: HashMap>, Vec<(u64, f64)>> = HashMap::new(); + panes_by_key.insert(Some(vec!["host-a".to_string()]), a_panes.clone()); + panes_by_key.insert(Some(vec!["host-b".to_string()]), b_panes.clone()); + + // See the comment on QUERY_END_MS above: start exactly at + // window_size_ms, the earliest step not affected by the separately + // tracked saturating_sub aliasing bug. + let query_start_ms = scenario.window_size_ms; + + let expected = simulate_sliding_window_keyed( + &panes_by_key, + scenario.window_size_ms, + scenario.slide_interval_ms, + query_start_ms, + QUERY_END_MS, + QUERY_STEP_MS, + ); + + let mut data: PaneData = Vec::new(); + for (ts, v) in &a_panes { + data.push(( + *ts, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(*v)) as Box, + )); + } + for (ts, v) in &b_panes { + data.push(( + *ts, + Some(vec!["host-b".to_string()]), + Box::new(SumAccumulator::with_sum(*v)) as Box, + )); + } + + let query = "sum_over_time(http_requests[1s])"; + let engine = create_engine_multi_timestamp_with_window( + "http_requests", + AggregationType::Sum, + vec!["host"], + data, + query, + scenario.window_size_ms, + scenario.slide_interval_ms, + WindowType::Sliding, + ); + + let result = engine.handle_range_query_promql( + query.to_string(), + query_start_ms as f64 / 1000.0, + QUERY_END_MS as f64 / 1000.0, + QUERY_STEP_MS as f64 / 1000.0, + ); + let (_, qr) = result.expect("range query failed"); + let elements = matrix_values(qr); + + let mut mismatches = Vec::new(); + for key_label in ["host-a", "host-b"] { + let mut expected_samples: Vec<(u64, f64)> = expected + .get(&Some(vec![key_label.to_string()])) + .cloned() + .unwrap_or_default(); + expected_samples.sort_by_key(|(t, _)| *t); + + let mut actual_samples: Vec<(u64, f64)> = elements + .iter() + .find(|e| e.labels.labels.contains(&key_label.to_string())) + .map(|e| e.samples.iter().map(|s| (s.timestamp, s.value)).collect()) + .unwrap_or_default(); + actual_samples.sort_by_key(|(t, _)| *t); + + let matches = actual_samples.len() == expected_samples.len() + && actual_samples + .iter() + .zip(expected_samples.iter()) + .all(|((at, av), (et, ev))| at == et && (av - ev).abs() < 1e-9); + + if !matches { + mismatches.push(format!( + "scenario [{}] key={}: expected={:?}, actual={:?}", + scenario.label, key_label, expected_samples, actual_samples + )); + } + } + mismatches + } + + #[tokio::test(flavor = "multi_thread")] + async fn sliding_window_keyed_property_sweep() { + let mut all_mismatches = Vec::new(); + for scenario in scenarios() { + all_mismatches.extend(check_scenario(&scenario)); + } + + assert!( + all_mismatches.is_empty(), + "sliding-window keyed oracle diverged from the real engine in {} case(s):\n{}", + all_mismatches.len(), + all_mismatches.join("\n") + ); + } + + /// Minimal reproducer for a real bug found while building the sweep + /// above (discovered via the [window=2000/slide=1000] x [always vs + /// appears_midway] scenario, which originally queried starting at + /// t=1000 -- see the comment on QUERY_END_MS): a Sliding-window range + /// query whose FIRST requested output step is earlier than the + /// aggregation's `window_size_ms` gets a phantom sample at that step, + /// wrongly duplicating the value of the first *legitimate* step + /// (current_time == window_size_ms) instead of correctly having no + /// sample there. + /// + /// Root cause (`execute_range_query_pipeline`, + /// `asap-query-engine/src/engines/simple_engine/mod.rs`): each step's + /// window_start is computed as `current_time.saturating_sub(lookback_ms)` + /// (`lookback_ms == window_size_ms` for Sliding, per the active assert + /// a few lines above that computation). For `current_time < + /// window_size_ms` this saturates to 0 -- the *same* window_start that + /// the legitimately-computed step `current_time == window_size_ms` + /// also resolves to (via ordinary, non-saturating subtraction). Since + /// `single_window` (used for `WindowType::Sliding`) does a bare + /// `bucket_map.get(&window_start)` with no independent check that + /// `current_time` actually had `window_size_ms` worth of history behind + /// it, both steps collide on the one store entry legitimately built for + /// `current_time == window_size_ms`, and the earlier step silently + /// receives that later step's value instead of "no data in window." + /// + /// Concretely here: window_size_ms=2000, slide_interval_ms=1000 (2 + /// panes/window), one key with panes at t=1000 (value 10.0) and t=2000 + /// (value 5.0). The only genuine full window is [0, 2000) -> merged + /// value 15.0, correctly surfaced at t=2000. Querying from t=1000 + /// should show NO sample at t=1000 (a window ending at 1000 would need + /// history back to t=-1000, which doesn't exist) -- but the engine + /// currently reports 15.0 there too, identical to t=2000. + /// + /// Do not weaken this assertion or patch around it if this test's + /// premise ever needs revisiting -- it is exact-equality against the + /// documented intended behavior (no sample when there isn't a full + /// window's worth of history), not an approximation. See #590. + #[ignore = "real bug, see #590: current_time.saturating_sub(lookback_ms) aliases every \ + pre-window_size_ms step onto the store's start=0 window instead of skipping it"] + #[tokio::test(flavor = "multi_thread")] + async fn sliding_window_range_query_start_before_window_size_ms_returns_wrong_value() { + let window_size_ms = 2000; + let slide_interval_ms = 1000; + + let data: PaneData = vec![ + ( + 1000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(10.0)) as Box, + ), + ( + 2000, + Some(vec!["host-a".to_string()]), + Box::new(SumAccumulator::with_sum(5.0)) as Box, + ), + ]; + + let query = "sum_over_time(http_requests[1s])"; + let engine = create_engine_multi_timestamp_with_window( + "http_requests", + AggregationType::Sum, + vec!["host"], + data, + query, + window_size_ms, + slide_interval_ms, + WindowType::Sliding, + ); + + // Query starts at t=1000ms, one slide interval before window_size_ms + // (2000ms) -- the earliest step for which a full window could + // possibly exist. + let result = engine.handle_range_query_promql(query.to_string(), 1.0, 2.0, 1.0); + let (_, qr) = result.expect("range query failed"); + let elements = matrix_values(qr); + + let host_a_samples: Vec<(u64, f64)> = elements + .iter() + .find(|e| e.labels.labels.contains(&"host-a".to_string())) + .map(|e| e.samples.iter().map(|s| (s.timestamp, s.value)).collect()) + .unwrap_or_default(); + + assert_eq!( + host_a_samples, + vec![(2000, 15.0)], + "expected only t=2000 (the first step with a full 2000ms window's worth of \ + history) to have a sample, with the two panes merged into 15.0; t=1000 should \ + have no sample at all (window would need history back to t=-1000). Got {:?} -- \ + if this now shows exactly [(2000, 15.0)], the saturating_sub aliasing bug has \ + been fixed and this test should be un-ignored.", + host_a_samples + ); + } +}