Skip to content

fix(query): dispatch Vector vs Matrix in asap_tier_result_to_query_result - #256

Merged
zzylol merged 1 commit into
mainfrom
fix-cardinality-response
May 16, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix-cardinality-response

Conversation

@zzylol

@zzylol zzylol commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the response-serialization gap that #255 flagged as the last blocker for Test 5 (HLL roundtrip). With #255's analyzer + reducer fixes, the ASAP-tier engine path went the full distance — idx.sids_for_policy(fp) resolved correctly, SketchReducer::evaluate returned Ok(...) — but the HTTP response came back empty (reqwest::Error: EOF while parsing a value).

Root cause

asap_tier_result_to_query_result unconditionally wrapped the result in QueryResult::matrix(...). The Prometheus adapter's format_success_response requires resultType: vector for instant queries (those the analyzer marked range_seconds == 0, i.e. no [range] selector — count(metric), quantile(...)) and resultType: matrix for range queries. A Matrix response for an instant request gets rejected with HTTP 500 and an empty body.

Fix

Thread the analyzer's range_seconds through to the result builder. If any candidate is range-shaped, build a Matrix; otherwise project the latest sample per series into an InstantVectorElement and wrap as Vector (with now_ms as the wire timestamp).

Test 5 (HLL e2e roundtrip) now passes strict success

The full chain — controller plan → POST /api/v1/streaming-config → OTLP HLL DPs → window close → GET /api/v1/query?query=count(metric)status: success — works end-to-end.

All 5 e2e tests now pass

Test Path Status
1 streaming-config round-trip (DDSketch)
2 grouping plumb (zone label)
3 DDSketch quantile full roundtrip
4 KLL quantile full roundtrip
5 HLL cardinality full roundtrip

Test plan

  • cargo test --test e2e_controller_plans_and_backend_serves: 5 passed; 0 failed; 0 ignored
  • Full sweep: lib 691 + bins 27 + integration tests all green

🤖 Generated with Claude Code

…sult

Closes the response-serialization gap that #255's PR-body flagged as
the last blocker for Test 5 (HLL roundtrip). With #255's analyzer +
reducer fixes in place, the ASAP-tier engine path went the full
distance — `idx.sids_for_policy(fp)` resolved correctly,
`SketchReducer::evaluate` returned `Ok(...)` — but the HTTP response
came back empty (`reqwest::Error: EOF while parsing a value`).

Root cause: `asap_tier_result_to_query_result` unconditionally wrapped
the result in `QueryResult::matrix(...)`. The Prometheus adapter's
`format_success_response` requires `resultType: vector` for instant
queries (those the analyzer marked `range_seconds == 0`, i.e. no
`[range]` selector — `count(metric)`, `quantile(...)`, etc.) and
`resultType: matrix` for range queries (`*_over_time(...)[range]`).
A Matrix response for an instant request gets rejected with HTTP 500
and an empty body.

Fix: thread the analyzer's `range_seconds` through to the result
builder. If any candidate is range-shaped, build a Matrix; otherwise
project the latest sample per series into an `InstantVectorElement`
and wrap as `Vector` (with `now_ms` as the wire timestamp).

`InstantVectorElement` doesn't carry a per-element
`label_keys_override` today (only `RangeVectorElement` does, for the
topk-`item`-key case) — labels render against the query-scoped
`KeyByLabelNames` the serializer holds. Correct for cardinality (the
only instant-vector consumer today); a future per-element override
on `InstantVectorElement` is plumbable when needed.

## Test 5 (HLL e2e roundtrip) now passes strict success

The full chain — controller plan → POST /api/v1/streaming-config →
OTLP HLL DPs → window close → GET /api/v1/query?query=count(metric)
→ status: success — works end-to-end. All 5 e2e tests now pass:

  * Test 1: streaming-config round-trip (DDSketch)
  * Test 2: grouping plumb (zone label)
  * Test 3: full roundtrip DDSketch quantile → strict success
  * Test 4: full roundtrip KLL quantile → strict success
  * Test 5: full roundtrip HLL cardinality → strict success

## Tests

- `cargo test --test e2e_controller_plans_and_backend_serves`:
  **5 passed; 0 failed; 0 ignored**.
- Full sweep: lib 691 + bins 27 + integration tests all green.

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

Mirrors `RangeVectorElement` — adds an optional `label_keys_override:
Option<Vec<String>>` field so per-element keys (synthesized by
ASAP-tier `topk(...)` as `"item": <key>`) surface through the
Prometheus adapter on instant-vector responses, not just range-vector.

Three-line change spanning three files:
  - `InstantVectorElement` gains the field + `with_label_keys_override`
    helper (query_result.rs).
  - `asap_tier_result_to_query_result`'s instant-vector branch stops
    discarding the BTreeMap keys (engine.rs:3443) — the same fix
    PR #256 applied to the range-vector branch.
  - `convert_query_result_to_prometheus` picks per-element override
    over the query-scoped `KeyByLabelNames`, mirroring
    `convert_range_result_to_prometheus` (http.rs).

Tests 8+9 in `e2e_controller_plans_and_backend_serves.rs` tighten
their `topk(3, top_endpoint_qps)` assertions to verify
`metric.item == "gamma"` is present in at least one returned series
— the strict invariant that previously couldn't be checked because
the adapter dropped the synthesized key.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the fix-cardinality-response 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