From 9331203eb31d9d09fc4044be9686c61c21c00139 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 28 Jul 2023 12:06:53 -0700 Subject: [PATCH 01/43] Implement an IMetricsListener over OpenTelemetry SDK. --- Directory.Packages.props | 12 +- examples/Directory.Packages.props | 2 + .../OpenTelemetryMetricListener.cs | 139 ++++++++ .../OpenTelemetry.Extensions.Hosting.csproj | 1 + .../OpenTelemetryBuilder.cs | 4 + .../OpenTelemetryMetricsBuilderExtensions.cs | 36 ++ src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 321 +++++++++--------- 7 files changed, 337 insertions(+), 178 deletions(-) create mode 100644 src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs create mode 100644 src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 626320dbe96..1bedeb999f0 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -2,7 +2,6 @@ true - @@ -13,9 +12,10 @@ + - + @@ -24,7 +24,6 @@ - - - - - - @@ -60,7 +54,7 @@ - + diff --git a/examples/Directory.Packages.props b/examples/Directory.Packages.props index 9a73f74f978..3c601684ac7 100644 --- a/examples/Directory.Packages.props +++ b/examples/Directory.Packages.props @@ -4,5 +4,7 @@ + + diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs new file mode 100644 index 00000000000..4801415c0cc --- /dev/null +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs @@ -0,0 +1,139 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Diagnostics; +using System.Diagnostics.Metrics; +using Microsoft.Extensions.Diagnostics.Metrics; + +namespace OpenTelemetry.Metrics; + +internal sealed class OpenTelemetryMetricListener : IMetricsListener +{ + private readonly MeterProviderSdk meterProviderSdk; + private IMetricsSource? metricsSource; + + public OpenTelemetryMetricListener(MeterProvider meterProvider) + { + var meterProviderSdk = meterProvider as MeterProviderSdk; + + Debug.Assert(meterProviderSdk != null, "meterProvider was not MeterProviderSdk"); + + this.meterProviderSdk = meterProviderSdk!; + + this.meterProviderSdk.OnCollectObservableInstruments += () => + { + this.metricsSource?.RecordObservableInstruments(); + }; + } + + public string Name => "OpenTelemetry"; + + public MeasurementCallback GetMeasurementHandler() + where T : struct + { + if (typeof(T) == typeof(byte)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedLong(instrument, (byte)(object)value, tags, state); + } + else if (typeof(T) == typeof(short)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedLong(instrument, (short)(object)value, tags, state); + } + else if (typeof(T) == typeof(int)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedLong(instrument, (int)(object)value, tags, state); + } + else if (typeof(T) == typeof(long)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedLong(instrument, (long)(object)value, tags, state); + } + else if (typeof(T) == typeof(float)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedDouble(instrument, (float)(object)value, tags, state); + } + else if (typeof(T) == typeof(double)) + { + return (instrument, value, tags, state) + => this.MeasurementRecordedDouble(instrument, (double)(object)value, tags, state); + } + else + { + return MeasurementRecordedUnknown; + } + } + + public object? InstrumentPublished(Instrument instrument) + { + var state = this.meterProviderSdk.InstrumentPublished(instrument, skipShouldListenToCheck: true); + return state; + } + + public void MeasurementsCompleted(Instrument instrument, object? userState) + { + this.meterProviderSdk.MeasurementsCompleted(instrument, userState); + } + + public void SetSource(IMetricsSource source) + { + this.metricsSource = source; + } + + private static void MeasurementRecordedUnknown(Instrument instrument, T value, ReadOnlySpan> tagsRos, object? state) + { + // todo: Log dropped metric + } + + private void MeasurementRecordedDouble(Instrument instrument, double value, ReadOnlySpan> tagsRos, object? state) + { + var meterProvider = this.meterProviderSdk!; + + if (state is List) + { + meterProvider.MeasurementRecordedDouble(instrument, value, tagsRos, state); + } + else if (state is Metric) + { + meterProvider.MeasurementRecordedDoubleSingleStream(instrument, value, tagsRos, state); + } + else + { + // todo: Log dropped metric + } + } + + private void MeasurementRecordedLong(Instrument instrument, long value, ReadOnlySpan> tagsRos, object? state) + { + var meterProvider = this.meterProviderSdk!; + + if (state is List) + { + meterProvider.MeasurementRecordedLong(instrument, value, tagsRos, state); + } + else if (state is Metric) + { + meterProvider.MeasurementRecordedLongSingleStream(instrument, value, tagsRos, state); + } + else + { + // todo: Log dropped metric + } + } +} diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj index 133f193acc6..e8ac7f30cbe 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj @@ -11,6 +11,7 @@ + diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 1f50f76637a..6507b89e4dc 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -15,6 +15,8 @@ // using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Diagnostics.Metrics; using OpenTelemetry.Internal; using OpenTelemetry.Logs; using OpenTelemetry.Metrics; @@ -99,6 +101,8 @@ public OpenTelemetryBuilder WithMetrics(Action configure) var builder = new MeterProviderBuilderBase(this.Services); + this.Services.TryAddSingleton(); + configure(builder); return this; diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs new file mode 100644 index 00000000000..20c0eaf5702 --- /dev/null +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -0,0 +1,36 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.Extensions.DependencyInjection; +using OpenTelemetry.Internal; +using OpenTelemetry.Metrics; + +namespace Microsoft.Extensions.Diagnostics.Metrics; + +public static class OpenTelemetryMetricsBuilderExtensions +{ + public static IMetricsBuilder AddOpenTelemetry(this IMetricsBuilder metricsBuilder) + => AddOpenTelemetry(metricsBuilder, b => { }); + + public static IMetricsBuilder AddOpenTelemetry(this IMetricsBuilder metricsBuilder, Action configure) + { + Guard.ThrowIfNull(metricsBuilder); + + metricsBuilder.Services.AddOpenTelemetry().WithMetrics(configure); + + return metricsBuilder; + } +} diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 73123215dad..fec31c49ab8 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -31,6 +31,7 @@ internal sealed class MeterProviderSdk : MeterProvider internal readonly IDisposable? OwnedServiceProvider; internal int ShutdownCount; internal bool Disposed; + internal Action? OnCollectObservableInstruments; private readonly List instrumentations = new(); private readonly List> viewConfigs; @@ -38,6 +39,7 @@ internal sealed class MeterProviderSdk : MeterProvider private readonly MeterListener listener; private readonly MetricReader? reader; private readonly CompositeMetricReader? compositeMetricReader; + private readonly Func shouldListenTo = instrument => false; internal MeterProviderSdk( IServiceProvider serviceProvider, @@ -136,16 +138,15 @@ internal MeterProviderSdk( } // Setup Listener - Func shouldListenTo = instrument => false; if (state.MeterSources.Any(s => WildcardHelper.ContainsWildcard(s))) { var regex = WildcardHelper.GetWildcardRegex(state.MeterSources); - shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); + this.shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); } else if (state.MeterSources.Any()) { var meterSourcesToSubscribe = new HashSet(state.MeterSources, StringComparer.OrdinalIgnoreCase); - shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); + this.shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); } OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Listening to following meters = \"{string.Join(";", state.MeterSources)}\"."); @@ -155,116 +156,19 @@ internal MeterProviderSdk( OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Number of views configured = {viewConfigCount}."); + this.listener.InstrumentPublished = (instrument, listener) => + { + object? state = this.InstrumentPublished(instrument, skipShouldListenToCheck: false); + if (state != null) + { + listener.EnableMeasurementEvents(instrument, state); + } + }; + // We expect that all the readers to be added are provided before MeterProviderSdk is built. // If there are no readers added, we do not enable measurements for the instruments. if (viewConfigCount > 0) { - this.listener.InstrumentPublished = (instrument, listener) => - { - bool enabledMeasurements = false; - - if (!shouldListenTo(instrument)) - { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter not subscribed by the provider.", "Use AddMeter to add the Meter to the provider."); - return; - } - - try - { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Started publishing Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\"."); - - // Creating list with initial capacity as the maximum - // possible size, to avoid any array resize/copy internally. - // There may be excess space wasted, but it'll eligible for - // GC right after this method. - var metricStreamConfigs = new List(viewConfigCount); - for (var i = 0; i < viewConfigCount; ++i) - { - var viewConfig = this.viewConfigs[i]; - MetricStreamConfiguration? metricStreamConfig = null; - - try - { - metricStreamConfig = viewConfig(instrument); - - // The SDK provides some static MetricStreamConfigurations. - // For example, the Drop configuration. The static ViewId - // should not be changed for these configurations. - if (metricStreamConfig != null && !metricStreamConfig.ViewId.HasValue) - { - metricStreamConfig.ViewId = i; - } - - if (metricStreamConfig is HistogramConfiguration - && instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>)) - { - metricStreamConfig = null; - - OpenTelemetrySdkEventSource.Log.MetricViewIgnored( - instrument.Name, - instrument.Meter.Name, - "The current SDK does not allow aggregating non-Histogram instruments as Histograms.", - "Fix the view configuration."); - } - } - catch (Exception ex) - { - OpenTelemetrySdkEventSource.Log.MetricViewIgnored(instrument.Name, instrument.Meter.Name, ex.Message, "Fix the view configuration."); - } - - if (metricStreamConfig != null) - { - metricStreamConfigs.Add(metricStreamConfig); - } - } - - if (metricStreamConfigs.Count == 0) - { - // No views matched. Add null - // which will apply defaults. - // Users can turn off this default - // by adding a view like below as the last view. - // .AddView(instrumentName: "*", MetricStreamConfiguration.Drop) - metricStreamConfigs.Add(null); - } - - if (this.reader != null) - { - if (this.compositeMetricReader == null) - { - var metrics = this.reader.AddMetricsListWithViews(instrument, metricStreamConfigs); - if (metrics.Count > 0) - { - listener.EnableMeasurementEvents(instrument, metrics); - enabledMeasurements = true; - } - } - else - { - var metricsSuperList = this.compositeMetricReader.AddMetricsSuperListWithViews(instrument, metricStreamConfigs); - if (metricsSuperList.Any(metrics => metrics.Count > 0)) - { - listener.EnableMeasurementEvents(instrument, metricsSuperList); - enabledMeasurements = true; - } - } - } - - if (enabledMeasurements) - { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be processed and aggregated by the SDK."); - } - else - { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be dropped by the SDK."); - } - } - catch (Exception) - { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "SDK internal error occurred.", "Contact SDK owners."); - } - }; - // Everything double this.listener.SetMeasurementEventCallback(this.MeasurementRecordedDouble); this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedDouble(instrument, value, tags, state)); @@ -279,92 +183,169 @@ internal MeterProviderSdk( } else { - this.listener.InstrumentPublished = (instrument, listener) => - { - bool enabledMeasurements = false; + // Everything double + this.listener.SetMeasurementEventCallback(this.MeasurementRecordedDoubleSingleStream); + this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedDoubleSingleStream(instrument, value, tags, state)); + + // Everything long + this.listener.SetMeasurementEventCallback(this.MeasurementRecordedLongSingleStream); + this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); + this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); + this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); + + this.listener.MeasurementsCompleted = (instrument, state) => this.MeasurementsCompletedSingleStream(instrument, state); + } + + this.listener.Start(); + + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("MeterProvider built successfully."); + } + + internal Resource Resource { get; } + + internal List Instrumentations => this.instrumentations; + + internal MetricReader? Reader => this.reader; + + internal object? InstrumentPublished(Instrument instrument, bool skipShouldListenToCheck) + { + if (!skipShouldListenToCheck && !this.shouldListenTo(instrument)) + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter not subscribed by the provider.", "Use AddMeter to add the Meter to the provider."); + return null; + } - if (!shouldListenTo(instrument)) + object? state = null; + var viewConfigCount = this.viewConfigs.Count; + + try + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Started publishing Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\"."); + + if (viewConfigCount <= 0) + { + if (!MeterProviderBuilderSdk.IsValidInstrumentName(instrument.Name)) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter not subscribed by the provider.", "Use AddMeter to add the Meter to the provider."); - return; + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( + instrument.Name, + instrument.Meter.Name, + "Instrument name is invalid.", + "The name must comply with the OpenTelemetry specification"); + return false; } - try + if (this.reader != null) { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Started publishing Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\"."); - - if (!MeterProviderBuilderSdk.IsValidInstrumentName(instrument.Name)) + if (this.compositeMetricReader == null) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( - instrument.Name, - instrument.Meter.Name, - "Instrument name is invalid.", - "The name must comply with the OpenTelemetry specification"); - - return; + state = this.reader.AddMetricWithNoViews(instrument); } + else + { + var metrics = this.compositeMetricReader.AddMetricsWithNoViews(instrument); + if (metrics.Any(metric => metric != null)) + { + state = metrics; + } + } + } + } + else + { + // Creating list with initial capacity as the maximum + // possible size, to avoid any array resize/copy internally. + // There may be excess space wasted, but it'll eligible for + // GC right after this method. + var metricStreamConfigs = new List(viewConfigCount); + for (var i = 0; i < viewConfigCount; ++i) + { + var viewConfig = this.viewConfigs[i]; + MetricStreamConfiguration? metricStreamConfig = null; - if (this.reader != null) + try { - if (this.compositeMetricReader == null) + metricStreamConfig = viewConfig(instrument); + + // The SDK provides some static MetricStreamConfigurations. + // For example, the Drop configuration. The static ViewId + // should not be changed for these configurations. + if (metricStreamConfig != null && !metricStreamConfig.ViewId.HasValue) { - var metric = this.reader.AddMetricWithNoViews(instrument); - if (metric != null) - { - listener.EnableMeasurementEvents(instrument, metric); - enabledMeasurements = true; - } + metricStreamConfig.ViewId = i; } - else + + if (metricStreamConfig is HistogramConfiguration + && instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>)) { - var metrics = this.compositeMetricReader.AddMetricsWithNoViews(instrument); - if (metrics.Any(metric => metric != null)) - { - listener.EnableMeasurementEvents(instrument, metrics); - enabledMeasurements = true; - } + metricStreamConfig = null; + + OpenTelemetrySdkEventSource.Log.MetricViewIgnored( + instrument.Name, + instrument.Meter.Name, + "The current SDK does not allow aggregating non-Histogram instruments as Histograms.", + "Fix the view configuration."); } } - - if (enabledMeasurements) + catch (Exception ex) { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be processed and aggregated by the SDK."); + OpenTelemetrySdkEventSource.Log.MetricViewIgnored(instrument.Name, instrument.Meter.Name, ex.Message, "Fix the view configuration."); } - else + + if (metricStreamConfig != null) { - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be dropped by the SDK."); + metricStreamConfigs.Add(metricStreamConfig); } } - catch (Exception) + + if (metricStreamConfigs.Count == 0) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "SDK internal error occurred.", "Contact SDK owners."); + // No views matched. Add null + // which will apply defaults. + // Users can turn off this default + // by adding a view like below as the last view. + // .AddView(instrumentName: "*", MetricStreamConfiguration.Drop) + metricStreamConfigs.Add(null); } - }; - - // Everything double - this.listener.SetMeasurementEventCallback(this.MeasurementRecordedDoubleSingleStream); - this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedDoubleSingleStream(instrument, value, tags, state)); - // Everything long - this.listener.SetMeasurementEventCallback(this.MeasurementRecordedLongSingleStream); - this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); - this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); - this.listener.SetMeasurementEventCallback((instrument, value, tags, state) => this.MeasurementRecordedLongSingleStream(instrument, value, tags, state)); + if (this.reader != null) + { + if (this.compositeMetricReader == null) + { + var metrics = this.reader.AddMetricsListWithViews(instrument, metricStreamConfigs); + if (metrics.Count > 0) + { + state = metrics; + } + } + else + { + var metricsSuperList = this.compositeMetricReader.AddMetricsSuperListWithViews(instrument, metricStreamConfigs); + if (metricsSuperList.Any(metrics => metrics.Count > 0)) + { + state = metricsSuperList; + } + } + } + } - this.listener.MeasurementsCompleted = (instrument, state) => this.MeasurementsCompletedSingleStream(instrument, state); + if (state != null) + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be processed and aggregated by the SDK."); + return state; + } + else + { + OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent($"Measurements for Instrument = \"{instrument.Name}\" of Meter = \"{instrument.Meter.Name}\" will be dropped by the SDK."); + return null; + } + } + catch (Exception) + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "SDK internal error occurred.", "Contact SDK owners."); + return null; } - - this.listener.Start(); - - OpenTelemetrySdkEventSource.Log.MeterProviderSdkEvent("MeterProvider built successfully."); } - internal Resource Resource { get; } - - internal List Instrumentations => this.instrumentations; - - internal MetricReader? Reader => this.reader; - internal void MeasurementsCompletedSingleStream(Instrument instrument, object? state) { Debug.Assert(instrument != null, "instrument must be non-null."); @@ -529,6 +510,8 @@ internal void CollectObservableInstruments() try { this.listener.RecordObservableInstruments(); + + this.OnCollectObservableInstruments?.Invoke(); } catch (Exception exception) { From b70f529d60858030793d908848938b742c164547 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 28 Jul 2023 12:12:08 -0700 Subject: [PATCH 02/43] Tweak. --- .../Implementation/OpenTelemetryMetricListener.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs index 4801415c0cc..07f6ed5b72d 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs @@ -82,8 +82,7 @@ public MeasurementCallback GetMeasurementHandler() public object? InstrumentPublished(Instrument instrument) { - var state = this.meterProviderSdk.InstrumentPublished(instrument, skipShouldListenToCheck: true); - return state; + return this.meterProviderSdk.InstrumentPublished(instrument, skipShouldListenToCheck: true); } public void MeasurementsCompleted(Instrument instrument, object? userState) From e411e224ffce38b951530279abc6a645c4bcfbfd Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 28 Jul 2023 13:21:54 -0700 Subject: [PATCH 03/43] Tweak. --- .../Implementation/OpenTelemetryMetricListener.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs index 07f6ed5b72d..7ab07295dcb 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs @@ -102,7 +102,7 @@ private static void MeasurementRecordedUnknown(Instrument instrument, T value private void MeasurementRecordedDouble(Instrument instrument, double value, ReadOnlySpan> tagsRos, object? state) { - var meterProvider = this.meterProviderSdk!; + var meterProvider = this.meterProviderSdk; if (state is List) { @@ -120,7 +120,7 @@ private void MeasurementRecordedDouble(Instrument instrument, double value, Read private void MeasurementRecordedLong(Instrument instrument, long value, ReadOnlySpan> tagsRos, object? state) { - var meterProvider = this.meterProviderSdk!; + var meterProvider = this.meterProviderSdk; if (state is List) { From 6db3bcbe829f564f30926b9c2fc525791ae1f1b8 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 9 Oct 2023 13:15:54 -0700 Subject: [PATCH 04/43] Manually fix merge conflicts and update API to released versions. --- Directory.Packages.props | 5 +- .../OpenTelemetryMetricListener.cs | 66 ++-- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 321 +++++++++--------- 3 files changed, 175 insertions(+), 217 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index fd00f3ef3dd..e3fa15ebabc 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -24,9 +24,10 @@ + - + disable + $(DefineConstants);BUILDING_HOSTING_TESTS @@ -17,6 +18,19 @@ + + + + + + + + + + + diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs index bfe2229dcf5..acc6cd2e7aa 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs @@ -50,10 +50,10 @@ public void MeasurementWithNullValuedTag() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counter = meter.CreateCounter("myCounter"); counter.Add(100, new KeyValuePair("tagWithNullValue", null)); @@ -83,10 +83,10 @@ public void ObserverCallbackTest() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter.CreateObservableGauge("myGauge", () => measurement); @@ -112,10 +112,10 @@ public void ObserverCallbackExceptionTest() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter.CreateObservableGauge("myGauge", () => measurement); @@ -146,11 +146,10 @@ public void MetricUnitIsExportedCorrectly(string unit) var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var counter = meter.CreateCounter("name1", unit); counter.Add(10); @@ -169,11 +168,10 @@ public void MetricDescriptionIsExportedCorrectly(string description) var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var counter = meter.CreateCounter("name1", null, description); counter.Add(10); @@ -189,11 +187,10 @@ public void DuplicateInstrumentRegistration_NoViews_IdenticalInstruments() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); var duplicateInstrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); @@ -223,11 +220,10 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription1"); var duplicateInstrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription2"); @@ -270,11 +266,10 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit1", "instrumentDescription"); var duplicateInstrument = meter.CreateCounter("instrumentName", "instrumentUnit2", "instrumentDescription"); @@ -317,11 +312,10 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); var duplicateInstrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); @@ -362,11 +356,10 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); var duplicateInstrument = meter.CreateHistogram("instrumentName", "instrumentUnit", "instrumentDescription"); @@ -409,12 +402,11 @@ public void DuplicateInstrumentNamesFromDifferentMetersWithSameNameDifferentVers using var meter1 = new Meter($"{Utils.GetCurrentMethodName()}", "1.0"); using var meter2 = new Meter($"{Utils.GetCurrentMethodName()}", "2.0"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddMeter(meter2.Name) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. var counterLong = meter1.CreateCounter("name1"); @@ -442,20 +434,22 @@ public void DuplicateInstrumentNamesFromDifferentMetersAreAllowed(MetricReaderTe using var meter1 = new Meter($"{Utils.GetCurrentMethodName()}.1.{temporality}"); using var meter2 = new Meter($"{Utils.GetCurrentMethodName()}.2.{temporality}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter1.Name) - .AddMeter(meter2.Name) - .AddInMemoryExporter(exportedItems, metricReaderOptions => - { - metricReaderOptions.TemporalityPreference = temporality; - }); - if (hasView) + using var container = BuildMeterProvider(out var meterProvider, builder => { - meterProviderBuilder.AddView("name1", new MetricStreamConfiguration() { Description = "description" }); - } + builder + .AddMeter(meter1.Name) + .AddMeter(meter2.Name) + .AddInMemoryExporter(exportedItems, metricReaderOptions => + { + metricReaderOptions.TemporalityPreference = temporality; + }); - using var meterProvider = meterProviderBuilder.Build(); + if (hasView) + { + builder.AddView("name1", new MetricStreamConfiguration() { Description = "description" }); + } + }); // Expecting one metric stream. var counterLong = meter1.CreateCounter("name1"); @@ -473,6 +467,7 @@ public void DuplicateInstrumentNamesFromDifferentMetersAreAllowed(MetricReaderTe Assert.Equal(2, exportedItems.Count); } +#if !BUILDING_HOSTING_TESTS [Theory] [InlineData(true)] [InlineData(false)] @@ -486,18 +481,20 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) using var meter6 = new Meter("SomeCompany.SomeProduct.SomeComponent"); var exportedItems = new List(); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter("AbcCompany.XyzProduct.Component?") - .AddMeter("DefCompany.*.ComponentC") - .AddMeter("GhiCompany.qweProduct.ComponentN") // Mixing of non-wildcard meter name and wildcard meter name. - .AddInMemoryExporter(exportedItems); - if (hasView) + using var container = BuildMeterProvider(out var meterProvider, builder => { - meterProviderBuilder.AddView("myGauge1", "newName"); - } + builder + .AddMeter("AbcCompany.XyzProduct.Component?") + .AddMeter("DefCompany.*.ComponentC") + .AddMeter("GhiCompany.qweProduct.ComponentN") // Mixing of non-wildcard meter name and wildcard meter name. + .AddInMemoryExporter(exportedItems); - using var meterProvider = meterProviderBuilder.Build(); + if (hasView) + { + builder.AddView("myGauge1", "newName"); + } + }); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter1.CreateObservableGauge("myGauge1", () => measurement); @@ -525,6 +522,7 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) Assert.Equal("myGauge4", exportedItems[3].Name); Assert.Equal("myGauge5", exportedItems[4].Name); } +#endif [Theory] [InlineData(true)] @@ -535,15 +533,18 @@ public void MeterSourcesWildcardSupportNegativeTestNoMeterAdded(bool hasView) using var meter2 = new Meter($"abcCompany.xYzProduct.componentC.{hasView}"); var exportedItems = new List(); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddInMemoryExporter(exportedItems); - if (hasView) + using var container = BuildMeterProvider(out var meterProvider, builder => { - meterProviderBuilder.AddView("gauge1", "renamed"); - } + builder + .AddInMemoryExporter(exportedItems); + + if (hasView) + { + builder.AddView("gauge1", "renamed"); + } + }); - using var meterProvider = meterProviderBuilder.Build(); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter1.CreateObservableGauge("myGauge1", () => measurement); @@ -564,13 +565,13 @@ public void CounterAggregationTest(bool exportDelta) using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{exportDelta}"); var counterLong = meter.CreateCounter("mycounter"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); counterLong.Add(10); counterLong.Add(10); @@ -666,13 +667,12 @@ public void ObservableCounterAggregationTest(bool exportDelta) }; }); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); meterProvider.ForceFlush(MaxTimeToAllowForFlush); long sumReceived = GetLongSum(exportedItems); @@ -740,13 +740,12 @@ public void ObservableCounterWithTagsAggregationTest(bool exportDelta) }; }); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); // Export 1 meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -837,14 +836,13 @@ public void ObservableCounterSpatialAggregationTest(bool exportDelta) }; }); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; }) - .AddView("requestCount", new MetricStreamConfiguration() { TagKeys = Array.Empty() }) - .Build(); + .AddView("requestCount", new MetricStreamConfiguration() { TagKeys = Array.Empty() })); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Single(exportedItems); @@ -878,13 +876,13 @@ public void UpDownCounterAggregationTest(bool exportDelta) using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{exportDelta}"); var counterLong = meter.CreateUpDownCounter("mycounter"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); counterLong.Add(10); counterLong.Add(-5); @@ -960,13 +958,12 @@ public void ObservableUpDownCounterAggregationTest(bool exportDelta) }; }); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); meterProvider.ForceFlush(MaxTimeToAllowForFlush); long sumReceived = GetLongSum(exportedItems); @@ -1024,13 +1021,12 @@ public void ObservableUpDownCounterWithTagsAggregationTest(bool exportDelta) }; }); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); // Export 1 meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -1094,13 +1090,13 @@ public void DimensionsAreOrderInsensitiveWithSortedKeysFirst(bool exportDelta) using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{exportDelta}"); var counterLong = meter.CreateCounter("Counter"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); // Emit the first metric with the sorted order of tag keys counterLong.Add(5, new("Key1", "Value1"), new("Key2", "Value2"), new("Key3", "Value3")); @@ -1185,13 +1181,13 @@ public void DimensionsAreOrderInsensitiveWithUnsortedKeysFirst(bool exportDelta) using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{exportDelta}"); var counterLong = meter.CreateCounter("Counter"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = exportDelta ? MetricReaderTemporalityPreference.Delta : MetricReaderTemporalityPreference.Cumulative; - }) - .Build(); + })); // Emit the first metric with the unsorted order of tag keys counterLong.Add(5, new("Key1", "Value1"), new("Key3", "Value3"), new("Key2", "Value2")); @@ -1278,14 +1274,14 @@ public void TestInstrumentDisposal(MetricReaderTemporalityPreference temporality var meter2 = new Meter($"{Utils.GetCurrentMethodName()}.{temporality}.2"); var counter1 = meter1.CreateCounter("counterFromMeter1"); var counter2 = meter2.CreateCounter("counterFromMeter2"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddMeter(meter2.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = temporality; - }) - .Build(); + })); counter1.Add(10, new KeyValuePair("key", "value")); counter2.Add(10, new KeyValuePair("key", "value")); @@ -1346,13 +1342,13 @@ int MetricPointCount() using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{temporality}"); var counterLong = meter.CreateCounter("mycounterCapTest"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = temporality; - }) - .Build(); + })); // Make one Add with no tags. // as currently we reserve 0th index @@ -1442,10 +1438,9 @@ public void InstrumentWithInvalidNameIsIgnoredTest(string instrumentName) using var meter = new Meter("InstrumentWithInvalidNameIsIgnoredTest"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counterLong = meter.CreateCounter(instrumentName); counterLong.Add(10); @@ -1464,10 +1459,9 @@ public void InstrumentWithValidNameIsExportedTest(string name) using var meter = new Meter("InstrumentValidNameIsExportedTest"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counterLong = meter.CreateCounter(name); counterLong.Add(10); @@ -1486,15 +1480,17 @@ public void SetupSdkProviderWithNoReader(bool hasViews) { // This test ensures that MeterProviderSdk can be set up without any reader using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{hasViews}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name); - if (hasViews) + using var container = BuildMeterProvider(out var meterProvider, builder => { - meterProviderBuilder.AddView("counter", "renamedCounter"); - } + builder + .AddMeter(meter.Name); - using var meterProvider = meterProviderBuilder.Build(); + if (hasViews) + { + builder.AddView("counter", "renamedCounter"); + } + }); var counter = meter.CreateCounter("counter"); @@ -1506,10 +1502,10 @@ public void UnsupportedMetricInstrument() { using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); + + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems)); using (var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log)) { @@ -1518,7 +1514,16 @@ public void UnsupportedMetricInstrument() // This validates that we log InstrumentIgnored event // and not something else. - Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 33)); + var instrumentIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 33); +#if BUILDING_HOSTING_TESTS + // Note: When using IMetricsListener this event is fired twice. Once + // for the SDK listener ignoring it because it isn't listening to + // the meter and then once for IMetricsListener ignoring it because + // decimal is not supported. + Assert.Equal(2, instrumentIgnoredEvents.Count()); +#else + Assert.Single(instrumentIgnoredEvents); +#endif } meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -1591,10 +1596,10 @@ private void MultithreadedCounterTest(T deltaValueUpdatedByEachCall) var metricItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{typeof(T).Name}.{deltaValueUpdatedByEachCall}"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddInMemoryExporter(metricItems) - .Build(); + .AddInMemoryExporter(metricItems)); var argToThread = new UpdateThreadArguments { @@ -1647,10 +1652,10 @@ private void MultithreadedHistogramTest(long[] expected, T[] values) var metricReader = new BaseExportingMetricReader(new InMemoryExporter(metrics)); using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{typeof(T).Name}"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) - .AddReader(metricReader) - .Build(); + .AddReader(metricReader)); var argsToThread = new UpdateThreadArguments { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs index d306e618de3..6daa4e52468 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs @@ -40,14 +40,14 @@ public void TestExemplarsCounter() using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); var counter = meter.CreateCounter("testCounter"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .SetExemplarFilter(new AlwaysOnExemplarFilter()) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = MetricReaderTemporalityPreference.Delta; - }) - .Build(); + })); var measurementValues = GenerateRandomValues(10); foreach (var value in measurementValues) @@ -94,14 +94,14 @@ public void TestExemplarsHistogram() using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); var histogram = meter.CreateHistogram("testHistogram"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .SetExemplarFilter(new AlwaysOnExemplarFilter()) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = MetricReaderTemporalityPreference.Delta; - }) - .Build(); + })); var measurementValues = GenerateRandomValues(10); foreach (var value in measurementValues) @@ -147,15 +147,15 @@ public void TestExemplarsFilterTags() using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); var histogram = meter.CreateHistogram("testHistogram"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .SetExemplarFilter(new AlwaysOnExemplarFilter()) .AddView(histogram.Name, new MetricStreamConfiguration() { TagKeys = new string[] { "key1" } }) .AddInMemoryExporter(exportedItems, metricReaderOptions => { metricReaderOptions.TemporalityPreference = MetricReaderTemporalityPreference.Delta; - }) - .Build(); + })); var measurementValues = GenerateRandomValues(10); foreach (var value in measurementValues) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs index 6cb5395ed29..4fdcb6f3e4a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -14,6 +14,12 @@ // limitations under the License. // +#if BUILDING_HOSTING_TESTS +using System.Diagnostics; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.Metrics; +using Microsoft.Extensions.Hosting; +#endif using Xunit; namespace OpenTelemetry.Metrics.Tests; @@ -22,6 +28,51 @@ public class MetricTestsBase { public const string EmitOverFlowAttributeConfigKey = "OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE"; + public static IDisposable BuildMeterProvider( + out MeterProvider meterProvider, + Action configure) + { + if (configure == null) + { + throw new ArgumentNullException(nameof(configure)); + } + +#if BUILDING_HOSTING_TESTS + var hostBuilder = new HostBuilder() + .ConfigureServices(services => + { + services.AddMetrics(builder => builder.UseOpenTelemetry( + metricsBuilder => + { + IServiceCollection localServices = null; + + metricsBuilder.ConfigureServices(services => localServices = services); + + Debug.Assert(localServices != null, "localServices was null"); + + var testBuilder = new HostingMeterProviderBuilder(localServices); + configure(testBuilder); + })); + + services.AddHostedService(); + }); + + var host = hostBuilder.Build(); + + host.Start(); + + meterProvider = host.Services.GetService(); + + return host; +#else + var builder = Sdk.CreateMeterProviderBuilder(); + + configure(builder); + + return meterProvider = builder.Build(); +#endif + } + // This method relies on the assumption that MetricPoints are exported in the order in which they are emitted. // For Delta AggregationTemporality, this holds true only until the AggregatorStore has not begun recaliming the MetricPoints. public static void ValidateMetricPointTags(List> expectedTags, ReadOnlyTagCollection actualTags) @@ -133,4 +184,59 @@ internal static Exemplar[] GetExemplars(MetricPoint mp) { return mp.GetExemplars().Where(exemplar => exemplar.Timestamp != default).ToArray(); } + +#if BUILDING_HOSTING_TESTS + private class MetricsSubscriptionManagerCleanupHostedService : IHostedService, IDisposable + { + private readonly object metricsSubscriptionManager; + + public MetricsSubscriptionManagerCleanupHostedService(IServiceProvider serviceProvider) + { + this.metricsSubscriptionManager = serviceProvider.GetService( + typeof(ConsoleMetrics).Assembly.GetType("Microsoft.Extensions.Diagnostics.Metrics.MetricsSubscriptionManager")); + + if (this.metricsSubscriptionManager == null) + { + throw new InvalidOperationException("MetricsSubscriptionManager could not be found reflectively."); + } + } + + public void Dispose() + { + // Note: The current version of MetricsSubscriptionManager seems to + // be bugged in that it doesn't implement IDisposable. This hack + // manually invokes Dispose so that tests don't clobber each other. + // See: https://github.com/dotnet/runtime/issues/94434. + this.metricsSubscriptionManager.GetType().GetMethod("Dispose").Invoke(this.metricsSubscriptionManager, null); + } + + public Task StartAsync(CancellationToken cancellationToken) + => Task.CompletedTask; + + public Task StopAsync(CancellationToken cancellationToken) + => Task.CompletedTask; + } + + private class HostingMeterProviderBuilder : MeterProviderBuilderBase + { + public HostingMeterProviderBuilder(IServiceCollection services) + : base(services) + { + } + + public override MeterProviderBuilder AddMeter(params string[] names) + { + return this.ConfigureServices(services => + { + foreach (var name in names) + { + // Note: The entire purpose of this class is to use the + // IMetricsBuilder API to enable Metrics and NOT the + // traditional AddMeter API. + services.AddMetrics(builder => builder.EnableMetrics(name)); + } + }); + } + } +#endif } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index df98fc6cc2a..f21ec118cc4 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -30,11 +30,11 @@ public void ViewToRenameMetric() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("name1", "renamed") - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. var counterLong = meter.CreateCounter("name1"); @@ -53,19 +53,17 @@ public void AddViewWithInvalidNameThrowsArgumentException(string viewNewName) using var meter1 = new Meter("AddViewWithInvalidNameThrowsArgumentException"); - var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + var ex = Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("name1", viewNewName) - .AddInMemoryExporter(exportedItems) - .Build()); + .AddInMemoryExporter(exportedItems))); Assert.Contains($"Custom view name {viewNewName} is invalid.", ex.Message); - ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + ex = Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("name1", new MetricStreamConfiguration() { Name = viewNewName }) - .AddInMemoryExporter(exportedItems) - .Build()); + .AddInMemoryExporter(exportedItems))); Assert.Contains($"Custom view name {viewNewName} is invalid.", ex.Message); } @@ -77,11 +75,10 @@ public void AddViewWithNullMetricStreamConfigurationThrowsArgumentnullException( using var meter1 = new Meter("AddViewWithInvalidNameThrowsArgumentException"); - Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("name1", (MetricStreamConfiguration)null) - .AddInMemoryExporter(exportedItems) - .Build()); + .AddInMemoryExporter(exportedItems))); } [Fact] @@ -91,11 +88,10 @@ public void AddViewWithNameThrowsInvalidArgumentExceptionWhenConflict() using var meter1 = new Meter("AddViewWithGuaranteedConflictThrowsInvalidArgumentException"); - Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("instrumenta.*", name: "newname") - .AddInMemoryExporter(exportedItems) - .Build()); + .AddInMemoryExporter(exportedItems))); } [Fact] @@ -105,11 +101,10 @@ public void AddViewWithNameInMetricStreamConfigurationThrowsInvalidArgumentExcep using var meter1 = new Meter("AddViewWithGuaranteedConflictThrowsInvalidArgumentException"); - Assert.Throws(() => Sdk.CreateMeterProviderBuilder() + Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("instrumenta.*", new MetricStreamConfiguration() { Name = "newname" }) - .AddInMemoryExporter(exportedItems) - .Build()); + .AddInMemoryExporter(exportedItems))); } [Fact] @@ -118,17 +113,18 @@ public void AddViewWithExceptionInUserCallbackAppliedDefault() var exportedItems = new List(); using var meter1 = new Meter("AddViewWithExceptionInUserCallback"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView((instrument) => { throw new Exception("bad"); }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); using (var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log)) { var counter1 = meter1.CreateCounter("counter1"); counter1.Add(1); - Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 41)); + + var metricViewIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 41); + Assert.Single(metricViewIgnoredEvents); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -144,12 +140,11 @@ public void AddViewWithExceptionInUserCallbackNoDefault() var exportedItems = new List(); using var meter1 = new Meter("AddViewWithExceptionInUserCallback"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView((instrument) => { throw new Exception("bad"); }) .AddView("*", MetricStreamConfiguration.Drop) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); using (var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log)) { @@ -172,18 +167,19 @@ public void AddViewsWithAndWithoutExceptionInUserCallback() var exportedItems = new List(); using var meter1 = new Meter("AddViewWithExceptionInUserCallback"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView((instrument) => { throw new Exception("bad"); }) .AddView((instrument) => { return new MetricStreamConfiguration() { Name = "newname" }; }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); using (var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log)) { var counter1 = meter1.CreateCounter("counter1"); counter1.Add(1); - Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 41)); + + var metricViewIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 41); + Assert.Single(metricViewIgnoredEvents); } meterProvider.ForceFlush(MaxTimeToAllowForFlush); @@ -198,8 +194,8 @@ public void AddViewsWithAndWithoutExceptionInUserCallback() [MemberData(nameof(MetricTestData.InvalidHistogramBoundaries), MemberType = typeof(MetricTestData))] public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] boundaries) { - var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() - .AddView("name1", new ExplicitBucketHistogramConfiguration { Boundaries = boundaries })); + var ex = Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder + .AddView("name1", new ExplicitBucketHistogramConfiguration { Boundaries = boundaries }))); Assert.Contains("Histogram boundaries must be in ascending order with distinct values", ex.Message); } @@ -210,8 +206,8 @@ public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] bo [InlineData(1)] public void AddViewWithInvalidExponentialHistogramMaxSizeConfigThrowsArgumentException(int maxSize) { - var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() - .AddView("name1", new Base2ExponentialBucketHistogramConfiguration { MaxSize = maxSize })); + var ex = Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder + .AddView("name1", new Base2ExponentialBucketHistogramConfiguration { MaxSize = maxSize }))); Assert.Contains("Histogram max size is invalid", ex.Message); } @@ -221,8 +217,8 @@ public void AddViewWithInvalidExponentialHistogramMaxSizeConfigThrowsArgumentExc [InlineData(21)] public void AddViewWithInvalidExponentialHistogramMaxScaleConfigThrowsArgumentException(int maxScale) { - var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() - .AddView("name1", new Base2ExponentialBucketHistogramConfiguration { MaxScale = maxScale })); + var ex = Assert.Throws(() => BuildMeterProvider(out var meterProvider, builder => builder + .AddView("name1", new Base2ExponentialBucketHistogramConfiguration { MaxScale = maxScale }))); Assert.Contains("Histogram max scale is invalid", ex.Message); } @@ -237,7 +233,7 @@ public void AddViewWithInvalidHistogramBoundsIgnored(double[] boundaries) var counter1 = meter1.CreateCounter("counter1"); - using (var provider = Sdk.CreateMeterProviderBuilder() + using (var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView((instrument) => { @@ -245,8 +241,7 @@ public void AddViewWithInvalidHistogramBoundsIgnored(double[] boundaries) ? new ExplicitBucketHistogramConfiguration() { Boundaries = boundaries } : null; }) - .AddInMemoryExporter(exportedItems) - .Build()) + .AddInMemoryExporter(exportedItems))) { counter1.Add(1); } @@ -263,11 +258,10 @@ public void ViewWithValidNameExported(string viewNewName) var exportedItems = new List(); using var meter1 = new Meter("ViewWithInvalidNameIgnoredTest"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView("name1", viewNewName) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counterLong = meter1.CreateCounter("name1"); counterLong.Add(10); @@ -287,7 +281,7 @@ public void ViewToRenameMetricConditionally() var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddMeter(meter2.Name) .AddView((instrument) => @@ -302,8 +296,7 @@ public void ViewToRenameMetricConditionally() return null; } }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Without views only 1 stream would be // exported (the 2nd one gets dropped due to @@ -327,7 +320,7 @@ public void ViewWithInvalidNameIgnoredConditionally(string viewNewName) { using var meter1 = new Meter("ViewToRenameMetricConditionallyTest"); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) // since here it's a func, we can't validate the name right away @@ -345,8 +338,7 @@ public void ViewWithInvalidNameIgnoredConditionally(string viewNewName) return null; } }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Because the MetricStreamName passed is invalid, the view is ignored, // and default aggregation is used. @@ -364,7 +356,7 @@ public void ViewWithValidNameConditionally(string viewNewName) { using var meter1 = new Meter("ViewToRenameMetricConditionallyTest"); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter1.Name) .AddView((instrument) => { @@ -379,8 +371,7 @@ public void ViewWithValidNameConditionally(string viewNewName) return null; } }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. var counter1 = meter1.CreateCounter("name1", "unit", "original_description"); @@ -401,7 +392,7 @@ public void ViewWithNullCustomNameTakesInstrumentName() using var meter = new Meter("ViewToRenameMetricConditionallyTest"); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -415,8 +406,7 @@ public void ViewWithNullCustomNameTakesInstrumentName() return null; } }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. // Since the View name was null, the instrument name was used instead @@ -436,12 +426,12 @@ public void ViewToProduceMultipleStreamsFromInstrument() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("name1", "renamedStream1") .AddView("name1", "renamedStream2") - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting two metric stream. var counterLong = meter.CreateCounter("name1"); @@ -457,13 +447,13 @@ public void ViewToProduceMultipleStreamsWithDuplicatesFromInstrument() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("name1", "renamedStream1") .AddView("name1", "renamedStream2") .AddView("name1", "renamedStream2") - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting three metric stream. // the second .AddView("name1", "renamedStream2") @@ -482,12 +472,12 @@ public void ViewWithHistogramConfigurationIgnoredWhenAppliedToNonHistogram() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("NotAHistogram", new ExplicitBucketHistogramConfiguration() { Name = "ImAnExplicitBoundsHistogram" }) .AddView("NotAHistogram", new Base2ExponentialBucketHistogramConfiguration() { Name = "ImAnExponentialHistogram" }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counter = meter.CreateCounter("NotAHistogram"); counter.Add(10); @@ -515,12 +505,12 @@ public void ViewToProduceCustomHistogramBound() using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); var boundaries = new double[] { 10, 20 }; - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("MyHistogram", new ExplicitBucketHistogramConfiguration() { Name = "MyHistogramDefaultBound" }) .AddView("MyHistogram", new ExplicitBucketHistogramConfiguration() { Boundaries = boundaries }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var histogram = meter.CreateHistogram("MyHistogram"); histogram.Record(-10); @@ -600,11 +590,11 @@ public void ViewToProduceExponentialHistogram() using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("MyHistogram", new Base2ExponentialBucketHistogramConfiguration()) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var histogram = meter.CreateHistogram("MyHistogram"); var expectedHistogram = new Base2ExponentialBucketHistogram(); @@ -648,11 +638,11 @@ public void HistogramMinMax(double[] values, HistogramConfiguration histogramCon using var meter = new Meter(Utils.GetCurrentMethodName()); var histogram = meter.CreateHistogram("MyHistogram"); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView(histogram.Name, histogramConfiguration) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); for (var i = 0; i < values.Length; i++) { @@ -686,11 +676,11 @@ public void HistogramMinMaxNotPresent(double[] values, HistogramConfiguration hi using var meter = new Meter(Utils.GetCurrentMethodName()); var histogram = meter.CreateHistogram("MyHistogram"); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView(histogram.Name, histogramConfiguration) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); for (var i = 0; i < values.Length; i++) { @@ -714,7 +704,8 @@ public void ViewToSelectTagKeys() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("FruitCounter", new MetricStreamConfiguration() { @@ -731,8 +722,7 @@ public void ViewToSelectTagKeys() TagKeys = Array.Empty(), Name = "NoTags", }) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); var counter = meter.CreateCounter("FruitCounter"); counter.Add(10, new("name", "apple"), new("color", "red"), new("size", "small")); @@ -785,11 +775,11 @@ public void ViewToDropSingleInstrument() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("counterNotInteresting", MetricStreamConfiguration.Drop) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. var counterInteresting = meter.CreateCounter("counterInteresting"); @@ -808,11 +798,11 @@ public void ViewToDropSingleInstrumentObservableCounter() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("observableCounterNotInteresting", MetricStreamConfiguration.Drop) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. meter.CreateObservableCounter("observableCounterNotInteresting", () => { return 10; }, "ms"); @@ -829,11 +819,11 @@ public void ViewToDropSingleInstrumentObservableGauge() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("observableGaugeNotInteresting", MetricStreamConfiguration.Drop) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream. meter.CreateObservableGauge("observableGaugeNotInteresting", () => { return 10; }, "ms"); @@ -850,11 +840,11 @@ public void ViewToDropMultipleInstruments() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("server*", MetricStreamConfiguration.Drop) - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting two client metric streams as both server* are dropped. var serverRequests = meter.CreateCounter("server.requests"); @@ -877,12 +867,12 @@ public void ViewToDropAndRetainInstrument() { using var meter = new Meter(Utils.GetCurrentMethodName()); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("server.requests", MetricStreamConfiguration.Drop) .AddView("server.requests", "server.request_renamed") - .AddInMemoryExporter(exportedItems) - .Build(); + .AddInMemoryExporter(exportedItems)); // Expecting one metric stream even though a View is asking // to drop the instrument, because another View is matching @@ -902,13 +892,12 @@ public void ViewConflict_OneInstrument_DifferentDescription() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription1" }) .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription2" }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); @@ -949,7 +938,8 @@ public void ViewConflict_TwoDistinctInstruments_ThreeStreams() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -961,9 +951,7 @@ public void ViewConflict_TwoDistinctInstruments_ThreeStreams() ? new MetricStreamConfiguration() { Name = "MetricStreamB" } : new MetricStreamConfiguration() { Name = "MetricStreamC" }; }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateCounter("name", "unit", "description1"); var instrument2 = meter.CreateCounter("name", "unit", "description2"); @@ -1006,7 +994,8 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentTags() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -1016,9 +1005,7 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentTags() { return new MetricStreamConfiguration { TagKeys = new[] { "key2" } }; }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateCounter("name"); var instrument2 = meter.CreateCounter("name"); @@ -1054,7 +1041,8 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_SameTags() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -1064,9 +1052,7 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_SameTags() { return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateCounter("name"); var instrument2 = meter.CreateCounter("name"); @@ -1103,7 +1089,8 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentHistogramBoun var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -1113,9 +1100,7 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentHistogramBoun { return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 10.0, 20.0 } }; }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateHistogram("name"); var instrument2 = meter.CreateHistogram("name"); @@ -1181,7 +1166,8 @@ public void ViewConflict_TwoInstruments_OneMatchesView() var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -1194,9 +1180,7 @@ public void ViewConflict_TwoInstruments_OneMatchesView() return null; } }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateCounter("name"); var instrument2 = meter.CreateCounter("othername"); @@ -1235,7 +1219,8 @@ public void ViewConflict_TwoInstruments_ConflictAvoidedBecauseSecondInstrumentIs var exportedItems = new List(); using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + + using var container = BuildMeterProvider(out var meterProvider, builder => builder .AddMeter(meter.Name) .AddView((instrument) => { @@ -1248,9 +1233,7 @@ public void ViewConflict_TwoInstruments_ConflictAvoidedBecauseSecondInstrumentIs return MetricStreamConfiguration.Drop; } }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); + .AddInMemoryExporter(exportedItems)); var instrument1 = meter.CreateCounter("name"); var instrument2 = meter.CreateCounter("othername"); From 24322e8b7157ac694b24c77cdf63f3270d28bd74 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 6 Nov 2023 13:20:38 -0800 Subject: [PATCH 15/43] Merge fixes. --- Directory.Packages.props | 3 ++- .../OpenTelemetry.Extensions.Hosting.Tests.csproj | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 21852b666b0..d3714215c8c 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -29,6 +29,7 @@ + @@ -77,7 +78,7 @@ - + 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 4b1fac3cb2a..e1d6d4fb496 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj @@ -23,6 +23,7 @@ IMetricsBuilder/IMetricsListener API added at the host level in .NET 8 instead of the direct lower-level MeterListener API added in .NET 6. --> + From 3fe5702e7d16565c5f2ccf78bbcac8c00790fab0 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 7 Nov 2023 12:11:41 -0800 Subject: [PATCH 16/43] Test coverage for the UseOpenTelemetry logging extension. --- .../OpenTelemetryLoggingExtensionsTests.cs | 109 ++++++++++++++++-- 1 file changed, 99 insertions(+), 10 deletions(-) diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index e4b8a33f919..376526a0f8f 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -25,16 +25,25 @@ namespace OpenTelemetry.Logs.Tests; public sealed class OpenTelemetryLoggingExtensionsTests { - [Fact] - public void ServiceCollectionAddOpenTelemetryNoParametersTest() + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ServiceCollectionAddOpenTelemetryNoParametersTest(bool callUseExtension) { bool optionsCallbackInvoked = false; var serviceCollection = new ServiceCollection(); - serviceCollection.AddLogging(configure => + serviceCollection.AddLogging(logging => { - configure.AddOpenTelemetry(); + if (callUseExtension) + { + logging.UseOpenTelemetry(); + } + else + { + logging.AddOpenTelemetry(); + } }); serviceCollection.Configure(options => @@ -52,10 +61,16 @@ public void ServiceCollectionAddOpenTelemetryNoParametersTest() } [Theory] - [InlineData(1, 0)] - [InlineData(1, 1)] - [InlineData(5, 5)] - public void ServiceCollectionAddOpenTelemetryConfigureActionTests(int numberOfBuilderRegistrations, int numberOfOptionsRegistrations) + [InlineData(false, 1, 0)] + [InlineData(false, 1, 1)] + [InlineData(false, 5, 5)] + [InlineData(true, 1, 0)] + [InlineData(true, 1, 1)] + [InlineData(true, 5, 5)] + public void ServiceCollectionAddOpenTelemetryConfigureActionTests( + bool callUseExtension, + int numberOfBuilderRegistrations, + int numberOfOptionsRegistrations) { int configureCallbackInvocations = 0; int optionsCallbackInvocations = 0; @@ -63,11 +78,18 @@ public void ServiceCollectionAddOpenTelemetryConfigureActionTests(int numberOfBu var serviceCollection = new ServiceCollection(); - serviceCollection.AddLogging(configure => + serviceCollection.AddLogging(logging => { for (int i = 0; i < numberOfBuilderRegistrations; i++) { - configure.AddOpenTelemetry(ConfigureCallback); + if (callUseExtension) + { + logging.UseOpenTelemetry(configureBuilder: null, configureOptions: ConfigureCallback); + } + else + { + logging.AddOpenTelemetry(ConfigureCallback); + } } }); @@ -116,6 +138,69 @@ void OptionsCallback(OpenTelemetryLoggerOptions options) } } + [Fact] + public void UseOpenTelemetryDependencyInjectionTest() + { + var serviceCollection = new ServiceCollection(); + + serviceCollection.AddLogging(logging => + { + logging.UseOpenTelemetry(builder => + { + builder.ConfigureServices(services => + { + services.AddSingleton(); + }); + + builder.ConfigureBuilder((sp, builder) => + { + builder.AddProcessor( + sp.GetRequiredService()); + }); + }); + }); + + using var sp = serviceCollection.BuildServiceProvider(); + + var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; + + Assert.NotNull(loggerProvider); + + Assert.NotNull(loggerProvider.Processor); + + Assert.True(loggerProvider.Processor is TestLogProcessor); + } + + [Fact] + public void UseOpenTelemetryOptionsOrderingTest() + { + int currentIndex = -1; + int beforeDelegateIndex = -1; + int extensionDelegateIndex = -1; + int afterDelegateIndex = -1; + + var serviceCollection = new ServiceCollection(); + + serviceCollection.Configure(o => beforeDelegateIndex = ++currentIndex); + + serviceCollection.AddLogging(logging => + { + logging.UseOpenTelemetry(configureBuilder: null, configureOptions: o => extensionDelegateIndex = ++currentIndex); + }); + + serviceCollection.Configure(o => afterDelegateIndex = ++currentIndex); + + using var sp = serviceCollection.BuildServiceProvider(); + + var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; + + Assert.NotNull(loggerProvider); + + Assert.Equal(0, beforeDelegateIndex); + Assert.Equal(1, extensionDelegateIndex); + Assert.Equal(2, afterDelegateIndex); + } + // This test validates that the OpenTelemetryLoggerOptions contains only primitive type properties. // This is necessary to ensure trim correctness since that class is effectively deserialized from // configuration. The top level properties are ensured via annotation on the RegisterProviderOptions API @@ -129,4 +214,8 @@ public void TestTrimmingCorrectnessOfOpenTelemetryLoggerOptions() Assert.True(prop.PropertyType.IsPrimitive, $"Property OpenTelemetryLoggerOptions.{prop.Name} doesn't have a primitive type. This is potentially a trim compatibility issue."); } } + + private sealed class TestLogProcessor : BaseProcessor + { + } } From bfec2751edf84d565a868d274de60ad2289f1115 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 9 Nov 2023 15:09:21 -0800 Subject: [PATCH 17/43] Ignore external subscriptions if AddMeter is also called. --- .../OpenTelemetryMetricListener.cs | 2 +- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 23 ++++- ...nTelemetryMetricsBuilderExtensionsTests.cs | 87 +++++++++++++++++ .../Metrics/MetricTestsBase.cs | 94 ++++++++++++------- 4 files changed, 165 insertions(+), 41 deletions(-) create mode 100644 test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs index 08c65e2f301..e2756d98ee6 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs @@ -62,7 +62,7 @@ public MeasurementHandlers GetMeasurementHandlers() public bool InstrumentPublished(Instrument instrument, out object? userState) { - userState = this.meterProviderSdk.InstrumentPublished(instrument, skipShouldListenToCheck: true); + userState = this.meterProviderSdk.InstrumentPublished(instrument, listeningIsManagedExternally: true); return userState != null; } diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 51804c25e0a..3d3c3529fcd 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -168,7 +168,7 @@ internal MeterProviderSdk( this.listener.InstrumentPublished = (instrument, listener) => { - object? state = this.InstrumentPublished(instrument, skipShouldListenToCheck: false); + object? state = this.InstrumentPublished(instrument, listeningIsManagedExternally: false); if (state != null) { listener.EnableMeasurementEvents(instrument, state); @@ -219,11 +219,26 @@ internal MeterProviderSdk( internal int ViewCount => this.viewConfigs.Count; - internal object? InstrumentPublished(Instrument instrument, bool skipShouldListenToCheck) + internal object? InstrumentPublished(Instrument instrument, bool listeningIsManagedExternally) { - if (!skipShouldListenToCheck && !this.shouldListenTo(instrument)) + var listenToInstrumentUsingSdkConfiguration = this.shouldListenTo(instrument); + + if (listeningIsManagedExternally && listenToInstrumentUsingSdkConfiguration) + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( + instrument.Name, + instrument.Meter.Name, + "Instrument belongs to a Meter which has been enabled externally and via a subscription on the provider. External subscription will be ignored in favor of the provider subscription.", + "Don't call AddMeter when also using external management."); + return null; + } + else if (!listeningIsManagedExternally && !listenToInstrumentUsingSdkConfiguration) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter not subscribed by the provider.", "Use AddMeter to add the Meter to the provider."); + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( + instrument.Name, + instrument.Meter.Name, + "Instrument belongs to a Meter not subscribed by the provider.", + "Use AddMeter to add the Meter to the provider."); return null; } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs new file mode 100644 index 00000000000..ca670113fb6 --- /dev/null +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -0,0 +1,87 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Diagnostics.Metrics; +using Microsoft.Extensions.Diagnostics.Metrics; +using OpenTelemetry.Logs; +using OpenTelemetry.Metrics; +using OpenTelemetry.Metrics.Tests; +using OpenTelemetry.Tests; +using OpenTelemetry.Trace; +using Xunit; + +namespace OpenTelemetry.Extensions.Hosting.Tests; + +public class OpenTelemetryMetricsBuilderExtensionsTests +{ + [Fact] + public void EnableMetricsTest() + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + List exportedItems = new(); + + using (var host = MetricTestsBase.BuildHost( + configureMetricsBuilder: builder => builder.EnableMetrics(meter.Name), + configureMeterProviderBuilder: builder => builder.AddInMemoryExporter(exportedItems))) + { + var counter = meter.CreateCounter("TestCounter"); + counter.Add(1); + } + + Assert.Single(exportedItems); + + List metricPoints = new(); + foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + + var metricPoint = metricPoints[0]; + Assert.Equal(1, metricPoint.GetSumLong()); + } + + [Fact] + public void EnableMetricsWithAddMeterTest() + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + List exportedItems = new(); + + using (var host = MetricTestsBase.BuildHost( + configureMetricsBuilder: builder => builder.EnableMetrics(meter.Name), + configureMeterProviderBuilder: builder => builder + .AddSdkMeter(meter.Name) + .AddInMemoryExporter(exportedItems))) + { + var counter = meter.CreateCounter("TestCounter"); + counter.Add(1); + } + + Assert.Single(exportedItems); + + List metricPoints = new(); + foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + + var metricPoint = metricPoints[0]; + Assert.Equal(1, metricPoint.GetSumLong()); + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs index 4fdcb6f3e4a..63b548f4786 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -38,11 +38,35 @@ public static IDisposable BuildMeterProvider( } #if BUILDING_HOSTING_TESTS + var host = BuildHost( + configureMetricsBuilder: null, + configureMeterProviderBuilder: configure); + + meterProvider = host.Services.GetService(); + + return host; +#else + var builder = Sdk.CreateMeterProviderBuilder(); + + configure(builder); + + return meterProvider = builder.Build(); +#endif + } + +#if BUILDING_HOSTING_TESTS + public static IHost BuildHost( + Action configureMetricsBuilder = null, + Action configureMeterProviderBuilder = null) + { var hostBuilder = new HostBuilder() .ConfigureServices(services => { - services.AddMetrics(builder => builder.UseOpenTelemetry( - metricsBuilder => + services.AddMetrics(builder => + { + configureMetricsBuilder?.Invoke(builder); + + builder.UseOpenTelemetry(metricsBuilder => { IServiceCollection localServices = null; @@ -51,8 +75,9 @@ public static IDisposable BuildMeterProvider( Debug.Assert(localServices != null, "localServices was null"); var testBuilder = new HostingMeterProviderBuilder(localServices); - configure(testBuilder); - })); + configureMeterProviderBuilder?.Invoke(testBuilder); + }); + }); services.AddHostedService(); }); @@ -61,17 +86,9 @@ public static IDisposable BuildMeterProvider( host.Start(); - meterProvider = host.Services.GetService(); - return host; -#else - var builder = Sdk.CreateMeterProviderBuilder(); - - configure(builder); - - return meterProvider = builder.Build(); -#endif } +#endif // This method relies on the assumption that MetricPoints are exported in the order in which they are emitted. // For Delta AggregationTemporality, this holds true only until the AggregatorStore has not begun recaliming the MetricPoints. @@ -186,7 +203,34 @@ internal static Exemplar[] GetExemplars(MetricPoint mp) } #if BUILDING_HOSTING_TESTS - private class MetricsSubscriptionManagerCleanupHostedService : IHostedService, IDisposable + public sealed class HostingMeterProviderBuilder : MeterProviderBuilderBase + { + public HostingMeterProviderBuilder(IServiceCollection services) + : base(services) + { + } + + public override MeterProviderBuilder AddMeter(params string[] names) + { + return this.ConfigureServices(services => + { + foreach (var name in names) + { + // Note: The entire purpose of this class is to use the + // IMetricsBuilder API to enable Metrics and NOT the + // traditional AddMeter API. + services.AddMetrics(builder => builder.EnableMetrics(name)); + } + }); + } + + public MeterProviderBuilder AddSdkMeter(params string[] names) + { + return base.AddMeter(names); + } + } + + private sealed class MetricsSubscriptionManagerCleanupHostedService : IHostedService, IDisposable { private readonly object metricsSubscriptionManager; @@ -216,27 +260,5 @@ public Task StartAsync(CancellationToken cancellationToken) public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; } - - private class HostingMeterProviderBuilder : MeterProviderBuilderBase - { - public HostingMeterProviderBuilder(IServiceCollection services) - : base(services) - { - } - - public override MeterProviderBuilder AddMeter(params string[] names) - { - return this.ConfigureServices(services => - { - foreach (var name in names) - { - // Note: The entire purpose of this class is to use the - // IMetricsBuilder API to enable Metrics and NOT the - // traditional AddMeter API. - services.AddMetrics(builder => builder.EnableMetrics(name)); - } - }); - } - } #endif } From c728717abb6102855cf1a10c7bd9cef287b3d711 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 10 Nov 2023 14:49:07 -0800 Subject: [PATCH 18/43] Verify hot reload of metrics via IConfiguration works and doesn't produce any strange warnings. --- .../Internal/OpenTelemetrySdkEventSource.cs | 12 +++ src/OpenTelemetry/Metrics/MetricReaderExt.cs | 92 +++++++++++++----- ...nTelemetryMetricsBuilderExtensionsTests.cs | 95 ++++++++++++++++--- .../Metrics/MetricTestsBase.cs | 11 ++- 4 files changed, 173 insertions(+), 37 deletions(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 1cbba26f9f8..89e58f20297 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -347,6 +347,18 @@ public void LoggerProcessStateSkipped(string type, string reason) this.WriteEvent(51, type, reason); } + [Event(52, Message = "Instrument '{0}', Meter '{1}' has been deactivated.", Level = EventLevel.Informational)] + public void MetricInstrumentDeactivated(string instrumentName, string meterName) + { + this.WriteEvent(52, instrumentName, meterName); + } + + [Event(53, Message = "A previously deactivated Instrument '{0}', Meter '{1}' has been reactivated.", Level = EventLevel.Informational)] + public void MetricInstrumentReactivated(string instrumentName, string meterName) + { + this.WriteEvent(53, instrumentName, meterName); + } + #if DEBUG public class OpenTelemetryEventListener : EventListener { diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 8a310352f2b..fb786e247cc 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -26,7 +26,7 @@ namespace OpenTelemetry.Metrics; /// public abstract partial class MetricReader { - private readonly HashSet metricStreamNames = new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary metricStreamNames = new(StringComparer.OrdinalIgnoreCase); private readonly ConcurrentDictionary instrumentIdentityToMetric = new(); private readonly object instrumentCreationLock = new(); private int maxMetricStreams; @@ -55,15 +55,6 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) return existingMetric; } - if (this.metricStreamNames.Contains(metricStreamIdentity.MetricStreamName)) - { - OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( - metricStreamIdentity.InstrumentName, - metricStreamIdentity.MeterName, - "Metric instrument has the same name as an existing one but differs by description, unit, or instrument type. Measurements from this instrument will still be exported but may result in conflicts.", - "Either change the name of the instrument or use MeterProviderBuilder.AddView to resolve the conflict."); - } - var index = ++this.metricIndex; if (index >= this.maxMetricStreams) { @@ -89,7 +80,9 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) this.instrumentIdentityToMetric[metricStreamIdentity] = metric; this.metrics![index] = metric; - this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName); + + this.CreateOrUpdateMetricStreamRegistration(in metricStreamIdentity); + return metric; } } @@ -140,15 +133,6 @@ internal List AddMetricsListWithViews(Instrument instrument, List AddMetricsListWithViews(Instrument instrument, List metrics, double value, ReadOn internal void CompleteSingleStreamMeasurement(Metric metric) { - metric.InstrumentDisposed = true; + DeactivateMetric(metric); } internal void CompleteMeasurement(List metrics) { foreach (var metric in metrics) { - metric.InstrumentDisposed = true; + DeactivateMetric(metric); } } @@ -250,6 +235,52 @@ internal void SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStre } } + private static void DeactivateMetric(Metric metric) + { + metric.InstrumentDisposed = true; + + var metricStreamIdentity = metric.InstrumentIdentity; + + OpenTelemetrySdkEventSource.Log.MetricInstrumentDeactivated( + metricStreamIdentity.InstrumentName, + metricStreamIdentity.MeterName); + } + + private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metricStreamIdentity) + { + var registration = this.metricStreamNames.GetOrAdd( + metricStreamIdentity.MetricStreamName, + CreateRegistration); + + var currentRegistrationCount = Interlocked.CompareExchange(ref registration.RegistrationCount, 1, -1); + + if (currentRegistrationCount == -1) + { + // Most common case where instrument being added is the first registration. + return; + } + + if (currentRegistrationCount > 0) + { + OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( + metricStreamIdentity.InstrumentName, + metricStreamIdentity.MeterName, + "Metric instrument has the same name as an existing one but differs by description, unit, or instrument type. Measurements from this instrument will still be exported but may result in conflicts.", + "Either change the name of the instrument or use MeterProviderBuilder.AddView to resolve the conflict."); + } + else + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( + metricStreamIdentity.InstrumentName, + metricStreamIdentity.MeterName); + } + + Interlocked.Increment(ref registration.RegistrationCount); + + static MetricStreamRegistration CreateRegistration(string metricStreamName) + => new() { RegistrationCount = -1 }; + } + private Batch GetMetricsBatch() { Debug.Assert(this.metrics != null, "this.metrics was null"); @@ -269,7 +300,15 @@ private Batch GetMetricsBatch() if (metric.InstrumentDisposed) { metricPointSize = metric.Snapshot(); - this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); + + var metricStreamNamesLookupResult = this.metricStreamNames.TryGetValue(metric.InstrumentIdentity.MetricStreamName, out var registration); + Debug.Assert(metricStreamNamesLookupResult, "result was false"); + Debug.Assert(registration != null, "registration was null"); + Interlocked.Decrement(ref registration!.RegistrationCount); + + var instrumentIdentityToMetricLookupResult = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); + Debug.Assert(instrumentIdentityToMetricLookupResult, "instrumentIdentityToMetricLookupResult was false"); + this.metrics[i] = null; } else @@ -292,4 +331,9 @@ private Batch GetMetricsBatch() return default; } } + + private sealed class MetricStreamRegistration + { + public int RegistrationCount; + } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index ca670113fb6..0db8bc434c2 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -15,7 +15,12 @@ // using System.Diagnostics.Metrics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.Memory; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.Metrics; +using Microsoft.Extensions.Options; +using OpenTelemetry.Internal; using OpenTelemetry.Logs; using OpenTelemetry.Metrics; using OpenTelemetry.Metrics.Tests; @@ -41,18 +46,7 @@ public void EnableMetricsTest() counter.Add(1); } - Assert.Single(exportedItems); - - List metricPoints = new(); - foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) - { - metricPoints.Add(mp); - } - - Assert.Single(metricPoints); - - var metricPoint = metricPoints[0]; - Assert.Equal(1, metricPoint.GetSumLong()); + AssertSingleMetricWithLongSumOfOne(exportedItems); } [Fact] @@ -71,6 +65,83 @@ public void EnableMetricsWithAddMeterTest() counter.Add(1); } + AssertSingleMetricWithLongSumOfOne(exportedItems); + } + + [Fact] + public void ReloadOfMetricsViaIConfigurationTest() + { + using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + List exportedItems = new(); + + var source = new MemoryConfigurationSource(); + var memory = new MemoryConfigurationProvider(source); + var configuration = new ConfigurationRoot(new[] { memory }); + + using var host = MetricTestsBase.BuildHost( + configureAppConfiguration: (context, builder) => builder.AddConfiguration(configuration), + configureMeterProviderBuilder: builder => builder + .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = MetricReaderTemporalityPreference.Delta)); + + var meterProvider = host.Services.GetRequiredService(); + var options = host.Services.GetRequiredService>(); + + var counter = meter.CreateCounter("TestCounter"); + counter.Add(1); + + meterProvider.ForceFlush(); + + Assert.Empty(exportedItems); + + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "true"); + + configuration.Reload(); + + counter.Add(1); + + meterProvider.ForceFlush(); + + AssertSingleMetricWithLongSumOfOne(exportedItems); + + exportedItems.Clear(); + + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "false"); + + configuration.Reload(); + + counter.Add(1); + + meterProvider.ForceFlush(); + + Assert.Empty(exportedItems); + + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "true"); + + configuration.Reload(); + + counter.Add(1); + + meterProvider.ForceFlush(); + + AssertSingleMetricWithLongSumOfOne(exportedItems); + + var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); + + Assert.Empty(duplicateMetricInstrumentEvents); + + var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); + + Assert.Single(metricInstrumentDeactivatedEvents); + + var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); + + Assert.Single(metricInstrumentReactivatedEvents); + } + + private static void AssertSingleMetricWithLongSumOfOne(List exportedItems) + { Assert.Single(exportedItems); List metricPoints = new(); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs index 63b548f4786..ef4f44d33e9 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -16,6 +16,7 @@ #if BUILDING_HOSTING_TESTS using System.Diagnostics; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.Metrics; using Microsoft.Extensions.Hosting; @@ -39,7 +40,6 @@ public static IDisposable BuildMeterProvider( #if BUILDING_HOSTING_TESTS var host = BuildHost( - configureMetricsBuilder: null, configureMeterProviderBuilder: configure); meterProvider = host.Services.GetService(); @@ -56,12 +56,21 @@ public static IDisposable BuildMeterProvider( #if BUILDING_HOSTING_TESTS public static IHost BuildHost( + Action configureAppConfiguration = null, + Action configureServices = null, Action configureMetricsBuilder = null, Action configureMeterProviderBuilder = null) { var hostBuilder = new HostBuilder() + .ConfigureDefaults(null) + .ConfigureAppConfiguration((context, builder) => + { + configureAppConfiguration?.Invoke(context, builder); + }) .ConfigureServices(services => { + configureServices?.Invoke(services); + services.AddMetrics(builder => { configureMetricsBuilder?.Invoke(builder); From 934e75bb4355f0937e4562be44e80ab40b71e381 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 13 Nov 2023 10:30:04 -0800 Subject: [PATCH 19/43] Switch WithLogging and WithMetrics to turn on hosting features. --- Directory.Packages.props | 2 +- examples/Directory.Packages.props | 2 - .../Experimental/PublicAPI.Unshipped.txt | 1 + .../OpenTelemetry.Extensions.Hosting.csproj | 2 +- .../OpenTelemetryBuilder.cs | 46 +++++++++++++++---- .../OpenTelemetryMetricsBuilderExtensions.cs | 5 +- .../Experimental/PublicAPI.Unshipped.txt | 2 +- .../ILogger/OpenTelemetryLoggingExtensions.cs | 12 +++-- ...nTelemetryMetricsBuilderExtensionsTests.cs | 23 +++++++--- .../Metrics/MetricTestsBase.cs | 34 ++++++++++---- 10 files changed, 94 insertions(+), 35 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index d3714215c8c..936d73a9f23 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -29,7 +29,7 @@ - + diff --git a/examples/Directory.Packages.props b/examples/Directory.Packages.props index 7e94cb469b9..902efc8cc04 100644 --- a/examples/Directory.Packages.props +++ b/examples/Directory.Packages.props @@ -2,7 +2,5 @@ - - diff --git a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt index 8617a4e70a1..f83d7ca4a0f 100644 --- a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,2 +1,3 @@ OpenTelemetry.OpenTelemetryBuilder.WithLogging() -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action! configure) -> OpenTelemetry.OpenTelemetryBuilder! +OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action? configureBuilder, System.Action? configureOptions) -> OpenTelemetry.OpenTelemetryBuilder! diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj index cc6b1b078a1..63e328228c9 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj @@ -10,7 +10,7 @@ - + diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 805808488a1..82bc29d9ce6 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -15,6 +15,8 @@ // using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.Metrics; +using Microsoft.Extensions.Logging; using OpenTelemetry.Internal; using OpenTelemetry.Logs; using OpenTelemetry.Metrics; @@ -95,11 +97,8 @@ public OpenTelemetryBuilder WithMetrics() /// calls. public OpenTelemetryBuilder WithMetrics(Action configure) { - Guard.ThrowIfNull(configure); - - var builder = new MeterProviderBuilderBase(this.Services); - - configure(builder); + this.Services.AddMetrics( + builder => builder.UseOpenTelemetry(configure)); return this; } @@ -163,7 +162,7 @@ public OpenTelemetryBuilder WithTracing(Action configure) internal #endif OpenTelemetryBuilder WithLogging() - => this.WithLogging(b => { }); + => this.WithLogging(configureBuilder: null, configureOptions: null); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -190,9 +189,40 @@ OpenTelemetryBuilder WithLogging(Action configure) { Guard.ThrowIfNull(configure); - var builder = new LoggerProviderBuilderBase(this.Services); + return this.WithLogging(configureBuilder: configure, configureOptions: null); + } - configure(builder); +#if EXPOSE_EXPERIMENTAL_FEATURES + /// + /// Adds logging services into the builder. + /// + /// + /// Optional configuration callback. + /// Optional configuration callback. + /// The supplied for chaining + /// calls. + public +#else + /// + /// Adds logging services into the builder. + /// + /// + /// Optional configuration callback. + /// Optional configuration callback. + /// The supplied for chaining + /// calls. + internal +#endif + OpenTelemetryBuilder WithLogging( + Action? configureBuilder, + Action? configureOptions) + { + this.Services.AddLogging( + logging => logging.UseOpenTelemetry(configureBuilder, configureOptions)); return this; } diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index b071278dc0b..2625056c5dc 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -60,11 +60,14 @@ public static IMetricsBuilder UseOpenTelemetry( Action configure) { Guard.ThrowIfNull(metricsBuilder); + Guard.ThrowIfNull(configure); + + var builder = new MeterProviderBuilderBase(metricsBuilder.Services); metricsBuilder.Services.TryAddEnumerable( ServiceDescriptor.Singleton()); - metricsBuilder.Services.AddOpenTelemetry().WithMetrics(configure); + configure(builder); return metricsBuilder; } diff --git a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt index 00f2b537065..21e3fb6a8d6 100644 --- a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -21,7 +21,7 @@ OpenTelemetry.Metrics.MetricPoint.GetExemplars() -> OpenTelemetry.Metrics.Exempl OpenTelemetry.Metrics.TraceBasedExemplarFilter OpenTelemetry.Metrics.TraceBasedExemplarFilter.TraceBasedExemplarFilter() -> void static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder) -> Microsoft.Extensions.Logging.ILoggingBuilder! -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder) -> Microsoft.Extensions.Logging.ILoggingBuilder! +static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action! configure) -> Microsoft.Extensions.Logging.ILoggingBuilder! static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder, System.Action? configureOptions) -> Microsoft.Extensions.Logging.ILoggingBuilder! static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, OpenTelemetry.BaseProcessor! processor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, System.Func!>! implementationFactory) -> OpenTelemetry.Logs.LoggerProviderBuilder! diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 320e35edc5a..0b1cb7d3d7d 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -123,7 +123,7 @@ static ILoggingBuilder UseOpenTelemetry( /// /// /// The to use. - /// Optional configuration action. + /// Optional configuration action. /// The supplied for call chaining. public #else @@ -132,14 +132,18 @@ static ILoggingBuilder UseOpenTelemetry( /// /// /// The to use. - /// Optional configuration action. + /// configuration action. /// The supplied for call chaining. internal #endif static ILoggingBuilder UseOpenTelemetry( this ILoggingBuilder builder, - Action? configureBuilder) - => AddOpenTelemetryInternal(builder, configureBuilder, configureOptions: null); + Action configure) + { + Guard.ThrowIfNull(configure); + + return AddOpenTelemetryInternal(builder, configureBuilder: configure, configureOptions: null); + } #if EXPOSE_EXPERIMENTAL_FEATURES /// diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index 0db8bc434c2..d03150a4dc8 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -32,13 +32,16 @@ namespace OpenTelemetry.Extensions.Hosting.Tests; public class OpenTelemetryMetricsBuilderExtensionsTests { - [Fact] - public void EnableMetricsTest() + [Theory] + [InlineData(false)] + [InlineData(true)] + public void EnableMetricsTest(bool useWithMetricsStyle) { using var meter = new Meter(Utils.GetCurrentMethodName()); List exportedItems = new(); using (var host = MetricTestsBase.BuildHost( + useWithMetricsStyle, configureMetricsBuilder: builder => builder.EnableMetrics(meter.Name), configureMeterProviderBuilder: builder => builder.AddInMemoryExporter(exportedItems))) { @@ -49,13 +52,16 @@ public void EnableMetricsTest() AssertSingleMetricWithLongSumOfOne(exportedItems); } - [Fact] - public void EnableMetricsWithAddMeterTest() + [Theory] + [InlineData(false)] + [InlineData(true)] + public void EnableMetricsWithAddMeterTest(bool useWithMetricsStyle) { using var meter = new Meter(Utils.GetCurrentMethodName()); List exportedItems = new(); using (var host = MetricTestsBase.BuildHost( + useWithMetricsStyle, configureMetricsBuilder: builder => builder.EnableMetrics(meter.Name), configureMeterProviderBuilder: builder => builder .AddSdkMeter(meter.Name) @@ -68,8 +74,10 @@ public void EnableMetricsWithAddMeterTest() AssertSingleMetricWithLongSumOfOne(exportedItems); } - [Fact] - public void ReloadOfMetricsViaIConfigurationTest() + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ReloadOfMetricsViaIConfigurationTest(bool useWithMetricsStyle) { using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); @@ -81,6 +89,7 @@ public void ReloadOfMetricsViaIConfigurationTest() var configuration = new ConfigurationRoot(new[] { memory }); using var host = MetricTestsBase.BuildHost( + useWithMetricsStyle, configureAppConfiguration: (context, builder) => builder.AddConfiguration(configuration), configureMeterProviderBuilder: builder => builder .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = MetricReaderTemporalityPreference.Delta)); @@ -117,7 +126,7 @@ public void ReloadOfMetricsViaIConfigurationTest() Assert.Empty(exportedItems); - memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "true"); + memory.Set($"Metrics:OpenTelemetry:EnabledMetrics:{meter.Name}:Default", "true"); configuration.Reload(); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs index ef4f44d33e9..fcd5ab3aeae 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -40,6 +40,7 @@ public static IDisposable BuildMeterProvider( #if BUILDING_HOSTING_TESTS var host = BuildHost( + useWithMetricsStyle: false, configureMeterProviderBuilder: configure); meterProvider = host.Services.GetService(); @@ -56,6 +57,7 @@ public static IDisposable BuildMeterProvider( #if BUILDING_HOSTING_TESTS public static IHost BuildHost( + bool useWithMetricsStyle, Action configureAppConfiguration = null, Action configureServices = null, Action configureMetricsBuilder = null, @@ -75,19 +77,19 @@ public static IHost BuildHost( { configureMetricsBuilder?.Invoke(builder); - builder.UseOpenTelemetry(metricsBuilder => + if (!useWithMetricsStyle) { - IServiceCollection localServices = null; - - metricsBuilder.ConfigureServices(services => localServices = services); - - Debug.Assert(localServices != null, "localServices was null"); - - var testBuilder = new HostingMeterProviderBuilder(localServices); - configureMeterProviderBuilder?.Invoke(testBuilder); - }); + builder.UseOpenTelemetry(metricsBuilder => ConfigureBuilder(metricsBuilder, configureMeterProviderBuilder)); + } }); + if (useWithMetricsStyle) + { + services + .AddOpenTelemetry() + .WithMetrics(metricsBuilder => ConfigureBuilder(metricsBuilder, configureMeterProviderBuilder)); + } + services.AddHostedService(); }); @@ -96,6 +98,18 @@ public static IHost BuildHost( host.Start(); return host; + + static void ConfigureBuilder(MeterProviderBuilder builder, Action configureMeterProviderBuilder) + { + IServiceCollection localServices = null; + + builder.ConfigureServices(services => localServices = services); + + Debug.Assert(localServices != null, "localServices was null"); + + var testBuilder = new HostingMeterProviderBuilder(localServices); + configureMeterProviderBuilder?.Invoke(testBuilder); + } } #endif From 467008ed1743d7e95a190b2b9ab2ec2614868695 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 13 Nov 2023 10:37:10 -0800 Subject: [PATCH 20/43] Make IMetricsBuilder.UseOpenTelemetry an experimental API. --- .../Experimental/PublicAPI.Unshipped.txt | 3 ++ .../.publicApi/Stable/PublicAPI.Unshipped.txt | 4 +- .../OpenTelemetryMetricsBuilderExtensions.cs | 44 +++++++++++++++++-- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt index f83d7ca4a0f..0437d9c23bb 100644 --- a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,3 +1,6 @@ OpenTelemetry.OpenTelemetryBuilder.WithLogging() -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action! configure) -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action? configureBuilder, System.Action? configureOptions) -> OpenTelemetry.OpenTelemetryBuilder! +Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions +static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! +static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder, System.Action! configure) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! diff --git a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Stable/PublicAPI.Unshipped.txt index 8fdc9f0ebc5..8b137891791 100644 --- a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -1,3 +1 @@ -Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions -static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! -static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder, System.Action! configure) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! + diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 2625056c5dc..0356c7eb32f 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -27,6 +27,21 @@ namespace Microsoft.Extensions.Diagnostics.Metrics; /// public static class OpenTelemetryMetricsBuilderExtensions { +#if EXPOSE_EXPERIMENTAL_FEATURES + /// + /// Adds an OpenTelemetry named 'OpenTelemetry' to the . + /// + /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. + /// Note: This is safe to be called multiple times and by library authors. + /// Only a single will be created for a given + /// . + /// + /// . + /// The supplied for chaining + /// calls. + public +#else /// /// Adds OpenTelemetry metric services into the builder. /// @@ -38,16 +53,35 @@ public static class OpenTelemetryMetricsBuilderExtensions /// . /// The supplied for chaining /// calls. - public static IMetricsBuilder UseOpenTelemetry( + internal +#endif + static IMetricsBuilder UseOpenTelemetry( this IMetricsBuilder metricsBuilder) => UseOpenTelemetry(metricsBuilder, b => { }); +#if EXPOSE_EXPERIMENTAL_FEATURES /// - /// Adds OpenTelemetry metric services into the builder. + /// Adds an OpenTelemetry named 'OpenTelemetry' to the . /// /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created for a given + /// Only a single will be created for a given + /// . + /// + /// . + /// + /// configuration callback. + /// The supplied for chaining + /// calls. + public +#else + /// + /// Adds an OpenTelemetry named 'OpenTelemetry' to the . + /// + /// + /// Note: This is safe to be called multiple times and by library authors. + /// Only a single will be created for a given /// . /// /// . @@ -55,7 +89,9 @@ public static IMetricsBuilder UseOpenTelemetry( /// configuration callback. /// The supplied for chaining /// calls. - public static IMetricsBuilder UseOpenTelemetry( + internal +#endif + static IMetricsBuilder UseOpenTelemetry( this IMetricsBuilder metricsBuilder, Action configure) { From f22cdf89d531aeb77d5eca7d6c966f741911f19b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 13 Nov 2023 10:40:12 -0800 Subject: [PATCH 21/43] XML comment tweaks. --- .../OpenTelemetryMetricsBuilderExtensions.cs | 13 ++---------- .../ILogger/OpenTelemetryLoggingExtensions.cs | 20 ++++--------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 0356c7eb32f..1909f9de8ad 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -63,12 +63,7 @@ static IMetricsBuilder UseOpenTelemetry( /// /// Adds an OpenTelemetry named 'OpenTelemetry' to the . /// - /// - /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. - /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created for a given - /// . - /// + /// /// . /// /// configuration callback. @@ -79,11 +74,7 @@ static IMetricsBuilder UseOpenTelemetry( /// /// Adds an OpenTelemetry named 'OpenTelemetry' to the . /// - /// - /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created for a given - /// . - /// + /// /// . /// /// configuration callback. diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 0b1cb7d3d7d..10c7f0074ae 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -81,15 +81,9 @@ public static ILoggingBuilder AddOpenTelemetry( /// /// /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. - /// Notes: - /// - /// This is safe to be called multiple times and by library authors. + /// Note: This is safe to be called multiple times and by library authors. /// Only a single will be created - /// for a given . - /// / - /// features (DI, Options, IConfiguration, etc.) are not available when - /// using . - /// + /// for a given . /// /// The to use. /// The supplied for call chaining. @@ -99,15 +93,9 @@ public static ILoggingBuilder AddOpenTelemetry( /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . /// /// - /// Notes: - /// - /// This is safe to be called multiple times and by library authors. + /// Note: This is safe to be called multiple times and by library authors. /// Only a single will be created - /// for a given . - /// / - /// features (DI, Options, IConfiguration, etc.) are not available when - /// using . - /// + /// for a given . /// /// The to use. /// The supplied for call chaining. From f5b2e6febc6ccad3885784efd2067b0db37af1d2 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 13 Nov 2023 11:35:52 -0800 Subject: [PATCH 22/43] Test fix. --- .../OpenTelemetryServicesExtensionsTests.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs index 181dec9e367..4c79604d106 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs @@ -381,7 +381,7 @@ public void AddOpenTelemetry_WithLogging_DisposalTest() } [Fact] - public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() + public void AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() { bool configureBuilderCalled = false; @@ -416,14 +416,8 @@ public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() var host = builder.Build(); - Assert.False(configureBuilderCalled); - - await host.StartAsync().ConfigureAwait(false); - Assert.True(configureBuilderCalled); - await host.StopAsync().ConfigureAwait(false); - host.Dispose(); } From ec19a81410e91df7c07421bde079a00245b6b376 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 13 Nov 2023 11:43:51 -0800 Subject: [PATCH 23/43] Warning cleanup. --- .../OpenTelemetryMetricsBuilderExtensions.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 1909f9de8ad..5ccca9d521f 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -25,7 +25,12 @@ namespace Microsoft.Extensions.Diagnostics.Metrics; /// Contains extension methods for registering OpenTelemetry metrics with an /// instance. /// -public static class OpenTelemetryMetricsBuilderExtensions +#if EXPOSE_EXPERIMENTAL_FEATURES +public +#else +internal +#endif + static class OpenTelemetryMetricsBuilderExtensions { #if EXPOSE_EXPERIMENTAL_FEATURES /// From 01114b33b631d9f62ac3163977696d41845ff14e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 14 Nov 2023 10:29:48 -0800 Subject: [PATCH 24/43] Update WithMetrics XML comments to capture the IMetricsListener behavior. --- .../OpenTelemetryBuilder.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 82bc29d9ce6..7aa9d5b8a7d 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -78,9 +78,16 @@ public OpenTelemetryBuilder ConfigureResource( /// Adds metric services into the builder. /// /// - /// Note: This is safe to be called multiple times and by library authors. + /// Notes: + /// + /// This is safe to be called multiple times and by library authors. /// Only a single will be created for a given - /// . + /// . + /// This method automatically calls and + /// registers an named "OpenTelemetry" into + /// the . + /// /// /// The supplied for chaining /// calls. From 4012f83bcd1d27b5507648af74e5fd61d44d04a5 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 14 Nov 2023 10:33:01 -0800 Subject: [PATCH 25/43] Make IMetricsBuilder.UseOpenTelemetry internal. --- .../Experimental/PublicAPI.Unshipped.txt | 3 -- .../OpenTelemetryBuilder.cs | 2 +- .../OpenTelemetryMetricsBuilderExtensions.cs | 42 ++----------------- 3 files changed, 4 insertions(+), 43 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt index 0437d9c23bb..f83d7ca4a0f 100644 --- a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,6 +1,3 @@ OpenTelemetry.OpenTelemetryBuilder.WithLogging() -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action! configure) -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action? configureBuilder, System.Action? configureOptions) -> OpenTelemetry.OpenTelemetryBuilder! -Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions -static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! -static Microsoft.Extensions.Diagnostics.Metrics.OpenTelemetryMetricsBuilderExtensions.UseOpenTelemetry(this Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! metricsBuilder, System.Action! configure) -> Microsoft.Extensions.Diagnostics.Metrics.IMetricsBuilder! diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 7aa9d5b8a7d..0458c8f7f0b 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -85,7 +85,7 @@ public OpenTelemetryBuilder ConfigureResource( /// . /// This method automatically calls and - /// registers an named "OpenTelemetry" into + /// registers an named 'OpenTelemetry' into /// the . /// /// diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 5ccca9d521f..92af3c929a6 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -25,32 +25,12 @@ namespace Microsoft.Extensions.Diagnostics.Metrics; /// Contains extension methods for registering OpenTelemetry metrics with an /// instance. /// -#if EXPOSE_EXPERIMENTAL_FEATURES -public -#else -internal -#endif - static class OpenTelemetryMetricsBuilderExtensions +internal static class OpenTelemetryMetricsBuilderExtensions { -#if EXPOSE_EXPERIMENTAL_FEATURES /// /// Adds an OpenTelemetry named 'OpenTelemetry' to the . /// /// - /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. - /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created for a given - /// . - /// - /// . - /// The supplied for chaining - /// calls. - public -#else - /// - /// Adds OpenTelemetry metric services into the builder. - /// - /// /// Note: This is safe to be called multiple times and by library authors. /// Only a single will be created for a given /// . @@ -58,24 +38,10 @@ static class OpenTelemetryMetricsBuilderExtensions /// . /// The supplied for chaining /// calls. - internal -#endif - static IMetricsBuilder UseOpenTelemetry( + public static IMetricsBuilder UseOpenTelemetry( this IMetricsBuilder metricsBuilder) => UseOpenTelemetry(metricsBuilder, b => { }); -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Adds an OpenTelemetry named 'OpenTelemetry' to the . - /// - /// - /// . - /// - /// configuration callback. - /// The supplied for chaining - /// calls. - public -#else /// /// Adds an OpenTelemetry named 'OpenTelemetry' to the . /// @@ -85,9 +51,7 @@ static IMetricsBuilder UseOpenTelemetry( /// configuration callback. /// The supplied for chaining /// calls. - internal -#endif - static IMetricsBuilder UseOpenTelemetry( + public static IMetricsBuilder UseOpenTelemetry( this IMetricsBuilder metricsBuilder, Action configure) { From cd393f7a7528f041c45914a13d81f1d29610cbc5 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 14 Nov 2023 12:19:40 -0800 Subject: [PATCH 26/43] Remove logging changes. --- .../Experimental/PublicAPI.Unshipped.txt | 1 - .../OpenTelemetryBuilder.cs | 38 +----- .../Experimental/PublicAPI.Unshipped.txt | 3 - .../ILogger/OpenTelemetryLoggingExtensions.cs | 94 --------------- .../OpenTelemetryServicesExtensionsTests.cs | 11 +- .../OpenTelemetryLoggingExtensionsTests.cs | 109 ++---------------- 6 files changed, 20 insertions(+), 236 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt index f83d7ca4a0f..8617a4e70a1 100644 --- a/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,3 +1,2 @@ OpenTelemetry.OpenTelemetryBuilder.WithLogging() -> OpenTelemetry.OpenTelemetryBuilder! OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action! configure) -> OpenTelemetry.OpenTelemetryBuilder! -OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action? configureBuilder, System.Action? configureOptions) -> OpenTelemetry.OpenTelemetryBuilder! diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 0458c8f7f0b..215d43bcbee 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -16,7 +16,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.Metrics; -using Microsoft.Extensions.Logging; using OpenTelemetry.Internal; using OpenTelemetry.Logs; using OpenTelemetry.Metrics; @@ -169,7 +168,7 @@ public OpenTelemetryBuilder WithTracing(Action configure) internal #endif OpenTelemetryBuilder WithLogging() - => this.WithLogging(configureBuilder: null, configureOptions: null); + => this.WithLogging(b => { }); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -196,40 +195,9 @@ OpenTelemetryBuilder WithLogging(Action configure) { Guard.ThrowIfNull(configure); - return this.WithLogging(configureBuilder: configure, configureOptions: null); - } + var builder = new LoggerProviderBuilderBase(this.Services); -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Adds logging services into the builder. - /// - /// - /// Optional configuration callback. - /// Optional configuration callback. - /// The supplied for chaining - /// calls. - public -#else - /// - /// Adds logging services into the builder. - /// - /// - /// Optional configuration callback. - /// Optional configuration callback. - /// The supplied for chaining - /// calls. - internal -#endif - OpenTelemetryBuilder WithLogging( - Action? configureBuilder, - Action? configureOptions) - { - this.Services.AddLogging( - logging => logging.UseOpenTelemetry(configureBuilder, configureOptions)); + configure(builder); return this; } diff --git a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt index 21e3fb6a8d6..862c997fe22 100644 --- a/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -20,9 +20,6 @@ OpenTelemetry.Metrics.ExemplarFilter.ExemplarFilter() -> void OpenTelemetry.Metrics.MetricPoint.GetExemplars() -> OpenTelemetry.Metrics.Exemplar[]! OpenTelemetry.Metrics.TraceBasedExemplarFilter OpenTelemetry.Metrics.TraceBasedExemplarFilter.TraceBasedExemplarFilter() -> void -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder) -> Microsoft.Extensions.Logging.ILoggingBuilder! -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action! configure) -> Microsoft.Extensions.Logging.ILoggingBuilder! -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder, System.Action? configureOptions) -> Microsoft.Extensions.Logging.ILoggingBuilder! static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, OpenTelemetry.BaseProcessor! processor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, System.Func!>! implementationFactory) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder) -> OpenTelemetry.Logs.LoggerProviderBuilder! diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 10c7f0074ae..e4ae10579d6 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -17,9 +17,6 @@ #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif -#if EXPOSE_EXPERIMENTAL_FEATURES -using System.ComponentModel; -#endif using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; @@ -52,9 +49,6 @@ public static class OpenTelemetryLoggingExtensions /// The to use. /// The supplied for call chaining. /// todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")] -#if EXPOSE_EXPERIMENTAL_FEATURES - [EditorBrowsable(EditorBrowsableState.Never)] -#endif public static ILoggingBuilder AddOpenTelemetry( this ILoggingBuilder builder) => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: null); @@ -67,99 +61,11 @@ public static ILoggingBuilder AddOpenTelemetry( /// Optional configuration action. /// The supplied for call chaining. /// todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")] -#if EXPOSE_EXPERIMENTAL_FEATURES - [EditorBrowsable(EditorBrowsableState.Never)] -#endif public static ILoggingBuilder AddOpenTelemetry( this ILoggingBuilder builder, Action? configure) => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: configure); -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. - /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created - /// for a given . - /// - /// The to use. - /// The supplied for call chaining. - public -#else - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// Note: This is safe to be called multiple times and by library authors. - /// Only a single will be created - /// for a given . - /// - /// The to use. - /// The supplied for call chaining. - internal -#endif - static ILoggingBuilder UseOpenTelemetry( - this ILoggingBuilder builder) - => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: null); - -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// The to use. - /// Optional configuration action. - /// The supplied for call chaining. - public -#else - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// The to use. - /// configuration action. - /// The supplied for call chaining. - internal -#endif - static ILoggingBuilder UseOpenTelemetry( - this ILoggingBuilder builder, - Action configure) - { - Guard.ThrowIfNull(configure); - - return AddOpenTelemetryInternal(builder, configureBuilder: configure, configureOptions: null); - } - -#if EXPOSE_EXPERIMENTAL_FEATURES - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// The to use. - /// Optional configuration action. - /// Optional configuration action. - /// The supplied for call chaining. - public -#else - /// - /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . - /// - /// - /// The to use. - /// Optional configuration action. - /// Optional configuration action. - /// The supplied for call chaining. - internal -#endif - static ILoggingBuilder UseOpenTelemetry( - this ILoggingBuilder builder, - Action? configureBuilder, - Action? configureOptions) - => AddOpenTelemetryInternal(builder, configureBuilder, configureOptions); - private static ILoggingBuilder AddOpenTelemetryInternal( ILoggingBuilder builder, Action? configureBuilder, diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs index 4c79604d106..146747f71cc 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs @@ -381,7 +381,7 @@ public void AddOpenTelemetry_WithLogging_DisposalTest() } [Fact] - public void AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() + public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() { bool configureBuilderCalled = false; @@ -403,11 +403,8 @@ public void AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() deferredLoggerProviderBuilder.Configure((sp, builder) => { configureBuilderCalled = true; - var configuration = sp.GetRequiredService(); - var testKeyValue = configuration.GetValue("TEST_KEY", null); - Assert.Equal("TEST_KEY_VALUE", testKeyValue); }); } @@ -416,8 +413,14 @@ public void AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() var host = builder.Build(); + Assert.False(configureBuilderCalled); + + await host.StartAsync().ConfigureAwait(false); + Assert.True(configureBuilderCalled); + await host.StopAsync().ConfigureAwait(false); + host.Dispose(); } diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index 376526a0f8f..e4b8a33f919 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -25,25 +25,16 @@ namespace OpenTelemetry.Logs.Tests; public sealed class OpenTelemetryLoggingExtensionsTests { - [Theory] - [InlineData(false)] - [InlineData(true)] - public void ServiceCollectionAddOpenTelemetryNoParametersTest(bool callUseExtension) + [Fact] + public void ServiceCollectionAddOpenTelemetryNoParametersTest() { bool optionsCallbackInvoked = false; var serviceCollection = new ServiceCollection(); - serviceCollection.AddLogging(logging => + serviceCollection.AddLogging(configure => { - if (callUseExtension) - { - logging.UseOpenTelemetry(); - } - else - { - logging.AddOpenTelemetry(); - } + configure.AddOpenTelemetry(); }); serviceCollection.Configure(options => @@ -61,16 +52,10 @@ public void ServiceCollectionAddOpenTelemetryNoParametersTest(bool callUseExtens } [Theory] - [InlineData(false, 1, 0)] - [InlineData(false, 1, 1)] - [InlineData(false, 5, 5)] - [InlineData(true, 1, 0)] - [InlineData(true, 1, 1)] - [InlineData(true, 5, 5)] - public void ServiceCollectionAddOpenTelemetryConfigureActionTests( - bool callUseExtension, - int numberOfBuilderRegistrations, - int numberOfOptionsRegistrations) + [InlineData(1, 0)] + [InlineData(1, 1)] + [InlineData(5, 5)] + public void ServiceCollectionAddOpenTelemetryConfigureActionTests(int numberOfBuilderRegistrations, int numberOfOptionsRegistrations) { int configureCallbackInvocations = 0; int optionsCallbackInvocations = 0; @@ -78,18 +63,11 @@ public void ServiceCollectionAddOpenTelemetryConfigureActionTests( var serviceCollection = new ServiceCollection(); - serviceCollection.AddLogging(logging => + serviceCollection.AddLogging(configure => { for (int i = 0; i < numberOfBuilderRegistrations; i++) { - if (callUseExtension) - { - logging.UseOpenTelemetry(configureBuilder: null, configureOptions: ConfigureCallback); - } - else - { - logging.AddOpenTelemetry(ConfigureCallback); - } + configure.AddOpenTelemetry(ConfigureCallback); } }); @@ -138,69 +116,6 @@ void OptionsCallback(OpenTelemetryLoggerOptions options) } } - [Fact] - public void UseOpenTelemetryDependencyInjectionTest() - { - var serviceCollection = new ServiceCollection(); - - serviceCollection.AddLogging(logging => - { - logging.UseOpenTelemetry(builder => - { - builder.ConfigureServices(services => - { - services.AddSingleton(); - }); - - builder.ConfigureBuilder((sp, builder) => - { - builder.AddProcessor( - sp.GetRequiredService()); - }); - }); - }); - - using var sp = serviceCollection.BuildServiceProvider(); - - var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; - - Assert.NotNull(loggerProvider); - - Assert.NotNull(loggerProvider.Processor); - - Assert.True(loggerProvider.Processor is TestLogProcessor); - } - - [Fact] - public void UseOpenTelemetryOptionsOrderingTest() - { - int currentIndex = -1; - int beforeDelegateIndex = -1; - int extensionDelegateIndex = -1; - int afterDelegateIndex = -1; - - var serviceCollection = new ServiceCollection(); - - serviceCollection.Configure(o => beforeDelegateIndex = ++currentIndex); - - serviceCollection.AddLogging(logging => - { - logging.UseOpenTelemetry(configureBuilder: null, configureOptions: o => extensionDelegateIndex = ++currentIndex); - }); - - serviceCollection.Configure(o => afterDelegateIndex = ++currentIndex); - - using var sp = serviceCollection.BuildServiceProvider(); - - var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; - - Assert.NotNull(loggerProvider); - - Assert.Equal(0, beforeDelegateIndex); - Assert.Equal(1, extensionDelegateIndex); - Assert.Equal(2, afterDelegateIndex); - } - // This test validates that the OpenTelemetryLoggerOptions contains only primitive type properties. // This is necessary to ensure trim correctness since that class is effectively deserialized from // configuration. The top level properties are ensured via annotation on the RegisterProviderOptions API @@ -214,8 +129,4 @@ public void TestTrimmingCorrectnessOfOpenTelemetryLoggerOptions() Assert.True(prop.PropertyType.IsPrimitive, $"Property OpenTelemetryLoggerOptions.{prop.Name} doesn't have a primitive type. This is potentially a trim compatibility issue."); } } - - private sealed class TestLogProcessor : BaseProcessor - { - } } From 1500698c36db5389514e7feef6f79cee0ee64661 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 14 Nov 2023 12:27:39 -0800 Subject: [PATCH 27/43] Remove more logging changes. --- .../Logs/ILogger/OpenTelemetryLoggingExtensions.cs | 2 -- .../OpenTelemetryServicesExtensionsTests.cs | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index e4ae10579d6..abd27cd70b4 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -48,7 +48,6 @@ public static class OpenTelemetryLoggingExtensions /// /// The to use. /// The supplied for call chaining. - /// todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")] public static ILoggingBuilder AddOpenTelemetry( this ILoggingBuilder builder) => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: null); @@ -60,7 +59,6 @@ public static ILoggingBuilder AddOpenTelemetry( /// The to use. /// Optional configuration action. /// The supplied for call chaining. - /// todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")] public static ILoggingBuilder AddOpenTelemetry( this ILoggingBuilder builder, Action? configure) diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs index 146747f71cc..181dec9e367 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs @@ -403,8 +403,11 @@ public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest() deferredLoggerProviderBuilder.Configure((sp, builder) => { configureBuilderCalled = true; + var configuration = sp.GetRequiredService(); + var testKeyValue = configuration.GetValue("TEST_KEY", null); + Assert.Equal("TEST_KEY_VALUE", testKeyValue); }); } From 0b623e965e97418ab89ef2ae15b6f7d2f4a36a2e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 15 Nov 2023 13:55:42 -0800 Subject: [PATCH 28/43] A few misc changes and fixes. --- .../OpenTelemetryBuilder.cs | 6 +-- .../Builder/MeterProviderBuilderExtensions.cs | 4 ++ .../OpenTelemetryBuilderTests.cs | 37 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 215d43bcbee..4f0b51a00b4 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -62,13 +62,13 @@ public OpenTelemetryBuilder ConfigureResource( Guard.ThrowIfNull(configure); this.Services.ConfigureOpenTelemetryMeterProvider( - (sp, builder) => builder.ConfigureResource(configure)); + builder => builder.ConfigureResource(configure)); this.Services.ConfigureOpenTelemetryTracerProvider( - (sp, builder) => builder.ConfigureResource(configure)); + builder => builder.ConfigureResource(configure)); this.Services.ConfigureOpenTelemetryLoggerProvider( - (sp, builder) => builder.ConfigureResource(configure)); + builder => builder.ConfigureResource(configure)); return this; } diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index d5c979604bf..767cf30714b 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -277,6 +277,8 @@ public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterP /// The supplied for chaining. public static MeterProviderBuilder SetResourceBuilder(this MeterProviderBuilder meterProviderBuilder, ResourceBuilder resourceBuilder) { + Guard.ThrowIfNull(resourceBuilder); + meterProviderBuilder.ConfigureBuilder((sp, builder) => { if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) @@ -297,6 +299,8 @@ public static MeterProviderBuilder SetResourceBuilder(this MeterProviderBuilder /// The supplied for chaining. public static MeterProviderBuilder ConfigureResource(this MeterProviderBuilder meterProviderBuilder, Action configure) { + Guard.ThrowIfNull(configure); + meterProviderBuilder.ConfigureBuilder((sp, builder) => { if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk) diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryBuilderTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryBuilderTests.cs index 4d31768c2ff..43da25a08b4 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryBuilderTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryBuilderTests.cs @@ -75,4 +75,41 @@ public void ConfigureResourceTest() loggerProvider.Resource.Attributes, kvp => kvp.Key == "l_key1" && (string)kvp.Value == "l_value1"); } + + [Fact] + public void ConfigureResourceServiceProviderTest() + { + var services = new ServiceCollection(); + + services.AddSingleton(); + + services.AddOpenTelemetry() + .ConfigureResource(r => r.AddDetector(sp => sp.GetRequiredService())) + .WithLogging() + .WithMetrics() + .WithTracing(); + + using var sp = services.BuildServiceProvider(); + + var tracerProvider = sp.GetRequiredService() as TracerProviderSdk; + var meterProvider = sp.GetRequiredService() as MeterProviderSdk; + var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; + + Assert.NotNull(tracerProvider); + Assert.NotNull(meterProvider); + Assert.NotNull(loggerProvider); + + Assert.Single(tracerProvider.Resource.Attributes, kvp => kvp.Key == "key1" && (string)kvp.Value == "value1"); + Assert.Single(meterProvider.Resource.Attributes, kvp => kvp.Key == "key1" && (string)kvp.Value == "value1"); + Assert.Single(loggerProvider.Resource.Attributes, kvp => kvp.Key == "key1" && (string)kvp.Value == "value1"); + } + + private sealed class TestResourceDetector : IResourceDetector + { + public Resource Detect() => ResourceBuilder.CreateEmpty().AddAttributes( + new Dictionary + { + ["key1"] = "value1", + }).Build(); + } } From 69055834524bf7b73fda2cee41169487d96cc265 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 16 Nov 2023 15:41:47 -0800 Subject: [PATCH 29/43] Refactor a little bit of duplicated code. --- .../Metrics/MetricApiTestsBase.cs | 2 +- .../Metrics/MetricSnapshotTestsBase.cs | 18 +++--------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs index 2a8af351757..8eb3a124dbf 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTestsBase.cs @@ -1527,7 +1527,7 @@ public void UnsupportedMetricInstrument() Assert.Empty(exportedItems); } - private static IConfiguration BuildConfiguration(bool emitOverflowAttribute, bool shouldReclaimUnusedMetricPoints) + internal static IConfiguration BuildConfiguration(bool emitOverflowAttribute, bool shouldReclaimUnusedMetricPoints) { var configurationData = new Dictionary(); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs index fad3663f773..9a22f7b6795 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricSnapshotTestsBase.cs @@ -31,21 +31,9 @@ public abstract class MetricSnapshotTestsBase protected MetricSnapshotTestsBase(bool emitOverflowAttribute, bool shouldReclaimUnusedMetricPoints) { - var configurationData = new Dictionary(); - - if (emitOverflowAttribute) - { - configurationData[MetricTestsBase.EmitOverFlowAttributeConfigKey] = "true"; - } - - if (shouldReclaimUnusedMetricPoints) - { - configurationData[MetricTestsBase.ReclaimUnusedMetricPointsConfigKey] = "true"; - } - - this.configuration = new ConfigurationBuilder() - .AddInMemoryCollection(configurationData) - .Build(); + this.configuration = MetricApiTestsBase.BuildConfiguration( + emitOverflowAttribute, + shouldReclaimUnusedMetricPoints); } [Fact] From cc195bacfafd5b400d4606606158dac51cc167d6 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 16 Nov 2023 21:28:37 -0800 Subject: [PATCH 30/43] Code review. --- ...tryMetricListener.cs => OpenTelemetryMetricsListener.cs} | 6 +++--- .../OpenTelemetryMetricsBuilderExtensions.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/OpenTelemetry.Extensions.Hosting/Implementation/{OpenTelemetryMetricListener.cs => OpenTelemetryMetricsListener.cs} (94%) diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricsListener.cs similarity index 94% rename from src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs rename to src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricsListener.cs index e2756d98ee6..51c8c0d1826 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricListener.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/OpenTelemetryMetricsListener.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,12 +20,12 @@ namespace OpenTelemetry.Metrics; -internal sealed class OpenTelemetryMetricListener : IMetricsListener, IDisposable +internal sealed class OpenTelemetryMetricsListener : IMetricsListener, IDisposable { private readonly MeterProviderSdk meterProviderSdk; private IObservableInstrumentsSource? observableInstrumentsSource; - public OpenTelemetryMetricListener(MeterProvider meterProvider) + public OpenTelemetryMetricsListener(MeterProvider meterProvider) { var meterProviderSdk = meterProvider as MeterProviderSdk; diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 92af3c929a6..57504cda4a9 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -61,7 +61,7 @@ public static IMetricsBuilder UseOpenTelemetry( var builder = new MeterProviderBuilderBase(metricsBuilder.Services); metricsBuilder.Services.TryAddEnumerable( - ServiceDescriptor.Singleton()); + ServiceDescriptor.Singleton()); configure(builder); From d99328926f82a41e851e965d4380cb9e9f27639a Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 09:35:02 -0800 Subject: [PATCH 31/43] Code review. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 918deb3a8e6..e9fd7689c7c 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -231,11 +231,11 @@ internal MeterProviderSdk( OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( instrument.Name, instrument.Meter.Name, - "Instrument belongs to a Meter which has been enabled externally and via a subscription on the provider. External subscription will be ignored in favor of the provider subscription.", + "Instrument belongs to a Meter which has been enabled both externally and via a subscription on the provider. External subscription will be ignored in favor of the provider subscription.", "Don't call AddMeter when also using external management."); return null; } - else if (!listeningIsManagedExternally && !listenToInstrumentUsingSdkConfiguration) + else if (!listenToInstrumentUsingSdkConfiguration && !listeningIsManagedExternally) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( instrument.Name, From 0862f7f6df664513d923ef8fac35089d180b1fce Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 11:37:27 -0800 Subject: [PATCH 32/43] Code review. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index e9fd7689c7c..7f9a9fa6627 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -232,7 +232,7 @@ internal MeterProviderSdk( instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter which has been enabled both externally and via a subscription on the provider. External subscription will be ignored in favor of the provider subscription.", - "Don't call AddMeter when also using external management."); + "Programmatic calls adding meters to the SDK (either by calling AddMeter directly or using 'AddInstrumentation' extensions) are always favored over external registrations. When also using external management (typically IMetricsBuilder or IMetricsListener) remove programmatic calls to the SDK to allow registrations to be added and removed dynamically."); return null; } else if (!listenToInstrumentUsingSdkConfiguration && !listeningIsManagedExternally) From f73d309c98ec73a29bc48f7d05463d2d19fad2ef Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 14:00:10 -0800 Subject: [PATCH 33/43] Code review and race condition cleanup. --- .../Internal/OpenTelemetrySdkEventSource.cs | 6 ++ src/OpenTelemetry/Metrics/Metric.cs | 10 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 95 ++++++++++++++----- ...nTelemetryMetricsBuilderExtensionsTests.cs | 74 +++++++++++++-- 4 files changed, 150 insertions(+), 35 deletions(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 89e58f20297..2a1c1ea0d6d 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -359,6 +359,12 @@ public void MetricInstrumentReactivated(string instrumentName, string meterName) this.WriteEvent(53, instrumentName, meterName); } + [Event(54, Message = "A deactivated Instrument '{0}', Meter '{1}' has been removed.", Level = EventLevel.Informational)] + public void MetricInstrumentRemoved(string instrumentName, string meterName) + { + this.WriteEvent(54, instrumentName, meterName); + } + #if DEBUG public class OpenTelemetryEventListener : EventListener { diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 6fe6036498e..27c55ead43a 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -27,6 +27,10 @@ public sealed class Metric internal const int DefaultExponentialHistogramMaxScale = 20; + internal const int MetricCleanupNoState = 0; + internal const int MetricCleanupPending = 1; + internal const int MetricCleanupComplete = 2; + internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 }; // Short default histogram bounds. Based on the recommended semantic convention values for http.server.request.duration. @@ -169,7 +173,7 @@ internal Metric( this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); this.Temporality = temporality; - this.InstrumentDisposed = false; + this.CleanupState = MetricCleanupNoState; } /// @@ -212,7 +216,9 @@ internal Metric( /// internal MetricStreamIdentity InstrumentIdentity { get; private set; } - internal bool InstrumentDisposed { get; set; } + internal object CleanupLock => this.aggStore; + + internal int CleanupState { get; set; } /// /// Get the metric points for the metric stream. diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 9e1b83730b9..8b6d887a93f 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -52,7 +52,34 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) { if (this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out var existingMetric)) { - return existingMetric; + if (existingMetric.CleanupState != Metric.MetricCleanupNoState) + { + bool activated = false; + + lock (existingMetric.CleanupLock) + { + if (existingMetric.CleanupState == Metric.MetricCleanupPending) + { + existingMetric.CleanupState = Metric.MetricCleanupNoState; + activated = true; + } + } + + if (activated) + { + // Note: This case here is a metric was deactivated + // and then reactivated before an export ran to + // finish the cleanup. + OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( + metricStreamIdentity.InstrumentName, + metricStreamIdentity.MeterName); + return existingMetric; + } + } + else + { + return existingMetric; + } } var index = ++this.metricIndex; @@ -239,13 +266,9 @@ internal void SetMaxMetricPointsPerMetricStream(int maxMetricPointsPerMetricStre private static void DeactivateMetric(Metric metric) { - metric.InstrumentDisposed = true; - - var metricStreamIdentity = metric.InstrumentIdentity; + metric.CleanupState = Metric.MetricCleanupPending; - OpenTelemetrySdkEventSource.Log.MetricInstrumentDeactivated( - metricStreamIdentity.InstrumentName, - metricStreamIdentity.MeterName); + OpenTelemetrySdkEventSource.Log.MetricInstrumentDeactivated(metric.Name, metric.MeterName); } private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metricStreamIdentity) @@ -272,6 +295,8 @@ private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metr } else { + // Note: This case here is a metric was deactivated and then + // reactivated after an export ran and finished the cleanup. OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName); @@ -295,32 +320,19 @@ private Batch GetMetricsBatch() int metricCountCurrentBatch = 0; for (int i = 0; i < target; i++) { - var metric = this.metrics![i]; - int metricPointSize = 0; + ref var metric = ref this.metrics![i]; if (metric != null) { - if (metric.InstrumentDisposed) - { - metricPointSize = metric.Snapshot(); - - var metricStreamNamesLookupResult = this.metricStreamNames.TryGetValue(metric.InstrumentIdentity.MetricStreamName, out var registration); - Debug.Assert(metricStreamNamesLookupResult, "result was false"); - Debug.Assert(registration != null, "registration was null"); - Interlocked.Decrement(ref registration!.RegistrationCount); + int metricPointSize = metric.Snapshot(); - var instrumentIdentityToMetricLookupResult = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); - Debug.Assert(instrumentIdentityToMetricLookupResult, "instrumentIdentityToMetricLookupResult was false"); - - this.metrics[i] = null; - } - else + if (metricPointSize > 0) { - metricPointSize = metric.Snapshot(); + this.metricsCurrentBatch![metricCountCurrentBatch++] = metric; } - if (metricPointSize > 0) + if (metric.CleanupState == Metric.MetricCleanupPending) { - this.metricsCurrentBatch![metricCountCurrentBatch++] = metric; + this.CleanupMetric(ref metric); } } } @@ -334,6 +346,37 @@ private Batch GetMetricsBatch() } } + private void CleanupMetric(ref Metric metric) + { + lock (metric.CleanupLock) + { + if (metric.CleanupState != Metric.MetricCleanupPending) + { + // Note: If we fall here it means the metric was reactivated + // while we were waiting on the lock. + return; + } + + var metricStreamNamesLookupResult = this.metricStreamNames.TryGetValue(metric.InstrumentIdentity.MetricStreamName, out var registration); + Debug.Assert(metricStreamNamesLookupResult, "result was false"); + Debug.Assert(registration != null, "registration was null"); + Interlocked.Decrement(ref registration!.RegistrationCount); + + metric.CleanupState = Metric.MetricCleanupComplete; + } + + var instrumentIdentityToMetricLookupResult = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); + Debug.Assert(instrumentIdentityToMetricLookupResult, "instrumentIdentityToMetricLookupResult was false"); + + OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric.Name, metric.MeterName); + + // Note: This is a pointer to the storage for the metric inside + // this.metrics array. Clearing using the pointer is faster than + // re-accessing it which will incur a bounds check (this.metrics[i] = + // null). + metric = null!; + } + private sealed class MetricStreamRegistration { public int RegistrationCount; diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index d03150a4dc8..0c2c53d69a6 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -49,7 +49,7 @@ public void EnableMetricsTest(bool useWithMetricsStyle) counter.Add(1); } - AssertSingleMetricWithLongSumOfOne(exportedItems); + AssertSingleMetricWithLongSum(exportedItems); } [Theory] @@ -71,13 +71,13 @@ public void EnableMetricsWithAddMeterTest(bool useWithMetricsStyle) counter.Add(1); } - AssertSingleMetricWithLongSumOfOne(exportedItems); + AssertSingleMetricWithLongSum(exportedItems); } [Theory] [InlineData(false)] [InlineData(true)] - public void ReloadOfMetricsViaIConfigurationTest(bool useWithMetricsStyle) + public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMetricsStyle) { using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); @@ -112,7 +112,7 @@ public void ReloadOfMetricsViaIConfigurationTest(bool useWithMetricsStyle) meterProvider.ForceFlush(); - AssertSingleMetricWithLongSumOfOne(exportedItems); + AssertSingleMetricWithLongSum(exportedItems); exportedItems.Clear(); @@ -134,7 +134,7 @@ public void ReloadOfMetricsViaIConfigurationTest(bool useWithMetricsStyle) meterProvider.ForceFlush(); - AssertSingleMetricWithLongSumOfOne(exportedItems); + AssertSingleMetricWithLongSum(exportedItems); var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); @@ -147,9 +147,69 @@ public void ReloadOfMetricsViaIConfigurationTest(bool useWithMetricsStyle) var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); Assert.Single(metricInstrumentReactivatedEvents); + + var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 54); + + Assert.Single(metricInstrumentRemovedEvents); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWithMetricsStyle) + { + using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + List exportedItems = new(); + + var source = new MemoryConfigurationSource(); + var memory = new MemoryConfigurationProvider(source); + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "true"); + var configuration = new ConfigurationRoot(new[] { memory }); + + using var host = MetricTestsBase.BuildHost( + useWithMetricsStyle, + configureAppConfiguration: (context, builder) => builder.AddConfiguration(configuration), + configureMeterProviderBuilder: builder => builder + .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = MetricReaderTemporalityPreference.Delta)); + + var meterProvider = host.Services.GetRequiredService(); + var options = host.Services.GetRequiredService>(); + + var counter = meter.CreateCounter("TestCounter"); + counter.Add(1); + + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "false"); + configuration.Reload(); + counter.Add(1); + + memory.Set($"Metrics:EnabledMetrics:{meter.Name}:Default", "true"); + configuration.Reload(); + counter.Add(1); + + meterProvider.ForceFlush(); + + AssertSingleMetricWithLongSum(exportedItems, expectedValue: 2); + + var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); + + Assert.Empty(duplicateMetricInstrumentEvents); + + var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); + + Assert.Single(metricInstrumentDeactivatedEvents); + + var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); + + Assert.Single(metricInstrumentReactivatedEvents); + + var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 54); + + Assert.Empty(metricInstrumentRemovedEvents); } - private static void AssertSingleMetricWithLongSumOfOne(List exportedItems) + private static void AssertSingleMetricWithLongSum(List exportedItems, long expectedValue = 1) { Assert.Single(exportedItems); @@ -162,6 +222,6 @@ private static void AssertSingleMetricWithLongSumOfOne(List exportedItem Assert.Single(metricPoints); var metricPoint = metricPoints[0]; - Assert.Equal(1, metricPoint.GetSumLong()); + Assert.Equal(expectedValue, metricPoint.GetSumLong()); } } From da5554b91da5198f87a1158344de1e6ab5da9cec Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 14:21:07 -0800 Subject: [PATCH 34/43] Tweak. --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 7f9a9fa6627..74405b02be2 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -232,7 +232,7 @@ internal MeterProviderSdk( instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter which has been enabled both externally and via a subscription on the provider. External subscription will be ignored in favor of the provider subscription.", - "Programmatic calls adding meters to the SDK (either by calling AddMeter directly or using 'AddInstrumentation' extensions) are always favored over external registrations. When also using external management (typically IMetricsBuilder or IMetricsListener) remove programmatic calls to the SDK to allow registrations to be added and removed dynamically."); + "Programmatic calls adding meters to the SDK (either by calling AddMeter directly or indirectly through helpers such as 'AddInstrumentation' extensions) are always favored over external registrations. When also using external management (typically IMetricsBuilder or IMetricsListener) remove programmatic calls to the SDK to allow registrations to be added and removed dynamically."); return null; } else if (!listenToInstrumentUsingSdkConfiguration && !listeningIsManagedExternally) From f748914eb5763e906fbe89c09f24c7e856ad1c1d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 21:36:02 -0800 Subject: [PATCH 35/43] Reuse metric instances when an instrument is reactivated. --- .../Internal/OpenTelemetrySdkEventSource.cs | 6 - src/OpenTelemetry/Metrics/Metric.cs | 13 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 124 ++++++------------ ...nTelemetryMetricsBuilderExtensionsTests.cs | 47 ++++--- .../Metrics/MeterProviderSdkTest.cs | 45 +++++++ 5 files changed, 120 insertions(+), 115 deletions(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 2a1c1ea0d6d..89e58f20297 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -359,12 +359,6 @@ public void MetricInstrumentReactivated(string instrumentName, string meterName) this.WriteEvent(53, instrumentName, meterName); } - [Event(54, Message = "A deactivated Instrument '{0}', Meter '{1}' has been removed.", Level = EventLevel.Informational)] - public void MetricInstrumentRemoved(string instrumentName, string meterName) - { - this.WriteEvent(54, instrumentName, meterName); - } - #if DEBUG public class OpenTelemetryEventListener : EventListener { diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 27c55ead43a..5162e40f49e 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -27,9 +27,9 @@ public sealed class Metric internal const int DefaultExponentialHistogramMaxScale = 20; - internal const int MetricCleanupNoState = 0; - internal const int MetricCleanupPending = 1; - internal const int MetricCleanupComplete = 2; + internal const int MetricStatusActive = 0; + internal const int MetricStatusDeactivating = 1; + internal const int MetricStatusInactive = 2; internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 }; @@ -57,6 +57,7 @@ public sealed class Metric ("System.Net.Http", "http.client.connection.duration"), }; + internal volatile int Status; private readonly AggregatorStore aggStore; internal Metric( @@ -173,7 +174,7 @@ internal Metric( this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); this.Temporality = temporality; - this.CleanupState = MetricCleanupNoState; + this.Status = MetricStatusActive; } /// @@ -216,9 +217,7 @@ internal Metric( /// internal MetricStreamIdentity InstrumentIdentity { get; private set; } - internal object CleanupLock => this.aggStore; - - internal int CleanupState { get; set; } + internal object StatusLock => this.aggStore; /// /// Get the metric points for the metric stream. diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 8b6d887a93f..d1d1af73aca 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -26,7 +26,7 @@ namespace OpenTelemetry.Metrics; /// public abstract partial class MetricReader { - private readonly ConcurrentDictionary metricStreamNames = new(StringComparer.OrdinalIgnoreCase); + private readonly HashSet metricStreamNames = new(StringComparer.OrdinalIgnoreCase); private readonly ConcurrentDictionary instrumentIdentityToMetric = new(); private readonly object instrumentCreationLock = new(); private int maxMetricStreams; @@ -52,34 +52,9 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) { if (this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out var existingMetric)) { - if (existingMetric.CleanupState != Metric.MetricCleanupNoState) - { - bool activated = false; - - lock (existingMetric.CleanupLock) - { - if (existingMetric.CleanupState == Metric.MetricCleanupPending) - { - existingMetric.CleanupState = Metric.MetricCleanupNoState; - activated = true; - } - } + ActivateMetricIfRequired(existingMetric); - if (activated) - { - // Note: This case here is a metric was deactivated - // and then reactivated before an export ran to - // finish the cleanup. - OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( - metricStreamIdentity.InstrumentName, - metricStreamIdentity.MeterName); - return existingMetric; - } - } - else - { - return existingMetric; - } + return existingMetric; } var index = ++this.metricIndex; @@ -157,6 +132,8 @@ internal List AddMetricsListWithViews(Instrument instrument, List 0) + private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metricStreamIdentity) + { + if (!this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName)) { OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( metricStreamIdentity.InstrumentName, @@ -293,19 +277,6 @@ private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metr "Metric instrument has the same name as an existing one but differs by description, unit, or instrument type. Measurements from this instrument will still be exported but may result in conflicts.", "Either change the name of the instrument or use MeterProviderBuilder.AddView to resolve the conflict."); } - else - { - // Note: This case here is a metric was deactivated and then - // reactivated after an export ran and finished the cleanup. - OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( - metricStreamIdentity.InstrumentName, - metricStreamIdentity.MeterName); - } - - Interlocked.Increment(ref registration.RegistrationCount); - - static MetricStreamRegistration CreateRegistration(string metricStreamName) - => new() { RegistrationCount = -1 }; } private Batch GetMetricsBatch() @@ -320,20 +291,26 @@ private Batch GetMetricsBatch() int metricCountCurrentBatch = 0; for (int i = 0; i < target; i++) { - ref var metric = ref this.metrics![i]; + var metric = this.metrics![i]; if (metric != null) { + var metricStatus = metric.Status; + if (metricStatus != Metric.MetricStatusActive) + { + if (metricStatus == Metric.MetricStatusInactive) + { + continue; + } + + this.CompleteMetricDeactivation(metric); + } + int metricPointSize = metric.Snapshot(); if (metricPointSize > 0) { this.metricsCurrentBatch![metricCountCurrentBatch++] = metric; } - - if (metric.CleanupState == Metric.MetricCleanupPending) - { - this.CleanupMetric(ref metric); - } } } @@ -346,39 +323,18 @@ private Batch GetMetricsBatch() } } - private void CleanupMetric(ref Metric metric) + private void CompleteMetricDeactivation(Metric metric) { - lock (metric.CleanupLock) + lock (metric.StatusLock) { - if (metric.CleanupState != Metric.MetricCleanupPending) + if (metric.Status != Metric.MetricStatusDeactivating) { // Note: If we fall here it means the metric was reactivated // while we were waiting on the lock. return; } - var metricStreamNamesLookupResult = this.metricStreamNames.TryGetValue(metric.InstrumentIdentity.MetricStreamName, out var registration); - Debug.Assert(metricStreamNamesLookupResult, "result was false"); - Debug.Assert(registration != null, "registration was null"); - Interlocked.Decrement(ref registration!.RegistrationCount); - - metric.CleanupState = Metric.MetricCleanupComplete; + metric.Status = Metric.MetricStatusInactive; } - - var instrumentIdentityToMetricLookupResult = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); - Debug.Assert(instrumentIdentityToMetricLookupResult, "instrumentIdentityToMetricLookupResult was false"); - - OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric.Name, metric.MeterName); - - // Note: This is a pointer to the storage for the metric inside - // this.metrics array. Clearing using the pointer is faster than - // re-accessing it which will incur a bounds check (this.metrics[i] = - // null). - metric = null!; - } - - private sealed class MetricStreamRegistration - { - public int RegistrationCount; } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index 0c2c53d69a6..a927a4e1cad 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -75,9 +75,11 @@ public void EnableMetricsWithAddMeterTest(bool useWithMetricsStyle) } [Theory] - [InlineData(false)] - [InlineData(true)] - public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMetricsStyle) + [InlineData(false, MetricReaderTemporalityPreference.Delta)] + [InlineData(true, MetricReaderTemporalityPreference.Delta)] + [InlineData(false, MetricReaderTemporalityPreference.Cumulative)] + [InlineData(true, MetricReaderTemporalityPreference.Cumulative)] + public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMetricsStyle, MetricReaderTemporalityPreference temporalityPreference) { using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); @@ -92,7 +94,7 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe useWithMetricsStyle, configureAppConfiguration: (context, builder) => builder.AddConfiguration(configuration), configureMeterProviderBuilder: builder => builder - .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = MetricReaderTemporalityPreference.Delta)); + .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = temporalityPreference)); var meterProvider = host.Services.GetRequiredService(); var options = host.Services.GetRequiredService>(); @@ -124,7 +126,20 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe meterProvider.ForceFlush(); - Assert.Empty(exportedItems); + if (temporalityPreference == MetricReaderTemporalityPreference.Cumulative) + { + // Note: When in Cumulative the metric shows up on the export + // immediately after being deactivated and then is ignored. + AssertSingleMetricWithLongSum(exportedItems); + + meterProvider.ForceFlush(); + exportedItems.Clear(); + Assert.Empty(exportedItems); + } + else + { + Assert.Empty(exportedItems); + } memory.Set($"Metrics:OpenTelemetry:EnabledMetrics:{meter.Name}:Default", "true"); @@ -134,7 +149,9 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe meterProvider.ForceFlush(); - AssertSingleMetricWithLongSum(exportedItems); + AssertSingleMetricWithLongSum( + exportedItems, + expectedValue: temporalityPreference == MetricReaderTemporalityPreference.Delta ? 1 : 2); var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); @@ -147,16 +164,14 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); Assert.Single(metricInstrumentReactivatedEvents); - - var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 54); - - Assert.Single(metricInstrumentRemovedEvents); } [Theory] - [InlineData(false)] - [InlineData(true)] - public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWithMetricsStyle) + [InlineData(false, MetricReaderTemporalityPreference.Delta)] + [InlineData(true, MetricReaderTemporalityPreference.Delta)] + [InlineData(false, MetricReaderTemporalityPreference.Cumulative)] + [InlineData(true, MetricReaderTemporalityPreference.Cumulative)] + public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWithMetricsStyle, MetricReaderTemporalityPreference temporalityPreference) { using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); @@ -172,7 +187,7 @@ public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWit useWithMetricsStyle, configureAppConfiguration: (context, builder) => builder.AddConfiguration(configuration), configureMeterProviderBuilder: builder => builder - .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = MetricReaderTemporalityPreference.Delta)); + .AddInMemoryExporter(exportedItems, reader => reader.TemporalityPreference = temporalityPreference)); var meterProvider = host.Services.GetRequiredService(); var options = host.Services.GetRequiredService>(); @@ -203,10 +218,6 @@ public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWit var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); Assert.Single(metricInstrumentReactivatedEvents); - - var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 54); - - Assert.Empty(metricInstrumentRemovedEvents); } private static void AssertSingleMetricWithLongSum(List exportedItems, long expectedValue = 1) diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs index 9a980e12ea4..5c4509307b9 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs @@ -14,6 +14,8 @@ // limitations under the License. // +using System.Diagnostics.Metrics; +using OpenTelemetry.Tests; using Xunit; namespace OpenTelemetry.Metrics.Tests; @@ -45,4 +47,47 @@ public void BuilderTypeDoesNotChangeTest() Assert.NotNull(provider); } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void TransientMeterReusesMetricTest(bool withView) + { + var meterName = Utils.GetCurrentMethodName(); + var exportedItems = new List(); + + var builder = Sdk.CreateMeterProviderBuilder() + .SetMaxMetricStreams(1) + .AddMeter(meterName) + .AddInMemoryExporter(exportedItems); + + if (withView) + { + builder.AddView(i => null); + } + + using var meterProvider = builder + .Build() as MeterProviderSdk; + + Assert.NotNull(meterProvider); + + RunTest(); + RunTest(); + + void RunTest() + { + exportedItems.Clear(); + + var meter = new Meter(meterName); + + var counter = meter.CreateCounter("Counter"); + counter.Add(1); + + meter.Dispose(); + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + } + } } From 7685521d8ac1e4dfa1ab7779932513f615b33144 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 17 Nov 2023 21:45:24 -0800 Subject: [PATCH 36/43] Expand TODOs. --- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index d1d1af73aca..5648847c2c8 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -245,6 +245,8 @@ private static void DeactivateMetric(Metric metric) { metric.Status = Metric.MetricStatusDeactivating; + // TODO: Should we set end time here? + OpenTelemetrySdkEventSource.Log.MetricInstrumentDeactivated( metric.Name, metric.MeterName); @@ -259,7 +261,9 @@ private static void ActivateMetricIfRequired(Metric metric) metric.Status = Metric.MetricStatusActive; } - // TODO: Should we reset metric data points & start time? + // TODO: Should we set start time here? Should we reset the data + // points? What if we are activating before an export/collect has + // had a chance to export the previously collected data? OpenTelemetrySdkEventSource.Log.MetricInstrumentReactivated( metric.Name, From 54da2d997ada4172ca774404c85cb6b5b7f69a22 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 20 Nov 2023 11:49:58 -0800 Subject: [PATCH 37/43] Lower reference to Microsoft.Extensions.Diagnostics.Abstractions. --- Directory.Packages.props | 2 +- .../OpenTelemetry.Extensions.Hosting.csproj | 2 +- .../OpenTelemetryBuilder.cs | 12 ++++++------ .../OpenTelemetryMetricsBuilderExtensions.cs | 19 +++++++++++++++---- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 5d4099ef9a0..13fb63b9f08 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -29,7 +29,7 @@ - + diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj index 63e328228c9..cc6b1b078a1 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetry.Extensions.Hosting.csproj @@ -10,7 +10,7 @@ - + diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 4f0b51a00b4..00af83db167 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -82,10 +82,9 @@ public OpenTelemetryBuilder ConfigureResource( /// This is safe to be called multiple times and by library authors. /// Only a single will be created for a given /// . - /// This method automatically calls and - /// registers an named 'OpenTelemetry' into - /// the . + /// This method automatically registers an named 'OpenTelemetry' into the . /// /// /// The supplied for chaining @@ -103,8 +102,9 @@ public OpenTelemetryBuilder WithMetrics() /// calls. public OpenTelemetryBuilder WithMetrics(Action configure) { - this.Services.AddMetrics( - builder => builder.UseOpenTelemetry(configure)); + OpenTelemetryMetricsBuilderExtensions.RegisterMetricsListener( + this.Services, + configure); return this; } diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 57504cda4a9..2d707fcf7c8 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using OpenTelemetry.Internal; @@ -56,15 +57,25 @@ public static IMetricsBuilder UseOpenTelemetry( Action configure) { Guard.ThrowIfNull(metricsBuilder); + + RegisterMetricsListener(metricsBuilder.Services, configure); + + return metricsBuilder; + } + + internal static void RegisterMetricsListener( + IServiceCollection services, + Action configure) + { + Debug.Assert(services != null, "services was null"); + Guard.ThrowIfNull(configure); - var builder = new MeterProviderBuilderBase(metricsBuilder.Services); + var builder = new MeterProviderBuilderBase(services); - metricsBuilder.Services.TryAddEnumerable( + services.TryAddEnumerable( ServiceDescriptor.Singleton()); configure(builder); - - return metricsBuilder; } } From 97f3bf502457521bfd618d0c96b5c5cbc23b88d4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 20 Nov 2023 13:50:12 -0800 Subject: [PATCH 38/43] Warning cleanup. --- .../OpenTelemetryMetricsBuilderExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs index 2d707fcf7c8..a34cea3ea3e 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryMetricsBuilderExtensions.cs @@ -71,9 +71,9 @@ internal static void RegisterMetricsListener( Guard.ThrowIfNull(configure); - var builder = new MeterProviderBuilderBase(services); + var builder = new MeterProviderBuilderBase(services!); - services.TryAddEnumerable( + services!.TryAddEnumerable( ServiceDescriptor.Singleton()); configure(builder); From abbcbb129647f8492f6e072a5d48aa40375b57b9 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 20 Nov 2023 16:00:16 -0800 Subject: [PATCH 39/43] Revert logging options ordering change. --- .../Logs/ILogger/OpenTelemetryLoggingExtensions.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index abd27cd70b4..e4cf260aa16 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -75,14 +75,17 @@ private static ILoggingBuilder AddOpenTelemetryInternal( var services = builder.Services; - // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions - RegisterLoggerProviderOptions(services); - if (configureOptions != null) { + // TODO: Move this below the RegisterLoggerProviderOptions call so + // that user-supplied delegate fires AFTER the options are bound to + // Logging:OpenTelemetry configuration. services.Configure(configureOptions); } + // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions + RegisterLoggerProviderOptions(services); + var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder( (sp, logging) => { From 7744d5ebd2ffda0d734a31d60548eba417292ca4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 Nov 2023 11:50:53 -0800 Subject: [PATCH 40/43] Code review. --- .../Internal/OpenTelemetrySdkEventSource.cs | 4 +- src/OpenTelemetry/Metrics/Metric.cs | 8 +- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 97 +++++++++---------- ...nTelemetryMetricsBuilderExtensionsTests.cs | 43 +++++--- .../Metrics/MeterProviderSdkTest.cs | 39 ++++++-- 5 files changed, 115 insertions(+), 76 deletions(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 89e58f20297..67a668cf3d5 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -353,8 +353,8 @@ public void MetricInstrumentDeactivated(string instrumentName, string meterName) this.WriteEvent(52, instrumentName, meterName); } - [Event(53, Message = "A previously deactivated Instrument '{0}', Meter '{1}' has been reactivated.", Level = EventLevel.Informational)] - public void MetricInstrumentReactivated(string instrumentName, string meterName) + [Event(53, Message = "Instrument '{0}', Meter '{1}' has been removed.", Level = EventLevel.Informational)] + public void MetricInstrumentRemoved(string instrumentName, string meterName) { this.WriteEvent(53, instrumentName, meterName); } diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 5162e40f49e..641996ec091 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -27,10 +27,6 @@ public sealed class Metric internal const int DefaultExponentialHistogramMaxScale = 20; - internal const int MetricStatusActive = 0; - internal const int MetricStatusDeactivating = 1; - internal const int MetricStatusInactive = 2; - internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 }; // Short default histogram bounds. Based on the recommended semantic convention values for http.server.request.duration. @@ -57,7 +53,6 @@ public sealed class Metric ("System.Net.Http", "http.client.connection.duration"), }; - internal volatile int Status; private readonly AggregatorStore aggStore; internal Metric( @@ -174,7 +169,6 @@ internal Metric( this.aggStore = new AggregatorStore(instrumentIdentity, aggType, temporality, maxMetricPointsPerMetricStream, emitOverflowAttribute, shouldReclaimUnusedMetricPoints, exemplarFilter); this.Temporality = temporality; - this.Status = MetricStatusActive; } /// @@ -217,7 +211,7 @@ internal Metric( /// internal MetricStreamIdentity InstrumentIdentity { get; private set; } - internal object StatusLock => this.aggStore; + internal bool Active { get; set; } = true; /// /// Get the metric points for the metric stream. diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 5648847c2c8..290bbbcdfed 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -16,6 +16,7 @@ using System.Collections.Concurrent; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; using OpenTelemetry.Internal; @@ -50,10 +51,8 @@ internal AggregationTemporality GetAggregationTemporality(Type instrumentType) var metricStreamIdentity = new MetricStreamIdentity(instrument, metricStreamConfiguration: null); lock (this.instrumentCreationLock) { - if (this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out var existingMetric)) + if (this.TryGetExistingMetric(in metricStreamIdentity, out var existingMetric)) { - ActivateMetricIfRequired(existingMetric); - return existingMetric; } @@ -130,10 +129,8 @@ internal List AddMetricsListWithViews(Instrument instrument, List GetMetricsBatch() int metricCountCurrentBatch = 0; for (int i = 0; i < target; i++) { - var metric = this.metrics![i]; + ref var metric = ref this.metrics![i]; if (metric != null) { - var metricStatus = metric.Status; - if (metricStatus != Metric.MetricStatusActive) - { - if (metricStatus == Metric.MetricStatusInactive) - { - continue; - } - - this.CompleteMetricDeactivation(metric); - } - int metricPointSize = metric.Snapshot(); if (metricPointSize > 0) { this.metricsCurrentBatch![metricCountCurrentBatch++] = metric; } + + if (!metric.Active) + { + this.RemoveMetric(ref metric); + } } } @@ -327,18 +318,24 @@ private Batch GetMetricsBatch() } } - private void CompleteMetricDeactivation(Metric metric) + private void RemoveMetric(ref Metric? metric) { - lock (metric.StatusLock) - { - if (metric.Status != Metric.MetricStatusDeactivating) - { - // Note: If we fall here it means the metric was reactivated - // while we were waiting on the lock. - return; - } + Debug.Assert(metric != null, "metric was null"); - metric.Status = Metric.MetricStatusInactive; - } + // TODO: This logic removes the metric. If the same + // metric is published again we will create a new metric + // for it. If this happens often we will run out of + // storage. Instead, should we keep the metric around + // and set a new start time + reset its data if it comes + // back? + + OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric!.Name, metric.MeterName); + + var result = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _); + Debug.Assert(result, "result was false"); + + // Note: metric is a reference to the array storage so + // this clears the metric out of the array. + metric = null; } } diff --git a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs index a927a4e1cad..66f2c11ee97 100644 --- a/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryMetricsBuilderExtensionsTests.cs @@ -149,21 +149,20 @@ public void ReloadOfMetricsViaIConfigurationWithExportCleanupTest(bool useWithMe meterProvider.ForceFlush(); - AssertSingleMetricWithLongSum( - exportedItems, - expectedValue: temporalityPreference == MetricReaderTemporalityPreference.Delta ? 1 : 2); + AssertSingleMetricWithLongSum(exportedItems); var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); - Assert.Empty(duplicateMetricInstrumentEvents); + // Note: We currently log a duplicate warning anytime a metric is reactivated. + Assert.Single(duplicateMetricInstrumentEvents); var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); Assert.Single(metricInstrumentDeactivatedEvents); - var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); + var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); - Assert.Single(metricInstrumentReactivatedEvents); + Assert.Single(metricInstrumentRemovedEvents); } [Theory] @@ -205,27 +204,49 @@ public void ReloadOfMetricsViaIConfigurationWithoutExportCleanupTest(bool useWit meterProvider.ForceFlush(); - AssertSingleMetricWithLongSum(exportedItems, expectedValue: 2); + // Note: We end up with 2 of the same metric being exported. This is + // because the current behavior when something is deactivated is to + // remove the metric. The next publish creates a new metric. + Assert.Equal(2, exportedItems.Count); + + AssertMetricWithLongSum(exportedItems[0]); + AssertMetricWithLongSum(exportedItems[1]); + + exportedItems.Clear(); + + counter.Add(1); + + meterProvider.ForceFlush(); + + AssertSingleMetricWithLongSum( + exportedItems, + expectedValue: temporalityPreference == MetricReaderTemporalityPreference.Delta ? 1 : 2); var duplicateMetricInstrumentEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 38); - Assert.Empty(duplicateMetricInstrumentEvents); + // Note: We currently log a duplicate warning anytime a metric is reactivated. + Assert.Single(duplicateMetricInstrumentEvents); var metricInstrumentDeactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 52); Assert.Single(metricInstrumentDeactivatedEvents); - var metricInstrumentReactivatedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); + var metricInstrumentRemovedEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 53); - Assert.Single(metricInstrumentReactivatedEvents); + Assert.Single(metricInstrumentRemovedEvents); } private static void AssertSingleMetricWithLongSum(List exportedItems, long expectedValue = 1) { Assert.Single(exportedItems); + AssertMetricWithLongSum(exportedItems[0], expectedValue); + } + + private static void AssertMetricWithLongSum(Metric metric, long expectedValue = 1) + { List metricPoints = new(); - foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) + foreach (ref readonly var mp in metric.GetMetricPoints()) { metricPoints.Add(mp); } diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs index 5c4509307b9..a6374a92565 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs @@ -15,6 +15,7 @@ // using System.Diagnostics.Metrics; +using OpenTelemetry.Internal; using OpenTelemetry.Tests; using Xunit; @@ -49,10 +50,14 @@ public void BuilderTypeDoesNotChangeTest() } [Theory] - [InlineData(false)] - [InlineData(true)] - public void TransientMeterReusesMetricTest(bool withView) + [InlineData(false, true)] + [InlineData(true, true)] + [InlineData(false, false)] + [InlineData(true, false)] + public void TransientMeterExhaustsMetricStorageTest(bool withView, bool forceFlushAfterEachTest) { + using var inMemoryEventListener = new InMemoryEventListener(OpenTelemetrySdkEventSource.Log); + var meterName = Utils.GetCurrentMethodName(); var exportedItems = new List(); @@ -72,8 +77,29 @@ public void TransientMeterReusesMetricTest(bool withView) Assert.NotNull(meterProvider); RunTest(); + + if (forceFlushAfterEachTest) + { + Assert.Single(exportedItems); + } + RunTest(); + if (forceFlushAfterEachTest) + { + Assert.Empty(exportedItems); + } + else + { + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + } + + var metricInstrumentIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 33); + + Assert.Single(metricInstrumentIgnoredEvents); + void RunTest() { exportedItems.Clear(); @@ -85,9 +111,10 @@ void RunTest() meter.Dispose(); - meterProvider.ForceFlush(); - - Assert.Single(exportedItems); + if (forceFlushAfterEachTest) + { + meterProvider.ForceFlush(); + } } } } From 9ab7c582ad098b825f2d46c6b3dbc4c948b4001c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 Nov 2023 11:54:51 -0800 Subject: [PATCH 41/43] CHANGELOG update. --- src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md index f9ab8b5d8ed..6b801fc615c 100644 --- a/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md @@ -6,6 +6,12 @@ version to `8.0.0`. ([#5051](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5051)) +* The `OpenTelemetryBuilder.WithMetrics` method will now register an + `IMetricsListener` named 'OpenTelemetry' into the `IServiceCollection` to + enable metric management via the new `Microsoft.Extensions.Diagnostics` .NET 8 + APIs. + ([#4958](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4958)) + ## 1.7.0-alpha.1 Released 2023-Oct-16 From f2fce4aff338a753ca2b4174e4d28b2ce5362f01 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 Nov 2023 12:16:48 -0800 Subject: [PATCH 42/43] Test tweak. --- test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs index a6374a92565..be2e564a3dd 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs @@ -96,9 +96,13 @@ public void TransientMeterExhaustsMetricStorageTest(bool withView, bool forceFlu Assert.Single(exportedItems); } +#if DEBUG + // Note: This is inside a debug block because when running in CI the + // event source sees events from other tests running in parallel. var metricInstrumentIgnoredEvents = inMemoryEventListener.Events.Where((e) => e.EventId == 33); Assert.Single(metricInstrumentIgnoredEvents); +#endif void RunTest() { From 1c264e820b91926b08230c0b300dbe36def9c9f3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 Nov 2023 15:49:06 -0800 Subject: [PATCH 43/43] Clean up nits. --- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 290bbbcdfed..0fede2ab672 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -256,15 +256,8 @@ private static void DeactivateMetric(Metric metric) } private bool TryGetExistingMetric(in MetricStreamIdentity metricStreamIdentity, [NotNullWhen(true)] out Metric? existingMetric) - { - if (!this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out existingMetric) - || !existingMetric.Active) - { - return false; - } - - return true; - } + => this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out existingMetric) + && existingMetric.Active; private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metricStreamIdentity) {