diff --git a/src/engine.rs b/src/engine.rs index 71f8144..ab23b34 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1222,7 +1222,7 @@ fn merge_series_locf(sub_series: &[Vec<(i64, f64)>], is_avg: bool) -> Vec<(i64, .collect() } -/// 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. fn evaluate_promql( index: &BTreeMap, parsed: &PromqlQuery, @@ -1313,20 +1313,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 = 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, 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 = 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() } @@ -2220,6 +2230,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); + 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) { declare_metric( index,