[OpenTelemetry.Instrumentation.AWS] Fix Memory Leak by Reusing Activi…

…tySources, Meters, and Instruments (#2039)
open-telemetry · Sep 10, 2024 · ce8c21e · ce8c21e
1 parent f0a5a6e
commit ce8c21e
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 23 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* Fix Memory Leak by Reusing ActivitySources, Meters, and Instruments
+  ([#2039](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2039))
 * Added instrumentation support for AWS Bedrock, BedrockRuntime, BedrockAgent, BedrockAgentRuntime.
   ([#1979](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1979))
 

diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSHistogram.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSHistogram.cs
@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Collections.Concurrent;
 using Amazon.Runtime.Telemetry;
 using Amazon.Runtime.Telemetry.Metrics;
 
@@ -9,11 +10,25 @@ namespace OpenTelemetry.Instrumentation.AWS.Implementation.Metrics;
 internal sealed class AWSHistogram<T> : Histogram<T>
     where T : struct
 {
+    private static readonly ConcurrentDictionary<string, System.Diagnostics.Metrics.Histogram<T>> HistogramsDictionary
+        = new ConcurrentDictionary<string, System.Diagnostics.Metrics.Histogram<T>>();
+
     private readonly System.Diagnostics.Metrics.Histogram<T> histogram;
 
-    public AWSHistogram(System.Diagnostics.Metrics.Histogram<T> histogram)
+    public AWSHistogram(
+        System.Diagnostics.Metrics.Meter meter,
+        string name,
+        string? units = null,
+        string? description = null)
     {
-        this.histogram = histogram;
+        if (HistogramsDictionary.TryGetValue(name, out System.Diagnostics.Metrics.Histogram<T>? histogram))
+        {
+            this.histogram = histogram;
+        }
+
+        this.histogram = HistogramsDictionary.GetOrAdd(
+            name,
+            meter.CreateHistogram<T>(name, units, description));
     }
 
     public override void Record(T value, Attributes? attributes = null)

diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMeter.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMeter.cs
@@ -24,8 +24,7 @@ public override UpDownCounter<T> CreateUpDownCounter<T>(
         string? description = null)
         where T : struct
     {
-        var upDownCounter = this.meter.CreateUpDownCounter<T>(name, units, description);
-        return new AWSUpDownCounter<T>(upDownCounter);
+        return new AWSUpDownCounter<T>(this.meter, name, units, description);
     }
 
     public override MonotonicCounter<T> CreateMonotonicCounter<T>(
@@ -34,8 +33,7 @@ public override MonotonicCounter<T> CreateMonotonicCounter<T>(
         string? description = null)
         where T : struct
     {
-        var counter = this.meter.CreateCounter<T>(name, units, description);
-        return new AWSMonotonicCounter<T>(counter);
+        return new AWSMonotonicCounter<T>(this.meter, name, units, description);
     }
 
     public override Histogram<T> CreateHistogram<T>(
@@ -44,8 +42,7 @@ public override Histogram<T> CreateHistogram<T>(
         string? description = null)
         where T : struct
     {
-        var histogram = this.meter.CreateHistogram<T>(name, units, description);
-        return new AWSHistogram<T>(histogram);
+        return new AWSHistogram<T>(this.meter, name, units, description);
     }
 
     protected override void Dispose(bool disposing)

diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMeterProvider.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMeterProvider.cs
@@ -1,13 +1,16 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Collections.Concurrent;
 using Amazon.Runtime.Telemetry;
 using Amazon.Runtime.Telemetry.Metrics;
 
 namespace OpenTelemetry.Instrumentation.AWS.Implementation.Metrics;
 
 internal sealed class AWSMeterProvider : MeterProvider
 {
+    private static readonly ConcurrentDictionary<string, AWSMeter> MetersDictionary = new ConcurrentDictionary<string, AWSMeter>();
+
     public override Meter GetMeter(string scope, Attributes? attributes = null)
     {
         // Passing attributes to the Meter is currently not possible due to version limitations
@@ -17,7 +20,15 @@ public override Meter GetMeter(string scope, Attributes? attributes = null)
         // update OpenTelemetry core component version(s) to `1.9.0` and allow passing tags to
         // the meter constructor.
 
-        var meter = new System.Diagnostics.Metrics.Meter(scope);
-        return new AWSMeter(meter);
+        if (MetersDictionary.TryGetValue(scope, out AWSMeter? meter))
+        {
+            return meter;
+        }
+
+        var awsMeter = MetersDictionary.GetOrAdd(
+            scope,
+            new AWSMeter(new System.Diagnostics.Metrics.Meter(scope)));
+
+        return awsMeter;
     }
 }
diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMonotonicCounter.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSMonotonicCounter.cs
@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Collections.Concurrent;
 using Amazon.Runtime.Telemetry;
 using Amazon.Runtime.Telemetry.Metrics;
 
@@ -9,23 +10,37 @@ namespace OpenTelemetry.Instrumentation.AWS.Implementation.Metrics;
 internal sealed class AWSMonotonicCounter<T> : MonotonicCounter<T>
     where T : struct
 {
-    private readonly System.Diagnostics.Metrics.Counter<T> counter;
+    private static readonly ConcurrentDictionary<string, System.Diagnostics.Metrics.Counter<T>> MonotonicCountersDictionary
+        = new ConcurrentDictionary<string, System.Diagnostics.Metrics.Counter<T>>();
 
-    public AWSMonotonicCounter(System.Diagnostics.Metrics.Counter<T> counter)
+    private readonly System.Diagnostics.Metrics.Counter<T> monotonicCounter;
+
+    public AWSMonotonicCounter(
+        System.Diagnostics.Metrics.Meter meter,
+        string name,
+        string? units = null,
+        string? description = null)
     {
-        this.counter = counter;
+        if (MonotonicCountersDictionary.TryGetValue(name, out System.Diagnostics.Metrics.Counter<T>? monotonicCounter))
+        {
+            this.monotonicCounter = monotonicCounter;
+        }
+
+        this.monotonicCounter = MonotonicCountersDictionary.GetOrAdd(
+            name,
+            meter.CreateCounter<T>(name, units, description));
     }
 
     public override void Add(T value, Attributes? attributes = null)
     {
         if (attributes != null)
         {
             // TODO: remove ToArray call and use when AttributesAsSpan expected to be added at AWS SDK v4.
-            this.counter.Add(value, attributes.AllAttributes.ToArray());
+            this.monotonicCounter.Add(value, attributes.AllAttributes.ToArray());
         }
         else
         {
-            this.counter.Add(value);
+            this.monotonicCounter.Add(value);
         }
     }
 }
diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSUpDownCounter.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Metrics/AWSUpDownCounter.cs
@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Collections.Concurrent;
 using Amazon.Runtime.Telemetry;
 using Amazon.Runtime.Telemetry.Metrics;
 
@@ -9,11 +10,25 @@ namespace OpenTelemetry.Instrumentation.AWS.Implementation.Metrics;
 internal sealed class AWSUpDownCounter<T> : UpDownCounter<T>
     where T : struct
 {
+    private static readonly ConcurrentDictionary<string, System.Diagnostics.Metrics.UpDownCounter<T>> UpDownCountersDictionary
+        = new ConcurrentDictionary<string, System.Diagnostics.Metrics.UpDownCounter<T>>();
+
     private readonly System.Diagnostics.Metrics.UpDownCounter<T> upDownCounter;
 
-    public AWSUpDownCounter(System.Diagnostics.Metrics.UpDownCounter<T> upDownCounter)
+    public AWSUpDownCounter(
+        System.Diagnostics.Metrics.Meter meter,
+        string name,
+        string? units = null,
+        string? description = null)
     {
-        this.upDownCounter = upDownCounter;
+        if (UpDownCountersDictionary.TryGetValue(name, out System.Diagnostics.Metrics.UpDownCounter<T>? upDownCounter))
+        {
+            this.upDownCounter = upDownCounter;
+        }
+
+        this.upDownCounter = UpDownCountersDictionary.GetOrAdd(
+            name,
+            meter.CreateUpDownCounter<T>(name, units, description));
     }
 
     public override void Add(T value, Attributes? attributes = null)

diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Tracing/AWSTracer.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Tracing/AWSTracer.cs
@@ -14,10 +14,10 @@ internal sealed class AWSTracer : Tracer
     /// <summary>
     /// Initializes a new instance of the <see cref="AWSTracer"/> class.
     /// </summary>
-    /// <param name="scope">The name of the instrumentation scope that uniquely identifies the tracer.</param>
-    public AWSTracer(string scope)
+    /// <param name="activitySource">The ActivitySource used for creating and tracking the activities.</param>
+    public AWSTracer(ActivitySource activitySource)
     {
-        this.activitySource = new ActivitySource(scope);
+        this.activitySource = activitySource ?? throw new ArgumentNullException(nameof(activitySource));
     }
 
     public override TraceSpan CreateSpan(

diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/Tracing/AWSTracerProvider.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/Tracing/AWSTracerProvider.cs
@@ -1,14 +1,27 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Collections.Concurrent;
+using System.Diagnostics;
 using Amazon.Runtime.Telemetry.Tracing;
 
 namespace OpenTelemetry.Instrumentation.AWS.Implementation.Tracing;
 
 internal sealed class AWSTracerProvider : TracerProvider
 {
+    private static readonly ConcurrentDictionary<string, AWSTracer> TracersDictionary = new ConcurrentDictionary<string, AWSTracer>();
+
     public override Tracer GetTracer(string scope)
     {
-        return new AWSTracer(scope);
+        if (TracersDictionary.TryGetValue(scope, out AWSTracer? awsTracer))
+        {
+            return awsTracer;
+        }
+
+        awsTracer = TracersDictionary.GetOrAdd(
+            scope,
+            new AWSTracer(new ActivitySource(scope)));
+
+        return awsTracer;
     }
 }
diff --git a/test/OpenTelemetry.Instrumentation.AWS.Tests/TestAWSClientMetricsInstrumentation.cs b/test/OpenTelemetry.Instrumentation.AWS.Tests/TestAWSClientMetricsInstrumentation.cs
@@ -186,15 +186,16 @@ public void TestAWSUpDownCounterIsntCalledAfterMeterDispose()
 
         var countAmount = 7;
         var counterName = "TestCounter";
-        var meter = AWSConfigs.TelemetryProvider.MeterProvider.GetMeter($"{TelemetryConstants.TelemetryScopePrefix}.TestDisposedMeter");
+        var meterName = $"{TelemetryConstants.TelemetryScopePrefix}.TestDisposedMeter";
+        var meter = AWSConfigs.TelemetryProvider.MeterProvider.GetMeter(meterName);
         var counter = meter.CreateUpDownCounter<long>(counterName);
 
         meter.Dispose();
         counter.Add(countAmount);
 
         meterProvider.ForceFlush();
 
-        var counterMetric = exportedItems.FirstOrDefault(i => i.Name == counterName);
+        var counterMetric = exportedItems.FirstOrDefault(i => i.MeterName == meterName && i.Name == counterName);
         Assert.Null(counterMetric);
     }