test: add runnable PromQL differential compliance suite - #742
milindsrivastava1997 wants to merge 29 commits into
Conversation
milindsrivastava1997
left a comment
milindsrivastava1997
left a comment
There was a problem hiding this comment.
Automated code review: 5 findings, posted inline.
| if err := l.runCompose(ctx, environment, "wait", "planner"); err != nil { | ||
| return fmt.Errorf("derive workload cost evidence: %w", err) | ||
| } | ||
| if err := l.runCompose(ctx, environment, "up", "-d", "data-plane"); err != nil { |
There was a problem hiding this comment.
Stale backend image (high). prometheus/planner get --build above, but data-plane is brought up without it, so Compose reuses the existing asapquery-backend-promql-compliance-data-plane image. After the first make run-all, subsequent runs on the same machine test the old data_plane binary (e.g. edit data_plane/src/..., rerun, and results still reflect the first build). Suggest adding --build here or building all three services in the first up.
| c := caseCard{Dataset: r.Dataset, Suite: r.Suite, Passed: r.Passed, Queries: len(r.Queries)} | ||
| for _, q := range r.Queries { | ||
| for _, i := range q.Instant { | ||
| if i.Responses.Backend.ServedBy != "" { |
There was a problem hiding this comment.
"Prometheus fallback" column is always 0 (medium). query.go sets ServedBy = "prometheus_fallback" when the provenance header is absent, so ServedBy is never empty for backend responses and this check counts fallback answers as ASAPQuery answers. The loop also only looks at q.Instant, skipping range responses. summary.md/summary.json therefore overstate local coverage.
| func main() { | ||
| dir := flag.String("reports-dir", "/tmp/asapquery-backend-promql-reports", "report directory") | ||
| flag.Parse() | ||
| files, err := filepath.Glob(filepath.Join(*dir, "*.json")) |
There was a problem hiding this comment.
Picks up stale reports (medium). *.json in REPORT_DIR (a fixed /tmp path by default) also matches the summary.json written by the previous run; it decodes as a runner.Report with empty dataset/suite and the previous overall passed, adding a blank row and possibly pinning the result to false. Likewise, a case that fails before writing its report (build error, health timeout) leaves the previous run's <case>.json counted as current. Suggest excluding summary.json and clearing/scoping REPORT_DIR per run.
| # not need credentials for ASAPCollector or Sketchlib while this dependency | ||
| # remains private. | ||
| RUN mkdir -p ASAPQuery-backend/.cargo && \ | ||
| printf '%s\n' \ |
There was a problem hiding this comment.
Overwrites .cargo/config.toml and patches all builds (medium). (Same block in control_plane/Dockerfile.) > replaces the checked-in config, dropping [net] git-fetch-with-cli = true. And since this applies to every image built from these Dockerfiles, not just the compliance suite, asap-precompute-rs/asap_sketchlib now compile from whatever sibling working trees are passed as build contexts rather than the revisions pinned in Cargo.lock, so production images can pick up uncommitted/unpinned code. Suggest appending (>>) and gating the patch behind a build arg used only by the compliance build.
| asap-precompute-rs: ${ASAP_PRECOMPUTE_RS_CONTEXT} | ||
| asap-sketchlib: ${ASAP_SKETCHLIB_CONTEXT} | ||
| asap-gorilla-rust: ${ASAP_GORILLA_RUST_CONTEXT} | ||
| command: ["--profile", "asapquery", "--planning-snapshot", "/config/planning-snapshot.json", "--prometheus-server", "http://prometheus:9090", "--forward-unsupported-queries", "--http-port", "9091", "--output-dir", "/tmp/asap"] |
There was a problem hiding this comment.
Fallback enabled but always fails the suite (low). --forward-unsupported-queries is on, but report.go:104 fails every response served by Prometheus fallback, so any query routed to the exact/fallback path fails rather than being compared. The design doc also says fallback should be disabled, which would make the validate_profile change in data_plane/src/main.rs pointless as written. Either drop the flag or accept fallback responses for those nodes.
1 participant
Why
Provide a repeatable way to compare ASAPQuery-backend with Prometheus across representative PromQL workloads before changing query execution.
What
Before this PR
There was no runnable end-to-end PromQL differential suite. A developer had no single command to ingest identical fixtures, start a backend-local plan, compare outputs, or inspect the serving path.
After this PR
make run-all starts a clean case, derives its plan, writes identical samples to both systems, compares every configured query, then writes summary.md, summary.json, per-case raw responses, and retained Compose logs under /tmp.
Design decisions discussed
Verification
Limitations
The first Docker build compiles the Rust images and can take several minutes. The suite currently reports backend serving provenance from X-ASAP-Data-Source; an absent value denotes Prometheus fallback in this Compose profile.