fix: planner binds all 6 contract metrics into routing table (HLL/CountSketch/CountMin) - #345
Merged
Merged
Conversation
…Sketch/CountMin) (#46) Two stitches were missing for MVP §46 entries 5–8 — the typed planner had to be reached with both the operator's `sketch_family_override` *and* the parsed-query `exact_required` flag turned off: 1. `WorkloadEntry` (config/workloads.rs) silently ignored the `sketch_family_override` / `target_path` keys (serde unknown-field default) so the registry-walk pre-pop in `main()` always set `QuerySpec.sketch_type = None`. Add the field with a case-insensitive `SketchType` deserialiser (the live YAML spells `KLL` / `HLL` / `CountSketch` / `CountMinSketch` mixed-case while the `#[serde(rename_all = "lowercase")]` enum would otherwise reject them) and thread it through `main`'s pre-pop loop. 2. `bind_workload_typed` (planner/rules.rs:81) early-returned `None` on `w.exact_required = true`. The query parser sets that flag when the inner walk synthesises a bare-VectorSelector → `AggFunc::Sum` (the path `count(metric)`, `topk(K, metric)`, and `rate(metric[5m])` all hit) — so for entries 6–8 the typed binder declined before the metric-name match ever ran. Carve out an exception: when the metric is a contract row (`classify_demo_metric` returns `Some`) or carries an explicit `sketch_type_override`, those signals win over the parser's `exact_required` flag because both are explicit "sketch this metric" instructions from the operator/contract. Tests added: - `config::workloads::tests::deserialize_sketch_family_override_*` — deserialisation accepts both mixed-case (`KLL`) and lowercase / alias spellings (`countmin`, `cms`). - `config::workloads::tests::live_mvp_workload_yaml_loads_with_overrides` — smoke-test the live `deploy/configs/mvp-workload.yaml`. - `config::runtime_tests::collect_metric_to_family_binds_all_six_…` — registry-walk path produces a 5-entry routing table (5 sketches, raw declines). - `api_tests::bootstrap_routing_table_covers_all_five_sketches_for_live_mvp_yaml` — end-to-end via the bootstrap GET endpoint, with metric names exactly as they appear in the live demo YAML. Curl evidence (controller running with `CONTROLLER_WORKLOADS= deploy/configs/mvp-workload.yaml USE_TYPED_STAGE_SPLIT=1`): routing: - route() where metric.name == "endpoint_request_freq" → metrics/countminsketch_path - route() where metric.name == "http_requests_total_latency_ms" → metrics/ddsketch_path - route() where metric.name == "request_size_bytes" → metrics/kll_path - route() where metric.name == "top_endpoint_qps" → metrics/countsketch_path - route() where metric.name == "unique_users_per_min" → metrics/hll_path - route() where metric.name == "http_freshness_probe_warm" → metrics/raw_passthrough - route() where metric.name == "http_freshness_probe_archive" → metrics/raw_passthrough before this change the routing table only carried the DDSketch and KLL rows; HLL / CountSketch / CountMinSketch silently dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 8, 2026
…equests_total_latency_ms) (#346) PR #337's SKETCH_FAMILIES table used `http_latency_ms` as the DDSketch metric, but the actual workload-spec entry (and fake-exporter emit) is `http_requests_total_latency_ms`. Result: post-PR-#345 demo runs reported DDSketch row n=0 even though warm tier was answering quantile queries — the rel-err values were attributed to no family. This aligns the table with the live workload-spec. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 8, 2026
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
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
The 5-sketch routing-connector wire YAML emitted by the bootstrap GET
(
/api/v1/collector-config/agent) was missing 3 of the 6 MVP §46 contractmetrics. Symptom from the live demo:
The workload registry loaded all 8 entries (
count=8), butcollect_metric_to_familywalked them and only emitted 2 sketched routes.Root cause — two missing stitches
WorkloadEntrysilently ignoredsketch_family_override. The struct incontroller/src/config/workloads.rshad no field for it; serde defaultsto ignoring unknown fields, so the operator's
sketch_family_override: HLL/CountSketch/CountMinSketchround-tripped toNone. Thepre-pop loop in
main()then passedQuerySpec.sketch_type = Nonetothe analyzer.
bind_workload_typedearly-returned onw.exact_required = trueatcontroller/src/planner/rules.rs:81. The PromQL parser sets that flagwhen the inner walk synthesises an
AggFunc::Sumover a bareVectorSelector— which happens forcount(metric),topk(K, metric), andrate(metric[5m])(thequery_strings onworkload entries 6, 7, 8). The metric-name match in
classify_demo_metricand thesketch_type_overridefield never gotconsulted because the early return happened first.
Carved out an exception: when the metric is a contract row
(
classify_demo_metricreturnsSome) or carries an explicit override,those signals win over
exact_requiredbecause both are explicit"sketch this metric" instructions.
Files
controller/src/config/workloads.rs— addsketch_family_override(case-insensitive
SketchTypedeserialiser) andtarget_pathfieldsto
WorkloadEntry; newWorkloadRegistry::from_entriesfor testing.controller/src/main.rs— threadentry.sketch_family_overrideintoQuerySpec.sketch_typein the registry pre-pop loop (lines ~234).controller/src/planner/rules.rs:81-99— guard theexact_requiredearly-return with the contract-row / operator-override carve-out.
controller/src/config/mod.rs+controller/src/main.rs— new tests.Curl evidence (after fix)
5 sketched metric routes (DDSketch / KLL / HLL / CountSketch /
CountMinSketch) + 2 freshness probes — full 5-sketch coverage.
Test plan
cargo test --release --bin controller— 608 passed (was 603 onorigin/main; +5 new tests, +0 regressions; same 10 pre-existing failures).
bootstrap_routing_table_covers_all_five_sketches_for_live_mvp_yamlpins the contract: registry-walk path produces a 5-entry routing
table for the live
deploy/configs/mvp-workload.yamlshape.confirms all 5 sketched metrics + both freshness probes route correctly.
live_mvp_workload_yaml_loads_with_overridessmoke-tests theactual deploy file deserialises with all four overrides intact.
Closes #46.
🤖 Generated with Claude Code