Skip to content

[OpenTelemetry] Fix metric storage leak#7466

Open
martincostello wants to merge 6 commits into
open-telemetry:mainfrom
martincostello:gh-5922
Open

[OpenTelemetry] Fix metric storage leak#7466
martincostello wants to merge 6 commits into
open-telemetry:mainfrom
martincostello:gh-5922

Conversation

@martincostello

Copy link
Copy Markdown
Member

Fixes #5922

Changes

Fix metric storage leak that occurs when meters and instruments are repeatedly created and disposed.

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)

Fix metric storage leak that occurs when meters and instruments are repeatedly created and disposed.

Resolves open-telemetry#5922.
Comment thread src/OpenTelemetry/CHANGELOG.md Outdated
@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Jun 29, 2026
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (fc89dbe) to head (7c54780).
⚠️ Report is 18 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7466      +/-   ##
==========================================
+ Coverage   90.22%   90.32%   +0.10%     
==========================================
  Files         287      287              
  Lines       15559    15798     +239     
==========================================
+ Hits        14038    14270     +232     
- Misses       1521     1528       +7     
Flag Coverage Δ
unittests-Project-Experimental 90.14% <100.00%> (-0.11%) ⬇️
unittests-Project-Stable 90.19% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...rc/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs 93.28% <100.00%> (+1.22%) ⬆️

... and 10 files with indirect coverage changes

@martincostello martincostello marked this pull request as ready for review June 29, 2026 16:17
@martincostello martincostello requested a review from a team as a code owner June 29, 2026 16:18
Copilot AI review requested due to automatic review settings June 29, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a metrics storage memory leak in the OpenTelemetry SDK that occurs when Meter/instruments are repeatedly created and disposed (as in dotnet/runtime meter unpublish notifications). It does so by reclaiming freed metric storage slots and by removing deactivated instrument entries from the identity-to-metric dictionary so it cannot grow without bound.

Changes:

  • Reclaim and reuse metric storage slots after deactivated metrics are removed during collection (availableMetricIndices + TryReserveMetricIndex).
  • Prevent unbounded growth of instrumentIdentityToMetric by removing deactivated entries using value-matching removal to remain race-safe.
  • Update/add unit tests validating storage reclamation and reuse across repeated collections; add a changelog entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTests.cs Updates existing test expectations and adds a new regression test ensuring metric storage is reused across many transient meters/collects.
src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs Implements index reuse for metric storage and removes deactivated metrics from the identity dictionary to eliminate leak vectors.
src/OpenTelemetry/CHANGELOG.md Adds an Unreleased entry documenting the metric storage leak fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@martincostello martincostello added this to the v1.17.0 milestone Jul 3, 2026
Reference-count the metrics to avoid unbounded cache growth.
Duplicate is no longer detected.
@github-actions github-actions Bot added the pkg:OpenTelemetry.Extensions.Hosting Issues related to OpenTelemetry.Extensions.Hosting NuGet package label Jul 3, 2026

@Kielek Kielek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but it will be great if @alanwest or @cijothomas look into this also.

@martincostello martincostello added the keep-open Prevents issues and pull requests being closed as stale label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

keep-open Prevents issues and pull requests being closed as stale pkg:OpenTelemetry.Extensions.Hosting Issues related to OpenTelemetry.Extensions.Hosting NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] - Memory leak

3 participants