Skip to content

Commit 5187e99

Browse files
LloydW93fisherdarling
authored andcommitted
Add compile-time validation for histogram buckets in foundations-macros
1 parent ccbdef2 commit 5187e99

File tree

2 files changed

+137
-1
lines changed

2 files changed

+137
-1
lines changed

foundations-macros/src/metrics/mod.rs

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use syn::{
99
};
1010

1111
mod parsing;
12+
mod validation;
1213

1314
#[derive(FromMeta)]
1415
struct MacroArgs {
@@ -279,6 +280,18 @@ fn metric_init(foundations: &Path, fn_: &ItemFn) -> proc_macro2::TokenStream {
279280
Span::call_site(),
280281
);
281282

283+
// Validate histogram buckets at compile time if this is a HistogramBuilder
284+
if let Some(ctor) = ctor {
285+
if let Some(path) = ctor.path.get_ident() {
286+
if path == "HistogramBuilder" {
287+
// Validate the histogram buckets
288+
if let Err(err) = validation::validate_histogram_buckets(ctor) {
289+
return err.to_compile_error();
290+
}
291+
}
292+
}
293+
}
294+
282295
let metric_init = match ctor {
283296
Some(ctor) if args.is_empty() => quote! {
284297
#reexports::prometheus_client::metrics::family::MetricConstructor::new_metric(&(#ctor))
@@ -378,7 +391,8 @@ fn metric_fn(foundations: &Path, metrics_struct: &Ident, fn_: &ItemFn) -> proc_m
378391
mod tests {
379392
use super::*;
380393
use crate::common::test_utils::{code_str, parse_attr};
381-
use syn::parse_quote;
394+
use crate::metrics::validation;
395+
use syn::{parse_quote, ExprStruct};
382396

383397
#[test]
384398
fn expand_empty() {
@@ -736,4 +750,64 @@ mod tests {
736750

737751
assert_eq!(actual, expected);
738752
}
753+
754+
#[test]
755+
fn test_valid_histogram_buckets() {
756+
// Valid strictly increasing buckets
757+
let expr: ExprStruct = parse_quote! {
758+
HistogramBuilder { buckets: &[0.1, 0.2, 0.3, 0.4] }
759+
};
760+
761+
assert!(validation::validate_histogram_buckets(&expr).is_ok());
762+
}
763+
764+
#[test]
765+
fn test_empty_histogram_buckets() {
766+
// Empty buckets are valid
767+
let expr: ExprStruct = parse_quote! {
768+
HistogramBuilder { buckets: &[] }
769+
};
770+
771+
assert!(validation::validate_histogram_buckets(&expr).is_ok());
772+
}
773+
774+
#[test]
775+
fn test_invalid_histogram_buckets_equal_values() {
776+
// Invalid buckets with equal values
777+
let expr: ExprStruct = parse_quote! {
778+
HistogramBuilder { buckets: &[0.1, 0.2, 0.2, 0.3] }
779+
};
780+
781+
assert!(validation::validate_histogram_buckets(&expr).is_err());
782+
}
783+
784+
#[test]
785+
fn test_invalid_histogram_buckets_decreasing_values() {
786+
// Invalid buckets with decreasing values
787+
let expr: ExprStruct = parse_quote! {
788+
HistogramBuilder { buckets: &[0.3, 0.2, 0.1] }
789+
};
790+
791+
assert!(validation::validate_histogram_buckets(&expr).is_err());
792+
}
793+
794+
#[test]
795+
fn test_skip_validation_for_variable_buckets() {
796+
// Variable buckets should skip validation
797+
let expr: ExprStruct = parse_quote! {
798+
HistogramBuilder { buckets: MY_BUCKETS }
799+
};
800+
801+
assert!(validation::validate_histogram_buckets(&expr).is_ok());
802+
}
803+
804+
#[test]
805+
fn test_skip_validation_for_function_buckets() {
806+
// Function call buckets should skip validation
807+
let expr: ExprStruct = parse_quote! {
808+
HistogramBuilder { buckets: get_buckets() }
809+
};
810+
811+
assert!(validation::validate_histogram_buckets(&expr).is_ok());
812+
}
739813
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
use syn::{Error, Expr, ExprLit, ExprStruct, Lit, Member, Result};
2+
3+
/// Validates that histogram bucket values are strictly increasing.
4+
/// Returns an error if the buckets are not valid.
5+
pub fn validate_histogram_buckets(expr: &ExprStruct) -> Result<()> {
6+
// Extract the buckets field from the HistogramBuilder struct
7+
let buckets_field = expr
8+
.fields
9+
.iter()
10+
.find(|field| {
11+
if let Member::Named(ident) = &field.member {
12+
ident == "buckets"
13+
} else {
14+
false
15+
}
16+
})
17+
.ok_or_else(|| Error::new_spanned(expr, "HistogramBuilder must have a 'buckets' field"))?;
18+
19+
// Extract the array expression from the buckets field
20+
let array_expr = match &buckets_field.expr {
21+
Expr::Reference(ref_expr) => match &*ref_expr.expr {
22+
Expr::Array(array) => array,
23+
// For non-array literals (like variables or function calls), skip validation
24+
_ => return Ok(()),
25+
},
26+
// For non-reference expressions, skip validation
27+
_ => return Ok(()),
28+
};
29+
30+
// Extract the f64 values from the array
31+
let mut values = Vec::new();
32+
for elem in &array_expr.elems {
33+
if let Expr::Lit(ExprLit {
34+
lit: Lit::Float(lit_float),
35+
..
36+
}) = elem
37+
{
38+
let value = lit_float.base10_parse::<f64>()?;
39+
values.push((value, lit_float.span()));
40+
} else {
41+
// For non-float literals (like variables or expressions), skip validation
42+
return Ok(());
43+
}
44+
}
45+
46+
// Check if the buckets are strictly increasing
47+
if !values.is_empty() {
48+
let mut prev_value = values[0].0;
49+
for (i, (value, span)) in values.iter().enumerate().skip(1) {
50+
if *value <= prev_value {
51+
let message = format!(
52+
"Histogram buckets must be strictly increasing. Found invalid bucket at position {}: {} <= {}",
53+
i, value, prev_value
54+
);
55+
return Err(Error::new(*span, message));
56+
}
57+
prev_value = *value;
58+
}
59+
}
60+
61+
Ok(())
62+
}

0 commit comments

Comments
 (0)