Add closing rollover behaviour for Step Histogram flavours

micrometer-metrics · Jul 19, 2024 · c61f1fe · c61f1fe
1 parent d01a5a1
commit c61f1fe
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 15 deletions.
diff --git a/...rometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpHistogramSupport.java b/...rometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpHistogramSupport.java
@@ -15,10 +15,12 @@
  */
 package io.micrometer.registry.otlp;
 
+import io.micrometer.common.lang.Nullable;
 import io.micrometer.registry.otlp.internal.ExponentialHistogramSnapShot;
 
 interface OtlpHistogramSupport {
 
+    @Nullable
     ExponentialHistogramSnapShot getExponentialHistogramSnapShot();
 
 }
diff --git a/...-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java b/...-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepDistributionSummary.java
@@ -15,7 +15,6 @@
  */
 package io.micrometer.registry.otlp;
 
-import io.micrometer.common.lang.Nullable;
 import io.micrometer.core.instrument.AbstractDistributionSummary;
 import io.micrometer.core.instrument.Clock;
 import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
@@ -78,7 +77,6 @@ public double max() {
     }
 
     @Override
-    @Nullable
     public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() {
         if (histogramFlavour == HistogramFlavour.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) {
             return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot();
@@ -98,6 +96,9 @@ void _closingRollover() {
         if (histogram instanceof OtlpStepBucketHistogram) { // can be noop
             ((OtlpStepBucketHistogram) histogram)._closingRollover();
         }
+        else if (histogram instanceof Base2ExponentialHistogram) {
+            histogram.close();
+        }
     }
 
 }
diff --git a/...ons/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java b/...ons/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpStepTimer.java
@@ -15,7 +15,6 @@
  */
 package io.micrometer.registry.otlp;
 
-import io.micrometer.common.lang.Nullable;
 import io.micrometer.core.instrument.AbstractTimer;
 import io.micrometer.core.instrument.Clock;
 import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
@@ -94,10 +93,12 @@ void _closingRollover() {
         if (histogram instanceof OtlpStepBucketHistogram) { // can be noop
             ((OtlpStepBucketHistogram) histogram)._closingRollover();
         }
+        else if (histogram instanceof Base2ExponentialHistogram) {
+            histogram.close();
+        }
     }
 
     @Override
-    @Nullable
     public ExponentialHistogramSnapShot getExponentialHistogramSnapShot() {
         if (histogramFlavour == HistogramFlavour.BASE2_EXPONENTIAL_BUCKET_HISTOGRAM) {
             return ((Base2ExponentialHistogram) histogram).getLatestExponentialHistogramSnapshot();

diff --git a/...ry-otlp/src/main/java/io/micrometer/registry/otlp/internal/Base2ExponentialHistogram.java b/...ry-otlp/src/main/java/io/micrometer/registry/otlp/internal/Base2ExponentialHistogram.java
@@ -104,13 +104,13 @@ int getScale() {
      * Provides a bridge to Micrometer {@link HistogramSnapshot}.
      */
     @Override
-    public synchronized HistogramSnapshot takeSnapshot(final long count, final double total, final double max) {
+    public HistogramSnapshot takeSnapshot(final long count, final double total, final double max) {
         this.takeExponentialHistogramSnapShot();
         return new HistogramSnapshot(count, total, max, null, null, null);
     }
 
     /**
-     * Returns the snapshot of current recorded values..
+     * Returns the snapshot of current recorded values.
      */
     ExponentialHistogramSnapShot getCurrentValuesSnapshot() {
         return (circularCountHolder.isEmpty() && zeroCount.longValue() == 0)
@@ -149,9 +149,12 @@ public void recordDouble(double value) {
 
         int index = base2IndexProvider.getIndexForValue(value);
         if (!circularCountHolder.increment(index, 1)) {
-            downScale(getDownScaleFactor(index));
-            index = base2IndexProvider.getIndexForValue(value);
-            circularCountHolder.increment(index, 1);
+            synchronized (this) {
+                int downScaleFactor = getDownScaleFactor(index);
+                downScale(downScaleFactor);
+                index = base2IndexProvider.getIndexForValue(value);
+                circularCountHolder.increment(index, 1);
+            }
         }
     }
 
@@ -160,7 +163,7 @@ public void recordDouble(double value) {
      * align with the exponential scale.
      * @param downScaleFactor - the factor to downscale this histogram.
      */
-    private synchronized void downScale(int downScaleFactor) {
+    private void downScale(int downScaleFactor) {
         if (downScaleFactor == 0) {
             return;
         }
@@ -195,7 +198,7 @@ private void updateScale(int newScale) {
      * @return a factor by which {@link Base2ExponentialHistogram#scale} should be
      * decreased.
      */
-    private synchronized int getDownScaleFactor(final long index) {
+    private int getDownScaleFactor(final long index) {
         long newStart = Math.min(index, circularCountHolder.getStartIndex());
         long newEnd = Math.max(index, circularCountHolder.getEndIndex());
 

diff --git a/...c/main/java/io/micrometer/registry/otlp/internal/DefaultExponentialHistogramSnapShot.java b/...c/main/java/io/micrometer/registry/otlp/internal/DefaultExponentialHistogramSnapShot.java
@@ -16,13 +16,20 @@
 package io.micrometer.registry.otlp.internal;
 
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
 class DefaultExponentialHistogramSnapShot implements ExponentialHistogramSnapShot {
 
-    private static final Map<Integer, ExponentialHistogramSnapShot> emptySnapshotCache = new HashMap<>();
+    private static final int MAX_ENTRIES = 50;
+
+    private static final Map<Integer, ExponentialHistogramSnapShot> emptySnapshotCache = new LinkedHashMap<Integer, ExponentialHistogramSnapShot>() {
+        @Override
+        protected boolean removeEldestEntry(final Map.Entry eldest) {
+            return size() > MAX_ENTRIES;
+        }
+    };
 
     private final int scale;
 

diff --git a/...lp/src/main/java/io/micrometer/registry/otlp/internal/DeltaBase2ExponentialHistogram.java b/...lp/src/main/java/io/micrometer/registry/otlp/internal/DeltaBase2ExponentialHistogram.java
@@ -31,7 +31,7 @@
  */
 public class DeltaBase2ExponentialHistogram extends Base2ExponentialHistogram {
 
-    private final StepValue<ExponentialHistogramSnapShot> stepExponentialHistogramSnapShot;
+    private final StepExponentialHistogramSnapShot stepExponentialHistogramSnapShot;
 
     /**
      * Creates an Base2ExponentialHistogram that record positive values and resets for
@@ -66,14 +66,19 @@ synchronized void takeExponentialHistogramSnapShot() {
         stepExponentialHistogramSnapShot.poll();
     }
 
+    @Override
+    public void close() {
+        stepExponentialHistogramSnapShot._closingRollover();
+    }
+
     private class StepExponentialHistogramSnapShot extends StepValue<ExponentialHistogramSnapShot> {
 
         public StepExponentialHistogramSnapShot(final Clock clock, final long stepMillis, final int maxScale) {
             super(clock, stepMillis, DefaultExponentialHistogramSnapShot.getEmptySnapshotForScale(maxScale));
         }
 
         @Override
-        protected Supplier<ExponentialHistogramSnapShot> valueSupplier() {
+        protected synchronized Supplier<ExponentialHistogramSnapShot> valueSupplier() {
             return () -> {
                 ExponentialHistogramSnapShot latestSnapShot = getCurrentValuesSnapshot();
                 reset();
@@ -86,6 +91,11 @@ protected ExponentialHistogramSnapShot noValue() {
             return DefaultExponentialHistogramSnapShot.getEmptySnapshotForScale(getScale());
         }
 
+        @Override
+        protected void _closingRollover() {
+            super._closingRollover();
+        }
+
     }
 
 }