From 0ba0f45edbfc086e97c240e2b2fc604d45e43261 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Fri, 6 May 2022 20:22:30 +0000 Subject: [PATCH 1/2] Prometheus exporter meters and instrument name --- .../exporters/prometheus/exporter_utils.h | 13 --- exporters/prometheus/src/exporter_utils.cc | 94 ++++++++----------- sdk/src/metrics/meter.cc | 22 ++++- 3 files changed, 56 insertions(+), 73 deletions(-) diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h index 1cb7202ce5..cfbbb9c48b 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter_utils.h @@ -34,12 +34,6 @@ class PrometheusExporterUtils const std::vector> &data); private: - /** - * Set value to metric family according to record - */ - static void SetMetricFamily(sdk::metrics::ResourceMetrics &data, - ::prometheus::MetricFamily *metric_family); - /** * Sanitize the given metric name or label according to Prometheus rule. * @@ -49,13 +43,6 @@ class PrometheusExporterUtils */ static std::string SanitizeNames(std::string name); - /** - * Set value to metric family for different aggregator - */ - static void SetMetricFamilyByAggregator(const sdk::metrics::ResourceMetrics &data, - std::string labels_str, - ::prometheus::MetricFamily *metric_family); - static opentelemetry::sdk::metrics::AggregationType getAggregationType( const opentelemetry::sdk::metrics::PointType &point_type); diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index 39131e210f..d78e2286c4 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -34,28 +34,52 @@ std::vector PrometheusExporterUtils::TranslateT } // initialize output vector - std::vector output(data.size()); + std::vector output; // iterate through the vector and set result data into it - int i = 0; for (const auto &r : data) { - SetMetricFamily(*r, &output[i]); - i++; + for (const auto &instrumentation_info : r->instrumentation_info_metric_data_) + { + for (const auto &metric_data : instrumentation_info.metric_data_) + { + auto origin_name = metric_data.instrument_descriptor.name_; + auto sanitized = SanitizeNames(origin_name); + std::puts(sanitized.c_str()); + prometheus_client::MetricFamily metric_family; + metric_family.name = sanitized; + metric_family.help = metric_data.instrument_descriptor.description_; + auto time = metric_data.start_ts.time_since_epoch(); + for (const auto &point_data_attr : metric_data.point_data_attr_) + { + auto kind = getAggregationType(point_data_attr.point_data); + const prometheus_client::MetricType type = TranslateType(kind); + metric_family.type = type; + if (type == prometheus_client::MetricType::Histogram) // Histogram + { + auto histogram_point_data = + nostd::get(point_data_attr.point_data); + auto boundaries = histogram_point_data.boundaries_; + auto counts = histogram_point_data.counts_; + SetData(std::vector{nostd::get(histogram_point_data.sum_), + (double)histogram_point_data.count_}, + boundaries, counts, "", time, &metric_family); + } + else // Counter, Untyped + { + auto sum_point_data = + nostd::get(point_data_attr.point_data); + std::vector values{sum_point_data.value_}; + SetData(values, "", type, time, &metric_family); + } + } + output.emplace_back(metric_family); + } + } } - return output; } -/** - * Set value to metric family according to record - */ -void PrometheusExporterUtils::SetMetricFamily(sdk::metrics::ResourceMetrics &data, - prometheus_client::MetricFamily *metric_family) -{ - SetMetricFamilyByAggregator(data, "", metric_family); -} - /** * Sanitize the given metric name or label according to Prometheus rule. * @@ -72,48 +96,6 @@ std::string PrometheusExporterUtils::SanitizeNames(std::string name) return name; } -/** - * Set value to metric family for different aggregator - */ -void PrometheusExporterUtils::SetMetricFamilyByAggregator( - const sdk::metrics::ResourceMetrics &data, - std::string labels_str, - prometheus_client::MetricFamily *metric_family) -{ - for (const auto &instrumentation_info : data.instrumentation_info_metric_data_) - { - auto origin_name = instrumentation_info.instrumentation_library_->GetName(); - auto sanitized = SanitizeNames(origin_name); - metric_family->name = sanitized; - for (const auto &metric_data : instrumentation_info.metric_data_) - { - auto time = metric_data.start_ts.time_since_epoch(); - for (const auto &point_data_attr : metric_data.point_data_attr_) - { - auto kind = getAggregationType(point_data_attr.point_data); - const prometheus_client::MetricType type = TranslateType(kind); - metric_family->type = type; - if (type == prometheus_client::MetricType::Histogram) // Histogram - { - auto histogram_point_data = - nostd::get(point_data_attr.point_data); - auto boundaries = histogram_point_data.boundaries_; - auto counts = histogram_point_data.counts_; - SetData(std::vector{nostd::get(histogram_point_data.sum_), - (double)histogram_point_data.count_}, - boundaries, counts, labels_str, time, metric_family); - } - else // Counter, Untyped - { - auto sum_point_data = nostd::get(point_data_attr.point_data); - std::vector values{sum_point_data.value_}; - SetData(values, labels_str, type, time, metric_family); - } - } - } - } -} - metric_sdk::AggregationType PrometheusExporterUtils::getAggregationType( const metric_sdk::PointType &point_type) { diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index c0b0ce7a57..712d8849be 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -188,10 +188,24 @@ std::unique_ptr Meter::RegisterMetricStorage( auto success = view_registry->FindViews( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor, &storages](const View &view) { - auto view_instr_desc = instrument_descriptor; - view_instr_desc.name_ = view.GetName(); - view_instr_desc.description_ = view.GetDescription(); - auto storage = std::shared_ptr(new SyncMetricStorage( + auto view_instr_desc = instrument_descriptor; + if (view.GetName().empty()) + { + view_instr_desc.name_ = instrument_descriptor.name_; + } + else + { + view_instr_desc.name_ = view.GetName(); + } + if (view.GetDescription().empty()) + { + view_instr_desc.description_ = instrument_descriptor.description_; + } + else + { + view_instr_desc.description_ = view.GetDescription(); + } + auto storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), NoExemplarReservoir::GetNoExemplarReservoir())); storage_registry_[instrument_descriptor.name_] = storage; From 9de5f2b4584c0d71a5d4cced03346ed729f28cef Mon Sep 17 00:00:00 2001 From: Oblivion Date: Sat, 7 May 2022 07:42:29 +0000 Subject: [PATCH 2/2] review comments --- exporters/prometheus/src/exporter_utils.cc | 1 - sdk/src/metrics/meter.cc | 12 ++---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index d78e2286c4..9979c9bebd 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -45,7 +45,6 @@ std::vector PrometheusExporterUtils::TranslateT { auto origin_name = metric_data.instrument_descriptor.name_; auto sanitized = SanitizeNames(origin_name); - std::puts(sanitized.c_str()); prometheus_client::MetricFamily metric_family; metric_family.name = sanitized; metric_family.help = metric_data.instrument_descriptor.description_; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 712d8849be..ce638c22e6 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -189,19 +189,11 @@ std::unique_ptr Meter::RegisterMetricStorage( instrument_descriptor, *instrumentation_library_, [this, &instrument_descriptor, &storages](const View &view) { auto view_instr_desc = instrument_descriptor; - if (view.GetName().empty()) - { - view_instr_desc.name_ = instrument_descriptor.name_; - } - else + if (!view.GetName().empty()) { view_instr_desc.name_ = view.GetName(); } - if (view.GetDescription().empty()) - { - view_instr_desc.description_ = instrument_descriptor.description_; - } - else + if (!view.GetDescription().empty()) { view_instr_desc.description_ = view.GetDescription(); }