From 4d2bc6dfbb1b196018e228a35d0465cdbf3ccb35 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 13 Jul 2023 22:13:30 -0700 Subject: [PATCH] changes --- examples/metrics_simple/metrics_ostream.cc | 16 ++++++----- .../sdk/metrics/view/instrument_selector.h | 9 ++++-- .../view/instrument_selector_factory.h | 3 +- .../opentelemetry/sdk/metrics/view/view.h | 3 ++ .../sdk/metrics/view/view_factory.h | 7 +++++ .../sdk/metrics/view/view_registry.h | 1 + .../view/instrument_selector_factory.cc | 5 ++-- sdk/src/metrics/view/view_factory.cc | 18 +++++++++--- .../metrics/histogram_aggregation_test.cc | 7 +++-- sdk/test/metrics/histogram_test.cc | 22 +++++++++------ sdk/test/metrics/meter_provider_sdk_test.cc | 2 +- sdk/test/metrics/sum_aggregation_test.cc | 28 ++++++++++++------- sdk/test/metrics/view_registry_test.cc | 5 ++-- 13 files changed, 86 insertions(+), 40 deletions(-) diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index a81937099d..7248bc9148 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -53,14 +53,15 @@ void InitMetrics(const std::string &name) // counter view std::string counter_name = name + "_counter"; + std::string unit = "counter-unit"; auto instrument_selector = metrics_sdk::InstrumentSelectorFactory::Create( - metrics_sdk::InstrumentType::kCounter, counter_name); + metrics_sdk::InstrumentType::kCounter, counter_name, unit); auto meter_selector = metrics_sdk::MeterSelectorFactory::Create(name, version, schema); - auto sum_view = - metrics_sdk::ViewFactory::Create(name, "description", metrics_sdk::AggregationType::kSum); + auto sum_view = metrics_sdk::ViewFactory::Create(name, "description", unit, + metrics_sdk::AggregationType::kSum); p->AddView(std::move(instrument_selector), std::move(meter_selector), std::move(sum_view)); @@ -68,11 +69,11 @@ void InitMetrics(const std::string &name) std::string observable_counter_name = name + "_observable_counter"; auto observable_instrument_selector = metrics_sdk::InstrumentSelectorFactory::Create( - metrics_sdk::InstrumentType::kObservableCounter, observable_counter_name); + metrics_sdk::InstrumentType::kObservableCounter, observable_counter_name, unit); auto observable_meter_selector = metrics_sdk::MeterSelectorFactory::Create(name, version, schema); - auto observable_sum_view = metrics_sdk::ViewFactory::Create(name, "test_description", + auto observable_sum_view = metrics_sdk::ViewFactory::Create(name, "test_description", unit, metrics_sdk::AggregationType::kSum); p->AddView(std::move(observable_instrument_selector), std::move(observable_meter_selector), @@ -80,9 +81,10 @@ void InitMetrics(const std::string &name) // histogram view std::string histogram_name = name + "_histogram"; + unit = "histogram-unit"; auto histogram_instrument_selector = metrics_sdk::InstrumentSelectorFactory::Create( - metrics_sdk::InstrumentType::kHistogram, histogram_name); + metrics_sdk::InstrumentType::kHistogram, histogram_name, unit); auto histogram_meter_selector = metrics_sdk::MeterSelectorFactory::Create(name, version, schema); @@ -96,7 +98,7 @@ void InitMetrics(const std::string &name) std::move(histogram_aggregation_config)); auto histogram_view = metrics_sdk::ViewFactory::Create( - name, "description", metrics_sdk::AggregationType::kHistogram, aggregation_config); + name, "description", unit, metrics_sdk::AggregationType::kHistogram, aggregation_config); p->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), std::move(histogram_view)); diff --git a/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector.h b/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector.h index 2055ed96fa..5f71de474c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector.h @@ -19,19 +19,24 @@ class InstrumentSelector { public: InstrumentSelector(opentelemetry::sdk::metrics::InstrumentType instrument_type, - opentelemetry::nostd::string_view name) + opentelemetry::nostd::string_view name, + opentelemetry::nostd::string_view units) : name_filter_{PredicateFactory::GetPredicate(name, PredicateType::kPattern)}, + unit_filter_{PredicateFactory::GetPredicate(units, PredicateType::kExact)}, instrument_type_{instrument_type} + {} // Returns name filter predicate. This shouldn't be deleted const opentelemetry::sdk::metrics::Predicate *GetNameFilter() const { return name_filter_.get(); } - + // Returns unit filter predicate. This shouldn't be deleted + const opentelemetry::sdk::metrics::Predicate *GetUnitFilter() const { return unit_filter_.get(); } // Returns instrument filter. InstrumentType GetInstrumentType() { return instrument_type_; } private: std::unique_ptr name_filter_; + std::unique_ptr unit_filter_; opentelemetry::sdk::metrics::InstrumentType instrument_type_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector_factory.h b/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector_factory.h index e5b8b83704..f30b1e27c1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector_factory.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/instrument_selector_factory.h @@ -19,7 +19,8 @@ class InstrumentSelectorFactory public: static std::unique_ptr Create( opentelemetry::sdk::metrics::InstrumentType instrument_type, - opentelemetry::nostd::string_view name); + opentelemetry::nostd::string_view name, + opentelemetry::nostd::string_view unit); }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/view/view.h b/sdk/include/opentelemetry/sdk/metrics/view/view.h index 2d3edcee0e..dc0888f47d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/view.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/view.h @@ -26,6 +26,7 @@ class View public: View(const std::string &name, const std::string &description = "", + const std::string &unit = "", AggregationType aggregation_type = AggregationType::kDefault, std::shared_ptr aggregation_config = nullptr, std::unique_ptr attributes_processor = @@ -33,6 +34,7 @@ class View new opentelemetry::sdk::metrics::DefaultAttributesProcessor())) : name_(name), description_(description), + unit_(unit), aggregation_type_{aggregation_type}, aggregation_config_{aggregation_config}, attributes_processor_{std::move(attributes_processor)} @@ -60,6 +62,7 @@ class View private: std::string name_; std::string description_; + std::string unit_; AggregationType aggregation_type_; std::shared_ptr aggregation_config_; std::unique_ptr attributes_processor_; diff --git a/sdk/include/opentelemetry/sdk/metrics/view/view_factory.h b/sdk/include/opentelemetry/sdk/metrics/view/view_factory.h index 5e7da2ea62..22d34bf506 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/view_factory.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/view_factory.h @@ -30,15 +30,22 @@ class OPENTELEMETRY_EXPORT ViewFactory static std::unique_ptr Create(const std::string &name, const std::string &description, + const std::string &unit); + + static std::unique_ptr Create(const std::string &name, + const std::string &description, + const std::string &unit, AggregationType aggregation_type); static std::unique_ptr Create(const std::string &name, const std::string &description, + const std::string &unit, AggregationType aggregation_type, std::shared_ptr aggregation_config); static std::unique_ptr Create(const std::string &name, const std::string &description, + const std::string &unit, AggregationType aggregation_type, std::shared_ptr aggregation_config, std::unique_ptr attributes_processor); diff --git a/sdk/include/opentelemetry/sdk/metrics/view/view_registry.h b/sdk/include/opentelemetry/sdk/metrics/view/view_registry.h index d017bd7107..40343c28de 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/view_registry.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/view_registry.h @@ -99,6 +99,7 @@ class ViewRegistry const opentelemetry::sdk::metrics::InstrumentDescriptor &instrument_descriptor) { return selector->GetNameFilter()->Match(instrument_descriptor.name_) && + selector->GetUnitFilter()->Match(instrument_descriptor.unit_) && (selector->GetInstrumentType() == instrument_descriptor.type_); } }; diff --git a/sdk/src/metrics/view/instrument_selector_factory.cc b/sdk/src/metrics/view/instrument_selector_factory.cc index 24542a57cd..98a587ea62 100644 --- a/sdk/src/metrics/view/instrument_selector_factory.cc +++ b/sdk/src/metrics/view/instrument_selector_factory.cc @@ -13,10 +13,11 @@ namespace metrics std::unique_ptr InstrumentSelectorFactory::Create( opentelemetry::sdk::metrics::InstrumentType instrument_type, - opentelemetry::nostd::string_view name) + opentelemetry::nostd::string_view name, + opentelemetry::nostd::string_view unit) { std::unique_ptr instrument_selector( - new InstrumentSelector(instrument_type, name)); + new InstrumentSelector(instrument_type, name, unit)); return instrument_selector; } diff --git a/sdk/src/metrics/view/view_factory.cc b/sdk/src/metrics/view/view_factory.cc index 2e410accc6..24d87769f9 100644 --- a/sdk/src/metrics/view/view_factory.cc +++ b/sdk/src/metrics/view/view_factory.cc @@ -18,36 +18,46 @@ std::unique_ptr ViewFactory::Create(const std::string &name) std::unique_ptr ViewFactory::Create(const std::string &name, const std::string &description) { - return Create(name, description, AggregationType::kDefault); + return Create(name, description, "", AggregationType::kDefault); } std::unique_ptr ViewFactory::Create(const std::string &name, const std::string &description, + const std::string &unit) +{ + return Create(name, description, unit, AggregationType::kDefault); +} + +std::unique_ptr ViewFactory::Create(const std::string &name, + const std::string &description, + const std::string &unit, AggregationType aggregation_type) { std::shared_ptr aggregation_config(nullptr); - return Create(name, description, aggregation_type, aggregation_config); + return Create(name, description, unit, aggregation_type, aggregation_config); } std::unique_ptr ViewFactory::Create(const std::string &name, const std::string &description, + const std::string &unit, AggregationType aggregation_type, std::shared_ptr aggregation_config) { auto attributes_processor = std::unique_ptr(new DefaultAttributesProcessor()); - return Create(name, description, aggregation_type, aggregation_config, + return Create(name, description, unit, aggregation_type, aggregation_config, std::move(attributes_processor)); } std::unique_ptr ViewFactory::Create(const std::string &name, const std::string &description, + const std::string &unit, AggregationType aggregation_type, std::shared_ptr aggregation_config, std::unique_ptr attributes_processor) { - std::unique_ptr view(new View(name, description, aggregation_type, aggregation_config, + std::unique_ptr view(new View(name, description, unit, aggregation_type, aggregation_config, std::move(attributes_processor))); return view; } diff --git a/sdk/test/metrics/histogram_aggregation_test.cc b/sdk/test/metrics/histogram_aggregation_test.cc index fada036747..6fd13c902c 100644 --- a/sdk/test/metrics/histogram_aggregation_test.cc +++ b/sdk/test/metrics/histogram_aggregation_test.cc @@ -72,13 +72,14 @@ TEST(CounterToHistogram, Double) std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; mp.AddMetricReader(reader); - std::unique_ptr view{new View("view1", "view1_description", AggregationType::kHistogram)}; + std::unique_ptr view{ + new View("view1", "view1_description", "unit", AggregationType::kHistogram)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kCounter, "counter1")}; + new InstrumentSelector(InstrumentType::kCounter, "counter1", "unit")}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); - auto h = m->CreateDoubleCounter("counter1", "counter1_description", "counter1_unit"); + auto h = m->CreateDoubleCounter("counter1", "counter1_description", "unit"); h->Add(5, {}); h->Add(10, {}); diff --git a/sdk/test/metrics/histogram_test.cc b/sdk/test/metrics/histogram_test.cc index ef2d023c92..a11a9c1f59 100644 --- a/sdk/test/metrics/histogram_test.cc +++ b/sdk/test/metrics/histogram_test.cc @@ -78,7 +78,10 @@ TEST(Histogram, Double) TEST(Histogram, DoubleCustomBuckets) { MeterProvider mp; - auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_unit = "ms"; + std::string instrument_name = "historgram1"; + std::string instrument_desc = "histogram metrics"; std::unique_ptr exporter(new MockMetricExporter()); std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; @@ -87,13 +90,13 @@ TEST(Histogram, DoubleCustomBuckets) std::shared_ptr config(new HistogramAggregationConfig()); config->boundaries_ = {10, 20, 30, 40}; std::unique_ptr view{ - new View("view1", "view1_description", AggregationType::kHistogram, config)}; + new View("view1", "view1_description", instrument_unit, AggregationType::kHistogram, config)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kHistogram, "histogram1")}; + new InstrumentSelector(InstrumentType::kHistogram, instrument_name, instrument_unit)}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); - auto h = m->CreateDoubleHistogram("histogram1", "histogram1_description", "histogram1_unit"); + auto h = m->CreateDoubleHistogram(instrument_name, instrument_desc, instrument_unit); h->Record(5, {}); h->Record(10, {}); @@ -190,7 +193,10 @@ TEST(Histogram, UInt64) TEST(Histogram, UInt64CustomBuckets) { MeterProvider mp; - auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_name = "historgram1"; + std::string instrument_desc = "histogram metrics"; + std::string instrument_unit = "ms"; std::unique_ptr exporter(new MockMetricExporter()); std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; @@ -199,13 +205,13 @@ TEST(Histogram, UInt64CustomBuckets) std::shared_ptr config(new HistogramAggregationConfig()); config->boundaries_ = {10, 20, 30, 40}; std::unique_ptr view{ - new View("view1", "view1_description", AggregationType::kHistogram, config)}; + new View("view1", "view1_description", "ms", AggregationType::kHistogram, config)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kHistogram, "histogram1")}; + new InstrumentSelector(InstrumentType::kHistogram, instrument_name, instrument_unit)}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); - auto h = m->CreateUInt64Histogram("histogram1", "histogram1_description", "histogram1_unit"); + auto h = m->CreateUInt64Histogram(instrument_name, instrument_desc, instrument_unit); h->Record(5, {}); h->Record(10, {}); diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index da75916555..a1ca8de330 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -49,7 +49,7 @@ TEST(MeterProvider, GetMeter) std::unique_ptr view{std::unique_ptr()}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kCounter, "instru1")}; + new InstrumentSelector(InstrumentType::kCounter, "instru1", "unit1")}; std::unique_ptr meter_selector{new MeterSelector("name1", "version1", "schema1")}; mp1.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); diff --git a/sdk/test/metrics/sum_aggregation_test.cc b/sdk/test/metrics/sum_aggregation_test.cc index 9fb1804022..4efad36f95 100644 --- a/sdk/test/metrics/sum_aggregation_test.cc +++ b/sdk/test/metrics/sum_aggregation_test.cc @@ -22,19 +22,23 @@ using namespace opentelemetry::sdk::metrics; TEST(HistogramToSum, Double) { MeterProvider mp; - auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_unit = "ms"; + std::string instrument_name = "historgram1"; + std::string instrument_desc = "histogram metrics"; std::unique_ptr exporter(new MockMetricExporter()); std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; mp.AddMetricReader(reader); - std::unique_ptr view{new View("view1", "view1_description", AggregationType::kSum)}; + std::unique_ptr view{ + new View("view1", "view1_description", instrument_unit, AggregationType::kSum)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kHistogram, "histogram1")}; + new InstrumentSelector(InstrumentType::kHistogram, instrument_name, instrument_unit)}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); - auto h = m->CreateDoubleHistogram("histogram1", "histogram1_description", "histogram1_unit"); + auto h = m->CreateDoubleHistogram(instrument_name, instrument_desc, instrument_unit); h->Record(5, {}); h->Record(10, {}); @@ -77,9 +81,9 @@ TEST(CounterToSum, Double) std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; mp.AddMetricReader(reader); - std::unique_ptr view{new View("view1", "view1_description", AggregationType::kSum)}; + std::unique_ptr view{new View("view1", "view1_description", "ms", AggregationType::kSum)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kCounter, "counter1")}; + new InstrumentSelector(InstrumentType::kCounter, "counter1", "ms")}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); @@ -120,19 +124,23 @@ TEST(CounterToSum, Double) TEST(UpDownCounterToSum, Double) { MeterProvider mp; - auto m = mp.GetMeter("meter1", "version1", "schema1"); + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_name = "updowncounter1"; + std::string instrument_desc = "updowncounter desc"; + std::string instrument_unit = "ms"; std::unique_ptr exporter(new MockMetricExporter()); std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; mp.AddMetricReader(reader); - std::unique_ptr view{new View("view1", "view1_description", AggregationType::kSum)}; + std::unique_ptr view{ + new View("view1", "view1_description", instrument_unit, AggregationType::kSum)}; std::unique_ptr instrument_selector{ - new InstrumentSelector(InstrumentType::kUpDownCounter, "counter1")}; + new InstrumentSelector(InstrumentType::kUpDownCounter, instrument_name, instrument_unit)}; std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); - auto h = m->CreateDoubleUpDownCounter("counter1", "counter1_description", "counter1_unit"); + auto h = m->CreateDoubleUpDownCounter(instrument_name, instrument_desc, instrument_desc); h->Add(5, {}); h->Add(10, {}); diff --git a/sdk/test/metrics/view_registry_test.cc b/sdk/test/metrics/view_registry_test.cc index 59899dedb0..49e475a9f1 100644 --- a/sdk/test/metrics/view_registry_test.cc +++ b/sdk/test/metrics/view_registry_test.cc @@ -55,9 +55,10 @@ TEST(ViewRegistry, FindNonExistingView) const std::string instrumentation_schema = "schema1"; const std::string instrument_name = "testname"; const InstrumentType instrument_type = InstrumentType::kCounter; + const std::string instrument_unit = "ms"; std::unique_ptr instrument_selector{ - new InstrumentSelector(instrument_type, instrument_name)}; + new InstrumentSelector(instrument_type, instrument_name, instrument_unit)}; std::unique_ptr meter_selector{ new MeterSelector(instrumentation_name, instrumentation_version, instrumentation_schema)}; std::unique_ptr view = std::unique_ptr(new View(view_name, view_description)); @@ -66,7 +67,7 @@ TEST(ViewRegistry, FindNonExistingView) registry.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); InstrumentDescriptor default_instrument_descriptor = {instrument_name, // name "test_descr", // description - "1", // unit + instrument_unit, // unit instrument_type, // instrument type InstrumentValueType::kLong};