From a1222b414fdbc702b8ecd2a52ab82492a06019d3 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Mon, 6 Jan 2025 11:46:46 +0200 Subject: [PATCH 1/5] Small metrics handling optimizations/cleanup --- ...njectionLoggerProviderBuilderExtensions.cs | 2 +- ...InjectionMeterProviderBuilderExtensions.cs | 2 +- ...njectionTracerProviderBuilderExtensions.cs | 2 +- .../Internal/OpenTelemetryApiEventSource.cs | 2 +- src/OpenTelemetry.Api/Trace/TracerProvider.cs | 4 +-- .../PersistentStorage/DirectorySizeTracker.cs | 4 +-- ...ersistentStorageAbstractionsEventSource.cs | 2 +- .../PersistentStorageEventSource.cs | 2 +- .../HostingExtensionsEventSource.cs | 2 +- .../Internal/InterlockedHelper.cs | 9 +++---- .../Internal/OpenTelemetrySdkEventSource.cs | 2 +- src/OpenTelemetry/Logs/LogRecord.cs | 6 ++--- .../Metrics/CircularBufferBuckets.cs | 9 ++++--- .../Metrics/Exemplar/Exemplar.cs | 4 +-- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 4 +-- .../Metrics/MetricPoint/MetricPoint.cs | 20 +++++++------- .../MetricPointOptionalComponents.cs | 2 +- .../MetricPoint/MetricPointsAccessor.cs | 10 +++---- .../Metrics/Reader/MetricReader.cs | 2 +- .../ExplicitBucketHistogramConfiguration.cs | 16 ++--------- .../Metrics/View/MetricStreamConfiguration.cs | 27 ++----------------- .../OpenTelemetrySdkExtensions.cs | 4 +-- 22 files changed, 48 insertions(+), 89 deletions(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggerProviderBuilderExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggerProviderBuilderExtensions.cs index 4a96f1b0818..51a1af5c5c5 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggerProviderBuilderExtensions.cs @@ -36,7 +36,7 @@ public static LoggerProviderBuilder AddInstrumentation< loggerProviderBuilder.ConfigureBuilder((sp, builder) => { - builder.AddInstrumentation(() => sp.GetRequiredService()); + builder.AddInstrumentation(sp.GetRequiredService); }); return loggerProviderBuilder; diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMeterProviderBuilderExtensions.cs index fea2275e9a3..ff52ee37cb3 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMeterProviderBuilderExtensions.cs @@ -36,7 +36,7 @@ public static MeterProviderBuilder AddInstrumentation< meterProviderBuilder.ConfigureBuilder((sp, builder) => { - builder.AddInstrumentation(() => sp.GetRequiredService()); + builder.AddInstrumentation(sp.GetRequiredService); }); return meterProviderBuilder; diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs index 2e6ab37628d..939fd34b366 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs @@ -36,7 +36,7 @@ public static TracerProviderBuilder AddInstrumentation< tracerProviderBuilder.ConfigureBuilder((sp, builder) => { - builder.AddInstrumentation(() => sp.GetRequiredService()); + builder.AddInstrumentation(sp.GetRequiredService); }); return tracerProviderBuilder; diff --git a/src/OpenTelemetry.Api/Internal/OpenTelemetryApiEventSource.cs b/src/OpenTelemetry.Api/Internal/OpenTelemetryApiEventSource.cs index 5ac56d362f3..c2e9cf9dfb2 100644 --- a/src/OpenTelemetry.Api/Internal/OpenTelemetryApiEventSource.cs +++ b/src/OpenTelemetry.Api/Internal/OpenTelemetryApiEventSource.cs @@ -12,7 +12,7 @@ namespace OpenTelemetry.Internal; [EventSource(Name = "OpenTelemetry-Api")] internal sealed class OpenTelemetryApiEventSource : EventSource { - public static OpenTelemetryApiEventSource Log = new(); + public static readonly OpenTelemetryApiEventSource Log = new(); [NonEvent] public void ActivityContextExtractException(string format, Exception ex) diff --git a/src/OpenTelemetry.Api/Trace/TracerProvider.cs b/src/OpenTelemetry.Api/Trace/TracerProvider.cs index 21197d0abdb..22413adc1c9 100644 --- a/src/OpenTelemetry.Api/Trace/TracerProvider.cs +++ b/src/OpenTelemetry.Api/Trace/TracerProvider.cs @@ -78,7 +78,7 @@ protected override void Dispose(bool disposing) { if (disposing) { - var tracers = Interlocked.CompareExchange(ref this.Tracers, null, this.Tracers); + var tracers = Interlocked.Exchange(ref this.Tracers, null); if (tracers != null) { lock (tracers) @@ -90,8 +90,6 @@ protected override void Dispose(bool disposing) tracer.ActivitySource = null; activitySource?.Dispose(); } - - tracers.Clear(); } } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs index 2bbc352817d..26c607abb7f 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs @@ -36,14 +36,14 @@ public DirectorySizeTracker(long maxSizeInBytes, string path) /// True if space is available else false. public bool IsSpaceAvailable(out long currentSizeInBytes) { - currentSizeInBytes = Interlocked.Read(ref this.directoryCurrentSizeInBytes); + currentSizeInBytes = Volatile.Read(ref this.directoryCurrentSizeInBytes); return currentSizeInBytes < this.maxSizeInBytes; } public void RecountCurrentSize() { var size = CalculateFolderSize(this.path); - Interlocked.Exchange(ref this.directoryCurrentSizeInBytes, size); + Volatile.Write(ref this.directoryCurrentSizeInBytes, size); } internal static long CalculateFolderSize(string path) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageAbstractionsEventSource.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageAbstractionsEventSource.cs index 0c8562ce319..b7494427763 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageAbstractionsEventSource.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageAbstractionsEventSource.cs @@ -9,7 +9,7 @@ namespace OpenTelemetry.PersistentStorage.Abstractions; [EventSource(Name = EventSourceName)] internal sealed class PersistentStorageAbstractionsEventSource : EventSource { - public static PersistentStorageAbstractionsEventSource Log = new PersistentStorageAbstractionsEventSource(); + public static readonly PersistentStorageAbstractionsEventSource Log = new PersistentStorageAbstractionsEventSource(); #if BUILDING_INTERNAL_PERSISTENT_STORAGE private const string EventSourceName = "OpenTelemetry-PersistentStorage-Abstractions-Otlp"; #else diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageEventSource.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageEventSource.cs index 69cdf419cac..554735b2501 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageEventSource.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/PersistentStorageEventSource.cs @@ -9,7 +9,7 @@ namespace OpenTelemetry.PersistentStorage.FileSystem; [EventSource(Name = EventSourceName)] internal sealed class PersistentStorageEventSource : EventSource { - public static PersistentStorageEventSource Log = new PersistentStorageEventSource(); + public static readonly PersistentStorageEventSource Log = new PersistentStorageEventSource(); #if BUILDING_INTERNAL_PERSISTENT_STORAGE private const string EventSourceName = "OpenTelemetry-PersistentStorage-FileSystem-Otlp"; #else diff --git a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs index 1d31393b0fa..af4bb57a7f2 100644 --- a/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions.Hosting/Implementation/HostingExtensionsEventSource.cs @@ -11,7 +11,7 @@ namespace OpenTelemetry.Extensions.Hosting.Implementation; [EventSource(Name = "OpenTelemetry-Extensions-Hosting")] internal sealed class HostingExtensionsEventSource : EventSource { - public static HostingExtensionsEventSource Log = new(); + public static readonly HostingExtensionsEventSource Log = new(); [Event(1, Message = "OpenTelemetry TracerProvider was not found in application services. Tracing will remain disabled.", Level = EventLevel.Warning)] public void TracerProviderNotRegistered() diff --git a/src/OpenTelemetry/Internal/InterlockedHelper.cs b/src/OpenTelemetry/Internal/InterlockedHelper.cs index 98639d234a4..2688e228ab8 100644 --- a/src/OpenTelemetry/Internal/InterlockedHelper.cs +++ b/src/OpenTelemetry/Internal/InterlockedHelper.cs @@ -18,29 +18,28 @@ public static void Add(ref double location, double value) var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); if (returnedValue != currentValue) { - AddRare(ref location, value, returnedValue); + AddRare(ref location, value); } } [MethodImpl(MethodImplOptions.AggressiveInlining)] public static double Read(ref double location) - => Interlocked.CompareExchange(ref location, double.NaN, double.NaN); + => Interlocked.CompareExchange(ref location, 0, 0); [MethodImpl(MethodImplOptions.NoInlining)] - private static void AddRare(ref double location, double value, double currentValue) + private static void AddRare(ref double location, double value) { var sw = default(SpinWait); while (true) { sw.SpinOnce(); + double currentValue = Volatile.Read(ref location); var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); if (returnedValue == currentValue) { break; } - - currentValue = returnedValue; } } } diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 88312e3a140..b3980c2a624 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -16,7 +16,7 @@ namespace OpenTelemetry.Internal; [EventSource(Name = "OpenTelemetry-Sdk")] internal sealed class OpenTelemetrySdkEventSource : EventSource, IConfigurationExtensionsLogger { - public static OpenTelemetrySdkEventSource Log = new(); + public static readonly OpenTelemetrySdkEventSource Log = new(); #if DEBUG public static OpenTelemetryEventListener Listener = new(); #endif diff --git a/src/OpenTelemetry/Logs/LogRecord.cs b/src/OpenTelemetry/Logs/LogRecord.cs index 8009687d551..824e95bf0c1 100644 --- a/src/OpenTelemetry/Logs/LogRecord.cs +++ b/src/OpenTelemetry/Logs/LogRecord.cs @@ -429,19 +429,17 @@ public void ForEachScope(Action callback, TState { Guard.ThrowIfNull(callback); - var forEachScopeState = new ScopeForEachState(callback, state); - var bufferedScopes = this.ILoggerData.BufferedScopes; if (bufferedScopes != null) { foreach (object? scope in bufferedScopes) { - ScopeForEachState.ForEachScope(scope, forEachScopeState); + callback(new(scope), state); } } else { - this.ILoggerData.ScopeProvider?.ForEachScope(ScopeForEachState.ForEachScope, forEachScopeState); + this.ILoggerData.ScopeProvider?.ForEachScope(ScopeForEachState.ForEachScope, new(callback, state)); } } diff --git a/src/OpenTelemetry/Metrics/CircularBufferBuckets.cs b/src/OpenTelemetry/Metrics/CircularBufferBuckets.cs index a7205a41eab..2ccede024b1 100644 --- a/src/OpenTelemetry/Metrics/CircularBufferBuckets.cs +++ b/src/OpenTelemetry/Metrics/CircularBufferBuckets.cs @@ -263,10 +263,11 @@ internal void Reset() { if (this.trait != null) { - for (var i = 0; i < this.trait.Length; ++i) - { - this.trait[i] = 0; - } +#if NET + Array.Clear(this.trait); +#else + Array.Clear(this.trait, 0, this.trait.Length); +#endif } } diff --git a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs index d1801afdd39..6f32e43d1d3 100644 --- a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs +++ b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs @@ -135,7 +135,7 @@ internal void Update(in ExemplarMeasurement measurement) this.StoreRawTags(measurement.Tags); } - Interlocked.Exchange(ref this.isCriticalSectionOccupied, 0); + Volatile.Write(ref this.isCriticalSectionOccupied, 0); } internal void Reset() @@ -168,7 +168,7 @@ internal void Collect(ref Exemplar destination, bool reset) destination.Reset(); } - Interlocked.Exchange(ref this.isCriticalSectionOccupied, 0); + Volatile.Write(ref this.isCriticalSectionOccupied, 0); } internal readonly void Copy(ref Exemplar destination) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index a07187de4ef..707c20b91c8 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -143,12 +143,12 @@ internal MeterProviderSdk( } // Setup Listener - if (state.MeterSources.Any(s => WildcardHelper.ContainsWildcard(s))) + if (state.MeterSources.Exists(WildcardHelper.ContainsWildcard)) { var regex = WildcardHelper.GetWildcardRegex(state.MeterSources); this.shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); } - else if (state.MeterSources.Any()) + else if (state.MeterSources.Count > 0) { var meterSourcesToSubscribe = new HashSet(state.MeterSources, StringComparer.OrdinalIgnoreCase); this.shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); diff --git a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs index 8c0ad5951fc..eea5c83bfa6 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs @@ -394,7 +394,7 @@ internal void Update(long number) case AggregationType.LongSumIncomingCumulative: case AggregationType.LongGauge: { - Interlocked.Exchange(ref this.runningValue.AsLong, number); + Volatile.Write(ref this.runningValue.AsLong, number); break; } @@ -451,7 +451,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan public double[]? Boundaries { - get - { - if (this.CopiedBoundaries != null) - { - double[] copy = new double[this.CopiedBoundaries.Length]; - this.CopiedBoundaries.AsSpan().CopyTo(copy); - return copy; - } - - return null; - } + get => this.CopiedBoundaries != null ? this.CopiedBoundaries.AsSpan().ToArray() : null; set { @@ -46,9 +36,7 @@ public double[]? Boundaries throw new ArgumentException($"Histogram boundaries are invalid. Histogram boundaries must be in ascending order with distinct values.", nameof(value)); } - double[] copy = new double[value.Length]; - value.AsSpan().CopyTo(copy); - this.CopiedBoundaries = copy; + this.CopiedBoundaries = value.AsSpan().ToArray(); } else { diff --git a/src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs index cf5e06661a0..5b80151a036 100644 --- a/src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs +++ b/src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs @@ -71,31 +71,8 @@ public string? Name /// public string[]? TagKeys { - get - { - if (this.CopiedTagKeys != null) - { - string[] copy = new string[this.CopiedTagKeys.Length]; - this.CopiedTagKeys.AsSpan().CopyTo(copy); - return copy; - } - - return null; - } - - set - { - if (value != null) - { - string[] copy = new string[value.Length]; - value.AsSpan().CopyTo(copy); - this.CopiedTagKeys = copy; - } - else - { - this.CopiedTagKeys = null; - } - } + get => this.CopiedTagKeys != null ? this.CopiedTagKeys.AsSpan().ToArray() : null; + set => this.CopiedTagKeys = value != null ? value.AsSpan().ToArray() : null; } /// diff --git a/src/OpenTelemetry/OpenTelemetrySdkExtensions.cs b/src/OpenTelemetry/OpenTelemetrySdkExtensions.cs index f1f01e0f20a..fc1782c864a 100644 --- a/src/OpenTelemetry/OpenTelemetrySdkExtensions.cs +++ b/src/OpenTelemetry/OpenTelemetrySdkExtensions.cs @@ -12,8 +12,6 @@ namespace OpenTelemetry; /// public static class OpenTelemetrySdkExtensions { - private static readonly NullLoggerFactory NoopLoggerFactory = new(); - /// /// Gets the contained in an instance. @@ -31,6 +29,6 @@ public static ILoggerFactory GetLoggerFactory(this OpenTelemetrySdk sdk) Guard.ThrowIfNull(sdk); return (ILoggerFactory?)sdk.Services.GetService(typeof(ILoggerFactory)) - ?? NoopLoggerFactory; + ?? NullLoggerFactory.Instance; } } From 421234598df603a808213426faaa6ff502fed780 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Tue, 7 Jan 2025 10:35:49 +0200 Subject: [PATCH 2/5] Remove spin wait from InterlockedHelper.Add --- .../Internal/InterlockedHelper.cs | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/OpenTelemetry/Internal/InterlockedHelper.cs b/src/OpenTelemetry/Internal/InterlockedHelper.cs index 2688e228ab8..596c7eb89c8 100644 --- a/src/OpenTelemetry/Internal/InterlockedHelper.cs +++ b/src/OpenTelemetry/Internal/InterlockedHelper.cs @@ -10,36 +10,20 @@ internal static class InterlockedHelper [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Add(ref double location, double value) { - // Note: Not calling InterlockedHelper.Read here on purpose because it - // is too expensive for fast/happy-path. If the first attempt fails - // we'll end up in an Interlocked.CompareExchange loop anyway. double currentValue = Volatile.Read(ref location); - - var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); - if (returnedValue != currentValue) - { - AddRare(ref location, value); - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static double Read(ref double location) - => Interlocked.CompareExchange(ref location, 0, 0); - - [MethodImpl(MethodImplOptions.NoInlining)] - private static void AddRare(ref double location, double value) - { - var sw = default(SpinWait); while (true) { - sw.SpinOnce(); - - double currentValue = Volatile.Read(ref location); var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); if (returnedValue == currentValue) { break; } + + currentValue = returnedValue; } } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static double Read(ref double location) + => Interlocked.CompareExchange(ref location, 0, 0); } From 3593cee083f1e994cfda99ba9873581d2e3fb6f5 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Wed, 8 Jan 2025 11:48:28 +0200 Subject: [PATCH 3/5] Remove Interlocked -> Volatile changes --- .../PersistentStorage/DirectorySizeTracker.cs | 4 +-- .../Internal/InterlockedHelper.cs | 25 ++++++++++++++++--- .../Metrics/Exemplar/Exemplar.cs | 4 +-- .../Metrics/MetricPoint/MetricPoint.cs | 20 +++++++-------- .../MetricPointOptionalComponents.cs | 2 +- 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs index 26c607abb7f..2bbc352817d 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/PersistentStorage/DirectorySizeTracker.cs @@ -36,14 +36,14 @@ public DirectorySizeTracker(long maxSizeInBytes, string path) /// True if space is available else false. public bool IsSpaceAvailable(out long currentSizeInBytes) { - currentSizeInBytes = Volatile.Read(ref this.directoryCurrentSizeInBytes); + currentSizeInBytes = Interlocked.Read(ref this.directoryCurrentSizeInBytes); return currentSizeInBytes < this.maxSizeInBytes; } public void RecountCurrentSize() { var size = CalculateFolderSize(this.path); - Volatile.Write(ref this.directoryCurrentSizeInBytes, size); + Interlocked.Exchange(ref this.directoryCurrentSizeInBytes, size); } internal static long CalculateFolderSize(string path) diff --git a/src/OpenTelemetry/Internal/InterlockedHelper.cs b/src/OpenTelemetry/Internal/InterlockedHelper.cs index 596c7eb89c8..98639d234a4 100644 --- a/src/OpenTelemetry/Internal/InterlockedHelper.cs +++ b/src/OpenTelemetry/Internal/InterlockedHelper.cs @@ -10,9 +10,30 @@ internal static class InterlockedHelper [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Add(ref double location, double value) { + // Note: Not calling InterlockedHelper.Read here on purpose because it + // is too expensive for fast/happy-path. If the first attempt fails + // we'll end up in an Interlocked.CompareExchange loop anyway. double currentValue = Volatile.Read(ref location); + + var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); + if (returnedValue != currentValue) + { + AddRare(ref location, value, returnedValue); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static double Read(ref double location) + => Interlocked.CompareExchange(ref location, double.NaN, double.NaN); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void AddRare(ref double location, double value, double currentValue) + { + var sw = default(SpinWait); while (true) { + sw.SpinOnce(); + var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); if (returnedValue == currentValue) { @@ -22,8 +43,4 @@ public static void Add(ref double location, double value) currentValue = returnedValue; } } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static double Read(ref double location) - => Interlocked.CompareExchange(ref location, 0, 0); } diff --git a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs index 6f32e43d1d3..d1801afdd39 100644 --- a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs +++ b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs @@ -135,7 +135,7 @@ internal void Update(in ExemplarMeasurement measurement) this.StoreRawTags(measurement.Tags); } - Volatile.Write(ref this.isCriticalSectionOccupied, 0); + Interlocked.Exchange(ref this.isCriticalSectionOccupied, 0); } internal void Reset() @@ -168,7 +168,7 @@ internal void Collect(ref Exemplar destination, bool reset) destination.Reset(); } - Volatile.Write(ref this.isCriticalSectionOccupied, 0); + Interlocked.Exchange(ref this.isCriticalSectionOccupied, 0); } internal readonly void Copy(ref Exemplar destination) diff --git a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs index eea5c83bfa6..8c0ad5951fc 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs @@ -394,7 +394,7 @@ internal void Update(long number) case AggregationType.LongSumIncomingCumulative: case AggregationType.LongGauge: { - Volatile.Write(ref this.runningValue.AsLong, number); + Interlocked.Exchange(ref this.runningValue.AsLong, number); break; } @@ -451,7 +451,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan Date: Wed, 8 Jan 2025 22:37:11 +0200 Subject: [PATCH 4/5] Adjust test --- test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs index 642795202d0..02fe1f3d746 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs @@ -404,7 +404,10 @@ static void InnerTest() getTracerThread.Join(); } - Assert.Empty(tracers); + foreach (var kvp in tracers) + { + Assert.Null(kvp.Value.ActivitySource); + } } } From c48b0ec19b9ddf1b9b9e6156341373ba8d2f0d18 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Tue, 4 Feb 2025 02:17:24 +0200 Subject: [PATCH 5/5] Address PR feedback --- src/OpenTelemetry.Api/Trace/TracerProvider.cs | 2 ++ test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs | 5 +---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Api/Trace/TracerProvider.cs b/src/OpenTelemetry.Api/Trace/TracerProvider.cs index 22413adc1c9..95b99be1ad7 100644 --- a/src/OpenTelemetry.Api/Trace/TracerProvider.cs +++ b/src/OpenTelemetry.Api/Trace/TracerProvider.cs @@ -90,6 +90,8 @@ protected override void Dispose(bool disposing) tracer.ActivitySource = null; activitySource?.Dispose(); } + + tracers.Clear(); } } } diff --git a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs index 02fe1f3d746..642795202d0 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs @@ -404,10 +404,7 @@ static void InnerTest() getTracerThread.Join(); } - foreach (var kvp in tracers) - { - Assert.Null(kvp.Value.ActivitySource); - } + Assert.Empty(tracers); } }