diff --git a/src/OpenTelemetry.Api/.publicApi/Stable/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api/.publicApi/Stable/net462/PublicAPI.Unshipped.txt index e69de29bb2d..b3db39f4c47 100644 --- a/src/OpenTelemetry.Api/.publicApi/Stable/net462/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/Stable/net462/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void diff --git a/src/OpenTelemetry.Api/.publicApi/Stable/net6.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api/.publicApi/Stable/net6.0/PublicAPI.Unshipped.txt index e69de29bb2d..b3db39f4c47 100644 --- a/src/OpenTelemetry.Api/.publicApi/Stable/net6.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/Stable/net6.0/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void diff --git a/src/OpenTelemetry.Api/.publicApi/Stable/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api/.publicApi/Stable/netstandard2.0/PublicAPI.Unshipped.txt index e69de29bb2d..b3db39f4c47 100644 --- a/src/OpenTelemetry.Api/.publicApi/Stable/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/Stable/netstandard2.0/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 619ad543626..36b39ea19bf 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -6,6 +6,10 @@ trace was running (`Activity.Current != null`). ([#4890](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4890)) +* Added a `Tracer` cache inside of `TracerProvider` to prevent repeated calls to + `GetTracer` from leaking memory. + ([#4906](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4906)) + ## 1.6.0 Released 2023-Sep-05 diff --git a/src/OpenTelemetry.Api/Trace/Tracer.cs b/src/OpenTelemetry.Api/Trace/Tracer.cs index af626aafc8b..91adfb8d38b 100644 --- a/src/OpenTelemetry.Api/Trace/Tracer.cs +++ b/src/OpenTelemetry.Api/Trace/Tracer.cs @@ -28,9 +28,9 @@ namespace OpenTelemetry.Trace; /// Tracer is a wrapper around class. public class Tracer { - internal readonly ActivitySource ActivitySource; + internal ActivitySource? ActivitySource; - internal Tracer(ActivitySource activitySource) + internal Tracer(ActivitySource? activitySource) { this.ActivitySource = activitySource; } @@ -213,7 +213,9 @@ private TelemetrySpan StartSpanHelper( IEnumerable? links = null, DateTimeOffset startTime = default) { - if (!this.ActivitySource.HasListeners()) + var activitySource = this.ActivitySource; + + if (!(activitySource?.HasListeners() ?? false)) { return TelemetrySpan.NoopInstance; } @@ -230,7 +232,7 @@ private TelemetrySpan StartSpanHelper( try { - var activity = this.ActivitySource.StartActivity(name, activityKind, parentContext.ActivityContext, initialAttributes?.Attributes ?? null, activityLinks, startTime); + var activity = activitySource.StartActivity(name, activityKind, parentContext.ActivityContext, initialAttributes?.Attributes ?? null, activityLinks, startTime); return activity == null ? TelemetrySpan.NoopInstance : new TelemetrySpan(activity); diff --git a/src/OpenTelemetry.Api/Trace/TracerProvider.cs b/src/OpenTelemetry.Api/Trace/TracerProvider.cs index c7611bff428..50d033cae51 100644 --- a/src/OpenTelemetry.Api/Trace/TracerProvider.cs +++ b/src/OpenTelemetry.Api/Trace/TracerProvider.cs @@ -16,7 +16,10 @@ #nullable enable -using System.Diagnostics; +using System.Collections.Concurrent; +#if NET6_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif namespace OpenTelemetry.Trace; @@ -25,6 +28,8 @@ namespace OpenTelemetry.Trace; /// public class TracerProvider : BaseProvider { + private ConcurrentDictionary? tracers = new(); + /// /// Initializes a new instance of the class. /// @@ -43,6 +48,81 @@ protected TracerProvider() /// Name identifying the instrumentation library. /// Version of the instrumentation library. /// Tracer instance. - public Tracer GetTracer(string name, string? version = null) - => new(new ActivitySource(name ?? string.Empty, version)); + public Tracer GetTracer( +#if NET6_0_OR_GREATER + [AllowNull] +#endif + string name, + string? version = null) + { + var tracers = this.tracers; + if (tracers == null) + { + // Note: Returns a no-op Tracer once dispose has been called. + return new(activitySource: null); + } + + var key = new TracerKey(name, version); + + if (!tracers.TryGetValue(key, out var tracer)) + { + lock (tracers) + { + if (this.tracers == null) + { + // Note: We check here for a race with Dispose and return a + // no-op Tracer in that case. + return new(activitySource: null); + } + + tracer = new(new(key.Name, key.Version)); +#if DEBUG + bool result = tracers.TryAdd(key, tracer); + System.Diagnostics.Debug.Assert(result, "Write into tracers cache failed"); +#else + tracers.TryAdd(key, tracer); +#endif + } + } + + return tracer; + } + + /// + protected override void Dispose(bool disposing) + { + if (disposing) + { + var tracers = Interlocked.CompareExchange(ref this.tracers, null, this.tracers); + if (tracers != null) + { + lock (tracers) + { + foreach (var kvp in tracers) + { + var tracer = kvp.Value; + var activitySource = tracer.ActivitySource; + tracer.ActivitySource = null; + activitySource?.Dispose(); + } + + tracers.Clear(); + } + } + } + + base.Dispose(disposing); + } + + private readonly record struct TracerKey + { + public readonly string Name; + public readonly string? Version; + + public TracerKey(string? name, string? version) + { + this.Name = name ?? string.Empty; + this.Version = version; + } + } } diff --git a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs index 22657b8f280..1e2e1f4579b 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs @@ -283,6 +283,32 @@ public void CreateSpan_NotSampled() Assert.False(span.IsRecording); } + [Fact] + public void TracerBecomesNoopWhenParentProviderIsDisposedTest() + { + TracerProvider provider = null; + Tracer tracer = null; + + using (var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddSource("mytracer") + .Build()) + { + provider = tracerProvider; + tracer = tracerProvider.GetTracer("mytracer"); + + var span1 = tracer.StartSpan("foo"); + Assert.True(span1.IsRecording); + } + + var span2 = tracer.StartSpan("foo"); + Assert.False(span2.IsRecording); + + var tracer2 = provider.GetTracer("mytracer"); + + var span3 = tracer2.StartSpan("foo"); + Assert.False(span3.IsRecording); + } + public void Dispose() { Activity.Current = null;