Skip to content

test(e2e): controller → backend streaming-config round-trip + grouping plumb - #247

Merged
zzylol merged 1 commit into
mainfrom
e2e-controller-backend-tests
May 15, 2026
Merged

zzylol merged 1 commit into
mainfrom
e2e-controller-backend-tests

Conversation

@zzylol

@zzylol zzylol commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

First end-to-end test of the gateway-less control-plane ↔ data-plane contract. Lives in data_plane/tests/ (control_plane is already a workspace dep — no Cargo.toml changes needed).

Two #[tokio::test]s:

1. controller_plans_ddsketch_quantile_and_backend_parses_streaming_config — smoke test for the full content-faithfulness contract. Workload { metric: http_latency_ms, agg: Quantile, sla: 0.01, window: 60s } flows through bind_workload_typedsplit_typed_three_stageemit_backend_streaming_config_json. JSON is POSTed to the in-process backend's /api/v1/streaming-config, and the GET endpoint reflects the parsed aggregation. Asserts the emitted JSON's shape: #244 content fields present (metric, windowSize, windowType, aggregationType), and #244/#246 aggregationId absent.

2. controller_plans_with_grouping_and_backend_parses_grouping_labels — same shape with group_by_labels: [”zone”]. Asserts the emitter surfaces labels.grouping: [”zone”] (#245), the POST succeeds, and the active-config snapshot contains ”zone” in the parsed AggregationConfig.grouping_labels.

Real bug the test caught

bind_workload_typed-produced PhysicalExpr doesn't surface metric_name or window_secs to BackendAggregation via extract_edge_facts. Likely root cause: the allocator paints the Logical(Scan{...}) chain at a stage other than Edge in some binder outputs, so the walk's (Logical, Edge) match arm doesn't fire — Step γ open-set label work (also blocking grouping fidelity) probably needs a coordinated fix.

Pragmatic workaround mirroring the #245 grouping patch — handle_plan's Backend stage arm now belt-and-braces the values from the workload spec:

for agg in &mut be.aggregations {
    if agg.metric_name.is_empty() {
        agg.metric_name = workload.metric_name.clone();
    }
    if agg.window_secs == 0 {
        agg.window_secs = workload.time_window.as_secs();
    }
    agg.grouping = workload.group_by_labels.clone();
}

Closes a real bug where fresh-deploy handle_plan POSTs would have failed the backend parser on Missing metric / Missing windowSize. PR #244 caught the JSON shape gap; this test caught the value-population gap.

Other change

HttpServer::start_test_server was #[cfg(test)]-gated to the lib's own unit tests — integration tests under data_plane/tests/ are compiled separately and couldn't see it. Dropped the gate. The method's name + doc-comment make the test-only intent explicit, and production code uses start() regardless.

Out of scope (deliberately)

Test plan

  • cargo test --test e2e_controller_plans_and_backend_serves: 2 passed; 0 failed
  • cargo test --lib -p control_plane: 687 passed; 0 failed
  • cargo test --tests --bins -p control_plane: 27 passed; 0 failed

🤖 Generated with Claude Code

…g plumb

First end-to-end test of the gateway-less control-plane ↔ data-plane
contract. Lives in `data_plane/tests/` since data_plane already owns
the OTLP/precompute/HTTP wiring; control_plane is already a
workspace dep so no Cargo.toml changes were needed.

## What it tests

Two `#[tokio::test]`s:

1. `controller_plans_ddsketch_quantile_and_backend_parses_streaming_config`
   — workload `{ metric: http_latency_ms, agg: Quantile, sla: 0.01,
   window: 60s }` flows through `bind_workload_typed` →
   `split_typed_three_stage` → `emit_backend_streaming_config_json`,
   the JSON is POSTed to the in-process backend's
   `/api/v1/streaming-config`, and the GET endpoint reflects the
   parsed aggregation. Asserts the emitted JSON's shape (#244 content
   fields present, #244/#246 `aggregationId` absent).

2. `controller_plans_with_grouping_and_backend_parses_grouping_labels`
   — same shape but with `group_by_labels: ["zone"]`. Asserts the
   emitter surfaces `labels.grouping: ["zone"]` (#245), the POST
   succeeds, and the active-config snapshot contains `"zone"` in the
   parsed `AggregationConfig.grouping_labels`.

## Findings the test caught

`bind_workload_typed`-produced PhysicalExpr doesn't surface
`metric_name` or `window_secs` to `BackendAggregation` via
`extract_edge_facts`. The underlying cause is likely that the
allocator paints the `Logical(Scan{...})` chain at a stage other than
Edge in some binder outputs, so the walk's `(Logical, Edge)` match arm
doesn't fire — Step γ open-set label work (also blocking grouping
fidelity) probably needs a coordinated fix. Pragmatic workaround
mirroring the #245 grouping patch:

  control_plane/src/main.rs (handle_plan, Backend stage match arm)
    for agg in &mut be.aggregations {
        if agg.metric_name.is_empty() {
            agg.metric_name = workload.metric_name.clone();
        }
        if agg.window_secs == 0 {
            agg.window_secs = workload.time_window.as_secs();
        }
        agg.grouping = workload.group_by_labels.clone();
    }

This is belt-and-braces — if the L5 walk DID surface the field,
the conditional `if … is_empty()` / `== 0` checks preserve it;
otherwise the workload spec wins. Closes a real bug where fresh-deploy
`handle_plan` POSTs would have failed the backend parser on
`Missing metric` / `Missing windowSize` (PR #244 caught the JSON shape
gap; this test caught the value-population gap).

## Other change

`HttpServer::start_test_server` had `#[cfg(test)]` gating it to the
lib's own unit tests — integration tests under `data_plane/tests/`
are compiled separately and couldn't see it. Dropped the gate; the
method's name + doc-comment make the test-only intent explicit, and
production code uses `start()` regardless.

## Test plan

- [x] `cargo test --test e2e_controller_plans_and_backend_serves`:
  **2 passed**.
- [x] `cargo test --lib -p control_plane`: **687 passed**.
- [x] `cargo test --tests --bins -p control_plane`: **27 passed**.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit c5c9b36 into main May 15, 2026
zzylol added a commit that referenced this pull request May 15, 2026
…stack (#248)

Follow-up to #247. Adds Test 3 — `controller_plan_to_query_full_roundtrip_ddsketch`
— exercising the entire gateway-less data path in-process:

  controller plan → POST /api/v1/streaming-config
                  → OTLP-encode `DdSketchDataPoint` via asap_sketchlib
                  → POST /v1/metrics
                  → watermark-advance DP
                  → wait for window close
                  → GET /api/v1/query

The full stack — `PrecomputeEngine` + `SketchStoreSink` + `OtlpReceiver`
+ `HttpServer` — all share one `SketchStore` and one
`HotReloadStreamingConfig` so a controller-posted streaming-config is
visible to ingest, the engine's window output lands in `SketchStore`,
and the query engine reads from the same store. Mirrors the wiring in
`data_plane/src/main.rs`.

## What's strictly asserted

- Controller-emitted streaming-config (content shape + grouping) is
  parseable on the backend (also covered by Tests 1+2; re-exercised
  with engine + receiver running).
- Modified-OTLP `DdSketchDataPoint` wire encoding is accepted by the
  HTTP receiver (no 4xx/5xx).
- The full stack stays up under POST + query traffic without panics.
- The query endpoint produces well-formed JSON with a `status` field.

## What's deliberately soft-checked

The query currently returns `errorType: bad_data` / "No result for
query" — same symptom that has the sibling
`e2e_dd_sketch_modified_otlp_path` test `#[ignore]`'d ("broken since
proto refactor"). The OTLP→precompute→`SketchStore` path is broken
*upstream* from this PR's scope: the DDSketch state never persists
in a queryable form even though the wire encoding round-trips
cleanly through the receiver. Tightening the assertion to
`status == "success"` + an exact-value-within-SLA check on the
returned quantile is deferred to whoever closes the proto path. The
test doc-comment captures the gap explicitly so future readers know
where to take it.

Build clean. 3 tests pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 15, 2026
…it order) (#249)

PR #247's belt-and-braces patches (`handle_plan` patching
`BackendAggregation.metric_name` / `window_secs` / `grouping` from the
workload spec) were load-bearing for `metric_name` and `window_secs`,
not no-ops. Root cause was a DAG-visit-order bug in
`ThreeStageEmitter::emit_per_stage`:

* The colored DAG's node order is depth-first parent-first
  (`SketchEstimate`, `SketchAgg`, `Logical`) so a SketchAgg appears
  BEFORE its `Logical(Window{Scan})` child in `dag.nodes`.
* Pass 2's SketchAgg arm captures `edge.source_metric` /
  `edge.window_secs` / `edge.label_filters` *at push time* while
  building `BackendAggregation`.
* But those fields are populated by Pass 2's Logical arm calling
  `extract_edge_facts(qe, &mut edge)` — which fires LATER in the
  same loop, after SketchAgg.
* Result: every `BackendAggregation` shipped with empty
  `metric_name` / `window_secs: 0`, which the backend's
  `AggregationConfig::from_yaml_data` rejects on
  `Missing metric` / `Missing windowSize`. The Replanner's separate
  legacy emitter (`generate_streaming_config_yaml`) papered over it
  by reading directly from `plan.agent_config`; `handle_plan` was
  papering over it with the workload-spec patch from #247.

Fix: add **Pass 0** that pre-walks the DAG looking only for
`Logical(qe) @ Edge` nodes and calls `extract_edge_facts` to populate
edge facts. Pass 2's Logical arm stays (it's idempotent — only sets
on `None` and dedupes label_filters), so the change is safe even if
node order changes upstream.

Grouping stays empty out of the L5 walk: canonical L3
`QueryExpr::Aggregate.by` is `Vec<ColumnId>` against a synthesized
schema with no label columns (Step γ TODO in
`intent_algebra::column_resolution`). The workload-spec patch in
`handle_plan` remains the source of truth for grouping today.

## Diagnostic tests added

`physical::stage_split::l5_walk_propagation_tests` — three
characterisation tests that pin the L5 walk's behaviour for
`bind_workload_typed` output:

1. `l5_walk_surfaces_metric_name_for_bind_workload_typed_output`
2. `l5_walk_surfaces_window_secs_for_bind_workload_typed_output`
3. `l5_walk_leaves_grouping_empty_pending_step_gamma`

The first two now pass (the fix). The third pins the current
limitation so a future Step γ fix fails it loudly and signals that
the handle_plan grouping patch can be retired alongside.

## Tests

- `cargo test --lib -p control_plane`: 690 passed (was 687; +3 new
  diagnostic tests).
- `cargo test --tests --bins -p control_plane`: 27 passed.
- `cargo test --test e2e_controller_plans_and_backend_serves`: 3
  passed (the e2e suite's metric_name / windowSize assertions now
  pass via the real fix, not the patch).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 15, 2026
…itations (#252)

Engine-path debug session diagnosis (post-#247#248#249#250).

The query layer's "No result for query" symptom for sketch-backed
metrics has a precise root cause: `query_precomputes_by_agg` (called
unconditionally by `execute_store_query`) filters its candidate-sid
scan with `matches!(&m.agg_kind, AggKind::ExactAgg { … })` — never
matching `AggKind::Sketch`. Since OTLP-arriving DDSketch / KLL / HLL /
CountSketch / CountMinSketch DPs are registered with
`AggKind::Sketch { kind, config, .. }`, the lookup always returns an
empty map for them, the engine bubbles up "No precomputed outputs
found", and `handle_query` returns `None` → HTTP responds
`errorType: bad_data` / `error: "No result for query"`.

Sketches DO reach `SketchStore` — they're readable via the sid-keyed
`query_range(sid, ...)` path which filters on `payload.as_sketch()`.
The agg-keyed precompute lookup is the gap.

This PR adds doc-comment blocks at both the call site
(`engine.rs::execute_store_query`) and the function definition
(`mod.rs::query_precomputes_by_agg`) flagging the gap with
file:line citations and pointing at the two viable fixes:
  * teach `query_precomputes_by_agg` to also collect sketch
    payloads (assemble `Box<dyn AggregateCore>` from
    `payload.as_sketch()`) — non-trivial; payload shapes diverge.
  * route the legacy `handle_query` path through the newer
    `ASAPQueryEngine::execute(&str)` trait dispatcher (around
    engine.rs:3430) which already handles sketches via
    `idx.sids_for_policy(fp)` + reducer dispatch.

No behaviour change. 690 lib + 27 binary tests pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the e2e-controller-backend-tests branch July 17, 2026 20:05
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