[Kielek]

Both PR and description AI assisted. I have reviewed both carefully.

Changes

Fix SuppressInstrumentationScope reference-count state leaking between inherited
ExecutionContext/async flows.

The fix stores immutable suppression states in the runtime context slot. Common
reference counts are cached, preserving most of the allocation benefit of the
current implementation without sharing mutable state between async flows.

This change also adds regression coverage for:

• Enter, IncrementIfTriggered, and DecrementIfTriggered isolation while
  parent and child async flows overlap.
• Restoring an explicit outer scope only when the inner scope is disposed.
• Reference counts beyond the cached-state boundary.

Why this is needed

#4304
changed the slot from an int to a mutable SuppressInstrumentationScope
instance as a performance optimization. This avoided boxing an integer and
updating AsyncLocal/ExecutionContext state for every reference-count change.

However, an AsyncLocal reference value flows into a child execution context as
the same object reference. Mutating Depth in place therefore mutates the value
observed by the parent and sibling flows as well. Suppression may consequently
end too early or remain active too long, causing telemetry to be incorrectly
recorded or suppressed.

A test that lets the child increment and then decrement before checking the
parent does not expose the bug: the shared counter has already returned to its
starting value. The new tests coordinate the flows so the parent observes its
own state while the child's mutation is still active. These tests fail against
the mutable implementation.

The mutable implementation also changes nested-scope semantics. When a
reference count reaches zero inside Begin(false), it restores the preceding
scope immediately instead of waiting for the explicit scope to be disposed.
The immutable-state implementation retains the original value-slot behavior.

Implementation

• Store a private immutable suppression-state reference in the runtime context
  slot.
• Use a singleton for the always-suppressed state.
• Cache reference-count states 1 through 16 to avoid boxing or allocating a new
  state for normal nesting depths.
• Allocate an immutable state for deeper counts, keeping the cache bounded.
• Call Slot.Set whenever the reference count changes so each execution context
  receives its own value.

Performance

The original #4304 results show the cost that a direct return to AsyncLocal<int>
would reintroduce:

Scenario	Before #4304 (int)	After #4304 (mutable reference)
Three nested Begin scopes	143.91 ns / 648 B	125.37 ns / 456 B
Enter reference-count cycle	99.49 ns / 384 B	59.17 ns / 104 B

New comparison benchmarks were run with BenchmarkDotNet 0.15.8 on .NET
10.0.12, Windows 11, and an Intel Core i9-13950HX. Runs were pinned to one
performance core, with 5 warmup and 15 measurement iterations.

Scenario	Mutable holder	int value	Immutable cached state
Enter + increment + two decrements	43.67 ns / 112 B	92.16 ns / 384 B	68.79 ns / 216 B
Three nested Begin scopes	119.75 ns / 480 B	135.91 ns / 576 B	49.37 ns / 72 B

For the active reference-count cycle, the immutable implementation costs about
25.1 ns and 104 B more than the unsafe mutable holder. A direct integer fix
would cost about 48.5 ns and 272 B more. The immutable cache therefore recovers
roughly half the time penalty and most of the additional allocation.

On .NET 10, nested scopes are faster than both alternatives because they reuse
the same immutable always-suppressed token. Redundant AsyncLocal writes become
no-ops and the JIT can eliminate the short-lived scope allocations.

These are focused microbenchmarks. Normal non-triggered suppression checks do
not update the runtime context, and exporter work will usually dominate the
absolute scope-management cost.

Testing

CI

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 (requires the new PR number)
• Changes in public API reviewed (no public API changes)

[opentelemetry-pr-dashboard]

Pull request dashboard status

Merged · refreshed 2026-09-24 21:16 UTC

Status above doesn't look right?

• Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

[Copilot]

Copilot review overview

🟡 Changes recommended

A core SDK behavioural bug fix requires an entry in src/OpenTelemetry/CHANGELOG.md under ## Unreleased, which is currently missing.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Add changelog entry for async suppression state bug fix · New

What changed in this PR

Fixes a correctness issue in the SDK’s suppression mechanism where a mutable suppression state stored in runtime context could be shared by reference across inherited ExecutionContext/async flows, causing suppression depth mutations in one flow to affect others.

Changes:

• Reworks SuppressInstrumentationScope to store immutable suppression state objects in the runtime context slot, with a bounded cache for common reference counts.
• Adds regression tests to validate isolation across overlapping parent/child async flows and correct nested-scope restoration semantics.
• Extends test coverage for deep reference-count nesting beyond the cached boundary.

File	Description
src/​OpenTelemetry/​SuppressInstrumentationScope.cs	Switches suppression tracking to immutable, cached state tokens to prevent cross-async-flow leakage while retaining performance benefits.
test/​OpenTelemetry.Tests/​SuppressInstrumentationTests.cs	Adds regression tests covering overlapping async flow isolation, explicit-scope restoration, and deep reference count behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.91%. Comparing base (44d841f) to head (330f61d).
⚠️ Report is 21 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7803   +/-   ##
=======================================
  Coverage   91.90%   91.91%           
=======================================
  Files         341      341           
  Lines       18615    18610    -5     
=======================================
- Hits        17109    17106    -3     
+ Misses       1506     1504    -2     

Flag	Coverage Δ
unittests-Project-Experimental	92.04% <100.00%> (+0.06%)	⬆️
unittests-Project-Stable	91.95% <100.00%> (-0.07%)	⬇️

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

Files with missing lines	Coverage Δ
src/OpenTelemetry/SuppressInstrumentationScope.cs	100.00% <100.00%> (+9.75%)	⬆️

... and 2 files with indirect coverage changes

[martincostello]

Just some comments on the tests.