[metrics] Fix perf regression introduced by metric point reclaim feat…

…ure (#5307) Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com>
open-telemetry · Feb 2, 2024 · ef942a1 · ef942a1
1 parent 71f9a42
commit ef942a1
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 22 deletions.
diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs
@@ -12,7 +12,7 @@ namespace OpenTelemetry.Metrics;
 internal sealed class AggregatorStore
 {
     internal readonly bool OutputDelta;
-    internal readonly bool ShouldReclaimUnusedMetricPoints;
+    internal readonly bool OutputDeltaWithUnusedMetricPointReclaimEnabled;
     internal long DroppedMeasurements = 0;
 
     private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
@@ -101,9 +101,9 @@ internal AggregatorStore(
             reservedMetricPointsCount++;
         }
 
-        this.ShouldReclaimUnusedMetricPoints = shouldReclaimUnusedMetricPoints;
+        this.OutputDeltaWithUnusedMetricPointReclaimEnabled = shouldReclaimUnusedMetricPoints && this.OutputDelta;
 
-        if (this.OutputDelta && shouldReclaimUnusedMetricPoints)
+        if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             this.availableMetricPoints = new Queue<int>(maxMetricPoints - reservedMetricPointsCount);
 
@@ -158,17 +158,14 @@ internal void Update(double value, ReadOnlySpan<KeyValuePair<string, object?>> t
     internal int Snapshot()
     {
         this.batchSize = 0;
-        if (this.OutputDelta)
+        if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
-            if (this.ShouldReclaimUnusedMetricPoints)
-            {
-                this.SnapshotDeltaWithMetricPointReclaim();
-            }
-            else
-            {
-                var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
-                this.SnapshotDelta(indexSnapshot);
-            }
+            this.SnapshotDeltaWithMetricPointReclaim();
+        }
+        else if (this.OutputDelta)
+        {
+            var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
+            this.SnapshotDelta(indexSnapshot);
         }
         else
         {

diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs
@@ -50,11 +50,7 @@ internal MetricPoint(
     {
         Debug.Assert(aggregatorStore != null, "AggregatorStore was null.");
         Debug.Assert(histogramExplicitBounds != null, "Histogram explicit Bounds was null.");
-
-        if (aggregatorStore!.OutputDelta && aggregatorStore.ShouldReclaimUnusedMetricPoints)
-        {
-            Debug.Assert(lookupData != null, "LookupData was null.");
-        }
+        Debug.Assert(!aggregatorStore!.OutputDeltaWithUnusedMetricPointReclaimEnabled || lookupData != null, "LookupData was null.");
 
         this.aggType = aggType;
         this.Tags = new ReadOnlyTagCollection(tagKeysAndValues);
@@ -445,7 +441,7 @@ internal void Update(long number)
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -570,7 +566,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string,
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -666,7 +662,7 @@ internal void Update(double number)
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -797,7 +793,7 @@ internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }