From e256ce4c50317a4c82f74db6c6476aeeab1722a8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 3 Aug 2022 19:06:04 -0700 Subject: [PATCH 1/5] agg tempo --- .../exporters/ostream/metric_exporter.h | 14 +++++++- exporters/ostream/src/metric_exporter.cc | 12 ++++++- .../otlp/otlp_grpc_metric_exporter.h | 8 +++++ .../otlp/otlp_grpc_metric_exporter_options.h | 3 ++ .../otlp/otlp_http_metric_exporter.h | 16 +++++++++ .../exporters/otlp/otlp_metric_utils.h | 7 ++++ .../otlp/src/otlp_grpc_metric_exporter.cc | 6 ++++ .../otlp/src/otlp_http_metric_exporter.cc | 8 ++++- exporters/otlp/src/otlp_metric_utils.cc | 35 ++++++++++++++++++- .../exporters/prometheus/exporter.h | 8 +++++ exporters/prometheus/src/exporter.cc | 7 ++++ .../export/periodic_exporting_metric_reader.h | 9 ++--- .../opentelemetry/sdk/metrics/instruments.h | 3 +- .../sdk/metrics/metric_exporter.h | 8 +++++ .../opentelemetry/sdk/metrics/metric_reader.h | 13 ++++--- .../sdk/metrics/state/metric_collector.h | 6 ++-- .../periodic_exporting_metric_reader.cc | 11 +++--- sdk/src/metrics/metric_reader.cc | 9 +---- sdk/src/metrics/state/metric_collector.cc | 5 +-- .../metrics/state/temporal_metric_storage.cc | 3 +- sdk/test/metrics/async_metric_storage_test.cc | 5 ++- sdk/test/metrics/meter_provider_sdk_test.cc | 11 ++++++ sdk/test/metrics/metric_reader_test.cc | 15 +++++--- .../periodic_exporting_metric_reader_test.cc | 6 ++++ sdk/test/metrics/sync_metric_storage_test.cc | 5 ++- 25 files changed, 196 insertions(+), 37 deletions(-) diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h index 5db433fe9a..b6a5f56d4e 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h @@ -9,6 +9,7 @@ # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" +# include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/version.h" @@ -29,7 +30,9 @@ class OStreamMetricExporter final : public opentelemetry::sdk::metrics::MetricEx * export() function will send metrics data into. * The default ostream is set to stdout */ - explicit OStreamMetricExporter(std::ostream &sout = std::cout) noexcept; + explicit OStreamMetricExporter(std::ostream &sout = std::cout, + sdk::metrics::AggregationTemporality aggregation_temporality = + sdk::metrics::AggregationTemporality::kCumulative) noexcept; /** * Export @@ -37,6 +40,14 @@ class OStreamMetricExporter final : public opentelemetry::sdk::metrics::MetricEx */ sdk::common::ExportResult Export(const sdk::metrics::ResourceMetrics &data) noexcept override; + /** + * Get the AggregationTemporality for ostream exporter + * + * @return AggregationTemporality + */ + sdk::metrics::AggregationTemporality GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept override; + /** * Force flush the exporter. */ @@ -55,6 +66,7 @@ class OStreamMetricExporter final : public opentelemetry::sdk::metrics::MetricEx std::ostream &sout_; bool is_shutdown_ = false; mutable opentelemetry::common::SpinLockMutex lock_; + sdk::metrics::AggregationTemporality aggregation_temporality_; bool isShutdown() const noexcept; void printInstrumentationInfoMetricData(const sdk::metrics::ScopeMetrics &info_metrics); void printPointData(const opentelemetry::sdk::metrics::PointType &point_data); diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 17f54bbb09..081f95185c 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -64,7 +64,17 @@ inline void printVec(std::ostream &os, Container &vec) os << ']'; } -OStreamMetricExporter::OStreamMetricExporter(std::ostream &sout) noexcept : sout_(sout) {} +OStreamMetricExporter::OStreamMetricExporter( + std::ostream &sout, + sdk::metrics::AggregationTemporality aggregation_temporality) noexcept + : sout_(sout), aggregation_temporality_(aggregation_temporality) +{} + +sdk::metrics::AggregationTemporality OStreamMetricExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + return aggregation_temporality_; +} sdk::common::ExportResult OStreamMetricExporter::Export( const sdk::metrics::ResourceMetrics &data) noexcept diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h index fa8ba66c14..1f107d209f 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h @@ -39,6 +39,14 @@ class OtlpGrpcMetricExporter : public opentelemetry::sdk::metrics::MetricExporte */ explicit OtlpGrpcMetricExporter(const OtlpGrpcMetricExporterOptions &options); + /** + * Get the AggregationTemporality for exporter + * + * @return AggregationTemporality + */ + sdk::metrics::AggregationTemporality GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept override; + opentelemetry::sdk::common::ExportResult Export( const opentelemetry::sdk::metrics::ResourceMetrics &data) noexcept override; diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h index a277052956..618a4a6821 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h @@ -21,6 +21,9 @@ struct OtlpGrpcMetricExporterOptions : public OtlpGrpcExporterOptions { opentelemetry::sdk::metrics::AggregationTemporality aggregation_temporality = opentelemetry::sdk::metrics::AggregationTemporality::kDelta; + + opentelemetry::sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = + OtlpMetricUtils::ChooseTemporalitySelector(); }; } // namespace otlp diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h index 311f68494e..b4530f115b 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h @@ -52,6 +52,11 @@ struct OtlpHttpMetricExporterOptions // Additional HTTP headers OtlpHeaders http_headers = GetOtlpDefaultMetricHeaders(); + // Preferred Aggregation Temporality + + sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = + otlp::OtlpMetricUtils::ChooseTemporalitySelector(); + # ifdef ENABLE_ASYNC_EXPORT // Concurrent requests // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md#otlpgrpc-concurrent-requests @@ -79,6 +84,14 @@ class OtlpHttpMetricExporter final : public opentelemetry::sdk::metrics::MetricE */ OtlpHttpMetricExporter(const OtlpHttpMetricExporterOptions &options); + /** + * Get the AggregationTemporality for exporter + * + * @return AggregationTemporality + */ + sdk::metrics::AggregationTemporality GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept override; + opentelemetry::sdk::common::ExportResult Export( const opentelemetry::sdk::metrics::ResourceMetrics &data) noexcept override; @@ -95,6 +108,9 @@ class OtlpHttpMetricExporter final : public opentelemetry::sdk::metrics::MetricE // Configuration options for the exporter const OtlpHttpMetricExporterOptions options_; + // Aggregation Temporality Selector + const sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector_; + // Object that stores the HTTP sessions that have been created std::unique_ptr http_client_; // For testing diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h index 87a972820f..97238b3773 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h @@ -52,6 +52,13 @@ class OtlpMetricUtils static void PopulateRequest( const opentelemetry::sdk::metrics::ResourceMetrics &data, proto::collector::metrics::v1::ExportMetricsServiceRequest *request) noexcept; + + static sdk::metrics::AggregationTemporalitySelector ChooseTemporalitySelector( + sdk::metrics::AggregationTemporality preferred_aggregation_temporality) noexcept; + static sdk::metrics::AggregationTemporality DeltaTemporalitySelector( + InstrumentType instrument_type) noexcept; + static sdk::metrics::AggregationTemporality CumulativeTemporalitySelector( + InstumentType instrument_type) noexcept; }; } // namespace otlp diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter.cc b/exporters/otlp/src/otlp_grpc_metric_exporter.cc index dfda869550..cfca13807a 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter.cc @@ -100,6 +100,12 @@ OtlpGrpcMetricExporter::OtlpGrpcMetricExporter( // ----------------------------- Exporter methods ------------------------------ +sdk::metrics::AggregationTemporality OtlpHttpMetricExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + return options_.aggregation_temporality_selector(instrument_type); +} + opentelemetry::sdk::common::ExportResult OtlpGrpcMetricExporter::Export( const opentelemetry::sdk::metrics::ResourceMetrics &data) noexcept { diff --git a/exporters/otlp/src/otlp_http_metric_exporter.cc b/exporters/otlp/src/otlp_http_metric_exporter.cc index fec53e8c1d..5cfbfa87c0 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter.cc @@ -43,7 +43,7 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter(const OtlpHttpMetricExporterOptio ))) {} -OtlpHttpMetricExporter::OtlpHttpMetricExporter(std::unique_ptr http_client) +OtlpHttpMetricExporter::(std::unique_ptr http_client) : options_(OtlpHttpMetricExporterOptions()), http_client_(std::move(http_client)) { OtlpHttpMetricExporterOptions &options = const_cast(options_); @@ -61,6 +61,12 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter(std::unique_ptr h } // ----------------------------- Exporter methods ------------------------------ +sdk::metrics::AggregationTemporality OtlpHttpMetricExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + return options_.aggregation_temporality_selector(instrument_type); +} + opentelemetry::sdk::common::ExportResult OtlpHttpMetricExporter::Export( const opentelemetry::sdk::metrics::ResourceMetrics &data) noexcept { diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 075df76b89..167c26a533 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -214,7 +214,6 @@ void OtlpMetricUtils::PopulateResourceMetrics( for (auto &metric_data : scope_metrics.metric_data_) { - PopulateInstrumentInfoMetrics(metric_data, scope_lib_metrics->add_metrics()); } } } @@ -231,6 +230,40 @@ void OtlpMetricUtils::PopulateRequest( auto resource_metrics = request->add_resource_metrics(); PopulateResourceMetrics(data, resource_metrics); } + +sdk::metrics::AggregationTemporalitySelector OtlpMetricUtils::ChooseTemporalitySelector( + sdk::metrics::AggregationTemporality preferred_aggregation_temporality) noexcept +{ + if (preferred_aggregation_temporality == sdk::metrics::AggregationTemporality::kDelta) + { + return DeltaTemporalitySelector; + } + return CumulativeTemporalitySelector; +} + +sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( + InstrumentType instrument_type) noexcept +{ + switch (instrument_type) + { + case sdk::metrics::InstrumentType::kCounter: + case sdk::metrics::InstrumentType::kObservableCounter: + case sdk::metrics::InstrumentType::kHistogram: + case sdk::metrics::InstrumentType::kObservableGauge: + return sdk::metrics::AggregationTemporality::kDelta; + break; + case sdk::metrics::InstrumentType::kUpDownCounter: + case sdk::metrics::InstrumentType::kObservableUpDownCounter: + return sdk::metrics::AggregationTemporality::kCumulative; + } +} + +sdk::metrics::AggregationTemporality OtlpMetricUtils::CumulativeTemporalitySelector( + InstumentType instrument_type) noexcept +{ + return sdk::metrics::AggregationTemporality::kCumulative; +} + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h index 59ef1a11ad..cbb34c7392 100644 --- a/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h +++ b/exporters/prometheus/include/opentelemetry/exporters/prometheus/exporter.h @@ -56,6 +56,14 @@ class PrometheusExporter : public sdk::metrics::MetricExporter */ PrometheusExporter(const PrometheusExporterOptions &options); + /** + * Get the AggregationTemporality for Prometheus exporter + * + * @return AggregationTemporality + */ + sdk::metrics::AggregationTemporality GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept override; + /** * Exports a batch of Metric Records. * @param records: a collection of records to export diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index c11ab8ed16..9d36cf00c9 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -33,6 +33,13 @@ PrometheusExporter::PrometheusExporter() : is_shutdown_(false) collector_ = std::unique_ptr(new PrometheusCollector(3)); } +sdk::metrics::AggregationTemporality PrometheusExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + // Prometheus exporter only support Cumulative + return sdk::metrics::AggregationTemporality::kCumulative; +} + /** * Exports a batch of Metric Records. * @param records: a collection of records to export diff --git a/sdk/include/opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader.h index 29125a6ea2..54c7c0536a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader.h @@ -40,10 +40,11 @@ class PeriodicExportingMetricReader : public MetricReader { public: - PeriodicExportingMetricReader( - std::unique_ptr exporter, - const PeriodicExportingMetricReaderOptions &option, - AggregationTemporality aggregation_temporality = AggregationTemporality::kCumulative); + PeriodicExportingMetricReader(std::unique_ptr exporter, + const PeriodicExportingMetricReaderOptions &option); + + AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept override; private: bool OnForceFlush(std::chrono::microseconds timeout) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 5d85357143..89b51bb330 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -52,7 +52,8 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; -using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; +using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; +using AggregationTemporalitySelector = std::function; /*class InstrumentSelector { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h b/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h index 3217b83df5..ec7831a32a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h @@ -35,6 +35,14 @@ class MetricExporter */ virtual opentelemetry::sdk::common::ExportResult Export(const ResourceMetrics &data) noexcept = 0; + /** + * Get the AggregationTemporality for given Instrument Type for this exporter. + * + * @return AggregationTemporality + */ + virtual AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept = 0; + /** * Force flush the exporter. */ diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index 94924315f8..5d78c4b388 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -25,8 +25,7 @@ namespace metrics class MetricReader { public: - MetricReader( - AggregationTemporality aggregation_temporality = AggregationTemporality::kCumulative); + MetricReader(); void SetMetricProducer(MetricProducer *metric_producer); @@ -36,7 +35,14 @@ class MetricReader */ bool Collect(nostd::function_ref callback) noexcept; - AggregationTemporality GetAggregationTemporality() const noexcept; + /** + * Get the AggregationTemporality for given Instrument Type for this reader. + * + * @return AggregationTemporality + */ + + virtual AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept = 0; /** * Shutdown the meter reader. @@ -62,7 +68,6 @@ class MetricReader private: MetricProducer *metric_producer_; - AggregationTemporality aggregation_temporality_; mutable opentelemetry::common::SpinLockMutex lock_; bool shutdown_; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h index 20372f5209..bac0d9966b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -18,7 +18,8 @@ class MeterContext; class CollectorHandle { public: - virtual AggregationTemporality GetAggregationTemporality() noexcept = 0; + virtual AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) noexcept = 0; }; /** @@ -33,7 +34,8 @@ class MetricCollector : public MetricProducer, public CollectorHandle MetricCollector(std::shared_ptr &&context, std::unique_ptr metric_reader); - AggregationTemporality GetAggregationTemporality() noexcept override; + AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) noexcept override; /** * The callback to be called for each metric exporter. This will only be those diff --git a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc index f11f84544f..0174be87ec 100644 --- a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc +++ b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc @@ -17,10 +17,8 @@ namespace metrics PeriodicExportingMetricReader::PeriodicExportingMetricReader( std::unique_ptr exporter, - const PeriodicExportingMetricReaderOptions &option, - AggregationTemporality aggregation_temporality) - : MetricReader(aggregation_temporality), - exporter_{std::move(exporter)}, + const PeriodicExportingMetricReaderOptions &option) + : exporter_{std::move(exporter)}, export_interval_millis_{option.export_interval_millis}, export_timeout_millis_{option.export_timeout_millis} { @@ -34,6 +32,11 @@ PeriodicExportingMetricReader::PeriodicExportingMetricReader( } } +AggregationTemporality PeriodicExportingMetricReader::GetAggregationTemporality( + InstrumentType instrument_type) const noexcept +{ + return exporter_->GetAggregationTemporality(instrument_type); +} void PeriodicExportingMetricReader::OnInitialized() noexcept { worker_thread_ = std::thread(&PeriodicExportingMetricReader::DoBackgroundWork, this); diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc index 2f44705e0e..6960e9a6fa 100644 --- a/sdk/src/metrics/metric_reader.cc +++ b/sdk/src/metrics/metric_reader.cc @@ -13,9 +13,7 @@ namespace sdk namespace metrics { -MetricReader::MetricReader(AggregationTemporality aggregation_temporality) - : metric_producer_(nullptr), aggregation_temporality_(aggregation_temporality), shutdown_(false) -{} +MetricReader::MetricReader() : metric_producer_(nullptr), shutdown_(false) {} void MetricReader::SetMetricProducer(MetricProducer *metric_producer) { @@ -23,11 +21,6 @@ void MetricReader::SetMetricProducer(MetricProducer *metric_producer) OnInitialized(); } -AggregationTemporality MetricReader::GetAggregationTemporality() const noexcept -{ - return aggregation_temporality_; -} - bool MetricReader::Collect( nostd::function_ref callback) noexcept { diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index 16012ca0f9..95a9567d13 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -24,9 +24,10 @@ MetricCollector::MetricCollector( metric_reader_->SetMetricProducer(this); } -AggregationTemporality MetricCollector::GetAggregationTemporality() noexcept +AggregationTemporality MetricCollector::GetAggregationTemporality( + InstrumentType instrument_type) noexcept { - return metric_reader_->GetAggregationTemporality(); + return metric_reader_->GetAggregationTemporality(instrument_type); } bool MetricCollector::Collect( diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc index bee2986f09..3eea56917b 100644 --- a/sdk/src/metrics/state/temporal_metric_storage.cc +++ b/sdk/src/metrics/state/temporal_metric_storage.cc @@ -32,7 +32,8 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector, { std::lock_guard guard(lock_); opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; - auto aggregation_temporarily = collector->GetAggregationTemporality(); + AggregationTemporality aggregation_temporarily = + collector->GetAggregationTemporality(instrument_descriptor_.type_); for (auto &col : collectors) { unreported_metrics_[col.get()].push_back(delta_metrics); diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 6e2e399e44..42aabb8733 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -29,7 +29,10 @@ class MockCollectorHandle : public CollectorHandle public: MockCollectorHandle(AggregationTemporality temp) : temporality(temp) {} - AggregationTemporality GetAggregationTemporality() noexcept override { return temporality; } + AggregationTemporality GetAggregationTemporality(InstrumentType instrument_type) noexcept override + { + return temporality; + } private: AggregationTemporality temporality; diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index b0fabe50bb..63db4bebf8 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -23,6 +23,12 @@ class MockMetricExporter : public MetricExporter return opentelemetry::sdk::common::ExportResult::kSuccess; } + AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept override + { + return AggregationTemporality::kCumulative; + } + bool ForceFlush( std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept override { @@ -39,6 +45,11 @@ class MockMetricReader : public MetricReader { public: MockMetricReader(std::unique_ptr exporter) : exporter_(std::move(exporter)) {} + AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept override + { + return exporter_->GetAggregationTemporality(instrument_type); + } virtual bool OnForceFlush(std::chrono::microseconds timeout) noexcept override { return true; } virtual bool OnShutDown(std::chrono::microseconds timeout) noexcept override { return true; } virtual void OnInitialized() noexcept override {} diff --git a/sdk/test/metrics/metric_reader_test.cc b/sdk/test/metrics/metric_reader_test.cc index 3023122bb0..82fca653f6 100644 --- a/sdk/test/metrics/metric_reader_test.cc +++ b/sdk/test/metrics/metric_reader_test.cc @@ -14,8 +14,12 @@ using namespace opentelemetry::sdk::metrics; class MockMetricReader : public MetricReader { public: - MockMetricReader(AggregationTemporality aggr_temporality) : MetricReader(aggr_temporality) {} - + MockMetricReader() = default; + AggregationTemporality GetAggregationTemporality( + InstrumentType instrument_type) const noexcept override + { + return AggregationTemporality::kCumulative; + } virtual bool OnForceFlush(std::chrono::microseconds timeout) noexcept override { return true; } virtual bool OnShutDown(std::chrono::microseconds timeout) noexcept override { return true; } virtual void OnInitialized() noexcept override {} @@ -24,13 +28,14 @@ class MockMetricReader : public MetricReader TEST(MetricReaderTest, BasicTests) { AggregationTemporality aggr_temporality = AggregationTemporality::kDelta; - std::unique_ptr metric_reader1(new MockMetricReader(aggr_temporality)); - EXPECT_EQ(metric_reader1->GetAggregationTemporality(), aggr_temporality); + std::unique_ptr metric_reader1(new MockMetricReader()); + EXPECT_EQ(metric_reader1->GetAggregationTemporality(InstrumentType::kCounter), + AggregationTemporality::kCumulative); std::shared_ptr meter_context1(new MeterContext()); EXPECT_NO_THROW(meter_context1->AddMetricReader(std::move(metric_reader1))); - std::unique_ptr metric_reader2(new MockMetricReader(aggr_temporality)); + std::unique_ptr metric_reader2(new MockMetricReader()); std::shared_ptr meter_context2(new MeterContext()); std::shared_ptr metric_producer{ new MetricCollector(std::move(meter_context2), std::move(metric_reader2))}; diff --git a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc index 4ff1d83634..ccf48bcead 100644 --- a/sdk/test/metrics/periodic_exporting_metric_reader_test.cc +++ b/sdk/test/metrics/periodic_exporting_metric_reader_test.cc @@ -28,6 +28,12 @@ class MockPushMetricExporter : public MetricExporter return false; } + sdk::metrics::AggregationTemporality GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept override + { + return sdk::metrics::AggregationTemporality::kCumulative; + } + bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { return true; diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index cb24ae5178..c2123d805b 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -23,7 +23,10 @@ class MockCollectorHandle : public CollectorHandle public: MockCollectorHandle(AggregationTemporality temp) : temporality(temp) {} - AggregationTemporality GetAggregationTemporality() noexcept override { return temporality; } + AggregationTemporality GetAggregationTemporality(InstrumentType instrument_type) noexcept override + { + return temporality; + } private: AggregationTemporality temporality; From 7bc83e2a145a1dfdd616345d3deb3b3542fbdd06 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 3 Aug 2022 20:15:02 -0700 Subject: [PATCH 2/5] fix --- .../include/opentelemetry/exporters/ostream/metric_exporter.h | 1 - 1 file changed, 1 deletion(-) diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h index b6a5f56d4e..7707591326 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h @@ -9,7 +9,6 @@ # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" -# include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/version.h" From 80573d4df5b3800f5ba65889a8d7d9144e4eced6 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 3 Aug 2022 21:16:18 -0700 Subject: [PATCH 3/5] fix --- .../exporters/otlp/otlp_grpc_metric_exporter.h | 3 +++ .../otlp/otlp_grpc_metric_exporter_options.h | 10 ++++++---- .../exporters/otlp/otlp_http_metric_exporter.h | 6 +++--- .../exporters/otlp/otlp_metric_utils.h | 4 ++-- exporters/otlp/src/otlp_grpc_metric_exporter.cc | 14 ++++++++++---- exporters/otlp/src/otlp_http_metric_exporter.cc | 12 +++++++++--- exporters/otlp/src/otlp_metric_utils.cc | 5 +++-- 7 files changed, 36 insertions(+), 18 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h index 1f107d209f..643a0cd3d5 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter.h @@ -60,6 +60,9 @@ class OtlpGrpcMetricExporter : public opentelemetry::sdk::metrics::MetricExporte // The configuration options associated with this exporter. const OtlpGrpcMetricExporterOptions options_; + // Aggregation Temporality selector + const sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector_; + // For testing friend class OtlpGrpcExporterTestPeer; diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h index 618a4a6821..30492850a0 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h @@ -19,11 +19,13 @@ namespace otlp */ struct OtlpGrpcMetricExporterOptions : public OtlpGrpcExporterOptions { - opentelemetry::sdk::metrics::AggregationTemporality aggregation_temporality = - opentelemetry::sdk::metrics::AggregationTemporality::kDelta; - opentelemetry::sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = - OtlpMetricUtils::ChooseTemporalitySelector(); + // Preferred Aggregation Temporality + sdk::metrics::AggregationTemporality aggregation_temporality = + sdk::metrics::AggregationTemporality::kCumulative; + + // opentelemetry::sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = + // OtlpMetricUtils::ChooseTemporalitySelector(); }; } // namespace otlp diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h index b4530f115b..f73966ee22 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter.h @@ -9,6 +9,7 @@ # include "opentelemetry/exporters/otlp/otlp_http_client.h" # include "opentelemetry/exporters/otlp/otlp_environment.h" +# include "opentelemetry/exporters/otlp/otlp_metric_utils.h" # include # include @@ -53,9 +54,8 @@ struct OtlpHttpMetricExporterOptions OtlpHeaders http_headers = GetOtlpDefaultMetricHeaders(); // Preferred Aggregation Temporality - - sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = - otlp::OtlpMetricUtils::ChooseTemporalitySelector(); + sdk::metrics::AggregationTemporality aggregation_temporality = + sdk::metrics::AggregationTemporality::kCumulative; # ifdef ENABLE_ASYNC_EXPORT // Concurrent requests diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h index 97238b3773..c7597638c1 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h @@ -56,9 +56,9 @@ class OtlpMetricUtils static sdk::metrics::AggregationTemporalitySelector ChooseTemporalitySelector( sdk::metrics::AggregationTemporality preferred_aggregation_temporality) noexcept; static sdk::metrics::AggregationTemporality DeltaTemporalitySelector( - InstrumentType instrument_type) noexcept; + sdk::metrics::InstrumentType instrument_type) noexcept; static sdk::metrics::AggregationTemporality CumulativeTemporalitySelector( - InstumentType instrument_type) noexcept; + sdk::metrics::InstrumentType instrument_type) noexcept; }; } // namespace otlp diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter.cc b/exporters/otlp/src/otlp_grpc_metric_exporter.cc index 69f8686bee..19b60f5637 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter.cc @@ -90,20 +90,26 @@ OtlpGrpcMetricExporter::OtlpGrpcMetricExporter() {} OtlpGrpcMetricExporter::OtlpGrpcMetricExporter(const OtlpGrpcMetricExporterOptions &options) - : options_(options), metrics_service_stub_(MakeMetricsServiceStub(options)) + : options_(options), + aggregation_temporality_selector_{ + OtlpMetricUtils::ChooseTemporalitySelector(options_.aggregation_temporality)}, + metrics_service_stub_(MakeMetricsServiceStub(options)) {} OtlpGrpcMetricExporter::OtlpGrpcMetricExporter( std::unique_ptr stub) - : options_(OtlpGrpcMetricExporterOptions()), metrics_service_stub_(std::move(stub)) + : options_(OtlpGrpcMetricExporterOptions()), + aggregation_temporality_selector_{ + OtlpMetricUtils::ChooseTemporalitySelector(options_.aggregation_temporality)}, + metrics_service_stub_(std::move(stub)) {} // ----------------------------- Exporter methods ------------------------------ -sdk::metrics::AggregationTemporality OtlpHttpMetricExporter::GetAggregationTemporality( +sdk::metrics::AggregationTemporality OtlpGrpcMetricExporter::GetAggregationTemporality( sdk::metrics::InstrumentType instrument_type) const noexcept { - return options_.aggregation_temporality_selector(instrument_type); + return aggregation_temporality_selector_(instrument_type); } opentelemetry::sdk::common::ExportResult OtlpGrpcMetricExporter::Export( diff --git a/exporters/otlp/src/otlp_http_metric_exporter.cc b/exporters/otlp/src/otlp_http_metric_exporter.cc index 5cfbfa87c0..62c16724f4 100644 --- a/exporters/otlp/src/otlp_http_metric_exporter.cc +++ b/exporters/otlp/src/otlp_http_metric_exporter.cc @@ -28,6 +28,8 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter() OtlpHttpMetricExporter::OtlpHttpMetricExporter(const OtlpHttpMetricExporterOptions &options) : options_(options), + aggregation_temporality_selector_{ + OtlpMetricUtils::ChooseTemporalitySelector(options_.aggregation_temporality)}, http_client_(new OtlpHttpClient(OtlpHttpClientOptions(options.url, options.content_type, options.json_bytes_mapping, @@ -43,8 +45,11 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter(const OtlpHttpMetricExporterOptio ))) {} -OtlpHttpMetricExporter::(std::unique_ptr http_client) - : options_(OtlpHttpMetricExporterOptions()), http_client_(std::move(http_client)) +OtlpHttpMetricExporter::OtlpHttpMetricExporter(std::unique_ptr http_client) + : options_(OtlpHttpMetricExporterOptions()), + aggregation_temporality_selector_{ + OtlpMetricUtils::ChooseTemporalitySelector(options_.aggregation_temporality)}, + http_client_(std::move(http_client)) { OtlpHttpMetricExporterOptions &options = const_cast(options_); options.url = http_client_->GetOptions().url; @@ -64,7 +69,8 @@ OtlpHttpMetricExporter::(std::unique_ptr http_client) sdk::metrics::AggregationTemporality OtlpHttpMetricExporter::GetAggregationTemporality( sdk::metrics::InstrumentType instrument_type) const noexcept { - return options_.aggregation_temporality_selector(instrument_type); + + return aggregation_temporality_selector_(instrument_type); } opentelemetry::sdk::common::ExportResult OtlpHttpMetricExporter::Export( diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 167c26a533..efd7ced707 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -242,7 +242,7 @@ sdk::metrics::AggregationTemporalitySelector OtlpMetricUtils::ChooseTemporalityS } sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( - InstrumentType instrument_type) noexcept + sdk::metrics::InstrumentType instrument_type) noexcept { switch (instrument_type) { @@ -256,10 +256,11 @@ sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( case sdk::metrics::InstrumentType::kObservableUpDownCounter: return sdk::metrics::AggregationTemporality::kCumulative; } + return sdk::metrics::AggregationTemporality::kUnspecified; } sdk::metrics::AggregationTemporality OtlpMetricUtils::CumulativeTemporalitySelector( - InstumentType instrument_type) noexcept + sdk::metrics::InstrumentType instrument_type) noexcept { return sdk::metrics::AggregationTemporality::kCumulative; } From b4ae52653f25b4945c28f56ce1285471aa808822 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 3 Aug 2022 22:03:06 -0700 Subject: [PATCH 4/5] fix --- exporters/otlp/src/otlp_metric_utils.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index efd7ced707..bc3632bf02 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -214,6 +214,7 @@ void OtlpMetricUtils::PopulateResourceMetrics( for (auto &metric_data : scope_metrics.metric_data_) { + PopulateInstrumentInfoMetrics(metric_data, scope_lib_metrics->add_metrics()); } } } From bc3e42dcb1df31c4a1a47ce3b56cdbafd098eea7 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 5 Aug 2022 11:05:09 -0700 Subject: [PATCH 5/5] fix --- .../exporters/otlp/otlp_grpc_metric_exporter_options.h | 3 --- exporters/otlp/src/otlp_metric_utils.cc | 1 - 2 files changed, 4 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h index 30492850a0..00c4412564 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_options.h @@ -23,9 +23,6 @@ struct OtlpGrpcMetricExporterOptions : public OtlpGrpcExporterOptions // Preferred Aggregation Temporality sdk::metrics::AggregationTemporality aggregation_temporality = sdk::metrics::AggregationTemporality::kCumulative; - - // opentelemetry::sdk::metrics::AggregationTemporalitySelector aggregation_temporality_selector = - // OtlpMetricUtils::ChooseTemporalitySelector(); }; } // namespace otlp diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index bc3632bf02..965279a673 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -252,7 +252,6 @@ sdk::metrics::AggregationTemporality OtlpMetricUtils::DeltaTemporalitySelector( case sdk::metrics::InstrumentType::kHistogram: case sdk::metrics::InstrumentType::kObservableGauge: return sdk::metrics::AggregationTemporality::kDelta; - break; case sdk::metrics::InstrumentType::kUpDownCounter: case sdk::metrics::InstrumentType::kObservableUpDownCounter: return sdk::metrics::AggregationTemporality::kCumulative;