address comments - allow empty and disallow nan

Author: cijothomas

opentelemetry-sdk/CHANGELOG.md

@@ -33,9 +33,10 @@
 
 
   - **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.
+    Histograms. The boundaries provided by the user must not contain `f64::NAN`,
+    `f64::INFINITY`, `f64::NEG_INFINITY` and must be 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

opentelemetry-sdk/src/metrics/meter.rs

@@ -395,7 +395,7 @@ impl SdkMeter {
         }
 
         if let Some(ref boundaries) = builder.boundaries {
-            let validation_result = validate_buckets(boundaries);
+            let validation_result = validate_bucket_boundaries(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
@@ -550,16 +550,19 @@ fn validate_instrument_config(name: &str, unit: &Option<Cow<'static, str>>) -> M
     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",
-        ));
+fn validate_bucket_boundaries(boundaries: &[f64]) -> MetricResult<()> {
+    // Validate boundaries do not contain f64::NAN, f64::INFINITY, or f64::NEG_INFINITY
+    for boundary in boundaries {
+        if boundary.is_nan() || boundary.is_infinite() {
+            return Err(MetricError::InvalidInstrumentConfiguration(
+                "Buckets must not contain NaN, +Inf, or -Inf",
+            ));
+        }
     }
 
     // validate that buckets are sorted and non-duplicate
-    for window in buckets.windows(2) {
-        if window[0] >= window[1] {
+    for i in 1..boundaries.len() {
+        if boundaries[i] <= boundaries[i - 1] {
             return Err(MetricError::InvalidInstrumentConfiguration(
                 "Buckets must be sorted and non-duplicate",
             ));

opentelemetry-sdk/src/metrics/mod.rs

@@ -179,24 +179,26 @@ mod tests {
             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
+        let invalid_bucket_boundaries = vec![
+            vec![1.0, 1.0],                          // duplicate boundaries
+            vec![1.0, 2.0, 3.0, 2.0],                // duplicate non consequent boundaries
+            vec![1.0, 2.0, 3.0, 4.0, 2.5],           // unsorted boundaries
+            vec![1.0, 2.0, 3.0, f64::INFINITY, 4.0], // boundaries with NaNs
+            vec![1.0, 2.0, 3.0, f64::NAN],           // boundaries with NaNs
+            vec![f64::NEG_INFINITY, 2.0, 3.0],       // boundaries with NaNs
         ];
-        for buckets in invalid_buckets {
+        for bucket_boundaries in invalid_bucket_boundaries {
             let test_context = TestContext::new(Temporality::Cumulative);
             let histogram = test_context
                 .meter()
                 .f64_histogram("test")
-                .with_boundaries(buckets)
+                .with_boundaries(bucket_boundaries)
                 .build();
             histogram.record(1.9, &[]);
             test_context.flush_metrics();
 
-            // As buckets via Advisory params are invalid, no metrics should be
-            // exported
+            // As bucket boundaires via Advisory params are invalid, no metrics
+            // should be exported
             test_context.check_no_metrics();
         }
     }