Code review.

open-telemetry · Feb 21, 2024 · fb9b07a · fb9b07a
1 parent 0ffba7d
commit fb9b07a
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 18 deletions.
diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs
@@ -17,7 +17,7 @@ internal sealed class AggregatorStore
     internal readonly int CardinalityLimit;
     internal readonly bool EmitOverflowAttribute;
     internal readonly ConcurrentDictionary<Tags, LookupData>? TagsToMetricPointIndexDictionaryDelta;
-    internal readonly ExemplarSamplingHelper ExemplarSampler;
+    internal readonly ExemplarFilteringHelper ExemplarFilter;
     internal readonly Func<ExemplarReservoir?>? ExemplarReservoirFactory;
     internal long DroppedMeasurements = 0;
 
@@ -45,7 +45,6 @@ internal sealed class AggregatorStore
     private readonly int exponentialHistogramMaxScale;
     private readonly UpdateLongDelegate updateLongCallback;
     private readonly UpdateDoubleDelegate updateDoubleCallback;
-    private readonly ExemplarFilter exemplarFilter;
     private readonly Func<KeyValuePair<string, object?>[], int, int> lookupAggregatorStore;
 
     private int metricPointIndex = 0;
@@ -76,7 +75,6 @@ internal AggregatorStore(
         this.exponentialHistogramMaxSize = metricStreamIdentity.ExponentialHistogramMaxSize;
         this.exponentialHistogramMaxScale = metricStreamIdentity.ExponentialHistogramMaxScale;
         this.StartTimeExclusive = DateTimeOffset.UtcNow;
-        this.exemplarFilter = exemplarFilter ?? DefaultExemplarFilter;
         this.ExemplarReservoirFactory = exemplarReservoirFactory;
         if (metricStreamIdentity.TagKeys == null)
         {
@@ -130,7 +128,7 @@ internal AggregatorStore(
             this.lookupAggregatorStore = this.LookupAggregatorStore;
         }
 
-        this.ExemplarSampler = new(this.exemplarFilter);
+        this.ExemplarFilter = new(exemplarFilter ?? DefaultExemplarFilter);
     }
 
     private delegate void UpdateLongDelegate(long value, ReadOnlySpan<KeyValuePair<string, object?>> tags);
@@ -144,11 +142,7 @@ internal AggregatorStore(
     internal double[] HistogramBounds => this.histogramBounds;
 
     internal bool IsExemplarEnabled()
-    {
-        // Using this filter to indicate On/Off
-        // instead of another separate flag.
-        return this.exemplarFilter is not AlwaysOffExemplarFilter;
-    }
+        => this.ExemplarFilter.Enabled;
 
     internal void Update(long value, ReadOnlySpan<KeyValuePair<string, object?>> tags)
     {
@@ -1141,30 +1135,34 @@ private int FindMetricAggregatorsCustomTag(ReadOnlySpan<KeyValuePair<string, obj
         return this.lookupAggregatorStore(tagKeysAndValues!, actualLength);
     }
 
-    internal sealed class ExemplarSamplingHelper
+    internal sealed class ExemplarFilteringHelper
     {
-        public bool? EarlySampleDecision;
-        public ShouldSampleFunc<long> ShouldSampleLong;
-        public ShouldSampleFunc<double> ShouldSampleDouble;
+        public readonly bool Enabled;
+        public readonly bool? EarlySampleDecision;
+        public readonly ShouldSampleFunc<long> ShouldSampleLong;
+        public readonly ShouldSampleFunc<double> ShouldSampleDouble;
 
-        public ExemplarSamplingHelper(ExemplarFilter exemplarFilter)
+        public ExemplarFilteringHelper(ExemplarFilter exemplarFilter)
         {
             Debug.Assert(exemplarFilter != null, "exemplarFilter was null");
 
             if (exemplarFilter is AlwaysOffExemplarFilter)
             {
+                this.Enabled = false;
                 this.EarlySampleDecision = false;
                 this.ShouldSampleLong = static (_, _) => false;
                 this.ShouldSampleDouble = static (_, _) => false;
             }
             else if (exemplarFilter is AlwaysOnExemplarFilter)
             {
+                this.Enabled = true;
                 this.EarlySampleDecision = true;
                 this.ShouldSampleLong = static (_, _) => true;
                 this.ShouldSampleDouble = static (_, _) => true;
             }
             else
             {
+                this.Enabled = true;
                 this.EarlySampleDecision = null;
                 this.ShouldSampleLong = exemplarFilter!.ShouldSample;
                 this.ShouldSampleDouble = exemplarFilter.ShouldSample;

diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs
@@ -383,8 +383,8 @@ internal void Update(long number, ReadOnlySpan<KeyValuePair<string, object?>> ta
     {
         this.Update(number, out var explicitBucketHistogramBucketIndex);
 
-        var exemplarSampler = this.aggregatorStore.ExemplarSampler;
-        if (exemplarSampler.EarlySampleDecision ?? exemplarSampler.ShouldSampleLong(number, tags))
+        var exemplarFilter = this.aggregatorStore.ExemplarFilter;
+        if (exemplarFilter.EarlySampleDecision ?? exemplarFilter.ShouldSampleLong(number, tags))
         {
             Debug.Assert(this.mpComponents?.ExemplarReservoir != null, "ExemplarReservoir was null");
 
@@ -399,8 +399,8 @@ internal void Update(double number, ReadOnlySpan<KeyValuePair<string, object?>>
     {
         this.Update(number, out var explicitBucketHistogramBucketIndex);
 
-        var exemplarSampler = this.aggregatorStore.ExemplarSampler;
-        if (exemplarSampler.EarlySampleDecision ?? exemplarSampler.ShouldSampleDouble(number, tags))
+        var exemplarFilter = this.aggregatorStore.ExemplarFilter;
+        if (exemplarFilter.EarlySampleDecision ?? exemplarFilter.ShouldSampleDouble(number, tags))
         {
             Debug.Assert(this.mpComponents?.ExemplarReservoir != null, "ExemplarReservoir was null");