[OpenTelemetry.Instrumentation.AWS] Fix Memory Leak by Reusing ActivitySources, Meters, and Instruments

[muhammad-othman]

Fixes #2025

Changes

This pull request addresses issue #2025 a memory leak issue in OpenTelemetry.Instrumentation.AWS, particularly related to the new metrics features.
The memory leak was identified after running a significant number of requests (around 10,000). The root cause was found to be the non-reuse of ActivitySources, Meters, and Metrics Instruments. These objects were being instantiated repeatedly, leading to a gradual increase in memory usage.
This PR includes refactoring the code to reuse ActivitySources, Meters, and Metrics Instruments instead of creating new instances for each request.

Here are memory graph screenshots to illustrate the impact of the fix:

• Before the Fix (with memory leak):

  • The first snapshot was taken after running 200 requests to warm up the application.
  • The second snapshot was taken after running 10,000 requests, showing a significant memory increase due to the leak.
    

• After the Fix (no memory leak):

  • The first snapshot was again taken after 200 requests.
  • The second snapshot was taken after 10,000 requests, showing stable memory usage with no signs of a leak related to OpenTelemetry.Instrumentation.AWS.
    

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Attention: Patch coverage is 98.24561% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.78%. Comparing base (71655ce) to head (90129f7).
Report is 414 commits behind head on main.

Files with missing lines	Patch %	Lines
....AWS/Implementation/Metrics/AWSMonotonicCounter.cs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2039       +/-   ##
===========================================
+ Coverage   73.91%   84.78%   +10.86%     
===========================================
  Files         267       20      -247     
  Lines        9615      368     -9247     
===========================================
- Hits         7107      312     -6795     
+ Misses       2508       56     -2452     

Flag	Coverage Δ
unittests-Instrumentation.AWS	84.78% <98.24%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ntation.AWS/Implementation/Metrics/AWSHistogram.cs	88.23% <100.00%> (ø)
...rumentation.AWS/Implementation/Metrics/AWSMeter.cs	100.00% <100.00%> (ø)
...ion.AWS/Implementation/Metrics/AWSMeterProvider.cs	100.00% <100.00%> (ø)
...ion.AWS/Implementation/Metrics/AWSUpDownCounter.cs	94.11% <100.00%> (ø)
...umentation.AWS/Implementation/Tracing/AWSTracer.cs	60.00% <100.00%> (ø)
...on.AWS/Implementation/Tracing/AWSTracerProvider.cs	100.00% <100.00%> (ø)
....AWS/Implementation/Metrics/AWSMonotonicCounter.cs	88.23% <92.85%> (ø)

... and 270 files with indirect coverage changes

[ppittle]

naive question - any concern with effectively caching Histograms, Meters, etc indefinitely? Looking to confirm it is necessary to clean up any old objects.

[muhammad-othman]

I don't think there are major side effects other than the memory overhead considering that other instrumentations packages do reuse the instruments too (ex:https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs#L36)
though we do store more than just one histogram and meter.

For cleaning up old objects, we do create one meter per service the first time it is used and around a dozen metrics instruments each, the names of these instruments are constants that doesn't include any user defined value, so there will always be a chance that the old objects are needed later.

[Kielek]

Approving per component owner review.

[Kielek]

Merging as agreed with @ppittle.

[cijothomas]

These are very common mistakes, and has now been explicitly called out in docs. See below for Metrics. Similar exists for Traces too.
https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics#metrics-api