Fix #1588 - Observable Gauge does not reflect updated values, and send the old value always

[lalitb]

Fixes #1588

Changes

Fixed the logic to convert monotonic increasing async counter values to the delta. Also fixed the ostream metric example to generate monotonic increasing values for the async counter.

More details -
While the observable instrument always records monotonically increasing values, it is possible for exporter/reader to request for either cumulative or delta of it. The AsyncMetricStorage::Record() method does conversion from recorded value to delta, and further passes it to TemporalMetricStorage during collection. TemporalMetricStorage further uses it to maintain the unreported values for each collector.

There was a problem in the conversion logic of recorded value -> delta in AsyncMetricStorage::Record(). This code is fixed now. And also added a test case to validate it.

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #1641 (a868b7c) into main (d127140) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1641      +/-   ##
==========================================
- Coverage   85.10%   85.08%   -0.02%     
==========================================
  Files         159      159              
  Lines        4991     4990       -1     
==========================================
- Hits         4247     4245       -2     
- Misses        744      745       +1     

Impacted Files	Coverage Δ
sdk/src/metrics/meter.cc	33.08% <ø> (ø)
sdk/src/metrics/state/temporal_metric_storage.cc	96.43% <ø> (ø)
...telemetry/sdk/metrics/state/async_metric_storage.h	86.12% <100.00%> (-0.37%)	⬇️
sdk/src/trace/batch_span_processor.cc	90.63% <0.00%> (-0.78%)	⬇️

[esigo]

LGTM
Thanks for the fix :)

[tauseef-ah]

@lalitb I was testing these changes. They work fine for ObservableCounter, but Observable Gauge still send the old value. Can you pls check for Observable Gauge as well

[lalitb]

@tauseef-ah - Thanks for testing, I felt the change should work for all the observable instruments, but let me test it for Observable Gauge too.

[lalitb]

@tauseef-ah - Can you test if by setting AggergationTemporality as kDelta (default is kCumulative for ostream exporter), as it gives the desired result. I still need to check specs guidelines for the desired effect if the Temporality is set as kCumulative.
You need to change the ostream exporter creation here to change the temporality -

opentelemetry-cpp/examples/metrics_simple/metrics_ostream.cc

Line 32 in f21b3a0

std::unique_ptr<metric_sdk::MetricExporter> exporter{new exportermetrics::OStreamMetricExporter};

std::unique_ptr<metric_sdk::MetricExporter> exporter{new exportermetrics::OStreamMetricExporter(std::cout, AggregationTemporality::kDelta};

[reyang]

@tauseef-ah - Can you test if by setting AggergationTemporality as kDelta (default is kCumulative for ostream exporter), as it gives the desired result.

I guess aggregation temporality doesn't apply to Gauge since it is non-additive?

[lalitb]

I guess aggregation temporality doesn't apply to Gauge since it is non-additive?

Yes, I realized it afterward. There seems some issue in the cumulative pipeline while handling Gauge, I am looking into it.

[lalitb]

@tauseef-ah This has been fixed in #1651 if you want to test in the latest commit.