[sdk-metrics] Improve exemplar tests (#5393)

open-telemetry · Feb 27, 2024 · e7cbbbb · e7cbbbb
1 parent 4443223
commit e7cbbbb
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 55 deletions.
diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs
@@ -17,6 +17,7 @@ internal sealed class AggregatorStore
     internal readonly int CardinalityLimit;
     internal readonly bool EmitOverflowAttribute;
     internal readonly ConcurrentDictionary<Tags, LookupData>? TagsToMetricPointIndexDictionaryDelta;
+    internal readonly Func<ExemplarReservoir?>? ExemplarReservoirFactory;
     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.";
@@ -59,7 +60,8 @@ internal AggregatorStore(
         int cardinalityLimit,
         bool emitOverflowAttribute,
         bool shouldReclaimUnusedMetricPoints,
-        ExemplarFilter? exemplarFilter = null)
+        ExemplarFilter? exemplarFilter = null,
+        Func<ExemplarReservoir?>? exemplarReservoirFactory = null)
     {
         this.name = metricStreamIdentity.InstrumentName;
         this.CardinalityLimit = cardinalityLimit;
@@ -74,6 +76,7 @@ internal AggregatorStore(
         this.exponentialHistogramMaxScale = metricStreamIdentity.ExponentialHistogramMaxScale;
         this.StartTimeExclusive = DateTimeOffset.UtcNow;
         this.exemplarFilter = exemplarFilter ?? DefaultExemplarFilter;
+        this.ExemplarReservoirFactory = exemplarReservoirFactory;
         if (metricStreamIdentity.TagKeys == null)
         {
             this.updateLongCallback = this.UpdateLong;

diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs
@@ -49,7 +49,8 @@ internal Metric(
         int cardinalityLimit,
         bool emitOverflowAttribute,
         bool shouldReclaimUnusedMetricPoints,
-        ExemplarFilter? exemplarFilter = null)
+        ExemplarFilter? exemplarFilter = null,
+        Func<ExemplarReservoir?>? exemplarReservoirFactory = null)
     {
         this.InstrumentIdentity = instrumentIdentity;
 
@@ -155,7 +156,15 @@ internal Metric(
             throw new NotSupportedException($"Unsupported Instrument Type: {instrumentIdentity.InstrumentType.FullName}");
         }
 
-        this.AggregatorStore = new AggregatorStore(instrumentIdentity, aggType, temporality, cardinalityLimit, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter);
+        this.AggregatorStore = new AggregatorStore(
+            instrumentIdentity,
+            aggType,
+            temporality,
+            cardinalityLimit,
+            emitOverflowAttribute,
+            shouldReclaimUnusedMetricPoints,
+            exemplarFilter,
+            exemplarReservoirFactory);
         this.Temporality = temporality;
     }
 

diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs
@@ -63,13 +63,25 @@ internal MetricPoint(
         this.ReferenceCount = 1;
         this.LookupData = lookupData;
 
-        ExemplarReservoir? reservoir = null;
+        var isExemplarEnabled = aggregatorStore!.IsExemplarEnabled();
+
+        ExemplarReservoir? reservoir;
+        try
+        {
+            reservoir = aggregatorStore.ExemplarReservoirFactory?.Invoke();
+        }
+        catch
+        {
+            // TODO : Log that the factory on view threw an exception, once view exposes that capability
+            reservoir = null;
+        }
+
         if (this.aggType == AggregationType.HistogramWithBuckets ||
             this.aggType == AggregationType.HistogramWithMinMaxBuckets)
         {
             this.mpComponents = new MetricPointOptionalComponents();
             this.mpComponents.HistogramBuckets = new HistogramBuckets(histogramExplicitBounds);
-            if (aggregatorStore!.IsExemplarEnabled())
+            if (isExemplarEnabled && reservoir == null)
             {
                 reservoir = new AlignedHistogramBucketExemplarReservoir(histogramExplicitBounds!.Length);
             }
@@ -91,7 +103,7 @@ internal MetricPoint(
             this.mpComponents = null;
         }
 
-        if (aggregatorStore!.IsExemplarEnabled() && reservoir == null)
+        if (isExemplarEnabled && reservoir == null)
         {
             reservoir = new SimpleFixedSizeExemplarReservoir(DefaultSimpleReservoirPoolSize);
         }

diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs
@@ -147,14 +147,14 @@ internal virtual List<Metric> AddMetricWithViews(Instrument instrument, List<Met
                 {
                     bool shouldReclaimUnusedMetricPoints = this.parentProvider is MeterProviderSdk meterProviderSdk && meterProviderSdk.ShouldReclaimUnusedMetricPoints;
 
-                    var cardinalityLimit = this.cardinalityLimit;
-
-                    if (metricStreamConfig != null && metricStreamConfig.CardinalityLimit != null)
-                    {
-                        cardinalityLimit = metricStreamConfig.CardinalityLimit.Value;
-                    }
-
-                    Metric metric = new(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), cardinalityLimit, this.emitOverflowAttribute, shouldReclaimUnusedMetricPoints, this.exemplarFilter);
+                    Metric metric = new(
+                        metricStreamIdentity,
+                        this.GetAggregationTemporality(metricStreamIdentity.InstrumentType),
+                        metricStreamConfig?.CardinalityLimit ?? this.cardinalityLimit,
+                        this.emitOverflowAttribute,
+                        shouldReclaimUnusedMetricPoints,
+                        this.exemplarFilter,
+                        metricStreamConfig?.ExemplarReservoirFactory);
 
                     this.instrumentIdentityToMetric[metricStreamIdentity] = metric;
                     this.metrics![index] = metric;

diff --git a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs
@@ -133,6 +133,10 @@ public string[]? TagKeys
         }
     }
 
+    // TODO: Expose this to be complaint with the spec:
+    // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#stream-configuration
+    internal Func<ExemplarReservoir?>? ExemplarReservoirFactory { get; set; }
+
     internal string[]? CopiedTagKeys { get; private set; }
 
     internal int? ViewId { get; set; }