From 8b61c99368466a546b29a41c8dc211faff375fd1 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 8 Aug 2022 08:57:19 -0700 Subject: [PATCH] [Metrics SDK] Metric aggregation temporality controls (#1541) --- .../exporters/ostream/metric_exporter.h | 13 ++++++- exporters/ostream/src/metric_exporter.cc | 12 ++++++- .../otlp/otlp_grpc_metric_exporter.h | 11 ++++++ .../otlp/otlp_grpc_metric_exporter_options.h | 6 ++-- .../otlp/otlp_http_metric_exporter.h | 16 +++++++++ .../exporters/otlp/otlp_metric_utils.h | 7 ++++ .../otlp/src/otlp_grpc_metric_exporter.cc | 16 +++++++-- .../otlp/src/otlp_http_metric_exporter.cc | 14 +++++++- exporters/otlp/src/otlp_metric_utils.cc | 34 +++++++++++++++++++ .../exporters/prometheus/exporter.h | 8 +++++ exporters/prometheus/src/exporter.cc | 7 ++++ .../export/periodic_exporting_metric_reader.h | 9 ++--- .../opentelemetry/sdk/metrics/instruments.h | 4 +-- .../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, 213 insertions(+), 41 deletions(-) diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h index 584b07db5d..2a9e9751ea 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h @@ -29,7 +29,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 +39,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 +65,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, const sdk::metrics::ResourceMetrics &data); diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 1c8307f111..e05597f6ec 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -65,7 +65,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..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 @@ -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; @@ -52,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 a277052956..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 @@ -19,8 +19,10 @@ namespace otlp */ struct OtlpGrpcMetricExporterOptions : public OtlpGrpcExporterOptions { - opentelemetry::sdk::metrics::AggregationTemporality aggregation_temporality = - opentelemetry::sdk::metrics::AggregationTemporality::kDelta; + + // Preferred Aggregation Temporality + sdk::metrics::AggregationTemporality aggregation_temporality = + sdk::metrics::AggregationTemporality::kCumulative; }; } // 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..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 @@ -52,6 +53,10 @@ struct OtlpHttpMetricExporterOptions // Additional HTTP headers OtlpHeaders http_headers = GetOtlpDefaultMetricHeaders(); + // Preferred Aggregation Temporality + sdk::metrics::AggregationTemporality aggregation_temporality = + sdk::metrics::AggregationTemporality::kCumulative; + # 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..c7597638c1 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( + sdk::metrics::InstrumentType instrument_type) noexcept; + static sdk::metrics::AggregationTemporality CumulativeTemporalitySelector( + 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 486d61dfd4..19b60f5637 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter.cc @@ -90,16 +90,28 @@ 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 OtlpGrpcMetricExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + return 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..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, @@ -44,7 +46,10 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter(const OtlpHttpMetricExporterOptio {} OtlpHttpMetricExporter::OtlpHttpMetricExporter(std::unique_ptr http_client) - : options_(OtlpHttpMetricExporterOptions()), http_client_(std::move(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; @@ -61,6 +66,13 @@ OtlpHttpMetricExporter::OtlpHttpMetricExporter(std::unique_ptr h } // ----------------------------- Exporter methods ------------------------------ +sdk::metrics::AggregationTemporality OtlpHttpMetricExporter::GetAggregationTemporality( + sdk::metrics::InstrumentType instrument_type) const noexcept +{ + + return 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 a74a246866..82c99684ef 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -250,6 +250,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( + sdk::metrics::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; + case sdk::metrics::InstrumentType::kUpDownCounter: + case sdk::metrics::InstrumentType::kObservableUpDownCounter: + return sdk::metrics::AggregationTemporality::kCumulative; + } + return sdk::metrics::AggregationTemporality::kUnspecified; +} + +sdk::metrics::AggregationTemporality OtlpMetricUtils::CumulativeTemporalitySelector( + sdk::metrics::InstrumentType 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 845485f70d..565fd05d95 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -58,6 +58,8 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; +using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; +using AggregationTemporalitySelector = std::function; static InstrumentClass GetInstrumentClass(InstrumentType type) { if (type == InstrumentType::kCounter || type == InstrumentType::kHistogram || @@ -71,8 +73,6 @@ static InstrumentClass GetInstrumentClass(InstrumentType type) } } -using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; - /*class InstrumentSelector { public: InstrumentSelector(opentelemetry::nostd::string_view name, 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 f41f40182e..729a793163 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -30,7 +30,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;