Skip to content

fix: warm-tier topk preserves per-element label keys - #127

Merged
zzylol merged 1 commit into
mainfrom
fix/topk-label-keys-and-mvp-report-resilience
May 11, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/topk-label-keys-and-mvp-report-resilience

Conversation

@zzylol

@zzylol zzylol commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

#124's warm-tier topk reducer synthesizes an "item" label key for each top-k entry (the item identity), but the warm_tier_result_to_query_result adapter collapsed the result's BTreeMap<key,value> to a KeyByLabelValues (values only — Vec<String>). The HTTP serializer pairs those values with KEYS from a query-scoped KeyByLabelNames, which carries the PromQL group-by clause — and topk(5, foo) has NO group-by. So the synthesized "item" key disappeared and the PromQL response showed "metric": {} for every top-k entry.

This is the bug surfaced in the MVP demo's smoke run: topk(5, top_endpoint_qps) returned 5 anonymous {} objects instead of named items.

Fix

  • RangeVectorElement gains an optional label_keys_override: Option<Vec<String>> field. Default None — all existing constructors unaffected, no breaking change.
  • RangeVectorElement::with_label_keys_override(keys) builder.
  • convert_range_result_to_prometheus uses the override when present, falls back to the query-scoped label_names otherwise.
  • warm_tier_result_to_query_result populates the override from the BTreeMap's keys (BTreeMap iteration is key-sorted, so it pairs correctly with the values pulled from into_values()).

This applies to every warm-tier path, not just topk — Capability::FrequencyTopk synthesizes "item", but QuantileApprox / CardinalityApprox paths also benefit when the sketch instance's group_by_keys differs from the query's group-by clause.

Verification

  • cargo build --release -p query_engine_rust clean
  • cargo test --release -p query_engine_rust --lib -- engines::warm_tier engines::query_result — 30/30 pass

Diff

asap-query-engine/src/engines/query_result.rs  | 18 ++++++++++
asap-query-engine/src/engines/simple/engine.rs | 25 +++++++------
asap-query-engine/src/utils/http.rs            | 14 +++++++-
3 files changed, 45 insertions(+), 12 deletions(-)

🤖 Generated with Claude Code

#124's warm-tier topk reducer synthesizes an `"item"` label key for
each top-k entry (the item identity), but the
`warm_tier_result_to_query_result` adapter then collapsed the
result's `BTreeMap<key,value>` to a `KeyByLabelValues` (values
only — `Vec<String>`). The HTTP serializer pairs those values with
KEYS from a query-scoped `KeyByLabelNames`, which carries the
PromQL group-by clause — and the PromQL `topk(5, foo)` has NO
group-by. So the synthesized `"item"` key disappeared and the
PromQL response showed `"metric": {}` for every top-k entry.

Fix:
- `RangeVectorElement` gains an optional
  `label_keys_override: Option<Vec<String>>` field. Default
  `None` — all existing constructors unaffected.
- `convert_range_result_to_prometheus` uses the override when
  present, falls back to the query-scoped `label_names` otherwise.
- `warm_tier_result_to_query_result` populates the override from
  the BTreeMap's keys (BTreeMap iteration is key-sorted, so it
  pairs correctly with the values).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit afbe0ef into main May 11, 2026
@zzylol
zzylol deleted the fix/topk-label-keys-and-mvp-report-resilience branch May 11, 2026 01:01
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