Skip to content

fix(deploy): add grouping_labels: [zone] to mvp-workload entries (B3 end-to-end) - #390

Merged
zzylol merged 1 commit into
mainfrom
mvp-workload-add-grouping-labels
May 18, 2026
Merged

zzylol merged 1 commit into
mainfrom
mvp-workload-add-grouping-labels

Conversation

@zzylol

@zzylol zzylol commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

ASAPQuery-backend PR #282 added a declarative `grouping_labels:` field to `WorkloadEntry` (replacing the PromQL-AST-walk attempt that couldn't extract grouping from `quantile_over_time(...)` shapes which have no `by(...)` clause). The `mvp-workload.yaml` file didn't have the new field on any of its 8 entries, so the controller-emitted `transform/keep_for_` blocks all had `keep_keys(datapoint.attributes, [])` — empty allowlist, strips all attrs.

Result observed in smoke test: 1 sid total, no per-zone breakdown.

After this PR: 21 sids, per-metric sid counts 4-5 (one per zone), and `quantile_over_time(0.99, http_requests_total_latency_ms[5m])` returns per-zone p99 results.

Test plan

  • `bash /mydata/mvp-smoke-test/run_smoke.sh` shows total_sids: 21, per-metric counts 4-5, quantile query returns per-zone results

🤖 Generated with Claude Code

ASAPQuery-backend PR #282 (bundle B3-pop + B4-window) made
`WorkloadEntry.grouping_labels` a declarative yaml field — the
controller stops trying to infer grouping from the PromQL `by(...)`
clause (which doesn't exist for `quantile_over_time(...)`-style
queries) and instead reads it from the yaml.

Without `grouping_labels:` on each entry, the controller emits
`transform/keep_for_<metric>` blocks with `keep_keys(datapoint.attributes, [])`
— strips ALL attrs instead of keeping the intended grouping.
Result in the smoke test: 1 sid registered (one per metric,
no per-zone breakdown).

Fix: add `grouping_labels:\n  - zone` to all 8 entries. After:
- Per-metric sids: 4-5 per metric (one per zone value)
- `quantile_over_time(0.99, http_requests_total_latency_ms[5m])`
  returns per-zone p99 results, e.g.:
    { "metric": {..., "zone": "z0"}, "value": [..., "93.7"] }
- Total: 21 sids across all metrics (was 1 pre-fix)

Empirically verified via the single-node MVP smoke test at
`/mydata/mvp-smoke-test/run_smoke.sh`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit f649ae2 into main May 18, 2026
@zzylol
zzylol deleted the mvp-workload-add-grouping-labels branch May 18, 2026 02:36
zzylol added a commit that referenced this pull request May 18, 2026
…ries-e2e wave queries (#397)

Three harness gaps closed (against asap arm post-#396 baseline 332/413):

1. WARMUP_S 30 → 60s in `topology.env`. The 30s warmup wasn't long enough
   for the agent's first 30s tumbling window to close + flush to backend
   before replay started. Result: 14 leading "error/200" on 4 queries
   per arm.

2. `deploy/mvp-multinode/configs/asap/mvp-workload.yaml` was missing
   `grouping_labels: [zone]` on all 8 entries. ASAPCollector PR #390
   added these to the singlenode workload but the multinode equivalent
   was never swept. Without grouping_labels, the controller's
   `attributes/keep` processor isn't injected, the agent emits full-attr
   sketches, the backend stores per-(zone,rack,node,pod) sids, and
   queries return per-series instead of per-zone.

3. Replaced broken `quantile by (zone) (0.99, http_requests_total_latency_ms)`
   in `queries-e2e.json` with
   `max by (zone) (quantile_over_time(0.99, http_requests_total_latency_ms[5m]))`.
   Original used PromQL's instant `quantile()` aggregator which expects
   scalar values across series — incompatible with DDSketch state.
   The replacement is valid PromQL on both raw and sketch tiers: inner
   computes per-series p99 over 5min (or per-zone p99 directly on asap
   thanks to the new grouping_labels), outer `max by (zone)` is the
   per-zone worst-case rollup (SLO-relevant tail).
   `histogram_quantile` was considered but doesn't apply — the
   fake-exporter emits a Float64Gauge per fake-exporter/main.go:446,
   not a Prometheus histogram with `_bucket{le=...}` series.

Results post-fix on asap arm: 6/7 wave queries now 100% success
(was 4/7 with 14-error leading pattern on the other 3). Baselines b0/b1
unchanged at 100%+empty on the queries the producer emits data for.

Known follow-up (asap engine limitation, not harness debt): the new
`max by (zone) (quantile_over_time(...))` query still errors on asap
because the engine doesn't compose outer aggregation operators on
function results — even when the inner already returns per-zone (which
makes the outer a no-op identity). The same query succeeds 118/118 on
b0/b1 via VictoriaMetrics. Worth a separate ASAPQuery-backend PR.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant