diff --git a/foundations-macros/src/metrics/mod.rs b/foundations-macros/src/metrics/mod.rs index 92c7645..d2a1d57 100644 --- a/foundations-macros/src/metrics/mod.rs +++ b/foundations-macros/src/metrics/mod.rs @@ -9,6 +9,7 @@ use syn::{ }; mod parsing; +mod validation; #[derive(FromMeta)] struct MacroArgs { @@ -279,6 +280,18 @@ fn metric_init(foundations: &Path, fn_: &ItemFn) -> proc_macro2::TokenStream { Span::call_site(), ); + // Validate histogram buckets at compile time if this is a HistogramBuilder + if let Some(ctor) = ctor { + if let Some(path) = ctor.path.get_ident() { + if path == "HistogramBuilder" { + // Validate the histogram buckets + if let Err(err) = validation::validate_histogram_buckets(ctor) { + return err.to_compile_error(); + } + } + } + } + let metric_init = match ctor { Some(ctor) if args.is_empty() => quote! { #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 mod tests { use super::*; use crate::common::test_utils::{code_str, parse_attr}; - use syn::parse_quote; + use crate::metrics::validation; + use syn::{parse_quote, ExprStruct}; #[test] fn expand_empty() { @@ -736,4 +750,64 @@ mod tests { assert_eq!(actual, expected); } + + #[test] + fn test_valid_histogram_buckets() { + // Valid strictly increasing buckets + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: &[0.1, 0.2, 0.3, 0.4] } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_ok()); + } + + #[test] + fn test_empty_histogram_buckets() { + // Empty buckets are valid + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: &[] } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_ok()); + } + + #[test] + fn test_invalid_histogram_buckets_equal_values() { + // Invalid buckets with equal values + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: &[0.1, 0.2, 0.2, 0.3] } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_err()); + } + + #[test] + fn test_invalid_histogram_buckets_decreasing_values() { + // Invalid buckets with decreasing values + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: &[0.3, 0.2, 0.1] } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_err()); + } + + #[test] + fn test_skip_validation_for_variable_buckets() { + // Variable buckets should skip validation + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: MY_BUCKETS } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_ok()); + } + + #[test] + fn test_skip_validation_for_function_buckets() { + // Function call buckets should skip validation + let expr: ExprStruct = parse_quote! { + HistogramBuilder { buckets: get_buckets() } + }; + + assert!(validation::validate_histogram_buckets(&expr).is_ok()); + } } diff --git a/foundations-macros/src/metrics/validation.rs b/foundations-macros/src/metrics/validation.rs new file mode 100644 index 0000000..ad269de --- /dev/null +++ b/foundations-macros/src/metrics/validation.rs @@ -0,0 +1,62 @@ +use syn::{Error, Expr, ExprLit, ExprStruct, Lit, Member, Result}; + +/// Validates that histogram bucket values are strictly increasing. +/// Returns an error if the buckets are not valid. +pub fn validate_histogram_buckets(expr: &ExprStruct) -> Result<()> { + // Extract the buckets field from the HistogramBuilder struct + let buckets_field = expr + .fields + .iter() + .find(|field| { + if let Member::Named(ident) = &field.member { + ident == "buckets" + } else { + false + } + }) + .ok_or_else(|| Error::new_spanned(expr, "HistogramBuilder must have a 'buckets' field"))?; + + // Extract the array expression from the buckets field + let array_expr = match &buckets_field.expr { + Expr::Reference(ref_expr) => match &*ref_expr.expr { + Expr::Array(array) => array, + // For non-array literals (like variables or function calls), skip validation + _ => return Ok(()), + }, + // For non-reference expressions, skip validation + _ => return Ok(()), + }; + + // Extract the f64 values from the array + let mut values = Vec::new(); + for elem in &array_expr.elems { + if let Expr::Lit(ExprLit { + lit: Lit::Float(lit_float), + .. + }) = elem + { + let value = lit_float.base10_parse::()?; + values.push((value, lit_float.span())); + } else { + // For non-float literals (like variables or expressions), skip validation + return Ok(()); + } + } + + // Check if the buckets are strictly increasing + if !values.is_empty() { + let mut prev_value = values[0].0; + for (i, (value, span)) in values.iter().enumerate().skip(1) { + if *value <= prev_value { + let message = format!( + "Histogram buckets must be strictly increasing. Found invalid bucket at position {}: {} <= {}", + i, value, prev_value + ); + return Err(Error::new(*span, message)); + } + prev_value = *value; + } + } + + Ok(()) +}