Fix: Observable instruments should not export stale series (#5950)

[EliaSaSe]

Aims to Fix #5950.

Changes

The https://opentelemetry.io/docs/specs/otel/metrics/sdk/#metricreader requires that for asynchronous instruments, MetricReader.Collect MUST only receive data points with measurements recorded since the previous collection. The SDK was instead re-exporting the last known value indefinitely when an observable callback stopped reporting a series.

I seems, the delta temporality path does not have this issue, because of its metric point reclaim mechanism. This PR changes the cumulative path by:

• Adding MetricStreamIdentity.IsAsynchronous to identify observable instruments (ObservableCounter, ObservableUpDownCounter, ObservableGauge)
• In SnapshotCumulative, skipping metric points not reported in the current callback cycle
• In MetricPoint.TakeSnapshot, resetting status to NoCollectPending after snapshot for asynchronous cumulative sum types, so unreported points are detected as stale in the next collection

No public API changes. But I'm not sure if the behavior change is considered as API change.

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)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: rajkumar-rangaraj / name: Rajkumar Rangaraj (931fc1d)

[EliaSaSe]

Hello @cijothomas, can You have a look at this PR to fix issue #5950? I tried make the most minimal code change. But I do not know the codebase enough to determine if this is a real fix or a hack. Should the MetricPointStatus be used for that? Have a nice day, Elia

[cijothomas]

@EliaSaSe I triggered CI runs, so we'll know if this fixes the issue without other regression. Once this is done, we can ask maintainers to review the change. (I'll review as well, but will be few days before I can commit time.)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (06f9384) to head (931fc1d).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6883      +/-   ##
==========================================
+ Coverage   88.51%   88.53%   +0.01%     
==========================================
  Files         263      263              
  Lines       12383    12408      +25     
==========================================
+ Hits        10961    10985      +24     
- Misses       1422     1423       +1     

Flag	Coverage Δ
unittests-Project-Experimental	88.22% <100.00%> (-0.25%)	⬇️
unittests-Project-Stable	88.31% <100.00%> (-0.01%)	⬇️

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

Files with missing lines	Coverage Δ
src/OpenTelemetry/Metrics/AggregatorStore.cs	87.59% <100.00%> (+0.09%)	⬆️
...c/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs	94.76% <100.00%> (+0.05%)	⬆️
src/OpenTelemetry/Metrics/MetricStreamIdentity.cs	91.66% <100.00%> (+1.19%)	⬆️

... and 3 files with indirect coverage changes

[rajkumar-rangaraj]

Left a minor test suggestion

[cijothomas]

open-telemetry/opentelemetry-specification#4861 consider adding a test for this issue. I don't know the actual recommendation from spec yet, but lets have a test and confirm and document the current behavior.

[cijothomas]

open-telemetry/opentelemetry-specification#4861 consider adding a test for this issue. I don't know the actual recommendation from spec yet, but lets have a test and confirm and document the current behavior.

open-telemetry/opentelemetry-rust#3334 you can steal this test case!

[EliaSaSe]

Thanks for the feedbacks, it's appreciated. I will look into it as soon as I can, it may take a few days.

[EliaSaSe]

open-telemetry/opentelemetry-specification#4861 consider adding a test for this issue. I don't know the actual recommendation from spec yet, but lets have a test and confirm and document the current behavior.

open-telemetry/opentelemetry-rust#3334 you can steal this test case!

I will look into it in the next few days.

[EliaSaSe]

@martincostello One of the builds failed, but because it was cancelled. Can the failed one be retriggered? I consider the code change as done if cjiothomas does not request further changes.

[cijothomas]

Thanks!

[github-actions]

Thank you for your contribution @EliaSaSe! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

[EliaSaSe]

Thank you all so much for your support. You made it easy to contribute.