[sdk-metrics] Add experimental envvar for setting ExemplarFilter for histograms (#5611)

CodeBlanch · cijothomas · web-flow · commit 1e065cbdaa4c · 2024-05-16T13:57:53.000-07:00
Co-authored-by: Cijo Thomas &lt;cijo.thomas@gmail.com&gt;
diff --git a/docs/metrics/customizing-the-sdk/README.md b/docs/metrics/customizing-the-sdk/README.md
@@ -352,26 +352,40 @@ tutorial](../exemplars/README.md) demonstrates how to use exemplars to achieve
 correlation from metrics to traces, which is one of the primary use cases for
 exemplars.
 
+#### Default behavior
+
+Exemplars in OpenTelemetry .NET are **off by default**
+(`ExemplarFilterType.AlwaysOff`). The [OpenTelemetry
+Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplarfilter)
+recommends Exemplars collection should be on by default
+(`ExemplarFilterType.TraceBased`) however there is a performance cost associated
+with Exemplars so OpenTelemetry .NET has taken a more conservative stance for
+its default behavior.
+
 #### ExemplarFilter
 
 `ExemplarFilter` determines which measurements are offered to the configured
 `ExemplarReservoir`, which makes the final decision about whether or not the
 offered measurement gets recorded as an `Exemplar`. Generally `ExemplarFilter`
-is a mechanism to control the overhead associated with `Exemplar` offering.
+is a mechanism to control the overhead associated with the offering and
+recording of `Exemplar`s.
 
-OpenTelemetry SDK comes with the following `ExemplarFilters` (defined on
+OpenTelemetry SDK comes with the following `ExemplarFilter`s (defined on
 `ExemplarFilterType`):
 
-* `AlwaysOff`: Makes no measurements eligible for becoming an `Exemplar`. Using
-  this is as good as turning off the `Exemplar` feature and is the current
-  default.
+* (Default behavior) `AlwaysOff`: Makes no measurements eligible for becoming an
+  `Exemplar`. Using this disables `Exemplar` collection and avoids all
+  performance costs associated with `Exemplar`s.
 * `AlwaysOn`: Makes all measurements eligible for becoming an `Exemplar`.
 * `TraceBased`: Makes those measurements eligible for becoming an `Exemplar`
   which are recorded in the context of a sampled `Activity` (span).
 
 The `SetExemplarFilter` extension method on `MeterProviderBuilder` can be used
 to set the desired `ExemplarFilterType` and enable `Exemplar` collection:
 
+> [!NOTE]
+> The `SetExemplarFilter` API was added in the `1.9.0` release.
+
 ```csharp
 using OpenTelemetry;
 using OpenTelemetry.Metrics;
@@ -382,6 +396,24 @@ using var meterProvider = Sdk.CreateMeterProviderBuilder()
     .Build();
 ```
 
+It is also possible to configure the `ExemplarFilter` by using following
+environmental variables:
+
+> [!NOTE]
+> Programmatically calling `SetExemplarFilter` will override any defaults set
+  using environment variables or configuration.
+
+| Environment variable       | Description                                        | Notes |
+| -------------------------- | -------------------------------------------------- |-------|
+| `OTEL_METRICS_EXEMPLAR_FILTER` | Sets the default `ExemplarFilter` to use for all metrics. | Added in `1.9.0` |
+| `OTEL_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER_HISTOGRAMS`        | Sets the default `ExemplarFilter` to use for histogram metrics. If set `OTEL_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER_HISTOGRAMS` takes precedence over `OTEL_METRICS_EXEMPLAR_FILTER` for histogram metrics. | Experimental key (may be removed or changed in the future). Added in `1.9.0` |
+
+Allowed values:
+
+* `always_off`: Equivalent to `ExemplarFilterType.AlwaysOff`
+* `always_on`: Equivalent to `ExemplarFilterType.AlwaysOn`
+* `trace_based`: Equivalent to `ExemplarFilterType.TraceBased`
+
 #### ExemplarReservoir
 
 `ExemplarReservoir` receives the measurements sampled by the `ExemplarFilter`
@@ -398,7 +430,8 @@ metrics except Histograms with buckets. It has a fixed reservoir pool, and
 implements the equivalent of [naive
 reservoir](https://en.wikipedia.org/wiki/Reservoir_sampling). The reservoir pool
 size (currently defaulting to 1) determines the maximum number of exemplars
-stored.
+stored. Exponential histograms use a `SimpleFixedSizeExemplarReservoir` with a
+pool size equal to the number of buckets up to a max of `20`.
 
 > [!NOTE]
 > Currently there is no ability to change or configure `ExemplarReservoir`.
diff --git a/examples/AspNetCore/Program.cs b/examples/AspNetCore/Program.cs
@@ -84,9 +84,7 @@
         // Ensure the MeterProvider subscribes to any custom Meters.
         builder
             .AddMeter(Instrumentation.MeterName)
-#if EXPOSE_EXPERIMENTAL_FEATURES
             .SetExemplarFilter(ExemplarFilterType.TraceBased)
-#endif
             .AddRuntimeInstrumentation()
             .AddHttpClientInstrumentation()
             .AddAspNetCoreInstrumentation();
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -30,6 +30,13 @@
   which has always been supported.
   ([#5614](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5614))
 
+* The `ExemplarFilter` used by SDK `MeterProvider`s for histogram metrics can
+  now be controlled via the experimental
+  `OTEL_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER_HISTOGRAMS` environment
+  variable. The supported values are: `always_off`, `always_on`, and
+  `trace_based`.
+  ([#5611](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5611))
+
 ## 1.8.1
 
 Released 2024-Apr-17
diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs
@@ -16,6 +16,7 @@ internal sealed class MeterProviderSdk : MeterProvider
     internal const string EmitOverFlowAttributeConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE";
     internal const string ReclaimUnusedMetricPointsConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_RECLAIM_UNUSED_METRIC_POINTS";
     internal const string ExemplarFilterConfigKey = "OTEL_METRICS_EXEMPLAR_FILTER";
+    internal const string ExemplarFilterHistogramsConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER_HISTOGRAMS";
 
     internal readonly IServiceProvider ServiceProvider;
     internal readonly IDisposable? OwnedServiceProvider;
@@ -24,6 +25,7 @@ internal sealed class MeterProviderSdk : MeterProvider
     internal bool EmitOverflowAttribute;
     internal bool ReclaimUnusedMetricPoints;
     internal ExemplarFilterType? ExemplarFilter;
+    internal ExemplarFilterType? ExemplarFilterForHistograms;
     internal Action? OnCollectObservableInstruments;
 
     private readonly List<object> instrumentations = new();
@@ -72,6 +74,9 @@ internal MeterProviderSdk(
 
         this.viewConfigs = state.ViewConfigs;
 
+        OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent(
+            $"MeterProvider configuration: {{MetricLimit={state.MetricLimit}, CardinalityLimit={state.CardinalityLimit}, EmitOverflowAttribute={this.EmitOverflowAttribute}, ReclaimUnusedMetricPoints={this.ReclaimUnusedMetricPoints}, ExemplarFilter={this.ExemplarFilter}, ExemplarFilterForHistograms={this.ExemplarFilterForHistograms}}}.");
+
         foreach (var reader in state.Readers)
         {
             Guard.ThrowIfNull(reader);
@@ -83,7 +88,8 @@ internal MeterProviderSdk(
                 state.CardinalityLimit,
                 this.EmitOverflowAttribute,
                 this.ReclaimUnusedMetricPoints,
-                this.ExemplarFilter);
+                this.ExemplarFilter,
+                this.ExemplarFilterForHistograms);
 
             if (this.reader == null)
             {
@@ -490,37 +496,70 @@ private void ApplySpecificationConfigurationKeys(IConfiguration configuration)
             OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("Reclaim unused metric point feature enabled via configuration.");
         }
 
+        var hasProgrammaticExemplarFilterValue = this.ExemplarFilter.HasValue;
+
         if (configuration.TryGetStringValue(ExemplarFilterConfigKey, out var configValue))
         {
-            if (this.ExemplarFilter.HasValue)
+            if (hasProgrammaticExemplarFilterValue)
             {
                 OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent(
                     $"Exemplar filter configuration value '{configValue}' has been ignored because a value '{this.ExemplarFilter}' was set programmatically.");
                 return;
             }
 
-            ExemplarFilterType? exemplarFilter;
+            if (!TryParseExemplarFilterFromConfigurationValue(configValue, out var exemplarFilter))
+            {
+                OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter configuration was found but the value '{configValue}' is invalid and will be ignored.");
+                return;
+            }
+
+            this.ExemplarFilter = exemplarFilter;
+
+            OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter set to '{exemplarFilter}' from configuration.");
+        }
+
+        if (configuration.TryGetStringValue(ExemplarFilterHistogramsConfigKey, out configValue))
+        {
+            if (hasProgrammaticExemplarFilterValue)
+            {
+                OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent(
+                    $"Exemplar filter histogram configuration value '{configValue}' has been ignored because a value '{this.ExemplarFilter}' was set programmatically.");
+                return;
+            }
+
+            if (!TryParseExemplarFilterFromConfigurationValue(configValue, out var exemplarFilter))
+            {
+                OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter histogram configuration was found but the value '{configValue}' is invalid and will be ignored.");
+                return;
+            }
+
+            this.ExemplarFilterForHistograms = exemplarFilter;
+
+            OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter for histograms set to '{exemplarFilter}' from configuration.");
+        }
+
+        static bool TryParseExemplarFilterFromConfigurationValue(string? configValue, out ExemplarFilterType? exemplarFilter)
+        {
             if (string.Equals("always_off", configValue, StringComparison.OrdinalIgnoreCase))
             {
                 exemplarFilter = ExemplarFilterType.AlwaysOff;
+                return true;
             }
-            else if (string.Equals("always_on", configValue, StringComparison.OrdinalIgnoreCase))
+
+            if (string.Equals("always_on", configValue, StringComparison.OrdinalIgnoreCase))
             {
                 exemplarFilter = ExemplarFilterType.AlwaysOn;
+                return true;
             }
-            else if (string.Equals("trace_based", configValue, StringComparison.OrdinalIgnoreCase))
+
+            if (string.Equals("trace_based", configValue, StringComparison.OrdinalIgnoreCase))
             {
                 exemplarFilter = ExemplarFilterType.TraceBased;
+                return true;
             }
-            else
-            {
-                OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter configuration was found but the value '{configValue}' is invalid and will be ignored.");
-                return;
-            }
-
-            this.ExemplarFilter = exemplarFilter;
 
-            OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter set to '{exemplarFilter}' from configuration.");
+            exemplarFilter = null;
+            return false;
         }
     }
 }
diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs
@@ -125,12 +125,7 @@ internal Metric(
             aggType = AggregationType.LongGauge;
             this.MetricType = MetricType.LongGauge;
         }
-        else if (instrumentIdentity.InstrumentType == typeof(Histogram<long>)
-            || instrumentIdentity.InstrumentType == typeof(Histogram<int>)
-            || instrumentIdentity.InstrumentType == typeof(Histogram<short>)
-            || instrumentIdentity.InstrumentType == typeof(Histogram<byte>)
-            || instrumentIdentity.InstrumentType == typeof(Histogram<float>)
-            || instrumentIdentity.InstrumentType == typeof(Histogram<double>))
+        else if (instrumentIdentity.IsHistogram)
         {
             var explicitBucketBounds = instrumentIdentity.HistogramBucketBounds;
             var exponentialMaxSize = instrumentIdentity.ExponentialHistogramMaxSize;
diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs
@@ -25,6 +25,7 @@ public abstract partial class MetricReader
     private bool emitOverflowAttribute;
     private bool reclaimUnusedMetricPoints;
     private ExemplarFilterType? exemplarFilter;
+    private ExemplarFilterType? exemplarFilterForHistograms;
 
     internal static void DeactivateMetric(Metric metric)
     {
@@ -54,6 +55,11 @@ internal virtual List<Metric> AddMetricWithNoViews(Instrument instrument)
         Debug.Assert(this.metrics != null, "this.metrics was null");
 
         var metricStreamIdentity = new MetricStreamIdentity(instrument!, metricStreamConfiguration: null);
+
+        var exemplarFilter = metricStreamIdentity.IsHistogram
+            ? this.exemplarFilterForHistograms ?? this.exemplarFilter
+            : this.exemplarFilter;
+
         lock (this.instrumentCreationLock)
         {
             if (this.TryGetExistingMetric(in metricStreamIdentity, out var existingMetric))
@@ -72,7 +78,13 @@ internal virtual List<Metric> AddMetricWithNoViews(Instrument instrument)
                 Metric? metric = null;
                 try
                 {
-                    metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.cardinalityLimit, this.emitOverflowAttribute, this.reclaimUnusedMetricPoints, this.exemplarFilter);
+                    metric = new Metric(
+                        metricStreamIdentity,
+                        this.GetAggregationTemporality(metricStreamIdentity.InstrumentType),
+                        this.cardinalityLimit,
+                        this.emitOverflowAttribute,
+                        this.reclaimUnusedMetricPoints,
+                        exemplarFilter);
                 }
                 catch (NotSupportedException nse)
                 {
@@ -114,6 +126,10 @@ internal virtual List<Metric> AddMetricWithViews(Instrument instrument, List<Met
                 var metricStreamConfig = metricStreamConfigs[i];
                 var metricStreamIdentity = new MetricStreamIdentity(instrument!, metricStreamConfig);
 
+                var exemplarFilter = metricStreamIdentity.IsHistogram
+                    ? this.exemplarFilterForHistograms ?? this.exemplarFilter
+                    : this.exemplarFilter;
+
                 if (!MeterProviderBuilderSdk.IsValidInstrumentName(metricStreamIdentity.InstrumentName))
                 {
                     OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
@@ -150,7 +166,7 @@ internal virtual List<Metric> AddMetricWithViews(Instrument instrument, List<Met
                         metricStreamConfig?.CardinalityLimit ?? this.cardinalityLimit,
                         this.emitOverflowAttribute,
                         this.reclaimUnusedMetricPoints,
-                        this.exemplarFilter,
+                        exemplarFilter,
                         metricStreamConfig?.ExemplarReservoirFactory);
 
                     this.instrumentIdentityToMetric[metricStreamIdentity] = metric;
@@ -170,7 +186,8 @@ internal void ApplyParentProviderSettings(
         int cardinalityLimit,
         bool emitOverflowAttribute,
         bool reclaimUnusedMetricPoints,
-        ExemplarFilterType? exemplarFilter)
+        ExemplarFilterType? exemplarFilter,
+        ExemplarFilterType? exemplarFilterForHistograms)
     {
         this.metricLimit = metricLimit;
         this.metrics = new Metric[metricLimit];
@@ -179,6 +196,7 @@ internal void ApplyParentProviderSettings(
         this.emitOverflowAttribute = emitOverflowAttribute;
         this.reclaimUnusedMetricPoints = reclaimUnusedMetricPoints;
         this.exemplarFilter = exemplarFilter;
+        this.exemplarFilterForHistograms = exemplarFilterForHistograms;
     }
 
     private bool TryGetExistingMetric(in MetricStreamIdentity metricStreamIdentity, [NotNullWhen(true)] out Metric? existingMetric)
diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs
@@ -115,6 +115,14 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me
 
     public bool HistogramRecordMinMax { get; }
 
+    public bool IsHistogram =>
+        this.InstrumentType == typeof(Histogram<long>)
+        || this.InstrumentType == typeof(Histogram<int>)
+        || this.InstrumentType == typeof(Histogram<short>)
+        || this.InstrumentType == typeof(Histogram<byte>)
+        || this.InstrumentType == typeof(Histogram<float>)
+        || this.InstrumentType == typeof(Histogram<double>);
+
     public static bool operator ==(MetricStreamIdentity metricIdentity1, MetricStreamIdentity metricIdentity2) => metricIdentity1.Equals(metricIdentity2);
 
     public static bool operator !=(MetricStreamIdentity metricIdentity1, MetricStreamIdentity metricIdentity2) => !metricIdentity1.Equals(metricIdentity2);
diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs

Original file line number	Diff line number	Diff line change
`@@ -125,12 +125,7 @@ internal Metric(`
`125`	`125`	`aggType = AggregationType.LongGauge;`
`126`	`126`	`this.MetricType = MetricType.LongGauge;`
`127`	`127`	`}`
`128`		`- else if (instrumentIdentity.InstrumentType == typeof(Histogram<long>)`
`129`		`- \|\| instrumentIdentity.InstrumentType == typeof(Histogram<int>)`
`130`		`- \|\| instrumentIdentity.InstrumentType == typeof(Histogram<short>)`
`131`		`- \|\| instrumentIdentity.InstrumentType == typeof(Histogram<byte>)`
`132`		`- \|\| instrumentIdentity.InstrumentType == typeof(Histogram<float>)`
`133`		`- \|\| instrumentIdentity.InstrumentType == typeof(Histogram<double>))`
	`128`	`+ else if (instrumentIdentity.IsHistogram)`
`134`	`129`	`{`
`135`	`130`	`var explicitBucketBounds = instrumentIdentity.HistogramBucketBounds;`
`136`	`131`	`var exponentialMaxSize = instrumentIdentity.ExponentialHistogramMaxSize;`