From 05fa66b51770cd118316b23f6f6ac8d929b31ed2 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 10:56:34 -0800 Subject: [PATCH 1/6] Support setting exemplar filter from envvar key. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 72 +++++++++++++++---- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 17 +++-- .../Metrics/MetricExemplarTests.cs | 40 +++++++++++ .../Metrics/MetricPointReclaimTestsBase.cs | 4 +- 4 files changed, 110 insertions(+), 23 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 430c7ecc0a..9a49947c68 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -13,16 +13,19 @@ namespace OpenTelemetry.Metrics; 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_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER"; + internal readonly IServiceProvider ServiceProvider; internal readonly IDisposable? OwnedServiceProvider; internal int ShutdownCount; internal bool Disposed; - internal bool ShouldReclaimUnusedMetricPoints; + internal bool EmitOverflowAttribute; + internal bool ReclaimUnusedMetricPoints; + internal ExemplarFilterType? ExemplarFilter; internal Action? OnCollectObservableInstruments; - private const string EmitOverFlowAttributeConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE"; - private const string ReclaimUnusedMetricPointsConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_RECLAIM_UNUSED_METRIC_POINTS"; - private readonly List instrumentations = new(); private readonly List> viewConfigs; private readonly object collectLock = new(); @@ -40,10 +43,6 @@ internal MeterProviderSdk( var state = serviceProvider!.GetRequiredService(); state.RegisterProvider(this); - var config = serviceProvider!.GetRequiredService(); - _ = config.TryGetBoolValue(EmitOverFlowAttributeConfigKey, out bool isEmitOverflowAttributeKeySet); - _ = config.TryGetBoolValue(ReclaimUnusedMetricPointsConfigKey, out this.ShouldReclaimUnusedMetricPoints); - this.ServiceProvider = serviceProvider!; if (ownsServiceProvider) @@ -54,14 +53,16 @@ internal MeterProviderSdk( OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("Building MeterProvider."); - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Metric overflow attribute key set to: {isEmitOverflowAttributeKeySet}"); - var configureProviderBuilders = serviceProvider!.GetServices(); foreach (var configureProviderBuilder in configureProviderBuilders) { configureProviderBuilder.ConfigureBuilder(serviceProvider!, state); } + this.ExemplarFilter = state.ExemplarFilter; + + this.ApplySpecificationConfigurationKeys(serviceProvider!.GetRequiredService()); + StringBuilder exportersAdded = new StringBuilder(); StringBuilder instrumentationFactoriesAdded = new StringBuilder(); @@ -80,8 +81,9 @@ internal MeterProviderSdk( reader.ApplyParentProviderSettings( state.MetricLimit, state.CardinalityLimit, - state.ExemplarFilter, - isEmitOverflowAttributeKeySet); + this.EmitOverflowAttribute, + this.ReclaimUnusedMetricPoints, + this.ExemplarFilter); if (this.reader == null) { @@ -475,4 +477,50 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + + private void ApplySpecificationConfigurationKeys(IConfiguration configuration) + { + if (configuration.TryGetBoolValue(EmitOverFlowAttributeConfigKey, out this.EmitOverflowAttribute)) + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("Overflow attribute feature enabled via configuration."); + } + + if (configuration.TryGetBoolValue(ReclaimUnusedMetricPointsConfigKey, out this.ReclaimUnusedMetricPoints)) + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("Reclaim unused metric point feature enabled via configuration."); + } + + if (configuration.TryGetStringValue(ExemplarFilterConfigKey, out var configValue)) + { + if (this.ExemplarFilter.HasValue) + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent( + $"Exemplar filter configuration value '{configValue}' has been ignored because a value '{this.ExemplarFilter}' was set programmatically."); + return; + } + + ExemplarFilterType? exemplarFilter; + if (string.Equals("always_off", configValue, StringComparison.OrdinalIgnoreCase)) + { + exemplarFilter = ExemplarFilterType.AlwaysOff; + } + else if (string.Equals("always_on", configValue, StringComparison.OrdinalIgnoreCase)) + { + exemplarFilter = ExemplarFilterType.AlwaysOn; + } + else if (string.Equals("trace_based", configValue, StringComparison.OrdinalIgnoreCase)) + { + exemplarFilter = ExemplarFilterType.TraceBased; + } + 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."); + } + } } diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 629ef4c33a..b64938e4c3 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -23,7 +23,7 @@ public abstract partial class MetricReader private Metric[]? metricsCurrentBatch; private int metricIndex = -1; private bool emitOverflowAttribute; - + private bool reclaimUnusedMetricPoints; private ExemplarFilterType? exemplarFilter; internal static void DeactivateMetric(Metric metric) @@ -72,8 +72,7 @@ internal virtual List AddMetricWithNoViews(Instrument instrument) Metric? metric = null; try { - bool shouldReclaimUnusedMetricPoints = this.parentProvider is MeterProviderSdk meterProviderSdk && meterProviderSdk.ShouldReclaimUnusedMetricPoints; - metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.cardinalityLimit, this.emitOverflowAttribute, shouldReclaimUnusedMetricPoints, this.exemplarFilter); + metric = new Metric(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.cardinalityLimit, this.emitOverflowAttribute, this.reclaimUnusedMetricPoints, this.exemplarFilter); } catch (NotSupportedException nse) { @@ -145,14 +144,12 @@ internal virtual List AddMetricWithViews(Instrument instrument, List AddMetricWithViews(Instrument instrument, List 1) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs index c0927683f8..c3c45d5845 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs @@ -5,6 +5,8 @@ using System.Diagnostics; using System.Diagnostics.Metrics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Tests; using Xunit; @@ -14,6 +16,44 @@ public class MetricExemplarTests : MetricTestsBase { private const int MaxTimeToAllowForFlush = 10000; + [Theory] + [InlineData(null, null, null)] + [InlineData(null, "always_off", ExemplarFilterType.AlwaysOff)] + [InlineData(null, "ALWays_ON", ExemplarFilterType.AlwaysOn)] + [InlineData(null, "trace_based", ExemplarFilterType.TraceBased)] + [InlineData(null, "invalid", null)] + [InlineData(ExemplarFilterType.AlwaysOn, "trace_based", ExemplarFilterType.AlwaysOn)] + public void TestExemplarFilterSetFromConfiguration( + ExemplarFilterType? programmaticValue, + string? configValue, + ExemplarFilterType? expectedValue) + { + var configBuilder = new ConfigurationBuilder(); + if (!string.IsNullOrEmpty(configValue)) + { + configBuilder.AddInMemoryCollection(new Dictionary + { + [MeterProviderSdk.ExemplarFilterConfigKey] = configValue, + }); + } + + using var container = this.BuildMeterProvider(out var meterProvider, b => + { + b.ConfigureServices( + s => s.AddSingleton(configBuilder.Build())); + + if (programmaticValue.HasValue) + { + b.SetExemplarFilter(programmaticValue.Value); + } + }); + + var meterProviderSdk = meterProvider as MeterProviderSdk; + + Assert.NotNull(meterProviderSdk); + Assert.Equal(expectedValue, meterProviderSdk.ExemplarFilter); + } + [Theory] [InlineData(MetricReaderTemporalityPreference.Cumulative)] [InlineData(MetricReaderTemporalityPreference.Delta)] diff --git a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs index a10849c405..fa33298643 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricPointReclaimTestsBase.cs @@ -57,7 +57,7 @@ public void TestReclaimAttributeConfigWithEnvVar(string value, bool isReclaimAtt .Build(); var meterProviderSdk = meterProvider as MeterProviderSdk; - Assert.Equal(isReclaimAttributeKeySet, meterProviderSdk.ShouldReclaimUnusedMetricPoints); + Assert.Equal(isReclaimAttributeKeySet, meterProviderSdk.ReclaimUnusedMetricPoints); } [Theory] @@ -87,7 +87,7 @@ public void TestReclaimAttributeConfigWithOtherConfigProvider(string value, bool .Build(); var meterProviderSdk = meterProvider as MeterProviderSdk; - Assert.Equal(isReclaimAttributeKeySet, meterProviderSdk.ShouldReclaimUnusedMetricPoints); + Assert.Equal(isReclaimAttributeKeySet, meterProviderSdk.ReclaimUnusedMetricPoints); } [Theory] From f5e60696b9ca3be16f8c5333879b95038c8170cc Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 11:07:08 -0800 Subject: [PATCH 2/6] Stable build fixes. --- .../Metrics/MetricExemplarTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs index c3c45d5845..d0bd4ff0eb 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs @@ -18,15 +18,15 @@ public class MetricExemplarTests : MetricTestsBase [Theory] [InlineData(null, null, null)] - [InlineData(null, "always_off", ExemplarFilterType.AlwaysOff)] - [InlineData(null, "ALWays_ON", ExemplarFilterType.AlwaysOn)] - [InlineData(null, "trace_based", ExemplarFilterType.TraceBased)] + [InlineData(null, "always_off", (int)ExemplarFilterType.AlwaysOff)] + [InlineData(null, "ALWays_ON", (int)ExemplarFilterType.AlwaysOn)] + [InlineData(null, "trace_based", (int)ExemplarFilterType.TraceBased)] [InlineData(null, "invalid", null)] - [InlineData(ExemplarFilterType.AlwaysOn, "trace_based", ExemplarFilterType.AlwaysOn)] + [InlineData((int)ExemplarFilterType.AlwaysOn, "trace_based", (int)ExemplarFilterType.AlwaysOn)] public void TestExemplarFilterSetFromConfiguration( - ExemplarFilterType? programmaticValue, + int? programmaticValue, string? configValue, - ExemplarFilterType? expectedValue) + int? expectedValue) { var configBuilder = new ConfigurationBuilder(); if (!string.IsNullOrEmpty(configValue)) @@ -44,14 +44,14 @@ public void TestExemplarFilterSetFromConfiguration( if (programmaticValue.HasValue) { - b.SetExemplarFilter(programmaticValue.Value); + b.SetExemplarFilter(((ExemplarFilterType?)programmaticValue).Value); } }); var meterProviderSdk = meterProvider as MeterProviderSdk; Assert.NotNull(meterProviderSdk); - Assert.Equal(expectedValue, meterProviderSdk.ExemplarFilter); + Assert.Equal((ExemplarFilterType?)expectedValue, meterProviderSdk.ExemplarFilter); } [Theory] From 2026e86413d840f4cb77d3cb1d692c1c7460d6e4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 12:17:06 -0800 Subject: [PATCH 3/6] Code review. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 4 ++- ...nTelemetry.Extensions.Hosting.Tests.csproj | 1 + .../Metrics/MetricExemplarTests.cs | 3 +- .../Shared/SkipUnlessTrueTheoryAttribute.cs | 33 +++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/OpenTelemetry.Tests/Shared/SkipUnlessTrueTheoryAttribute.cs diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 9a49947c68..0e5614abbe 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -15,7 +15,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_DOTNET_EXPERIMENTAL_METRICS_EXEMPLAR_FILTER"; + internal const string ExemplarFilterConfigKey = "OTEL_METRICS_EXEMPLAR_FILTER"; internal readonly IServiceProvider ServiceProvider; internal readonly IDisposable? OwnedServiceProvider; @@ -490,6 +490,7 @@ private void ApplySpecificationConfigurationKeys(IConfiguration configuration) OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("Reclaim unused metric point feature enabled via configuration."); } +#if EXPOSE_EXPERIMENTAL_FEATURES if (configuration.TryGetStringValue(ExemplarFilterConfigKey, out var configValue)) { if (this.ExemplarFilter.HasValue) @@ -522,5 +523,6 @@ private void ApplySpecificationConfigurationKeys(IConfiguration configuration) OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter set to '{exemplarFilter}' from configuration."); } +#endif } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj index 8ae85542eb..1ea17a421d 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj @@ -14,6 +14,7 @@ + diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs index d0bd4ff0eb..4c8a4c4153 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs @@ -15,8 +15,9 @@ namespace OpenTelemetry.Metrics.Tests; public class MetricExemplarTests : MetricTestsBase { private const int MaxTimeToAllowForFlush = 10000; + private static readonly Func IsExemplarApiExposed = () => typeof(ExemplarFilterType).IsVisible; - [Theory] + [SkipUnlessTrueTheory(typeof(MetricExemplarTests), nameof(IsExemplarApiExposed), "ExemplarFilter config tests skipped for stable builds")] [InlineData(null, null, null)] [InlineData(null, "always_off", (int)ExemplarFilterType.AlwaysOff)] [InlineData(null, "ALWays_ON", (int)ExemplarFilterType.AlwaysOn)] diff --git a/test/OpenTelemetry.Tests/Shared/SkipUnlessTrueTheoryAttribute.cs b/test/OpenTelemetry.Tests/Shared/SkipUnlessTrueTheoryAttribute.cs new file mode 100644 index 0000000000..087bff4366 --- /dev/null +++ b/test/OpenTelemetry.Tests/Shared/SkipUnlessTrueTheoryAttribute.cs @@ -0,0 +1,33 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Reflection; +using OpenTelemetry.Internal; +using Xunit; + +namespace OpenTelemetry.Tests; + +internal sealed class SkipUnlessTrueTheoryAttribute : TheoryAttribute +{ + public SkipUnlessTrueTheoryAttribute(Type typeContainingTest, string testFieldName, string skipMessage) + { + Guard.ThrowIfNull(typeContainingTest); + Guard.ThrowIfNullOrEmpty(testFieldName); + Guard.ThrowIfNullOrEmpty(skipMessage); + + var field = typeContainingTest.GetField(testFieldName, BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic) + ?? throw new InvalidOperationException($"Static field '{testFieldName}' could not be found on '{typeContainingTest}' type."); + + if (field.FieldType != typeof(Func)) + { + throw new InvalidOperationException($"Field '{testFieldName}' on '{typeContainingTest}' type should be defined as '{typeof(Func)}'."); + } + + var testFunc = (Func)field.GetValue(null); + + if (!testFunc()) + { + this.Skip = skipMessage; + } + } +} From ec4849e8f581bc72fa7f3513d26b888860521777 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 12:20:11 -0800 Subject: [PATCH 4/6] Update CHANGELOG. --- src/OpenTelemetry/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 2958b18487..3d3ad14430 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -70,6 +70,12 @@ Specification](https://github.com/open-telemetry/opentelemetry-specification/pull/3820). ([#5404](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5404)) +* **Experimental (pre-release builds only):** The `ExemplarFilter` used by SDK + `MeterProvider`s can now be controlled via the `OTEL_METRICS_EXEMPLAR_FILTER` + environment variable. For details see: [OpenTelemetry Environment Variable + Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#exemplar). + ([#5412](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5412)) + ## 1.7.0 Released 2023-Dec-08 From d648b021a016a4c612a5dfb0151314367f1b7a01 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 13:34:28 -0800 Subject: [PATCH 5/6] Code review. --- src/OpenTelemetry/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 3d3ad14430..b3e5a11e44 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -72,7 +72,8 @@ * **Experimental (pre-release builds only):** The `ExemplarFilter` used by SDK `MeterProvider`s can now be controlled via the `OTEL_METRICS_EXEMPLAR_FILTER` - environment variable. For details see: [OpenTelemetry Environment Variable + environment variable. The supported values are: `always_off`, `always_on`, and + `trace_based`. For details see: [OpenTelemetry Environment Variable Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#exemplar). ([#5412](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5412)) From aa7d236399cfd4acb97fb03b5ad7bc471bc0e845 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 5 Mar 2024 13:38:46 -0800 Subject: [PATCH 6/6] Code review. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 0e5614abbe..22f49cb986 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -523,6 +523,12 @@ private void ApplySpecificationConfigurationKeys(IConfiguration configuration) OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Exemplar filter set to '{exemplarFilter}' from configuration."); } +#else + if (configuration.TryGetStringValue(ExemplarFilterConfigKey, out var configValue)) + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent( + $"Exemplar filter configuration value '{configValue}' has been ignored because exemplars are an experimental feature not available in stable builds."); + } #endif } }