test(e2e): 5-sketch coverage — add KLL strict-success + HLL ignored-with-gap - #254
Merged
Merged
Conversation
…ith-gap Extends the e2e test suite to cover the second and third sketch families (DDSketch is already strict-success-asserted in Test 3). ## Test 4 — controller_plan_to_query_full_roundtrip_kll (passes) Mirrors Test 3 with `sketch_type_override: Some(SketchType::KLL)`. Builds a `KllState` via `asap_sketchlib::proto::sketchlib::KllState`, wraps in `KllSketchDataPoint`, OTLP-POSTs, watermark-advances, queries `quantile_over_time(0.5, request_size_bytes[10s])`, asserts `status == "success"`. Also asserts the controller emits `aggregationType: DatasketchesKLL` for the override. This validates the second of the five sketch families on the gateway-less data path. The `execute(&str)` trait dispatcher (#253's fallback) handles KLL identically to DDSketch — both route through `SketchReducer::evaluate` with quantile readout dispatch. ## Test 5 — controller_plan_to_query_full_roundtrip_hll (#[ignore]'d) Workload pins HLL with `AggType::Cardinality`. The streaming-config registration succeeds (`aggregationType: HLL` confirmed) and the sketch state lands in `SketchStore` — but the PromQL query path for HLL needs a query shape the analyzer recognises as `Capability::CardinalityApprox`. `count(metric)` doesn't map today: `resolve_sketch_metric_alias` only rewrites `count(metric)` → `count(metric_hll)` when the bare metric is ABSENT (a deploy-time aliasing tactic). With bare registered, alias is a no-op and the analyzer doesn't bind `count(...)` to the cardinality capability. `#[ignore]`'d with a precise note pointing at the gap. The OTLP ingest path for HLL works (no `#[ignore]` on ingest); the readout side needs follow-up analyzer / alias work. ## Helper changes - Splits `build_workload` into `build_workload` (no override) and `build_workload_with_override` (with `sketch_type_override`). - Adds `build_kll_state` / `build_kll_export` / `build_hll_state` / `build_hll_export` mirrors of the existing DDSketch helpers. ## Tests - `cargo test --test e2e_controller_plans_and_backend_serves`: **4 passed; 0 failed; 1 ignored** (HLL). ## What's left for full 5-sketch coverage CountSketch (topk) and CountMinSketch (frequency / count) need their own tests + likely similar analyzer routing work as HLL. Two paths to ship those: 1. Wait for the analyzer-side fix on HLL — same fix likely unblocks all three. 2. Use the demo-contract metric names (`top_endpoint_qps`, `endpoint_request_freq`) which `classify_demo_metric` routes directly, bypassing the analyzer's bare-name handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the e2e test suite to cover the second and third sketch families. DDSketch is strict-success in Test 3 (post-#253); this PR adds KLL (passing strict success) and HLL (ignored with a precise gap note for follow-up).
Test 4 —
controller_plan_to_query_full_roundtrip_kll(passes)Mirrors Test 3 with
sketch_type_override: Some(SketchType::KLL). Builds aKllState, wraps inKllSketchDataPoint, OTLP-POSTs, queriesquantile_over_time(0.5, request_size_bytes[10s]), assertsstatus == "success". Also asserts the controller emitsaggregationType: DatasketchesKLLfor the override.The
execute(&str)trait dispatcher from #253 handles KLL identically to DDSketch — both route throughSketchReducer::evaluatewith quantile readout dispatch.Test 5 —
controller_plan_to_query_full_roundtrip_hll(#[ignore]'d)Workload pins HLL with
AggType::Cardinality. Streaming-config registers correctly (aggregationType: HLL); sketch state lands inSketchStore— but the PromQL query path for HLL needs a query shape the analyzer recognises asCapability::CardinalityApprox.count(metric)doesn't map today:resolve_sketch_metric_aliasonly rewritescount(metric)→count(metric_hll)when the bare metric is ABSENT (a deploy-time aliasing tactic). With bare registered, alias is a no-op and the analyzer doesn't bindcount(...)to cardinality capability.#[ignore]'d with a precise doc-comment. The OTLP ingest path for HLL is unaffected (works fine; verifiable viaruntime_info); the readout side needs follow-up analyzer / alias work.Test plan
cargo test --test e2e_controller_plans_and_backend_serves: 4 passed; 0 failed; 1 ignored (HLL).What's left for full 5-sketch coverage
CountSketch (topk) and CountMinSketch (frequency / count) need their own tests + likely similar analyzer routing work. Two paths to ship them:
top_endpoint_qps,endpoint_request_freq) whichclassify_demo_metricroutes directly, bypassing the analyzer's bare-name handling.🤖 Generated with Claude Code