Bugfix - add validation for custom buckets provided for Histograms

opentelemetry-sdk/CHANGELOG.md

@@ -32,6 +32,12 @@
         - These methods are intended for log appenders. Keep the clone of the provider handle, instead of depending on above methods.
 
 
+  - **Bug Fix:** Validates the `with_boundaries` bucket boundaries used in
+    Histograms. The boundaries provided by the user must be a non-empty vector,
+    sorted in strictly increasing order, and contain no duplicates. Instruments
+    will not record measurements if the boundaries are invalid.
+    [#2351](https://github.com/open-telemetry/opentelemetry-rust/pull/2351)
+
 ## 0.27.0
 
 Released 2024-Nov-11

opentelemetry-sdk/src/metrics/meter.rs

@@ -394,6 +394,23 @@
             return Histogram::new(Arc::new(NoopSyncInstrument::new()));
         }
 
+        if let Some(ref boundaries) = builder.boundaries {
+            let validation_result = validate_buckets(boundaries);
+            if let Err(err) = validation_result {
+                // TODO: Include the buckets too in the error message.
+                // TODO: This validation is not done when Views are used to
+                // provide boundaries, and that should be fixed.
+                otel_error!(
+                    name: "InstrumentCreationFailed",
+                    meter_name = self.scope.name(),
+                    instrument_name = builder.name.as_ref(),
+                    message = "Measurements from this Histogram will be ignored.",
+                    reason = format!("{}", err)
+                );
+                return Histogram::new(Arc::new(NoopSyncInstrument::new()));
+            }
+        }
+
         match resolver
             .lookup(
                 InstrumentKind::Histogram,
@@ -533,6 +550,25 @@
     validate_instrument_name(name).and_then(|_| validate_instrument_unit(unit))
 }
 
+fn validate_buckets(buckets: &[f64]) -> MetricResult<()> {
+    if buckets.is_empty() {
+        return Err(MetricError::InvalidInstrumentConfiguration(
+            "Buckets must not be empty",
+        ));
+    }
+
+    // validate that buckets are sorted and non-duplicate
+    for window in buckets.windows(2) {
+        if window[0] >= window[1] {
+            return Err(MetricError::InvalidInstrumentConfiguration(
+                "Buckets must be sorted and non-duplicate",
+            ));
+        }
+    }
+
+    Ok(())
+}
+
 fn validate_instrument_name(name: &str) -> MetricResult<()> {
     if name.is_empty() {
         return Err(MetricError::InvalidInstrumentConfiguration(

opentelemetry-sdk/src/metrics/mod.rs

@@ -178,6 +178,27 @@ mod tests {
             // As instrument name is invalid, no metrics should be exported
             test_context.check_no_metrics();
         }
+
+        let invalid_buckets = vec![
+            vec![],                        // empty buckets
+            vec![1.0, 1.0],                // duplicate buckets
+            vec![1.0, 2.0, 3.0, 2.0],      // duplicate non consequent buckets
+            vec![1.0, 2.0, 3.0, 4.0, 2.5], // unsorted buckets
+        ];
+        for buckets in invalid_buckets {
+            let test_context = TestContext::new(Temporality::Cumulative);
+            let histogram = test_context
+                .meter()
+                .f64_histogram("test")
+                .with_boundaries(buckets)
+                .build();
+            histogram.record(1.9, &[]);
+            test_context.flush_metrics();
+
+            // As buckets via Advisory params are invalid, no metrics should be
+            // exported
+            test_context.check_no_metrics();
+        }
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]