PR feedback

Author: jack-berg

sdk/all/src/jmh/java/io/opentelemetry/sdk/MetricRecordBenchmark.java

@@ -119,9 +119,10 @@ public static class BenchmarkState {
 
     // Whether to record through bound instruments (Extended*#bind(Attributes)), which resolve the
     // timeseries once up front, or unbound instruments, which look up the timeseries by Attributes
-    // on every record.
-    @Param({"false", "true"})
-    boolean bound;
+    // on every record. Uncomment to evaluate.
+    // @Param({"false", "true"})
+    // boolean bound;
+    boolean bound = false;
 
     // The following parameters are excluded from the benchmark to reduce combinatorial explosion
     // but can optionally be enabled for adhoc evaluation.

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/CumulativeSynchronousMetricStorage.java

@@ -78,7 +78,7 @@ private final class CumulativeBoundHandle implements BoundStorageHandle {
 
     @Override
     public void recordLong(long value, Context context) {
-      if (!recordingEnabled()) {
+      if (!isEnabled()) {
         return;
       }
       handle.recordLong(value, attributes, context);

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DefaultSynchronousMetricStorage.java

@@ -101,11 +101,6 @@ public void recordDouble(double value, Attributes attributes, Context context) {
     doRecordDouble(value, attributes, context);
   }
 
-  /** Returns whether recording is currently enabled. Used by bound record paths. */
-  final boolean recordingEnabled() {
-    return enabled;
-  }
-
   /**
    * Returns true if a double {@code value} should be recorded. Returns false (dropping the
    * measurement) when recording is disabled, or when {@code value} is NaN, logging in the latter

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DeltaSynchronousMetricStorage.java

@@ -232,29 +232,20 @@ public MetricData collect(
     // Seed the new holder with the bound handles, so that (a) the bound wrappers survive the swap
     // (their bound instruments hold direct references), (b) a series recorded both bound and
     // unbound continues to share one wrapper, and (c) bound series are collected every interval.
-    ConcurrentHashMap<Attributes, DeltaAggregatorHandle<T>> newHolderHandles;
-    if (memoryMode == REUSABLE_DATA) {
-      // Ping-pong between two maps. Copy bound wrappers into the next map so they appear every
-      // interval rather than every other interval.
-      holder.aggregatorHandles.forEach(
-          (attributes, handle) -> {
-            if (handle.bound) {
-              previousCollectionAggregatorHandles.put(attributes, handle);
-            }
-          });
-      newHolderHandles = previousCollectionAggregatorHandles;
-    } else {
-      // IMMUTABLE_DATA: unbound series start fresh each interval (the old map is abandoned), so
-      // seed
-      // the new holder with only the bound wrappers.
-      newHolderHandles = new ConcurrentHashMap<>();
-      holder.aggregatorHandles.forEach(
-          (attributes, handle) -> {
-            if (handle.bound) {
-              newHolderHandles.put(attributes, handle);
-            }
-          });
-    }
+    // In REUSABLE_DATA we ping-pong between two maps, so the bound wrappers are copied into the
+    // next
+    // map; in IMMUTABLE_DATA the old map is abandoned and the new map starts with only the bound
+    // wrappers.
+    ConcurrentHashMap<Attributes, DeltaAggregatorHandle<T>> newHolderHandles =
+        (memoryMode == REUSABLE_DATA)
+            ? previousCollectionAggregatorHandles
+            : new ConcurrentHashMap<>();
+    holder.aggregatorHandles.forEach(
+        (attributes, handle) -> {
+          if (handle.bound) {
+            newHolderHandles.put(attributes, handle);
+          }
+        });
     this.aggregatorHolder = new AggregatorHolder<>(newHolderHandles);
 
     ConcurrentHashMap<Attributes, DeltaAggregatorHandle<T>> aggregatorHandles =
@@ -403,7 +394,7 @@ private final class DeltaBoundHandle implements BoundStorageHandle {
 
     @Override
     public void recordLong(long value, Context context) {
-      if (!recordingEnabled()) {
+      if (!isEnabled()) {
         return;
       }
       wrapper.recordLong(value, attributes, context);