diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/aggregator.h index b997514208..77c167cc57 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/aggregator.h @@ -97,6 +97,14 @@ class Aggregator */ virtual AggregatorKind get_aggregator_kind() final { return agg_kind_; } + /** + * Getter function for updated_ protected var + * + * @return A bool indicating wether or not this aggregator has been updated + * in the most recent collection interval. + */ + virtual bool is_updated() final { return updated_; } + // virtual function to be overriden for the Histogram Aggregator virtual std::vector get_boundaries() { return std::vector(); } @@ -134,6 +142,7 @@ class Aggregator opentelemetry::metrics::InstrumentKind kind_; std::mutex mu_; AggregatorKind agg_kind_; + bool updated_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/counter_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/counter_aggregator.h index ebd0201ef0..a82cf40766 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/counter_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/counter_aggregator.h @@ -36,6 +36,7 @@ class CounterAggregator final : public Aggregator void update(T val) override { this->mu_.lock(); + this->updated_ = true; this->values_[0] += val; // atomic operation this->mu_.unlock(); } @@ -50,6 +51,7 @@ class CounterAggregator final : public Aggregator void checkpoint() override { this->mu_.lock(); + this->updated_ = false; this->checkpoint_ = this->values_; this->values_[0] = 0; this->mu_.unlock(); diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h index 00ec1ae12d..f1a5242c48 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/exact_aggregator.h @@ -62,6 +62,7 @@ class ExactAggregator : public Aggregator void update(T val) override { this->mu_.lock(); + this->updated_ = true; this->values_.push_back(val); this->mu_.unlock(); } @@ -74,6 +75,7 @@ class ExactAggregator : public Aggregator void checkpoint() override { this->mu_.lock(); + this->updated_ = false; if (quant_estimation_) { std::sort(this->values_.begin(), this->values_.end()); diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h index 7925f5e205..5046928bac 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/gauge_aggregator.h @@ -59,6 +59,7 @@ class GaugeAggregator : public Aggregator void update(T val) override { this->mu_.lock(); + this->updated_ = true; this->values_[0] = val; current_timestamp_ = core::SystemTimestamp(std::chrono::system_clock::now()); this->mu_.unlock(); @@ -75,6 +76,7 @@ class GaugeAggregator : public Aggregator { this->mu_.lock(); + this->updated_ = false; this->checkpoint_ = this->values_; // Reset the values to default diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h index 9d5aafa127..07b05c247c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/histogram_aggregator.h @@ -63,7 +63,8 @@ class HistogramAggregator final : public Aggregator void update(T val) override { this->mu_.lock(); - int bucketID = boundaries_.size(); + this->updated_ = true; + int bucketID = boundaries_.size(); for (size_t i = 0; i < boundaries_.size(); i++) { if (val < boundaries_[i]) // concurrent read is thread-safe @@ -93,6 +94,7 @@ class HistogramAggregator final : public Aggregator void checkpoint() override { this->mu_.lock(); + this->updated_ = false; this->checkpoint_ = this->values_; this->values_[0] = 0; this->values_[1] = 0; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h index b47cf0df06..08c57c998d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/min_max_sum_count_aggregator.h @@ -60,6 +60,7 @@ class MinMaxSumCountAggregator : public Aggregator void update(T val) override { this->mu_.lock(); + this->updated_ = true; if (this->values_[CountValueIndex] == 0 || val < this->values_[MinValueIndex]) // set min this->values_[MinValueIndex] = val; @@ -80,6 +81,7 @@ class MinMaxSumCountAggregator : public Aggregator void checkpoint() override { this->mu_.lock(); + this->updated_ = false; this->checkpoint_ = this->values_; // Reset the values this->values_[MinValueIndex] = 0; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h index fe0b4737ec..bc29868320 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregator/sketch_aggregator.h @@ -64,6 +64,7 @@ class SketchAggregator final : public Aggregator void update(T val) override { this->mu_.lock(); + this->updated_ = true; int idx; if (val == 0) { @@ -130,6 +131,7 @@ class SketchAggregator final : public Aggregator void checkpoint() override { this->mu_.lock(); + this->updated_ = false; this->checkpoint_ = this->values_; checkpoint_raw_ = raw_; this->values_[0] = 0; diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index 2c0931eeb2..aab0c4e86f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -149,8 +149,11 @@ class Counter final : public SynchronousInstrument, public metrics_api::Count toDelete.push_back(x.first); } auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + if (agg_ptr->is_updated()) + { + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } } for (const auto &x : toDelete) { @@ -273,8 +276,11 @@ class UpDownCounter final : public SynchronousInstrument, public metrics_api: toDelete.push_back(x.first); } auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + if (agg_ptr->is_updated()) + { + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } } for (const auto &x : toDelete) { @@ -396,8 +402,11 @@ class ValueRecorder final : public SynchronousInstrument, public metrics_api: toDelete.push_back(x.first); } auto agg_ptr = dynamic_cast *>(x.second.get())->GetAggregator(); - agg_ptr->checkpoint(); - ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + if (agg_ptr->is_updated()) + { + agg_ptr->checkpoint(); + ret.push_back(Record(x.second->GetName(), x.second->GetDescription(), x.first, agg_ptr)); + } } for (const auto &x : toDelete) { diff --git a/sdk/test/metrics/metric_instrument_test.cc b/sdk/test/metrics/metric_instrument_test.cc index 3518c54604..3caf526010 100644 --- a/sdk/test/metrics/metric_instrument_test.cc +++ b/sdk/test/metrics/metric_instrument_test.cc @@ -248,6 +248,7 @@ TEST(Counter, getAggsandnewupdate) auto labelkv = trace::KeyValueIterableView{labels}; auto beta = alpha.bindCounter(labelkv); + beta->add(1); beta->unbind(); EXPECT_EQ(alpha.boundInstruments_[KvToString(labelkv)]->get_ref(), 0); @@ -435,7 +436,34 @@ TEST(IntValueRecorder, StressRecord) 125); // count } +TEST(Instruments, NoUpdateNoRecord) +{ + // This test verifies that instruments that have received no updates + // in the last collection period are not made into records for export. + + Counter alpha("alpha", "no description", "unitless", true); + + std::map labels = {{"key", "value"}}; + + auto labelkv = trace::KeyValueIterableView{labels}; + + EXPECT_EQ(alpha.GetRecords().size(), 0); + alpha.add(1, labelkv); + EXPECT_EQ(alpha.GetRecords().size(), 1); + + UpDownCounter beta("beta", "no description", "unitless", true); + + EXPECT_EQ(beta.GetRecords().size(), 0); + beta.add(1, labelkv); + EXPECT_EQ(beta.GetRecords().size(), 1); + + ValueRecorder gamma("gamma", "no description", "unitless", true); + + EXPECT_EQ(gamma.GetRecords().size(), 0); + gamma.record(1, labelkv); + EXPECT_EQ(gamma.GetRecords().size(), 1); +} + } // namespace metrics } // namespace sdk - OPENTELEMETRY_END_NAMESPACE