Skip to content

[METRICS] Support synchronous gauge with delta temporality#4152

Open
pranitaurlam wants to merge 2 commits into
open-telemetry:mainfrom
pranitaurlam:fix/gauge-delta-temporality
Open

[METRICS] Support synchronous gauge with delta temporality#4152
pranitaurlam wants to merge 2 commits into
open-telemetry:mainfrom
pranitaurlam:fix/gauge-delta-temporality

Conversation

@pranitaurlam

Copy link
Copy Markdown
Contributor

Fixes #4146

Summary

Removes the block in MetricCollector::GetAggregationTemporality that logged an error and silently fell back to cumulative when delta was requested for a synchronous gauge instrument.

The existing delta fast-path in TemporalMetricStorage::buildMetrics already provides the correct window-scoped last-value semantics for gauge: because SyncMetricStorage::Collect swaps and resets the live attributes_hashmap_ on every collection, delta_metrics contains only values recorded since the previous collection. Stale attribute sets that were not updated in the current window are simply absent from the snapshot and are never re-emitted.

Changes:

  • metric_collector.cc: remove the gauge+delta error/fallback; return the reader-requested temporality directly.
  • temporal_metric_storage.cc: update fast-path comment to mention gauge and its window-scoped last-value behavior.
  • sync_metric_storage_gauge_test.cc:
    • Add kDelta to both INSTANTIATE_TEST_SUITE_P suites and assert values for both temporalities.
    • Add GaugeDeltaWindowTest.NoRecordWindowEmitsNothing: record → collect → no record → collect emits nothing.
    • Add GaugeDeltaWindowTest.StaleAttributeNotReemitted: record A+B → collect → record A only → collect emits only A.
    • Add GaugeDeltaWindowTest.RecordAfterEmptyWindowEmitsNewValue: empty window followed by a new recording is exported correctly.

Test plan

  • Existing cumulative gauge tests still pass with both temporalities.
  • NoRecordWindowEmitsNothing: second collection with no new recordings emits 0 points.
  • StaleAttributeNotReemitted: attribute not recorded in the current window is not re-exported.
  • RecordAfterEmptyWindowEmitsNewValue: new recording after an empty window is exported correctly.

@pranitaurlam

Copy link
Copy Markdown
Contributor Author

Hi @lalitb @marcalff @ThomsonTan — I've opened #4152 to address this.

The fix removes the error/fallback in MetricCollector::GetAggregationTemporality and relies on the existing delta fast-path in TemporalMetricStorage, which already provides the correct window-scoped last-value semantics (the live attributes_hashmap_ is swapped and reset on every collection, so stale attribute sets are never in the snapshot).

Tests added for the scenarios you mentioned: record → collect → no record → collect (emits nothing), stale attribute not re-emitted, and recovery after an empty window. Happy to adjust based on your feedback.

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.09%. Comparing base (4352a63) to head (66744a6).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4152      +/-   ##
==========================================
+ Coverage   82.01%   82.09%   +0.08%     
==========================================
  Files         385      386       +1     
  Lines       16093    16207     +114     
==========================================
+ Hits        13197    13303     +106     
- Misses       2896     2904       +8     
Files with missing lines Coverage Δ
sdk/src/metrics/state/metric_collector.cc 96.62% <100.00%> (+2.96%) ⬆️
sdk/src/metrics/state/temporal_metric_storage.cc 97.78% <ø> (+0.06%) ⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support synchronous gauge instrument with delta aggregation temporality in C++ SDK

1 participant