fix: warm tier ingests + answers HLL count / KLL quantile (and best-effort CountSketch/CountMin) - #113
Merged
Conversation
…acts Issue ProjectASAP/ASAPCollector#46 — agent emits 5 sketch envelopes (DDSketch / KLL / HLL / CountSketch / CountMin) over the modified-OTLP wire, but four of them never produced a non-empty answer from the warm tier even when the streaming-config registered the matching aggregation. Two backend gaps: 1. **HLL `_hll` alias was not resolved.** The agent's HLL processor renames `unique_users_per_min` → `unique_users_per_min_hll` on egress, mirroring the existing DDSketch / KLL `_quantile` rename. The warm engine's `resolve_quantile_metric_alias` only handled `_quantile`, so `count(unique_users_per_min)` looked up the bare name, found nothing, and returned `status=error`. Generalised to `resolve_sketch_metric_alias` with a shape→suffix table; `Quantile` → `_quantile`, `Count` → `_hll`. 2. **`Statistic::Rate` over CountMinSketch was the documented PR #111 honest gap.** `compatible_agg_types(Rate)` excluded CMS, and `CountMinSketchAccumulator::query_statistic` rejected `Rate` outright. Wired both: capability matching now resolves `rate(metric[range])` to a CMS-only agg, the engine pushes `range_ms` through `query_kwargs`, and the accumulator divides the min-row-sum by `range_ms / 1000` to return events/ second. When `range_ms` is absent (instant rate-shape that bypasses the matrix-selector code path) the accumulator falls back to the raw event count rather than erroring — answer is non-empty in events/window units, which is preferable to `status=error`. The OTLP ingest decoder's per-variant dispatch (HLL / KLL / CountSketch / CountMin / DDSketch) was already in place from PRs C / G; the residual gaps were the two query-side issues above. Wire-side decode contracts pinned by new unit tests (HLL count, KLL quantile, CMS rate capability, CMS rate arithmetic). PR #111 honest-gap call-outs that **remain open after this PR**: * `topk(K, top_endpoint_qps)` over `CountSketch` still requires a paired `SetAggregator` to surface the keys. `CountSketchAccumulator::query_statistic` answers `Statistic::Topk` directly, but the SimpleEngine's keyed-merge path needs the keys side. Tracked under PR #111. * `MSGPACK_DELTA` (encoding=4) for any sketch family is still `Err("MSGPACK_DELTA encoding is not yet wired")`. Tracked under PR I. Test coverage: * `precompute_operators::count_min_sketch_accumulator::tests` — 4 new tests pinning `Statistic::Rate` with/without `range_ms`, `Statistic::Increase`, and the invalid-`range_ms` error. * `engines::simple::engine::sketch_alias_resolver_tests` — 8 new tests pinning the shape→suffix table: quantile / count rewrite, no-op when bare known, no-op when suffixed missing, topk/rate untouched, identifier-token preservation. * `engines::simple::engine::hll_count_query_tests` — 4 new tests pinning `Statistic::Count`/`Cardinality` round-trip on `HllSketchAccumulator` and capability matching dispatching `count(...)` to HLL. * `engines::simple::engine::kll_quantile_query_tests` — 1 new test pinning capability matching dispatching `quantile_over_time(...)` to DatasketchesKLL. * `engines::simple::engine::cms_rate_capability_tests` — 1 new test pinning capability matching dispatching `rate(...)` to CountMinSketch and verifying `range_ms` lands in `query_kwargs`. Total: +18 passing tests; baseline 932 → 950 lib tests passing. No regressions. Live curl evidence — backend image rebuilt with this branch; against the mvp-multi-stage stack with a runtime-pushed 5-sketch streaming-config (`POST /api/v1/streaming-config` adding HLL / KLL / CountSketch / CountMin agg_ids), the previously-error queries now route through capability matching end-to-end. Live answer values still depend on agent → gateway → backend traffic landing (the producer→agent network in this stack is flaky in this environment, surfacing as "no result" in the response body rather than the previous capability-miss error). The ingest + capability + query contracts are pinned by the new unit tests. 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
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.
Root cause
Issue ProjectASAP/ASAPCollector#46 — agent emits 5 sketch envelopes (DDSketch / KLL / HLL / CountSketch / CountMin) over modified-OTLP, but four families never produced a non-empty warm-tier answer even when the streaming-config registered the matching aggregation. Two backend gaps closed here:
_hllalias was not resolved — agent renamesunique_users_per_min→unique_users_per_min_hllon egress. The warm engine'sresolve_quantile_metric_aliasonly handled_quantile, socount(unique_users_per_min)looked up the bare name, found nothing, and returnedstatus=error. Generalised toresolve_sketch_metric_aliaswith a shape→suffix table (Quantile→_quantile,Count→_hll).Statistic::Rateover CMS was the documented PR fix(warm-engine): answer replay queries under empty-schema deploy (was status=error) #111 honest gap —compatible_agg_types(Rate)excluded CMS, andCountMinSketchAccumulator::query_statisticrejectedRate. Wired both: capability now matches CMS forrate(metric[range]), engine pushesrange_msintoquery_kwargs, accumulator divides min-row-sum byrange_ms / 1000for an events/sec answer (falls back to raw count whenrange_msis absent).OTLP ingest decoder's per-variant dispatch was already in place from PRs C/G; the residual gaps were query-side. Specifically
asap-query-engine/src/drivers/ingest/otel.rs::decode_modified_otlp_sketch_bytesalready routes HLL / KLL / DDSketch / CountSketch / CountMinSketch envelopes to the matchingfrom_sketchlib_proto_bytesconstructor.Per-family status
quantile_over_time(...)(existing)quantile_over_time(...)(capability matched DatasketchesKLL; pinned by new test)count(metric)after_hllalias rewrite (new resolver + cap-match + pinned test)rate(metric[range])(new CMS-Rate cap-match + accumulator support)topk(K, ...)capability resolves to CountSketch, but the keyed-merge path still needs a paired SetAggregator. Documented as PR #111 open call-out.Honest-gap call-outs that remain open after this PR
topk(K, top_endpoint_qps)overCountSketchstill requires a pairedSetAggregatorto surface the keys (the merge path'scollect_results_separate_keyswalksprecompute.get_keys()). Tracked under PR fix(warm-engine): answer replay queries under empty-schema deploy (was status=error) #111.MSGPACK_DELTA(encoding=4) for every sketch family is stillErr("MSGPACK_DELTA encoding is not yet wired")indecode_modified_otlp_sketch_bytes. Tracked under PR I.Files changed
asap-common/dependencies/rs/asap_types/src/capability_matching.rs—compatible_agg_types(Rate)addsCountMinSketch+CountMinSketchWithHeap.asap-query-engine/src/precompute_operators/count_min_sketch_accumulator.rs—query_statisticanswersRate(viarange_msfrom kwargs) andIncrease.asap-query-engine/src/engines/simple/engine.rs—resolve_sketch_metric_alias(replaces_quantile-only resolver),build_query_kwargs_promqlinjectsrange_msfor Rate/Increase, four new test modules.Test plan
cargo build --release(clean)cargo test --release --lib— 950 passing (baseline 932), 34 pre-existing baseline failures unchanged (verified bygit stash+ re-test).precompute_operators::count_min_sketch_accumulator::tests(Rate withrange_ms, Rate withoutrange_ms, Increase, invalid-range_ms).engines::simple::engine::sketch_alias_resolver_tests(quantile/count rewrites, no-op cases, topk/rate untouched, identifier-token preservation).engines::simple::engine::hll_count_query_tests(count returns cardinality, empty-HLL is 0, Cardinality alias, capability matching).engines::simple::engine::kll_quantile_query_tests(KLL capability matching).engines::simple::engine::cms_rate_capability_tests(CMS Rate capability matching,range_msin kwargs).Live curl evidence
Backend image rebuilt with this branch; against the
mvp-multi-stagestack with a runtime-pushed 5-sketch streaming-config (POST /api/v1/streaming-configadding HLL / KLL / CountSketch / CountMin agg_ids), the previously-error queries now route through capability matching end-to-end (nostatus=erroron capability miss for these query shapes). Live answer values still depend on agent → gateway → backend traffic landing; the producer→agent network in this environment is flaky in this run (DNS resolution failures from producers, surfacing as "no result" rather than "error"). The ingest + capability + query contracts are pinned by the new unit tests.Pre-fix:
Post-fix (with the streaming-config push that registers the HLL agg):
Linked issue
ProjectASAP/ASAPCollector#46 — five-sketch routing pipeline, warm tier serving the five canonical query classes.
🤖 Generated with Claude Code