From 40032c51085afca6e019b4f7ab9ae7ddea2f9c85 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 7 Feb 2022 16:04:13 -0800 Subject: [PATCH 1/2] writable storage --- .../aggregator/histogram_aggregator.h | 1 + .../sdk/metrics/aggregation/aggregation.h | 4 +- .../metrics/aggregation/drop_aggregation.h | 4 +- .../aggregation/histogram_aggregation.h | 8 +- .../aggregation/lastvalue_aggregation.h | 8 +- .../sdk/metrics/aggregation/sum_aggregation.h | 8 +- .../sdk/metrics/data/metric_data.h | 3 +- .../sdk/metrics/data/point_data.h | 8 +- .../opentelemetry/sdk/metrics/instruments.h | 22 +++ .../sdk/metrics/state/metric_storage.h | 73 ++++++++++ .../sdk/metrics/state/sync_metric_storage.h | 137 ++++++++++++++++++ .../sdk/metrics/view/attributes_processor.h | 8 +- .../opentelemetry/sdk/metrics/view/view.h | 13 +- .../aggregation/histogram_aggregation.cc | 4 +- .../aggregation/lastvalue_aggregation.cc | 4 +- .../metrics/aggregation/sum_aggregation.cc | 4 +- sdk/test/metrics/aggregation_test.cc | 32 ++-- sdk/test/metrics/view_registry_test.cc | 1 + 18 files changed, 284 insertions(+), 58 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h create mode 100644 sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h diff --git a/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h b/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h index 13e1e1244a..27ff29a42c 100644 --- a/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h +++ b/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h @@ -9,6 +9,7 @@ # include # include # include "opentelemetry/_metrics/instrument.h" +# include "opentelemetry/sdk/metrics/" # include "opentelemetry/sdk/_metrics/aggregator/aggregator.h" # include "opentelemetry/version.h" diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index 3bb133ac15..01a84b785b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -24,9 +24,9 @@ class InstrumentMonotonicityAwareAggregation class Aggregation { public: - virtual void Aggregate(long value, const PointAttributes &attributes = {}) noexcept = 0; + virtual void Aggregate(long value) noexcept = 0; - virtual void Aggregate(double value, const PointAttributes &attributes = {}) noexcept = 0; + virtual void Aggregate(double value) noexcept = 0; virtual PointType Collect() noexcept = 0; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h index 0af5dd43a5..1898aec7f8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h @@ -19,9 +19,9 @@ class DropAggregation : public Aggregation public: DropAggregation() = default; - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(long value) noexcept override {} - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(double value) noexcept override {} PointType Collect() noexcept override { return DropPointData(); } }; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 5a66ecb243..31ecdba40b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -33,9 +33,9 @@ class LongHistogramAggregation : public Aggregation public: LongHistogramAggregation(); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(long value) noexcept override; - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(double value) noexcept override {} PointType Collect() noexcept override; @@ -52,9 +52,9 @@ class DoubleHistogramAggregation : public Aggregation public: DoubleHistogramAggregation(); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(long value) noexcept override {} - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(double value) noexcept override; PointType Collect() noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h index 092a0df30e..c9bdf4477c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h @@ -18,9 +18,9 @@ class LongLastValueAggregation : public Aggregation public: LongLastValueAggregation(); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(long value) noexcept override; - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(double value) noexcept override {} PointType Collect() noexcept override; @@ -35,9 +35,9 @@ class DoubleLastValueAggregation : public Aggregation public: DoubleLastValueAggregation(); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(long value) noexcept override {} - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(double value) noexcept override; PointType Collect() noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index c8b2eff33c..75358507db 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -33,9 +33,9 @@ class LongSumAggregation : public Aggregation, InstrumentMonotonicityAwareAggreg public: LongSumAggregation(bool is_monotonic); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(long value) noexcept override; - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(double value) noexcept override {} PointType Collect() noexcept override; @@ -50,9 +50,9 @@ class DoubleSumAggregation : public Aggregation, InstrumentMonotonicityAwareAggr public: DoubleSumAggregation(bool is_monotonic); - void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} + void Aggregate(long value) noexcept override {} - void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(double value) noexcept override; PointType Collect() noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h index cb7bc4cf80..2add5859ce 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h @@ -17,7 +17,6 @@ namespace sdk namespace metrics { -using PointAttributes = opentelemetry::sdk::common::AttributeMap; using PointType = opentelemetry::nostd:: variant; @@ -26,7 +25,7 @@ class MetricData public: opentelemetry::sdk::resource::Resource *resource_; opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library_; - PointAttributes attributes_; + MetricAttributes attributes_; InstrumentDescriptor instrument_descriptor; PointType point_data_; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index e95a329e44..b49f6f4af6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -5,6 +5,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/timestamp.h" # include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/version.h" # include @@ -18,13 +19,6 @@ namespace metrics using ValueType = nostd::variant; using ListType = nostd::variant, std::vector>; -enum class AggregationTemporarily -{ - kUnspecified, - kDelta, - kCummulative -}; - class SumPointData { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 88d432a136..42a1217bf9 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -4,11 +4,16 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/common/attribute_utils.h" + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { + +typedef opentelemetry::sdk::common::OrderedAttributeMap MetricAttributes; + enum class InstrumentType { kCounter, @@ -27,6 +32,22 @@ enum class InstrumentValueType kDouble }; +enum class AggregationType +{ + kDefault, + kDrop, + kLastValue, + kSum, + kHistogram +}; + +enum class AggregationTemporarily +{ + kUnspecified, + kDelta, + kCummulative +}; + struct InstrumentDescriptor { std::string name_; @@ -36,6 +57,7 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; + /*class InstrumentSelector { public: InstrumentSelector(opentelemetry::nostd::string_view name, diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h new file mode 100644 index 0000000000..a168987020 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -0,0 +1,73 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/common/key_value_iterable_view.h" +#include "opentelemetry/sdk/metrics/data/metric_data.h" +# include "opentelemetry/sdk/metrics/instruments.h" +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +/* Represent the storage from which to collect the metrics */ + +class MetricStorage +{ +public: + /* collect the metrics from this storage */ + virtual bool Collect(AggregationTemporarily aggregation_temporarily, + nostd::function_ref callback) noexcept = 0; +}; + +class WritableMetricStorage +{ +public: + virtual void RecordLong(long value) noexcept = 0; + + virtual void RecordLong(long value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept = 0; + + virtual void RecordDouble(double value) noexcept = 0; + + virtual void RecordDouble(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept = 0; +}; + +class NoopMetricStorage : public MetricStorage +{ +public: + bool Collect(AggregationTemporarily aggregation_temporarily, + nostd::function_ref callback) noexcept override + { + MetricData metric_data; + if (callback(metric_data)) + { + return true; + } + return false; + } +}; + +class NoopWritableMetricStorage : public WritableMetricStorage +{ +public: + void RecordLong(long value) noexcept = 0; + + void RecordLong(long value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override + {} + + void RecordDouble(double value) noexcept override {} + + void RecordDouble(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override + {} +}; + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h new file mode 100644 index 0000000000..ce07843e98 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -0,0 +1,137 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/sdk/common/attributemap_hash.h" +# include "opentelemetry/sdk/metrics/state/metric_storage.h" +# include "opentelemetry/sdk/metrics/view/attributes_processor.h" +#include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" +# include "opentelemetry/sdk/metrics/view/view.h" +# include "opentelemetry/sdk/resource/resource.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +class SyncMetricStorage : public MetricStorage, public WritableMetricStorage +{ + +public: + SyncMetricStorage( + InstrumentDescriptor instrument_descriptor, + const AggregationType aggregation_type, + const AttributesProcessor* attributes_processor = new DefaultAttributesProcessor() + ): aggregation_type_{aggregation_type}, instrument_descriptor_{instrument_descriptor}, + attributes_processor_{attributes_processor} + {} + + void RecordLong(long value) noexcept override + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { + return ; + } + size_t hashcode = 0 ; //for empty attributs + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); + } + + void RecordLong(long value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { + return ; + } + auto attr = attributes_processor_->process(attributes); + auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); + } + + void RecordDouble(double value) noexcept override + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) { + return ; + } + size_t hashcode = 0 ; //for empty attributs + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); + } + + void RecordDouble(double value, + const opentelemetry::common::KeyValueIterable &attributes) noexcept override + { + if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) { + return ; + } + + auto attr = attributes_processor_->process(attributes); + auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); + } + + bool Collect(AggregationTemporarily aggregation_temporarily, + nostd::function_ref callback) noexcept override + { + if (callback(MetricData())) + { + return true; + } + return false; + } + +private: + InstrumentDescriptor instrument_descriptor_; + AggregationType aggregation_type_; + std::unordered_map> attributes_aggregation_; + const AttributesProcessor* attributes_processor_; + + std::unique_ptr create_aggregation() { + switch (aggregation_type_) { + case AggregationType::kDrop: + return std::move(std::unique_ptr(new DropAggregation())); + break; + case AggregationType::kHistogram: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { + return std::move(std::unique_ptr(new LongHistogramAggregation())); + } else { + return std::move(std::unique_ptr(new DoubleHistogramAggregation())); + } + break; + case AggregationType::kLastValue: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { + return std::move(std::unique_ptr(new LongLastValueAggregation())); + } else { + return std::move(std::unique_ptr(new DoubleLastValueAggregation())); + } + break; + case AggregationType::kSum: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { + return std::move(std::unique_ptr(new LongSumAggregation(true))); + } else { + return std::move(std::unique_ptr(new DoubleSumAggregation(true))); + } + break; + default: + return std::move(DefaultAggregation::CreateAggregation(instrument_descriptor_)); + } + } + +}; + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h index 6611abe97f..756e97eb20 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h @@ -4,12 +4,12 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/common/attribute_utils.h" +# include "opentelemetry/sdk/metrics/instruments.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { -using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; /** * The AttributesProcessor is responsible for customizing which @@ -22,7 +22,7 @@ class AttributesProcessor // Process the metric instrument attributes. // @returns The processed attributes virtual MetricAttributes process( - const opentelemetry::common::KeyValueIterable &attributes) noexcept = 0; + const opentelemetry::common::KeyValueIterable &attributes) const noexcept = 0; }; /** @@ -33,7 +33,7 @@ class AttributesProcessor class DefaultAttributesProcessor : public AttributesProcessor { MetricAttributes process( - const opentelemetry::common::KeyValueIterable &attributes) noexcept override + const opentelemetry::common::KeyValueIterable &attributes) const noexcept override { MetricAttributes result(attributes); return result; @@ -54,7 +54,7 @@ class FilteringAttributesProcessor : public AttributesProcessor {} MetricAttributes process( - const opentelemetry::common::KeyValueIterable &attributes) noexcept override + const opentelemetry::common::KeyValueIterable &attributes) const noexcept override { MetricAttributes result; attributes.ForEachKeyValue( diff --git a/sdk/include/opentelemetry/sdk/metrics/view/view.h b/sdk/include/opentelemetry/sdk/metrics/view/view.h index 4d1119d894..a0f1066a76 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/view.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/view.h @@ -4,7 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/nostd/string_view.h" -# include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" +# include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/view/attributes_processor.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -22,14 +22,13 @@ class View public: View(const std::string &name, const std::string &description = "", - std::unique_ptr aggregation = - std::unique_ptr(new DropAggregation()), + AggregationType aggregation_type = AggregationType::kDefault, std::unique_ptr attributes_processor = std::unique_ptr( new opentelemetry::sdk::metrics::DefaultAttributesProcessor())) : name_(name), description_(description), - aggregation_{std::move(aggregation)}, + aggregation_type_{aggregation_type}, attributes_processor_{std::move(attributes_processor)} {} @@ -37,9 +36,9 @@ class View virtual std::string GetDescription() const noexcept { return description_; } - virtual const opentelemetry::sdk::metrics::Aggregation &GetAggregation() const noexcept + virtual const AggregationType &GetAggregationType() const noexcept { - return *aggregation_.get(); + return aggregation_type_; } virtual const opentelemetry::sdk::metrics::AttributesProcessor &GetAttributesProcessor() @@ -51,7 +50,7 @@ class View private: std::string name_; std::string description_; - std::unique_ptr aggregation_; + AggregationType aggregation_type_; std::unique_ptr attributes_processor_; }; } // namespace metrics diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index 2ebe3715bb..27df0a72af 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -19,7 +19,7 @@ LongHistogramAggregation::LongHistogramAggregation() count_(0) {} -void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept +void LongHistogramAggregation::Aggregate(long value) noexcept { const std::lock_guard locked(lock_); count_ += 1; @@ -50,7 +50,7 @@ DoubleHistogramAggregation::DoubleHistogramAggregation() count_(0) {} -void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept +void DoubleHistogramAggregation::Aggregate(double value) noexcept { const std::lock_guard locked(lock_); count_ += 1; diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index 290b526344..ecee258682 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -16,7 +16,7 @@ namespace metrics LongLastValueAggregation::LongLastValueAggregation() : is_lastvalue_valid_(false) {} -void LongLastValueAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept +void LongLastValueAggregation::Aggregate(long value) noexcept { const std::lock_guard locked(lock_); is_lastvalue_valid_ = true; @@ -37,7 +37,7 @@ PointType LongLastValueAggregation::Collect() noexcept DoubleLastValueAggregation::DoubleLastValueAggregation() : is_lastvalue_valid_(false) {} -void DoubleLastValueAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept +void DoubleLastValueAggregation::Aggregate(double value) noexcept { const std::lock_guard locked(lock_); is_lastvalue_valid_ = true; diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index e0d1330f68..5809a32413 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -19,7 +19,7 @@ LongSumAggregation::LongSumAggregation(bool is_monotonic) sum_(0l) {} -void LongSumAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept +void LongSumAggregation::Aggregate(long value) noexcept { const std::lock_guard locked(lock_); sum_ += value; @@ -44,7 +44,7 @@ DoubleSumAggregation::DoubleSumAggregation(bool is_monotonic) sum_(0L) {} -void DoubleSumAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept +void DoubleSumAggregation::Aggregate(double value) noexcept { const std::lock_guard locked(lock_); sum_ += value; diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index ec753111ff..fdf9b6090d 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -20,8 +20,8 @@ TEST(Aggregation, LongSumAggregation) ASSERT_TRUE(nostd::holds_alternative(sum_data.value_)); EXPECT_EQ(nostd::get(sum_data.value_), 0l); EXPECT_EQ(sum_data.is_monotonic_, true); - EXPECT_NO_THROW(aggr.Aggregate(12l, {})); - EXPECT_NO_THROW(aggr.Aggregate(0l, {})); + EXPECT_NO_THROW(aggr.Aggregate(12l)); + EXPECT_NO_THROW(aggr.Aggregate(0l)); sum_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(sum_data.value_), 12l); } @@ -35,8 +35,8 @@ TEST(Aggregation, DoubleSumAggregation) ASSERT_TRUE(nostd::holds_alternative(sum_data.value_)); EXPECT_EQ(nostd::get(sum_data.value_), 0); EXPECT_EQ(sum_data.is_monotonic_, true); - EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); - EXPECT_NO_THROW(aggr.Aggregate(1.0, {})); + EXPECT_NO_THROW(aggr.Aggregate(12.0)); + EXPECT_NO_THROW(aggr.Aggregate(1.0)); sum_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(sum_data.value_), 13.0); } @@ -49,8 +49,8 @@ TEST(Aggregation, LongLastValueAggregation) auto lastvalue_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(lastvalue_data.value_)); EXPECT_EQ(lastvalue_data.is_lastvalue_valid_, false); - EXPECT_NO_THROW(aggr.Aggregate(12l, {})); - EXPECT_NO_THROW(aggr.Aggregate(1l, {})); + EXPECT_NO_THROW(aggr.Aggregate(12l)); + EXPECT_NO_THROW(aggr.Aggregate(1l)); lastvalue_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(lastvalue_data.value_), 1.0); } @@ -63,8 +63,8 @@ TEST(Aggregation, DoubleLastValueAggregation) auto lastvalue_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(lastvalue_data.value_)); EXPECT_EQ(lastvalue_data.is_lastvalue_valid_, false); - EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); - EXPECT_NO_THROW(aggr.Aggregate(1.0, {})); + EXPECT_NO_THROW(aggr.Aggregate(12.0)); + EXPECT_NO_THROW(aggr.Aggregate(1.0)); lastvalue_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(lastvalue_data.value_), 1.0); } @@ -79,15 +79,15 @@ TEST(Aggregation, LongHistogramAggregation) ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); EXPECT_EQ(nostd::get(histogram_data.sum_), 0); EXPECT_EQ(histogram_data.count_, 0); - EXPECT_NO_THROW(aggr.Aggregate(12l, {})); // lies in fourth bucket - EXPECT_NO_THROW(aggr.Aggregate(100l, {})); // lies in eight bucket + EXPECT_NO_THROW(aggr.Aggregate(12l)); // lies in fourth bucket + EXPECT_NO_THROW(aggr.Aggregate(100l)); // lies in eight bucket histogram_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(histogram_data.sum_), 112); EXPECT_EQ(histogram_data.count_, 2); EXPECT_EQ(histogram_data.counts_[3], 1); EXPECT_EQ(histogram_data.counts_[7], 1); - EXPECT_NO_THROW(aggr.Aggregate(13l, {})); // lies in fourth bucket - EXPECT_NO_THROW(aggr.Aggregate(252l, {})); // lies in ninth bucket + EXPECT_NO_THROW(aggr.Aggregate(13l)); // lies in fourth bucket + EXPECT_NO_THROW(aggr.Aggregate(252l)); // lies in ninth bucket histogram_data = nostd::get(aggr.Collect()); EXPECT_EQ(histogram_data.count_, 4); EXPECT_EQ(histogram_data.counts_[3], 2); @@ -104,15 +104,15 @@ TEST(Aggregation, DoubleHistogramAggregation) ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); EXPECT_EQ(nostd::get(histogram_data.sum_), 0); EXPECT_EQ(histogram_data.count_, 0); - EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); // lies in fourth bucket - EXPECT_NO_THROW(aggr.Aggregate(100.0, {})); // lies in eight bucket + EXPECT_NO_THROW(aggr.Aggregate(12.0)); // lies in fourth bucket + EXPECT_NO_THROW(aggr.Aggregate(100.0)); // lies in eight bucket histogram_data = nostd::get(aggr.Collect()); EXPECT_EQ(nostd::get(histogram_data.sum_), 112); EXPECT_EQ(histogram_data.count_, 2); EXPECT_EQ(histogram_data.counts_[3], 1); EXPECT_EQ(histogram_data.counts_[7], 1); - EXPECT_NO_THROW(aggr.Aggregate(13.0, {})); // lies in fourth bucket - EXPECT_NO_THROW(aggr.Aggregate(252.0, {})); // lies in ninth bucket + EXPECT_NO_THROW(aggr.Aggregate(13.0)); // lies in fourth bucket + EXPECT_NO_THROW(aggr.Aggregate(252.0)); // lies in ninth bucket histogram_data = nostd::get(aggr.Collect()); EXPECT_EQ(histogram_data.count_, 4); EXPECT_EQ(histogram_data.counts_[3], 2); diff --git a/sdk/test/metrics/view_registry_test.cc b/sdk/test/metrics/view_registry_test.cc index c3a9923c50..7828f48be7 100644 --- a/sdk/test/metrics/view_registry_test.cc +++ b/sdk/test/metrics/view_registry_test.cc @@ -6,6 +6,7 @@ # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/view/predicate.h" +#include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" # include From 77171831b99959a54f99c312cd5617396afc237d Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 7 Feb 2022 16:13:26 -0800 Subject: [PATCH 2/2] format --- .../aggregator/histogram_aggregator.h | 2 +- .../sdk/metrics/data/metric_data.h | 2 +- .../sdk/metrics/data/point_data.h | 2 +- .../opentelemetry/sdk/metrics/instruments.h | 3 +- .../sdk/metrics/state/metric_storage.h | 2 +- .../sdk/metrics/state/sync_metric_storage.h | 163 ++++++++++-------- .../sdk/metrics/view/attributes_processor.h | 2 +- .../opentelemetry/sdk/metrics/view/view.h | 7 +- sdk/test/metrics/view_registry_test.cc | 2 +- 9 files changed, 100 insertions(+), 85 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h b/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h index 27ff29a42c..17effcf6e4 100644 --- a/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h +++ b/sdk/include/opentelemetry/sdk/_metrics/aggregator/histogram_aggregator.h @@ -9,8 +9,8 @@ # include # include # include "opentelemetry/_metrics/instrument.h" -# include "opentelemetry/sdk/metrics/" # include "opentelemetry/sdk/_metrics/aggregator/aggregator.h" +# include "opentelemetry/sdk/metrics/" # include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h index 2add5859ce..2c54307510 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h @@ -17,7 +17,7 @@ namespace sdk namespace metrics { -using PointType = opentelemetry::nostd:: +using PointType = opentelemetry::nostd:: variant; class MetricData diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index b49f6f4af6..603bcc30bc 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -5,7 +5,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/timestamp.h" # include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/version.h" # include diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 42a1217bf9..07e832bad2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -4,7 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/sdk/common/attribute_utils.h" +# include "opentelemetry/sdk/common/attribute_utils.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -57,7 +57,6 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; - /*class InstrumentSelector { public: InstrumentSelector(opentelemetry::nostd::string_view name, diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index a168987020..7282a90a3f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -4,7 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/key_value_iterable_view.h" -#include "opentelemetry/sdk/metrics/data/metric_data.h" +# include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index ce07843e98..8c9c242877 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -4,10 +4,10 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/key_value_iterable_view.h" -# include "opentelemetry/sdk/common/attributemap_hash.h" +# include "opentelemetry/sdk/common/attributemap_hash.h" +# include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" # include "opentelemetry/sdk/metrics/state/metric_storage.h" # include "opentelemetry/sdk/metrics/view/attributes_processor.h" -#include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" # include "opentelemetry/sdk/metrics/view/view.h" # include "opentelemetry/sdk/resource/resource.h" @@ -24,62 +24,71 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage SyncMetricStorage( InstrumentDescriptor instrument_descriptor, const AggregationType aggregation_type, - const AttributesProcessor* attributes_processor = new DefaultAttributesProcessor() - ): aggregation_type_{aggregation_type}, instrument_descriptor_{instrument_descriptor}, - attributes_processor_{attributes_processor} + const AttributesProcessor *attributes_processor = new DefaultAttributesProcessor()) + : aggregation_type_{aggregation_type}, + instrument_descriptor_{instrument_descriptor}, + attributes_processor_{attributes_processor} {} void RecordLong(long value) noexcept override { - if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { - return ; - } - size_t hashcode = 0 ; //for empty attributs - if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ - attributes_aggregation_[hashcode] = std::move(create_aggregation()); - } - attributes_aggregation_[hashcode]->Aggregate(value); + if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) + { + return; + } + size_t hashcode = 0; // for empty attributs + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()) + { + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); } void RecordLong(long value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override { - if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) { - return ; - } - auto attr = attributes_processor_->process(attributes); - auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); - if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ - attributes_aggregation_[hashcode] = std::move(create_aggregation()); - } - attributes_aggregation_[hashcode]->Aggregate(value); + if (instrument_descriptor_.value_type_ != InstrumentValueType::kLong) + { + return; + } + auto attr = attributes_processor_->process(attributes); + auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()) + { + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); } void RecordDouble(double value) noexcept override { - if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) { - return ; - } - size_t hashcode = 0 ; //for empty attributs - if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ - attributes_aggregation_[hashcode] = std::move(create_aggregation()); - } - attributes_aggregation_[hashcode]->Aggregate(value); + if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) + { + return; + } + size_t hashcode = 0; // for empty attributs + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()) + { + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); } void RecordDouble(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override { - if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) { - return ; - } + if (instrument_descriptor_.value_type_ != InstrumentValueType::kDouble) + { + return; + } - auto attr = attributes_processor_->process(attributes); - auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); - if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()){ - attributes_aggregation_[hashcode] = std::move(create_aggregation()); - } - attributes_aggregation_[hashcode]->Aggregate(value); + auto attr = attributes_processor_->process(attributes); + auto hashcode = opentelemetry::sdk::common::GetHashForAttributeMap(attr); + if (attributes_aggregation_.find(hashcode) == attributes_aggregation_.end()) + { + attributes_aggregation_[hashcode] = std::move(create_aggregation()); + } + attributes_aggregation_[hashcode]->Aggregate(value); } bool Collect(AggregationTemporarily aggregation_temporarily, @@ -93,42 +102,52 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } private: - InstrumentDescriptor instrument_descriptor_; - AggregationType aggregation_type_; - std::unordered_map> attributes_aggregation_; - const AttributesProcessor* attributes_processor_; + InstrumentDescriptor instrument_descriptor_; + AggregationType aggregation_type_; + std::unordered_map> attributes_aggregation_; + const AttributesProcessor *attributes_processor_; - std::unique_ptr create_aggregation() { - switch (aggregation_type_) { - case AggregationType::kDrop: - return std::move(std::unique_ptr(new DropAggregation())); - break; - case AggregationType::kHistogram: - if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { - return std::move(std::unique_ptr(new LongHistogramAggregation())); - } else { - return std::move(std::unique_ptr(new DoubleHistogramAggregation())); - } - break; - case AggregationType::kLastValue: - if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { - return std::move(std::unique_ptr(new LongLastValueAggregation())); - } else { - return std::move(std::unique_ptr(new DoubleLastValueAggregation())); - } - break; - case AggregationType::kSum: - if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { - return std::move(std::unique_ptr(new LongSumAggregation(true))); - } else { - return std::move(std::unique_ptr(new DoubleSumAggregation(true))); - } - break; - default: - return std::move(DefaultAggregation::CreateAggregation(instrument_descriptor_)); + std::unique_ptr create_aggregation() + { + switch (aggregation_type_) + { + case AggregationType::kDrop: + return std::move(std::unique_ptr(new DropAggregation())); + break; + case AggregationType::kHistogram: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) + { + return std::move(std::unique_ptr(new LongHistogramAggregation())); + } + else + { + return std::move(std::unique_ptr(new DoubleHistogramAggregation())); } + break; + case AggregationType::kLastValue: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) + { + return std::move(std::unique_ptr(new LongLastValueAggregation())); + } + else + { + return std::move(std::unique_ptr(new DoubleLastValueAggregation())); + } + break; + case AggregationType::kSum: + if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) + { + return std::move(std::unique_ptr(new LongSumAggregation(true))); + } + else + { + return std::move(std::unique_ptr(new DoubleSumAggregation(true))); + } + break; + default: + return std::move(DefaultAggregation::CreateAggregation(instrument_descriptor_)); } - + } }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h index 756e97eb20..7979c79767 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h @@ -4,7 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/common/attribute_utils.h" -# include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/instruments.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { diff --git a/sdk/include/opentelemetry/sdk/metrics/view/view.h b/sdk/include/opentelemetry/sdk/metrics/view/view.h index a0f1066a76..a758ec6a20 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/view.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/view.h @@ -21,7 +21,7 @@ class View { public: View(const std::string &name, - const std::string &description = "", + const std::string &description = "", AggregationType aggregation_type = AggregationType::kDefault, std::unique_ptr attributes_processor = std::unique_ptr( @@ -36,10 +36,7 @@ class View virtual std::string GetDescription() const noexcept { return description_; } - virtual const AggregationType &GetAggregationType() const noexcept - { - return aggregation_type_; - } + virtual const AggregationType &GetAggregationType() const noexcept { return aggregation_type_; } virtual const opentelemetry::sdk::metrics::AttributesProcessor &GetAttributesProcessor() const noexcept diff --git a/sdk/test/metrics/view_registry_test.cc b/sdk/test/metrics/view_registry_test.cc index 7828f48be7..1de8615214 100644 --- a/sdk/test/metrics/view_registry_test.cc +++ b/sdk/test/metrics/view_registry_test.cc @@ -5,8 +5,8 @@ # include "opentelemetry/sdk/metrics/view/view_registry.h" # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" # include "opentelemetry/sdk/metrics/view/predicate.h" -#include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" # include