Merge ticket 043 (histogram by() support), resolve doc-comment conflict

Also fix 043's histogram-by-clause test to unwrap the Result now returned
by evaluate_promql (introduced by ticket 041).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
Hugh Kaznowski
2026-09-05 22:45:55 +01:00
co-authored by Claude Sonnet 5
+59 -13
View File
@@ -1354,7 +1354,7 @@ fn declared_value_kind(
}
}
/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point.
/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point; `histogram()` groups by `by_labels` the same way sum()/avg() do, then buckets each group's values independently, so `histogram(m) by (label)` yields one histogram (a family of `le`-labeled series carrying the group's own labels) per label group instead of collapsing all groups together.
/// `sum()`/`avg()` over a `String`-kind metric are rejected up front — `coerce_to_f64` has no
/// numeric reading for a string, so silently treating every occurrence as `1.0` would let a sum
/// pass for a count with no signal to the caller. `sum(rate(...))`/`avg(rate(...))` are exempt:
@@ -1479,20 +1479,30 @@ fn evaluate_promql(
.flat_map(|points| points.iter().map(|(ts, _)| *ts))
.max()
.unwrap_or(0);
// Buckets every value (across all label combinations, collapsing the label dimension like sum()/avg()) into fixed-width buckets, labeled `le` (Prometheus's less-than-or-equal convention).
let mut counts: BTreeMap<i64, u64> = BTreeMap::new();
for points in series_points.values() {
for (_, value) in points {
let bucket_index = (value / bucket_width).floor() as i64;
*counts.entry(bucket_index).or_insert(0) += 1;
}
// Groups series by `by_labels` first (empty `by_labels` collapses to a single group,
// matching sum()/avg()'s behavior), then buckets each group's values independently into
// fixed-width buckets, labeled `le` (Prometheus's less-than-or-equal convention) alongside
// the group's own by_labels values.
let mut groups: BTreeMap<BTreeMap<String, String>, Vec<&(i64, f64)>> = BTreeMap::new();
for (labels, points) in &series_points {
let group_key = project_labels(labels, &parsed.by_labels);
groups.entry(group_key).or_default().extend(points.iter());
}
counts
groups
.into_iter()
.map(|(bucket_index, count)| {
let upper_bound = (bucket_index + 1) as f64 * bucket_width;
let labels = BTreeMap::from([("le".to_string(), upper_bound.to_string())]);
PromqlSeriesDto { labels, points: vec![(last_ts, count as f64)] }
.flat_map(|(group_labels, points)| {
let mut counts: BTreeMap<i64, u64> = BTreeMap::new();
for (_, value) in points {
let bucket_index = (value / bucket_width).floor() as i64;
*counts.entry(bucket_index).or_insert(0) += 1;
}
counts.into_iter().map(move |(bucket_index, count)| {
let upper_bound = (bucket_index + 1) as f64 * bucket_width;
let mut labels = group_labels.clone();
labels.insert("le".to_string(), upper_bound.to_string());
PromqlSeriesDto { labels, points: vec![(last_ts, count as f64)] }
})
})
.collect()
}
@@ -2433,6 +2443,42 @@ mod tests {
assert_eq!(series[2].points[0].1, 1.0);
}
#[test]
fn histogram_with_by_clause_buckets_each_label_group_separately() {
let mut index = BTreeMap::new();
declare_durations_metric(&mut index);
// path=/a: 2,4 -> le=10 (count 2); 12 -> le=20 (count 1).
// path=/b: 21 -> le=30 (count 1).
for (seq, path, duration) in [
(0, "/a", 2),
(1, "/a", 4),
(2, "/a", 12),
(3, "/b", 21),
] {
ingest_log(&mut index, seq, &format!("request path={path} duration={duration}"));
}
let parsed = promql::parse_query("histogram(durations[10]) by (path)").unwrap();
let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
series.sort_by(|a, b| {
a.labels.get("path").cmp(&b.labels.get("path")).then(a.labels.get("le").cmp(&b.labels.get("le")))
});
assert_eq!(series.len(), 3);
assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
assert_eq!(series[0].labels.get("le").map(String::as_str), Some("10"));
assert_eq!(series[0].points[0].1, 2.0);
assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/a"));
assert_eq!(series[1].labels.get("le").map(String::as_str), Some("20"));
assert_eq!(series[1].points[0].1, 1.0);
assert_eq!(series[2].labels.get("path").map(String::as_str), Some("/b"));
assert_eq!(series[2].labels.get("le").map(String::as_str), Some("30"));
assert_eq!(series[2].points[0].1, 1.0);
}
fn declare_login_duration_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
let pattern = r"user=(?P<user>\S+) logged in";
declare_test_param(index, "user", pattern, "user", MetricPrimitiveKind::String);