Stabilize explicit bucket boundaries advice API (#5897)

open-telemetry · Nov 6, 2023 · 19196a0 · 19196a0
1 parent 7fd46f0
commit 19196a0
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 47 deletions.
diff --git a/api/all/src/main/java/io/opentelemetry/api/metrics/DoubleHistogramBuilder.java b/api/all/src/main/java/io/opentelemetry/api/metrics/DoubleHistogramBuilder.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.api.metrics;
 
+import java.util.List;
+
 /**
  * Builder class for {@link DoubleHistogram}.
  *
@@ -32,6 +34,20 @@ public interface DoubleHistogramBuilder {
    */
   DoubleHistogramBuilder setUnit(String unit);
 
+  /**
+   * Set the explicit bucket buckets boundaries advice, which suggests the recommended set of
+   * explicit bucket boundaries for this histogram.
+   *
+   * @param bucketBoundaries The explicit bucket boundaries advice.
+   * @see <a
+   *     href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument-advisory-parameter-explicitbucketboundaries">Explicit
+   *     bucket boundaries advisory parameter</a>
+   * @since 1.32.0
+   */
+  default DoubleHistogramBuilder setExplicitBucketBoundariesAdvice(List<Double> bucketBoundaries) {
+    return this;
+  }
+
   /** Sets the Counter for recording {@code long} values. */
   LongHistogramBuilder ofLongs();
 

diff --git a/api/all/src/main/java/io/opentelemetry/api/metrics/LongHistogramBuilder.java b/api/all/src/main/java/io/opentelemetry/api/metrics/LongHistogramBuilder.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.api.metrics;
 
+import java.util.List;
+
 /**
  * Builder class for {@link LongHistogram}.
  *
@@ -31,6 +33,20 @@ public interface LongHistogramBuilder {
    */
   LongHistogramBuilder setUnit(String unit);
 
+  /**
+   * Set the explicit bucket buckets boundaries advice, which suggests the recommended set of
+   * explicit bucket boundaries for this histogram.
+   *
+   * @param bucketBoundaries The explicit bucket boundaries advice.
+   * @see <a
+   *     href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument-advisory-parameter-explicitbucketboundaries">Explicit
+   *     bucket boundaries advisory parameter</a>
+   * @since 1.32.0
+   */
+  default LongHistogramBuilder setExplicitBucketBoundariesAdvice(List<Long> bucketBoundaries) {
+    return this;
+  }
+
   /**
    * Builds and returns a Histogram instrument with the configuration.
    *

diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-api.txt b/docs/apidiffs/current_vs_latest/opentelemetry-api.txt
@@ -1,2 +1,7 @@
 Comparing source compatibility of  against 
-No changes.
+***  MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.metrics.DoubleHistogramBuilder  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.api.metrics.DoubleHistogramBuilder setExplicitBucketBoundariesAdvice(java.util.List<java.lang.Double>)
+***  MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.metrics.LongHistogramBuilder  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.api.metrics.LongHistogramBuilder setExplicitBucketBoundariesAdvice(java.util.List<java.lang.Long>)
diff --git a/...ain/java/io/opentelemetry/extension/incubator/metrics/ExtendedDoubleHistogramBuilder.java b/...ain/java/io/opentelemetry/extension/incubator/metrics/ExtendedDoubleHistogramBuilder.java
@@ -12,15 +12,6 @@
 /** Extended {@link DoubleHistogramBuilder} with experimental APIs. */
 public interface ExtendedDoubleHistogramBuilder extends DoubleHistogramBuilder {
 
-  /**
-   * Specify the explicit bucket buckets boundaries advice, which suggests the recommended set of
-   * explicit bucket boundaries for this histogram.
-   */
-  default ExtendedDoubleHistogramBuilder setExplicitBucketBoundariesAdvice(
-      List<Double> bucketBoundaries) {
-    return this;
-  }
-
   /**
    * Specify the attribute advice, which suggests the recommended set of attribute keys to be used
    * for this histogram.

diff --git a/.../main/java/io/opentelemetry/extension/incubator/metrics/ExtendedLongHistogramBuilder.java b/.../main/java/io/opentelemetry/extension/incubator/metrics/ExtendedLongHistogramBuilder.java
@@ -12,15 +12,6 @@
 /** Extended {@link LongHistogramBuilder} with experimental APIs. */
 public interface ExtendedLongHistogramBuilder extends LongHistogramBuilder {
 
-  /**
-   * Specify the explicit bucket buckets boundaries advice, which suggests the recommended set of
-   * explicit bucket boundaries for this histogram.
-   */
-  default ExtendedLongHistogramBuilder setExplicitBucketBoundariesAdvice(
-      List<Long> bucketBoundaries) {
-    return this;
-  }
-
   /**
    * Specify the attribute advice, which suggests the recommended set of attribute keys to be used
    * for this histogram.

diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java
@@ -13,11 +13,13 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder;
 import io.opentelemetry.sdk.internal.ThrottlingLogger;
+import io.opentelemetry.sdk.metrics.internal.aggregator.ExplicitBucketHistogramUtils;
 import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
 import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.WriteableMetricStorage;
 import java.util.List;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -97,6 +99,13 @@ public LongHistogramBuilder ofLongs() {
     @Override
     public ExtendedDoubleHistogramBuilder setExplicitBucketBoundariesAdvice(
         List<Double> bucketBoundaries) {
+      try {
+        Objects.requireNonNull(bucketBoundaries, "bucketBoundaries must not be null");
+        ExplicitBucketHistogramUtils.validateBucketBoundaries(bucketBoundaries);
+      } catch (IllegalArgumentException | NullPointerException e) {
+        logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage());
+        return this;
+      }
       builder.setExplicitBucketBoundaries(bucketBoundaries);
       return this;
     }

diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java
@@ -12,12 +12,14 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.extension.incubator.metrics.ExtendedLongHistogramBuilder;
 import io.opentelemetry.sdk.internal.ThrottlingLogger;
+import io.opentelemetry.sdk.metrics.internal.aggregator.ExplicitBucketHistogramUtils;
 import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
 import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
 import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.WriteableMetricStorage;
 import java.util.List;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -99,9 +101,16 @@ public SdkLongHistogram build() {
     @Override
     public ExtendedLongHistogramBuilder setExplicitBucketBoundariesAdvice(
         List<Long> bucketBoundaries) {
-      List<Double> doubleBoundaries =
-          bucketBoundaries.stream().map(Long::doubleValue).collect(Collectors.toList());
-      builder.setExplicitBucketBoundaries(doubleBoundaries);
+      List<Double> boundaries;
+      try {
+        Objects.requireNonNull(bucketBoundaries, "bucketBoundaries must not be null");
+        boundaries = bucketBoundaries.stream().map(Long::doubleValue).collect(Collectors.toList());
+        ExplicitBucketHistogramUtils.validateBucketBoundaries(boundaries);
+      } catch (IllegalArgumentException | NullPointerException e) {
+        logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage());
+        return this;
+      }
+      builder.setExplicitBucketBoundaries(boundaries);
       return this;
     }
 

diff --git a/...n/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java b/...n/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java
@@ -26,7 +26,8 @@ private ExplicitBucketHistogramUtils() {}
 
   /** Converts bucket boundary "convenient" configuration into the "more efficient" array. */
   public static double[] createBoundaryArray(List<Double> boundaries) {
-    return validateBucketBoundaries(boundaries.stream().mapToDouble(i -> i).toArray());
+    validateBucketBoundaries(boundaries);
+    return boundaries.stream().mapToDouble(i -> i).toArray();
   }
 
   /**
@@ -51,32 +52,30 @@ public static int findBucketIndex(double[] boundaries, double value) {
    * Validates errors in boundary configuration.
    *
    * @param boundaries The array of bucket boundaries.
-   * @return The original boundaries.
    * @throws IllegalArgumentException if boundaries are not specified correctly.
    */
-  public static double[] validateBucketBoundaries(double[] boundaries) {
+  public static void validateBucketBoundaries(List<Double> boundaries) {
     for (double v : boundaries) {
       if (Double.isNaN(v)) {
         throw new IllegalArgumentException("invalid bucket boundary: NaN");
       }
     }
-    for (int i = 1; i < boundaries.length; ++i) {
-      if (boundaries[i - 1] >= boundaries[i]) {
+    for (int i = 1; i < boundaries.size(); ++i) {
+      if (boundaries.get(i - 1) >= boundaries.get(i)) {
         throw new IllegalArgumentException(
             "Bucket boundaries must be in increasing order: "
-                + boundaries[i - 1]
+                + boundaries.get(i - 1)
                 + " >= "
-                + boundaries[i]);
+                + boundaries.get(i));
       }
     }
-    if (boundaries.length > 0) {
-      if (boundaries[0] == Double.NEGATIVE_INFINITY) {
+    if (!boundaries.isEmpty()) {
+      if (boundaries.get(0) == Double.NEGATIVE_INFINITY) {
         throw new IllegalArgumentException("invalid bucket boundary: -Inf");
       }
-      if (boundaries[boundaries.length - 1] == Double.POSITIVE_INFINITY) {
+      if (boundaries.get(boundaries.size() - 1) == Double.POSITIVE_INFINITY) {
         throw new IllegalArgumentException("invalid bucket boundary: +Inf");
       }
     }
-    return boundaries;
   }
 }