Use sdk_start_ts for MetricData start_ts for instruments having cumul…

…ative aggregation temporality. (#2086) Co-authored-by: Marc Alff <marc.alff@oracle.com>
open-telemetry · Apr 12, 2023 · e71e28c · e71e28c
1 parent efb1a26
commit e71e28c
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,8 @@ Increment the:
   [#2088](https://github.com/open-telemetry/opentelemetry-cpp/pull/2088)
 * [EXPORTER] Add OTLP HTTP SSL support
   [#1793](https://github.com/open-telemetry/opentelemetry-cpp/pull/1793)
+* [METRICS SDK] Use sdk_start_ts for MetricData start_ts for instruments having
+  cumulative aggregation temporality. [#2086](https://github.com/open-telemetry/opentelemetry-cpp/pull/2086)
 
 Important changes:
 

diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc
@@ -89,7 +89,6 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
   auto reported = last_reported_metrics_.find(collector);
   if (reported != last_reported_metrics_.end())
   {
-    last_collection_ts     = last_reported_metrics_[collector].collection_ts;
     auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map);
     if (aggregation_temporarily == AggregationTemporality::kCumulative)
     {
@@ -111,6 +110,10 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
             return true;
           });
     }
+    else
+    {
+      last_collection_ts = last_reported_metrics_[collector].collection_ts;
+    }
     last_reported_metrics_[collector] =
         LastReportedMetrics{std::move(merged_metrics), collection_ts};
   }

diff --git a/sdk/test/metrics/sync_metric_storage_counter_test.cc b/sdk/test/metrics/sync_metric_storage_counter_test.cc
@@ -95,6 +95,10 @@ TEST_P(WritableMetricStorageTestFixture, LongCounterSumAggregation)
   count_attributes = 0;
   storage.Collect(
       collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) {
+        if (temporality == AggregationTemporality::kCumulative)
+        {
+          EXPECT_EQ(metric_data.start_ts, sdk_start_ts);
+        }
         for (const auto &data_attr : metric_data.point_data_attr_)
         {
           const auto &data = opentelemetry::nostd::get<SumPointData>(data_attr.point_data);
@@ -234,6 +238,10 @@ TEST_P(WritableMetricStorageTestFixture, DoubleCounterSumAggregation)
   count_attributes = 0;
   storage.Collect(
       collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) {
+        if (temporality == AggregationTemporality::kCumulative)
+        {
+          EXPECT_EQ(metric_data.start_ts, sdk_start_ts);
+        }
         for (const auto &data_attr : metric_data.point_data_attr_)
         {
           const auto &data = opentelemetry::nostd::get<SumPointData>(data_attr.point_data);

diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc
@@ -97,6 +97,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram)
   count_attributes = 0;
   storage.Collect(
       collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) {
+        if (temporality == AggregationTemporality::kCumulative)
+        {
+          EXPECT_EQ(metric_data.start_ts, sdk_start_ts);
+        }
         for (const auto &data_attr : metric_data.point_data_attr_)
         {
           const auto &data = opentelemetry::nostd::get<HistogramPointData>(data_attr.point_data);
@@ -236,6 +240,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram)
   count_attributes = 0;
   storage.Collect(
       collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) {
+        if (temporality == AggregationTemporality::kCumulative)
+        {
+          EXPECT_EQ(metric_data.start_ts, sdk_start_ts);
+        }
         for (const auto &data_attr : metric_data.point_data_attr_)
         {
           const auto &data = opentelemetry::nostd::get<HistogramPointData>(data_attr.point_data);

diff --git a/sdk/test/metrics/sync_metric_storage_up_down_counter_test.cc b/sdk/test/metrics/sync_metric_storage_up_down_counter_test.cc
@@ -99,6 +99,10 @@ TEST_P(WritableMetricStorageTestFixture, LongUpDownCounterSumAggregation)
   count_attributes = 0;
   storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts,
                   [&](const MetricData data) {
+                    if (temporality == AggregationTemporality::kCumulative)
+                    {
+                      EXPECT_EQ(data.start_ts, sdk_start_ts);
+                    }
                     for (auto data_attr : data.point_data_attr_)
                     {
                       auto sum_data = opentelemetry::nostd::get<SumPointData>(data_attr.point_data);
@@ -246,6 +250,10 @@ TEST_P(WritableMetricStorageTestFixture, DoubleUpDownCounterSumAggregation)
   count_attributes = 0;
   storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts,
                   [&](const MetricData data) {
+                    if (temporality == AggregationTemporality::kCumulative)
+                    {
+                      EXPECT_EQ(data.start_ts, sdk_start_ts);
+                    }
                     for (auto data_attr : data.point_data_attr_)
                     {
                       auto sum_data = opentelemetry::nostd::get<SumPointData>(data_attr.point_data);