From 2dfcdaaa81dadff21f73571c7c0e57c364141b05 Mon Sep 17 00:00:00 2001 From: zz_y Date: Sun, 10 May 2026 19:01:04 -0600 Subject: [PATCH] fix: warm-tier topk preserves per-element label keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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` to a `KeyByLabelValues` (values only — `Vec`). 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>` 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) --- asap-query-engine/src/engines/query_result.rs | 18 +++++++++++++ .../src/engines/simple/engine.rs | 25 +++++++++++-------- asap-query-engine/src/utils/http.rs | 14 +++++++++-- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/asap-query-engine/src/engines/query_result.rs b/asap-query-engine/src/engines/query_result.rs index 9c89d152e..ea4d04c4d 100644 --- a/asap-query-engine/src/engines/query_result.rs +++ b/asap-query-engine/src/engines/query_result.rs @@ -181,6 +181,16 @@ pub struct RangeVector { pub struct RangeVectorElement { pub labels: KeyByLabelValues, pub samples: Vec, + /// Optional per-element label-key override. When `Some`, the + /// HTTP serializer uses these keys for the PromQL response's + /// `"metric"` object instead of the query-scoped + /// `KeyByLabelNames` argument. Used by warm-tier `topk` (whose + /// reducer synthesizes an `"item"` key not present in the + /// query's group-by clause) and other adapters that materialize + /// labels the caller doesn't know about. `None` for everyone + /// else — the existing serializer path is unaffected. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub label_keys_override: Option>, } /// A single sample (timestamp, value) pair @@ -201,9 +211,17 @@ impl RangeVectorElement { Self { labels, samples: Vec::new(), + label_keys_override: None, } } + /// Attach a per-element label-key override (see field doc on + /// `RangeVectorElement::label_keys_override`). + pub fn with_label_keys_override(mut self, keys: Vec) -> Self { + self.label_keys_override = Some(keys); + self + } + pub fn add_sample(&mut self, timestamp: u64, value: f64) { self.samples.push(Sample::new(timestamp, value)); } diff --git a/asap-query-engine/src/engines/simple/engine.rs b/asap-query-engine/src/engines/simple/engine.rs index 7735f837c..27c8e8ac8 100644 --- a/asap-query-engine/src/engines/simple/engine.rs +++ b/asap-query-engine/src/engines/simple/engine.rs @@ -3585,16 +3585,21 @@ fn warm_tier_result_to_query_result( let mut elements: Vec = Vec::with_capacity(result.series.len()); for (label_values, samples) in result.series { - // `KeyByLabelValues` is a `Vec` carrying VALUES only. - // We project the BTreeMap's values in key-sorted order - // (BTreeMap iteration order matches the `group_by_keys` - // BTreeSet iteration order, so the result preserves the - // sketch instance's group-by-key projection without - // re-emitting the keys). - let labels = KeyByLabelValues::new_with_labels( - label_values.into_values().collect::>(), - ); - let mut element = RangeVectorElement::new(labels); + // `KeyByLabelValues` is a `Vec` carrying VALUES only; + // the serializer pairs them with KEYS from a query-scoped + // `KeyByLabelNames`. For most queries the keys ARE the + // query's group-by clause, so the default path works. But + // warm-tier `topk` synthesizes an `"item"` key (the top-k + // entry name) that the original query's group-by doesn't + // carry — without an override the serializer drops it and + // the response shows `"metric": {}`. Project the BTreeMap's + // VALUES in key-sorted order (BTreeMap iteration is + // key-sorted), and stash the BTreeMap's KEYS in the + // per-element override so the serializer can pair them + // correctly. + let (keys, values): (Vec, Vec) = label_values.into_iter().unzip(); + let labels = KeyByLabelValues::new_with_labels(values); + let mut element = RangeVectorElement::new(labels).with_label_keys_override(keys); for (window_end_ms, value) in samples { // `window_end_ms` is i64 from the index; cast to u64 // for the wire format (window_end is monotonic + post- diff --git a/asap-query-engine/src/utils/http.rs b/asap-query-engine/src/utils/http.rs index 9b5c4fd68..cadf82c99 100644 --- a/asap-query-engine/src/utils/http.rs +++ b/asap-query-engine/src/utils/http.rs @@ -175,9 +175,19 @@ pub fn convert_range_result_to_prometheus( .values .iter() .map(|element| { - // Build metric labels object + // Build metric labels object. Per-element override + // (`element.label_keys_override`) wins when the + // adapter knows the keys at materialization time — + // e.g. warm-tier `topk` synthesizes an `"item"` key + // that's not in the query's group-by clause, so the + // outer `label_names` doesn't carry it. Falls back + // to the query-scoped key list for everyone else. let mut metric = serde_json::Map::new(); - for (i, label_name) in label_names.labels.iter().enumerate() { + let effective_keys: &[String] = element + .label_keys_override + .as_deref() + .unwrap_or(&label_names.labels); + for (i, label_name) in effective_keys.iter().enumerate() { if i < element.labels.labels.len() { metric.insert( label_name.clone(),