[api] Fix memory leaks in TracerProvider.GetTracer API (#4906)
This commit is contained in:
parent
3e885c77f2
commit
d4d5122316
|
|
@ -0,0 +1 @@
|
|||
override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void
|
||||
|
|
@ -0,0 +1 @@
|
|||
override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void
|
||||
|
|
@ -0,0 +1 @@
|
|||
override OpenTelemetry.Trace.TracerProvider.Dispose(bool disposing) -> void
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -28,9 +28,9 @@ namespace OpenTelemetry.Trace;
|
|||
/// <remarks>Tracer is a wrapper around <see cref="System.Diagnostics.ActivitySource"/> class.</remarks>
|
||||
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 @@ public class Tracer
|
|||
IEnumerable<Link>? links = null,
|
||||
DateTimeOffset startTime = default)
|
||||
{
|
||||
if (!this.ActivitySource.HasListeners())
|
||||
var activitySource = this.ActivitySource;
|
||||
|
||||
if (!(activitySource?.HasListeners() ?? false))
|
||||
{
|
||||
return TelemetrySpan.NoopInstance;
|
||||
}
|
||||
|
|
@ -230,7 +232,7 @@ public class Tracer
|
|||
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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;
|
|||
/// </summary>
|
||||
public class TracerProvider : BaseProvider
|
||||
{
|
||||
private ConcurrentDictionary<TracerKey, Tracer>? tracers = new();
|
||||
|
||||
/// <summary>
|
||||
/// Initializes a new instance of the <see cref="TracerProvider"/> class.
|
||||
/// </summary>
|
||||
|
|
@ -43,6 +48,81 @@ public class TracerProvider : BaseProvider
|
|||
/// <param name="name">Name identifying the instrumentation library.</param>
|
||||
/// <param name="version">Version of the instrumentation library.</param>
|
||||
/// <returns>Tracer instance.</returns>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <inheritdoc/>
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -283,6 +283,32 @@ public class TracerTest : IDisposable
|
|||
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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue