Add compile-time validation for histogram buckets in foundations-macros

Author: LloydW93

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());
+    }
 }

foundations-macros/src/metrics/validation.rs

@@ -0,0 +1,61 @@
+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(crate) 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::<f64>()?;
+            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(())
+}

foundations/src/telemetry/tracing/live/event_output.rs

@@ -93,8 +93,7 @@ impl TraceLogBuilder {
 
     fn finalize(mut self, end_timestamp: u64) -> String {
         self.out.push_str(&format!(
-            "{{\"pid\":1,\"name\":\"Trace dump requested\",\"ph\":\"i\",\"ts\":{},\"s\":\"g\"}}",
-            end_timestamp,
+            "{{\"pid\":1,\"name\":\"Trace dump requested\",\"ph\":\"i\",\"ts\":{end_timestamp},\"s\":\"g\"}}",
         ));
 
         self.out.push(']');