From c08cf3844549eb9a3ff5d1b40d795ef89f670ea3 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 1 Mar 2022 23:53:40 -0800 Subject: [PATCH 01/32] collector and reader --- .../sdk/metrics/async_instruments.h | 55 ++----------- .../sdk/metrics/export/metric_producer.h | 36 +++++++++ .../opentelemetry/sdk/metrics/instruments.h | 7 -- sdk/include/opentelemetry/sdk/metrics/meter.h | 8 +- .../opentelemetry/sdk/metrics/meter_context.h | 50 ++++++++---- .../sdk/metrics/meter_provider.h | 18 ++--- .../sdk/metrics/metric_exporter.h | 8 +- .../opentelemetry/sdk/metrics/metric_reader.h | 53 +++++-------- .../sdk/metrics/state/metric_collector.h | 48 ++++++++++++ .../sdk/metrics/state/metric_storage.h | 1 + .../sdk/metrics/sync_instruments.h | 5 +- sdk/src/metrics/CMakeLists.txt | 2 + sdk/src/metrics/meter.cc | 28 +++---- sdk/src/metrics/meter_context.cc | 43 ++++++---- sdk/src/metrics/meter_provider.cc | 24 +++--- sdk/src/metrics/metric_reader.cc | 78 +++++++++++++++++++ sdk/src/metrics/state/metric_collector.cc | 44 +++++++++++ sdk/src/metrics/sync_instruments.cc | 1 + sdk/test/metrics/async_instruments_test.cc | 21 ++--- sdk/test/metrics/meter_provider_sdk_test.cc | 12 ++- 20 files changed, 352 insertions(+), 190 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/metrics/export/metric_producer.h create mode 100644 sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h create mode 100644 sdk/src/metrics/metric_reader.cc create mode 100644 sdk/src/metrics/state/metric_collector.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h index d66bb69890..f38ad6a3b4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h @@ -5,10 +5,8 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/metrics/async_instruments.h" # include "opentelemetry/metrics/observer_result.h" -# include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" -# include "opentelemetry/sdk/metrics/measurement_processor.h" - # include "opentelemetry/nostd/string_view.h" +# include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/instruments.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -23,13 +21,11 @@ class Asynchronous Asynchronous(nostd::string_view name, const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - MeasurementProcessor *measurement_processor, void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") : name_(name), instrumentation_library_{instrumentation_library}, - measurement_processor_{measurement_processor}, callback_(callback), description_(description), unit_(unit) @@ -39,7 +35,6 @@ class Asynchronous std::string name_; const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library_; - const MeasurementProcessor *measurement_processor_; void (*callback_)(opentelemetry::metrics::ObserverResult &); std::string description_; std::string unit_; @@ -52,16 +47,10 @@ class LongObservableCounter : public opentelemetry::metrics::ObservableCounter &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; @@ -73,16 +62,10 @@ class DoubleObservableCounter : public opentelemetry::metrics::ObservableCounter DoubleObservableCounter(nostd::string_view name, const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - MeasurementProcessor *measurement_processor, void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; @@ -94,16 +77,10 @@ class LongObservableGauge : public opentelemetry::metrics::ObservableGauge LongObservableGauge(nostd::string_view name, const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - MeasurementProcessor *measurement_processor, void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; @@ -115,16 +92,10 @@ class DoubleObservableGauge : public opentelemetry::metrics::ObservableGauge &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; @@ -137,16 +108,10 @@ class LongObservableUpDownCounter : public opentelemetry::metrics::ObservableUpD nostd::string_view name, const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - MeasurementProcessor *measurement_processor, void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; @@ -160,16 +125,10 @@ class DoubleObservableUpDownCounter nostd::string_view name, const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - MeasurementProcessor *measurement_processor, void (*callback)(opentelemetry::metrics::ObserverResult &), nostd::string_view description = "", nostd::string_view unit = "") - : Asynchronous(name, - instrumentation_library, - measurement_processor, - callback, - description, - unit) + : Asynchronous(name, instrumentation_library, callback, description, unit) {} }; diff --git a/sdk/include/opentelemetry/sdk/metrics/export/metric_producer.h b/sdk/include/opentelemetry/sdk/metrics/export/metric_producer.h new file mode 100644 index 0000000000..7c9ee57755 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/export/metric_producer.h @@ -0,0 +1,36 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/data/metric_data.h" +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +/** + * MetricProducer is the interface that is used to make metric data available to the + * OpenTelemetry exporters. Implementations should be stateful, in that each call to + * `Collect` will return any metric generated since the last call was made. + * + *

Implementations must be thread-safe. + */ + +class MetricProducer +{ +public: + /** + * The callback to be called for each metric exporter. This will only be those + * metrics that have been produced since the last time this method was called. + * + * @return a status of completion of method. + */ + virtual bool Collect(nostd::function_ref callback) noexcept = 0; +}; + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index ff8ed297fc..ad64ce718b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -54,13 +54,6 @@ struct InstrumentDescriptor using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; -// TBD -> Remove once MetricCollector is imoplemeted -class MetricCollector -{ -public: - AggregationTemporarily aggregation_temporarily_; -}; - /*class InstrumentSelector { public: InstrumentSelector(opentelemetry::nostd::string_view name, diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 24c60b454a..43e979c955 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -7,7 +7,6 @@ # include "opentelemetry/metrics/meter.h" # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/instruments.h" -# include "opentelemetry/sdk/metrics/measurement_processor.h" # include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/resource/resource.h" # include "opentelemetry/version.h" @@ -17,6 +16,10 @@ namespace sdk { namespace metrics { + +class MetricStorage; +class WritableMetricStorage; + class Meter final : public opentelemetry::metrics::Meter { public: @@ -99,9 +102,6 @@ class Meter final : public opentelemetry::metrics::Meter const sdk::instrumentationlibrary::InstrumentationLibrary *GetInstrumentationLibrary() const noexcept; - /** Returns the associated measurement processor */ - MeasurementProcessor *GetMeasurementProcessor() const noexcept; - private: // order of declaration is important here - instrumentation library should destroy after // meter-context. diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index f0fdc6d946..c1063427cc 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -3,27 +3,34 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW -# include -# include -# include -# include "opentelemetry/sdk/metrics/metric_exporter.h" -# include "opentelemetry/sdk/metrics/metric_reader.h" + # include "opentelemetry/sdk/metrics/view/instrument_selector.h" # include "opentelemetry/sdk/metrics/view/meter_selector.h" # include "opentelemetry/sdk/metrics/view/view_registry.h" # include "opentelemetry/sdk/resource/resource.h" # include "opentelemetry/version.h" +# include +# include +# include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { + +// forward declaration +class Meter; +class MetricCollector; +class MetricExporter; +class MetricReader; + /** * A class which stores the MeterProvider context. */ -class MeterContext +class MeterContext : public std::enable_shared_from_this { public: /** @@ -35,7 +42,6 @@ class MeterContext */ MeterContext( std::vector> &&exporters, - std::vector> &&readers, std::unique_ptr views = std::unique_ptr(new ViewRegistry()), opentelemetry::sdk::resource::Resource resource = opentelemetry::sdk::resource::Resource::Create({})) noexcept; @@ -46,18 +52,18 @@ class MeterContext */ const opentelemetry::sdk::resource::Resource &GetResource() const noexcept; - /** - * Obtain the reference of measurement_processor. - * - */ - MeasurementProcessor *GetMeasurementProcessor() const noexcept; - /** * Obtain the View Registry configured * @return The reference to view registry */ ViewRegistry *GetViewRegistry() const noexcept; + /** + * Obtain the configured meters. + * + */ + nostd::span> GetMeters() noexcept; + /** * Attaches a metric exporter to list of configured exporters for this Meter context. * @param exporter The metric exporter for this meter context. This @@ -91,23 +97,33 @@ class MeterContext std::unique_ptr view) noexcept; /** - * Force all active Exporters and Readers to flush any buffered meter data + * Adds a meter to the list of configured meters. + * + * Note: This method is not thread safe, and should ideally be called from main thread in + * thread-safe manner. + * + * @param meter + */ + void AddMeter(std::shared_ptr meter); + + /** + * Force all active Exporters and Collectors to flush any buffered meter data * within the given timeout. */ bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; /** - * Shutdown the Exporters and Readers associated with this meter provider. + * Shutdown the Exporters and Collectors associated with this meter provider. */ bool Shutdown() noexcept; private: + std::vector> meters_; opentelemetry::sdk::resource::Resource resource_; std::vector> exporters_; - std::vector> readers_; + std::vector> collectors_; std::unique_ptr views_; - std::unique_ptr measurement_processor_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h index 7d7966147f..c6efba6222 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h @@ -6,9 +6,9 @@ # include # include # include +# include "opentelemetry/metrics/meter.h" # include "opentelemetry/metrics/meter_provider.h" # include "opentelemetry/nostd/shared_ptr.h" -# include "opentelemetry/sdk/metrics/measurement_processor.h" # include "opentelemetry/sdk/metrics/meter.h" # include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/resource/resource.h" @@ -19,19 +19,23 @@ namespace sdk { namespace metrics { + +// forward declaration +class MetricCollector; +class MetricExporter; +class MetricReader; + class MeterProvider final : public opentelemetry::metrics::MeterProvider { public: /** * Initialize a new meter provider * @param exporters The span exporters for this meter provider - * @param readers The readers for this meter provider. * @param views The views for this meter provider * @param resource The resources for this meter provider. */ MeterProvider( std::vector> &&exporters, - std::vector> &&readers, std::unique_ptr views = std::unique_ptr(new ViewRegistry()), sdk::resource::Resource resource = sdk::resource::Resource::Create({})) noexcept; @@ -52,12 +56,6 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider */ const sdk::resource::Resource &GetResource() const noexcept; - /** - * Obtain the reference of measurement processor. - * - */ - MeasurementProcessor *GetMeasurementProcessor() const noexcept; - /** * Attaches a metric exporter to list of configured exporters for this Meter provider. * @param exporter The metric exporter for this meter provider. This @@ -101,8 +99,6 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; private: - // // order of declaration is important here - meter should destroy only after resource. - std::vector> meters_; std::shared_ptr context_; std::mutex lock_; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h b/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h index 039d3819ec..96898c9ad1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_exporter.h @@ -3,19 +3,21 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW -# include -# include + # include "opentelemetry/nostd/span.h" # include "opentelemetry/sdk/common/exporter_utils.h" -# include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/version.h" +# include +# include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { +class MetricData; /** * MetricExporter defines the interface to be used by metrics libraries to * push metrics data to the OpenTelemetry exporters. diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index 16a1880113..4b04b569d4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -9,12 +9,15 @@ # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/version.h" +# include + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { +class MetricProducer; /** * MetricReader defines the interface to collect metrics from SDK */ @@ -22,56 +25,38 @@ class MetricReader { public: MetricReader( - AggregationTemporarily aggregation_temporarily = AggregationTemporarily::kCummulative) - : aggregation_temporarily_(aggregation_temporarily), measurement_processor_callback_({}) - {} + AggregationTemporarily aggregation_temporarily = AggregationTemporarily::kCummulative); - virtual ~MetricReader() = default; + void SetMetricProducer(std::shared_ptr &&metric_producer); /** * Collect the metrics from SDK. * @return return the status of the operation. */ - bool Collect() noexcept - { - if (!measurement_processor_callback_) - { - OTEL_INTERNAL_LOG_WARN( - "Cannot invoke Collect() for MetricReader. No collection callback registered!") - } - return measurement_processor_callback_( - *this, aggregation_temporarily_, - [&](MetricData metric_data) noexcept { return ProcessReceivedMetrics(metric_data); }); - } + bool Collect(nostd::function_ref callback) noexcept; - /** - * Register the callback to Measurement Processor - * This function is internal to SDK. - */ - void RegisterCollectorCallback( - std::function)> measurement_processor_callback) - { - measurement_processor_callback_ = measurement_processor_callback; - } + AggregationTemporarily GetAggregationTemporarily() const noexcept; /** - * Process the metrics received through Measurement Processor. + * Shutdown the meter reader. */ - virtual bool ProcessReceivedMetrics(MetricData &metric_data) noexcept = 0; + bool Shutdown() noexcept; /** - * Shut down the metric reader. - * @param timeout an optional timeout. - * @return return the status of the operation. + * Force flush the metric read by the reader. */ - virtual bool Shutdown() noexcept = 0; + bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + + virtual bool OnForceFlush() noexcept = 0; + + virtual bool OnShutDown() noexcept = 0; + + virtual void OnInitialized() noexcept {} private: - std::function)> - measurement_processor_callback_; + std::shared_ptr metric_producer_; AggregationTemporarily aggregation_temporarily_; + bool shutdown_; }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h new file mode 100644 index 0000000000..a9e5fd7469 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -0,0 +1,48 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/data/metric_data.h" +# include "opentelemetry/sdk/metrics/export/metric_producer.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +class MetricReader; +class MeterContext; +/** + * An internal opaque interface that the MetricReader receives as + * MetricProducer. It acts as the storage key to the internal metric stream + * state for each MetricReader. + */ + +class MetricCollector : public MetricProducer, public std::enable_shared_from_this +{ +public: + MetricCollector(std::shared_ptr &&context, + std::unique_ptr metric_reader); + + /** + * The callback to be called for each metric exporter. This will only be those + * metrics that have been produced since the last time this method was called. + * + * @return a status of completion of method. + */ + bool Collect(nostd::function_ref callback) noexcept override; + + bool ForceFlush(std::chrono::microseconds timeout) noexcept; + + bool Shutdown() noexcept; + +private: + std::shared_ptr meter_context_; + std::unique_ptr metric_reader_; +}; +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index c2213a701b..8bb011696e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -13,6 +13,7 @@ namespace metrics { /* Represent the storage from which to collect the metrics */ +class MetricCollector; class MetricStorage { diff --git a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h index aa0348f0b0..3e8c5c8623 100644 --- a/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/sync_instruments.h @@ -7,7 +7,6 @@ # include "opentelemetry/metrics/sync_instruments.h" # include "opentelemetry/nostd/string_view.h" # include "opentelemetry/sdk/metrics/instruments.h" -# include "opentelemetry/sdk/metrics/measurement_processor.h" # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" @@ -16,6 +15,10 @@ namespace sdk { namespace metrics { + +// forward declaration +class WritableMetricStorage; + class Synchronous { public: diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index f9b6d546ed..119477eb06 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -3,6 +3,8 @@ add_library( meter_provider.cc meter.cc meter_context.cc + metric_reader.cc + state/metric_collector.cc aggregation/histogram_aggregation.cc aggregation/lastvalue_aggregation.cc aggregation/sum_aggregation.cc diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 52ecfc9e4e..ed83a14955 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -7,6 +7,7 @@ # include "opentelemetry/nostd/shared_ptr.h" # include "opentelemetry/sdk/metrics/async_instruments.h" # include "opentelemetry/sdk/metrics/state/multi_metric_storage.h" +# include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" # include "opentelemetry/sdk/metrics/sync_instruments.h" # include "opentelemetry/sdk_config.h" @@ -61,8 +62,8 @@ nostd::shared_ptr> Meter::CreateLongObservableC nostd::string_view description, nostd::string_view unit) noexcept { - return nostd::shared_ptr>{new LongObservableCounter( - name, GetInstrumentationLibrary(), GetMeasurementProcessor(), callback, description, unit)}; + return nostd::shared_ptr>{ + new LongObservableCounter(name, GetInstrumentationLibrary(), callback, description, unit)}; } nostd::shared_ptr> Meter::CreateDoubleObservableCounter( @@ -71,8 +72,8 @@ nostd::shared_ptr> Meter::CreateDoubleObserva nostd::string_view description, nostd::string_view unit) noexcept { - return nostd::shared_ptr>{new DoubleObservableCounter( - name, GetInstrumentationLibrary(), GetMeasurementProcessor(), callback, description, unit)}; + return nostd::shared_ptr>{ + new DoubleObservableCounter(name, GetInstrumentationLibrary(), callback, description, unit)}; } nostd::shared_ptr> Meter::CreateLongHistogram( @@ -109,8 +110,8 @@ nostd::shared_ptr> Meter::CreateLongObservableGau nostd::string_view description, nostd::string_view unit) noexcept { - return nostd::shared_ptr>{new LongObservableGauge( - name, GetInstrumentationLibrary(), GetMeasurementProcessor(), callback, description, unit)}; + return nostd::shared_ptr>{ + new LongObservableGauge(name, GetInstrumentationLibrary(), callback, description, unit)}; } nostd::shared_ptr> Meter::CreateDoubleObservableGauge( @@ -119,8 +120,8 @@ nostd::shared_ptr> Meter::CreateDoubleObservabl nostd::string_view description, nostd::string_view unit) noexcept { - return nostd::shared_ptr>{new DoubleObservableGauge( - name, GetInstrumentationLibrary(), GetMeasurementProcessor(), callback, description, unit)}; + return nostd::shared_ptr>{ + new DoubleObservableGauge(name, GetInstrumentationLibrary(), callback, description, unit)}; } nostd::shared_ptr> Meter::CreateLongUpDownCounter( @@ -158,7 +159,7 @@ nostd::shared_ptr> Meter::CreateLongObser nostd::string_view unit) noexcept { return nostd::shared_ptr>{new LongObservableUpDownCounter( - name, GetInstrumentationLibrary(), GetMeasurementProcessor(), callback, description, unit)}; + name, GetInstrumentationLibrary(), callback, description, unit)}; } nostd::shared_ptr> @@ -168,8 +169,8 @@ Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name, nostd::string_view unit) noexcept { return nostd::shared_ptr>{ - new DoubleObservableUpDownCounter(name, GetInstrumentationLibrary(), - GetMeasurementProcessor(), callback, description, unit)}; + new DoubleObservableUpDownCounter(name, GetInstrumentationLibrary(), callback, description, + unit)}; } const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumentationLibrary() @@ -178,11 +179,6 @@ const sdk::instrumentationlibrary::InstrumentationLibrary *Meter::GetInstrumenta return instrumentation_library_.get(); } -MeasurementProcessor *Meter::GetMeasurementProcessor() const noexcept -{ - return meter_context_->GetMeasurementProcessor(); -} - std::unique_ptr Meter::RegisterMetricStorage( InstrumentDescriptor &instrument_descriptor) { diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index bc2f75de2a..3280b20fe9 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -2,8 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 #ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/common/global_log_handler.h" -# include "opentelemetry/sdk/metrics/meter.h" +# include "opentelemetry/sdk/metrics/metric_exporter.h" +# include "opentelemetry/sdk/metrics/metric_exporter.h" +# include "opentelemetry/sdk/metrics/metric_reader.h" +# include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk_config.h" # include "opentelemetry/version.h" @@ -14,13 +18,9 @@ namespace metrics { MeterContext::MeterContext(std::vector> &&exporters, - std::vector> &&readers, std::unique_ptr views, opentelemetry::sdk::resource::Resource resource) noexcept - : exporters_(std::move(exporters)), - readers_(std::move(readers)), - views_(std::move(views)), - resource_{resource} + : exporters_(std::move(exporters)), views_(std::move(views)), resource_{resource} {} const resource::Resource &MeterContext::GetResource() const noexcept @@ -28,14 +28,14 @@ const resource::Resource &MeterContext::GetResource() const noexcept return resource_; } -MeasurementProcessor *MeterContext::GetMeasurementProcessor() const noexcept +ViewRegistry *MeterContext::GetViewRegistry() const noexcept { - return measurement_processor_.get(); + return views_.get(); } -ViewRegistry *MeterContext::GetViewRegistry() const noexcept +nostd::span> MeterContext::GetMeters() noexcept { - return views_.get(); + return nostd::span>{meters_}; } void MeterContext::AddMetricExporter(std::unique_ptr exporter) noexcept @@ -45,7 +45,9 @@ void MeterContext::AddMetricExporter(std::unique_ptr exporter) n void MeterContext::AddMetricReader(std::unique_ptr reader) noexcept { - readers_.push_back(std::move(reader)); + auto collector = + std::shared_ptr{new MetricCollector(shared_from_this(), std::move(reader))}; + collectors_.push_back(collector); } void MeterContext::AddView(std::unique_ptr instrument_selector, @@ -55,10 +57,17 @@ void MeterContext::AddView(std::unique_ptr instrument_select views_->AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); } +void MeterContext::AddMeter(std::shared_ptr meter) +{ + meters_.push_back(meter); +} + bool MeterContext::Shutdown() noexcept { - bool result_exporter = true; - bool result_reader = true; + bool result_exporter = true; + bool result_reader = true; + bool result_collector = true; + for (auto &exporter : exporters_) { bool status = exporter->Shutdown(); @@ -68,12 +77,12 @@ bool MeterContext::Shutdown() noexcept { OTEL_INTERNAL_LOG_WARN("[MeterContext::Shutdown] Unable to shutdown all metric exporters"); } - for (auto &reader : readers_) + for (auto &collector : collectors_) { - bool status = reader->Shutdown(); - result_reader = result_reader && status; + bool status = collector->Shutdown(); + result_collector = result_reader && status; } - if (!result_reader) + if (!result_collector) { OTEL_INTERNAL_LOG_WARN("[MeterContext::Shutdown] Unable to shutdown all metric readers"); } diff --git a/sdk/src/metrics/meter_provider.cc b/sdk/src/metrics/meter_provider.cc index db1737ce94..8a9572c88c 100644 --- a/sdk/src/metrics/meter_provider.cc +++ b/sdk/src/metrics/meter_provider.cc @@ -3,6 +3,10 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/meter_provider.h" +# include "opentelemetry/metrics/meter.h" +# include "opentelemetry/sdk/metrics/metric_exporter.h" +# include "opentelemetry/sdk/metrics/metric_reader.h" + # include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk_config.h" # include "opentelemetry/version.h" @@ -20,13 +24,9 @@ namespace metrics_api = opentelemetry::metrics; MeterProvider::MeterProvider(std::shared_ptr context) noexcept : context_{context} {} MeterProvider::MeterProvider(std::vector> &&exporters, - std::vector> &&readers, std::unique_ptr views, sdk::resource::Resource resource) noexcept - : context_(std::make_shared(std::move(exporters), - std::move(readers), - std::move(views), - resource)) + : context_(std::make_shared(std::move(exporters), std::move(views), resource)) {} nostd::shared_ptr MeterProvider::GetMeter( @@ -42,7 +42,7 @@ nostd::shared_ptr MeterProvider::GetMeter( const std::lock_guard guard(lock_); - for (auto &meter : meters_) + for (auto &meter : context_->GetMeters()) { auto meter_lib = meter->GetInstrumentationLibrary(); if (meter_lib->equal(name, version, schema_url)) @@ -50,9 +50,10 @@ nostd::shared_ptr MeterProvider::GetMeter( return nostd::shared_ptr{meter}; } } - auto lib = instrumentationlibrary::InstrumentationLibrary::Create(name, version, schema_url); - meters_.push_back(std::shared_ptr(new Meter(context_, std::move(lib)))); - return nostd::shared_ptr{meters_.back()}; + auto lib = instrumentationlibrary::InstrumentationLibrary::Create(name, version, schema_url); + auto meter = std::shared_ptr(new Meter(context_, std::move(lib))); + context_->AddMeter(meter); + return nostd::shared_ptr{meter}; } const resource::Resource &MeterProvider::GetResource() const noexcept @@ -60,11 +61,6 @@ const resource::Resource &MeterProvider::GetResource() const noexcept return context_->GetResource(); } -MeasurementProcessor *MeterProvider::GetMeasurementProcessor() const noexcept -{ - return context_->GetMeasurementProcessor(); -} - void MeterProvider::AddMetricExporter(std::unique_ptr exporter) noexcept { return context_->AddMetricExporter(std::move(exporter)); diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc new file mode 100644 index 0000000000..47ee90e6ab --- /dev/null +++ b/sdk/src/metrics/metric_reader.cc @@ -0,0 +1,78 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/metric_reader.h" +# include "opentelemetry/sdk/metrics/export/metric_producer.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +MetricReader::MetricReader(AggregationTemporarily aggregation_temporarily) + : aggregation_temporarily_(aggregation_temporarily) +{} + +void MetricReader::SetMetricProducer(std::shared_ptr &&metric_producer) +{ + metric_producer_ = metric_producer; +} + +AggregationTemporarily MetricReader::GetAggregationTemporarily() const noexcept +{ + return aggregation_temporarily_; +} + +bool MetricReader::Collect(nostd::function_ref callback) noexcept +{ + if (!metric_producer_) + { + OTEL_INTERNAL_LOG_WARN( + "MetricReader::Collect Cannot invoke Collect(). No MetricProducer registered for " + "collection!") + } + if (shutdown_) + { + OTEL_INTERNAL_LOG_WARN("MetricReader::Collect Cannot invoke Collect(). Shutdown in progress!"); + } + + return metric_producer_->Collect(callback); +} + +bool MetricReader::Shutdown() noexcept +{ + bool status = true; + if (shutdown_) + { + OTEL_INTERNAL_LOG_WARN("MetricReader::Shutdown Cannot invoke shutown twice!"); + } + if (!OnShutDown()) + { + status = false; + OTEL_INTERNAL_LOG_ERROR("MetricReader::OnShutDown Shutdown failed. Will not be tried again!"); + } + shutdown_ = true; + return status; +} + +/** Flush metric read by this reader **/ +bool MetricReader::ForceFlush(std::chrono::microseconds timeout) noexcept +{ + bool status = true; + if (shutdown_) + { + OTEL_INTERNAL_LOG_WARN("MetricReader::Shutdown Cannot invoke Force flush on shutdown reader!"); + } + if (!OnForceFlush()) + { + status = false; + OTEL_INTERNAL_LOG_ERROR("MetricReader::OnForceFlush failed!"); + } + return status; +} +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc new file mode 100644 index 0000000000..89c6c14a56 --- /dev/null +++ b/sdk/src/metrics/state/metric_collector.cc @@ -0,0 +1,44 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/state/metric_collector.h" +# include "opentelemetry/sdk/common/global_log_handler.h" +# include "opentelemetry/sdk/metrics/meter_context.h" +# include "opentelemetry/sdk/metrics/metric_reader.h" +# include "opentelemetry/sdk_config.h" +# include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +MetricCollector::MetricCollector( + std::shared_ptr &&context, + std::unique_ptr metric_reader) + : meter_context_{context}, metric_reader_{std::move(metric_reader)} +{ + metric_reader_->SetMetricProducer(this->shared_from_this()); +} + +bool MetricCollector::Collect(nostd::function_ref callback) noexcept +{ + return metric_reader_->Collect(callback); +} + +bool MetricCollector::ForceFlush(std::chrono::microseconds timeout) noexcept +{ + return metric_reader_->ForceFlush(timeout); +} + +bool MetricCollector::Shutdown() noexcept +{ + return metric_reader_->Shutdown(); +} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif \ No newline at end of file diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 85e770a715..863e5b6323 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -3,6 +3,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/sync_instruments.h" +# include "opentelemetry/sdk/metrics/state/metric_storage.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/test/metrics/async_instruments_test.cc b/sdk/test/metrics/async_instruments_test.cc index abaeb83028..ad3a81b031 100644 --- a/sdk/test/metrics/async_instruments_test.cc +++ b/sdk/test/metrics/async_instruments_test.cc @@ -4,7 +4,6 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/async_instruments.h" # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" -# include "opentelemetry/sdk/metrics/measurement_processor.h" # include @@ -13,7 +12,6 @@ using namespace opentelemetry::sdk::instrumentationlibrary; using namespace opentelemetry::sdk::metrics; auto instrumentation_library = InstrumentationLibrary::Create("opentelemetry-cpp", "0.1.0"); -DefaultMeasurementProcessor measurement_processor; using M = std::map; @@ -25,48 +23,43 @@ TEST(AsyncInstruments, LongObservableCounter) { auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; EXPECT_NO_THROW(LongObservableCounter counter("long_counter", instrumentation_library.get(), - &measurement_processor, asyc_generate_meas_long, - "description", "1")); + asyc_generate_meas_long, "description", "1")); } TEST(AsyncInstruments, DoubleObservableCounter) { auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; EXPECT_NO_THROW(DoubleObservableCounter counter("long_counter", instrumentation_library.get(), - &measurement_processor, asyc_generate_meas_double, - "description", "1")); + asyc_generate_meas_double, "description", "1")); } TEST(AsyncInstruments, LongObservableGauge) { auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; EXPECT_NO_THROW(LongObservableGauge counter("long_counter", instrumentation_library.get(), - &measurement_processor, asyc_generate_meas_long, - "description", "1")); + asyc_generate_meas_long, "description", "1")); } TEST(AsyncInstruments, DoubleObservableGauge) { auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; EXPECT_NO_THROW(DoubleObservableGauge counter("long_counter", instrumentation_library.get(), - &measurement_processor, asyc_generate_meas_double, - "description", "1")); + asyc_generate_meas_double, "description", "1")); } TEST(AsyncInstruments, LongObservableUpDownCounter) { auto asyc_generate_meas_long = [](opentelemetry::metrics::ObserverResult &observer) {}; EXPECT_NO_THROW(LongObservableUpDownCounter counter("long_counter", instrumentation_library.get(), - &measurement_processor, asyc_generate_meas_long, "description", "1")); } TEST(AsyncInstruments, DoubleObservableUpDownCounter) { auto asyc_generate_meas_double = [](opentelemetry::metrics::ObserverResult &observer) {}; - EXPECT_NO_THROW(DoubleObservableUpDownCounter counter( - "long_counter", instrumentation_library.get(), &measurement_processor, - asyc_generate_meas_double, "description", "1")); + EXPECT_NO_THROW( + DoubleObservableUpDownCounter counter("long_counter", instrumentation_library.get(), + asyc_generate_meas_double, "description", "1")); } #endif diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index e321a29a96..9b6ea95f48 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -3,8 +3,11 @@ #ifndef ENABLE_METRICS_PREVIEW # include +# include "opentelemetry/sdk/metrics/export/metric_producer.h" # include "opentelemetry/sdk/metrics/meter.h" # include "opentelemetry/sdk/metrics/meter_provider.h" +# include "opentelemetry/sdk/metrics/metric_exporter.h" +# include "opentelemetry/sdk/metrics/metric_reader.h" # include "opentelemetry/sdk/metrics/view/instrument_selector.h" # include "opentelemetry/sdk/metrics/view/meter_selector.h" @@ -36,17 +39,18 @@ class MockMetricExporter : public MetricExporter class MockMetricReader : public MetricReader { public: - bool ProcessReceivedMetrics(MetricData &metric_data) noexcept override { return true; } + virtual bool OnForceFlush() noexcept override { return true; } - bool Shutdown() noexcept override { return true; } + virtual bool OnShutDown() noexcept override { return true; } + + virtual void OnInitialized() noexcept override {} }; TEST(MeterProvider, GetMeter) { std::vector> exporters; - std::vector> readers; - MeterProvider mp1(std::move(exporters), std::move(readers)); + MeterProvider mp1(std::move(exporters)); // std::unique_ptr view{std::unique_ptr()}; // MeterProvider mp1(std::move(exporters), std::move(readers), std::move(views); auto m1 = mp1.GetMeter("test"); From c729c6ad9427d55b228064e410a06d35ed05ef25 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 2 Mar 2022 00:47:02 -0800 Subject: [PATCH 02/32] fix format, and memory corruption --- sdk/src/metrics/meter_context.cc | 1 - sdk/src/metrics/metric_reader.cc | 2 +- sdk/src/metrics/state/metric_collector.cc | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 3280b20fe9..62851e7870 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -5,7 +5,6 @@ # include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" -# include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/sdk/metrics/metric_reader.h" # include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk_config.h" diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc index 47ee90e6ab..180b33c695 100644 --- a/sdk/src/metrics/metric_reader.cc +++ b/sdk/src/metrics/metric_reader.cc @@ -17,7 +17,7 @@ MetricReader::MetricReader(AggregationTemporarily aggregation_temporarily) void MetricReader::SetMetricProducer(std::shared_ptr &&metric_producer) { - metric_producer_ = metric_producer; + metric_producer_ = std::move(metric_producer); } AggregationTemporarily MetricReader::GetAggregationTemporarily() const noexcept diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index 89c6c14a56..246e906358 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -18,7 +18,7 @@ namespace metrics MetricCollector::MetricCollector( std::shared_ptr &&context, std::unique_ptr metric_reader) - : meter_context_{context}, metric_reader_{std::move(metric_reader)} + : meter_context_{std::move(context)}, metric_reader_{std::move(metric_reader)} { metric_reader_->SetMetricProducer(this->shared_from_this()); } From f692e1572811db01474ba31c8f3c4b4300604b50 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 2 Mar 2022 22:28:26 -0800 Subject: [PATCH 03/32] changes for metric flow --- sdk/include/opentelemetry/sdk/metrics/meter.h | 5 +++ .../opentelemetry/sdk/metrics/meter_context.h | 17 ++++++- .../opentelemetry/sdk/metrics/metric_reader.h | 5 ++- .../sdk/metrics/state/metric_collector.h | 15 +++++-- .../sdk/metrics/state/metric_storage.h | 31 +++++++------ .../sdk/metrics/state/sync_metric_storage.h | 18 ++++---- sdk/src/metrics/meter.cc | 20 +++++++++ sdk/src/metrics/meter_context.cc | 18 ++++++-- sdk/src/metrics/metric_reader.cc | 6 +-- sdk/src/metrics/state/metric_collector.cc | 17 +++++-- sdk/test/metrics/BUILD | 16 +++++++ sdk/test/metrics/CMakeLists.txt | 3 +- sdk/test/metrics/metric_reader_test.cc | 45 +++++++++++++++++++ 13 files changed, 178 insertions(+), 38 deletions(-) create mode 100644 sdk/test/metrics/metric_reader_test.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 43e979c955..fc9fb36503 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -102,6 +102,11 @@ class Meter final : public opentelemetry::metrics::Meter const sdk::instrumentationlibrary::InstrumentationLibrary *GetInstrumentationLibrary() const noexcept; + /** collect metrics across all the meters **/ + bool collect(CollectorHandle *collector, + opentelemetry::common::SystemTimestamp collect_ts, + nostd::function_ref callback) noexcept; + private: // order of declaration is important here - instrumentation library should destroy after // meter-context. diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index c1063427cc..46f9b12825 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -4,6 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk/metrics/view/instrument_selector.h" # include "opentelemetry/sdk/metrics/view/meter_selector.h" # include "opentelemetry/sdk/metrics/view/view_registry.h" @@ -22,7 +23,6 @@ namespace metrics // forward declaration class Meter; -class MetricCollector; class MetricExporter; class MetricReader; @@ -64,6 +64,18 @@ class MeterContext : public std::enable_shared_from_this */ nostd::span> GetMeters() noexcept; + /** + * Obtain the configured collectors. + * + */ + nostd::span> GetCollectors() noexcept; + + /** + * GET SDK Start time + * + */ + opentelemetry::common::SystemTimestamp GetSDKStartTime() noexcept; + /** * Attaches a metric exporter to list of configured exporters for this Meter context. * @param exporter The metric exporter for this meter context. This @@ -122,8 +134,9 @@ class MeterContext : public std::enable_shared_from_this std::vector> meters_; opentelemetry::sdk::resource::Resource resource_; std::vector> exporters_; - std::vector> collectors_; + std::vector> collectors_; std::unique_ptr views_; + opentelemetry::common::SystemTimestamp sdk_start_ts_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index 4b04b569d4..ccabe7d616 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -27,7 +27,7 @@ class MetricReader MetricReader( AggregationTemporarily aggregation_temporarily = AggregationTemporarily::kCummulative); - void SetMetricProducer(std::shared_ptr &&metric_producer); + void SetMetricProducer(MetricProducer *metric_producer); /** * Collect the metrics from SDK. @@ -47,6 +47,7 @@ class MetricReader */ bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; +private: virtual bool OnForceFlush() noexcept = 0; virtual bool OnShutDown() noexcept = 0; @@ -54,7 +55,7 @@ class MetricReader virtual void OnInitialized() noexcept {} private: - std::shared_ptr metric_producer_; + MetricProducer *metric_producer_; AggregationTemporarily aggregation_temporarily_; 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 a9e5fd7469..799f752e62 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -14,18 +14,27 @@ namespace metrics class MetricReader; class MeterContext; + +class CollectorHandle +{ +public: + virtual AggregationTemporarily GetAggregationTemporarily() noexcept = 0; +}; + /** * An internal opaque interface that the MetricReader receives as * MetricProducer. It acts as the storage key to the internal metric stream * state for each MetricReader. */ -class MetricCollector : public MetricProducer, public std::enable_shared_from_this +class MetricCollector : public MetricProducer, public CollectorHandle { public: MetricCollector(std::shared_ptr &&context, std::unique_ptr metric_reader); + AggregationTemporarily GetAggregationTemporarily() noexcept override; + /** * The callback to be called for each metric exporter. This will only be those * metrics that have been produced since the last time this method was called. @@ -40,9 +49,9 @@ class MetricCollector : public MetricProducer, public std::enable_shared_from_th private: std::shared_ptr meter_context_; - std::unique_ptr metric_reader_; + std::shared_ptr metric_reader_; }; } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE -#endif \ No newline at end of file +#endif diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index 8bb011696e..d64aff40e1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -4,6 +4,7 @@ #pragma once #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/common/key_value_iterable_view.h" +# include "opentelemetry/common/timestamp.h" # include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -13,18 +14,20 @@ namespace metrics { /* Represent the storage from which to collect the metrics */ -class MetricCollector; +class CollectorHandle; class MetricStorage { public: /* collect the metrics from this storage */ - virtual bool Collect( - MetricCollector *collector, - nostd::span collectors, - opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - opentelemetry::sdk::resource::Resource *resource, - nostd::function_ref callback) noexcept = 0; + virtual bool Collect(CollectorHandle *collector, + nostd::span> collectors, + const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary + &instrumentation_library, + const opentelemetry::sdk::resource::Resource &resource, + opentelemetry::common::SystemTimestamp sdk_start_ts, + opentelemetry::common::SystemTimestamp collection_ts, + nostd::function_ref callback) noexcept = 0; }; class WritableMetricStorage @@ -44,12 +47,14 @@ class WritableMetricStorage class NoopMetricStorage : public MetricStorage { public: - bool Collect( - MetricCollector *collector, - nostd::span collectors, - opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - opentelemetry::sdk::resource::Resource *resource, - nostd::function_ref callback) noexcept override + bool Collect(CollectorHandle *collector, + nostd::span> collectors, + const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary + &instrumentation_library, + const opentelemetry::sdk::resource::Resource &resource, + opentelemetry::common::SystemTimestamp sdk_start_ts, + opentelemetry::common::SystemTimestamp collection_ts, + nostd::function_ref callback) noexcept override { MetricData metric_data; if (callback(metric_data)) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index ee64c5fba4..3744328270 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -86,15 +86,17 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage aggregation->Aggregate(value); } - bool Collect( - MetricCollector *collector, - nostd::span collectors, - opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - opentelemetry::sdk::resource::Resource *resource, - nostd::function_ref callback) noexcept override + bool Collect(CollectorHandle *collector, + nostd::span> collectors, + const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary + &instrumentation_library, + const opentelemetry::sdk::resource::Resource &resource, + opentelemetry::common::SystemTimestamp sdk_start_ts, + opentelemetry::common::SystemTimestamp collection_ts, + nostd::function_ref callback) noexcept override { - - if (callback(MetricData())) + MetricData data; + if (callback(data)) { return true; } diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index ed83a14955..0db8ca59af 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -208,6 +208,26 @@ std::unique_ptr Meter::RegisterMetricStorage( return std::move(storages); } +/** collect metrics across all the meters **/ +bool Meter::collect(CollectorHandle *collector, + opentelemetry::common::SystemTimestamp collect_ts, + nostd::function_ref callback) noexcept +{ + std::vector data; + for (auto &metric_storage : storage_registry_) + { + // TBD - this needs to be asynchronous + metric_storage.second->Collect(collector, meter_context_->GetCollectors(), + *instrumentation_library_, meter_context_->GetResource(), + meter_context_->GetSDKStartTime(), collect_ts, + [&callback](MetricData &metric_data) { + callback(metric_data); + return true; + }); + } + return true; +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 62851e7870..7ec523c463 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -6,7 +6,6 @@ # include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/sdk/metrics/metric_reader.h" -# include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk_config.h" # include "opentelemetry/version.h" @@ -19,7 +18,10 @@ namespace metrics MeterContext::MeterContext(std::vector> &&exporters, std::unique_ptr views, opentelemetry::sdk::resource::Resource resource) noexcept - : exporters_(std::move(exporters)), views_(std::move(views)), resource_{resource} + : exporters_(std::move(exporters)), + views_(std::move(views)), + resource_{resource}, + sdk_start_ts_{std::chrono::system_clock::now()} {} const resource::Resource &MeterContext::GetResource() const noexcept @@ -37,6 +39,16 @@ nostd::span> MeterContext::GetMeters() noexcept return nostd::span>{meters_}; } +nostd::span> MeterContext::GetCollectors() noexcept +{ + return nostd::span>(collectors_); +} + +opentelemetry::common::SystemTimestamp MeterContext::GetSDKStartTime() noexcept +{ + return sdk_start_ts_; +} + void MeterContext::AddMetricExporter(std::unique_ptr exporter) noexcept { exporters_.push_back(std::move(exporter)); @@ -78,7 +90,7 @@ bool MeterContext::Shutdown() noexcept } for (auto &collector : collectors_) { - bool status = collector->Shutdown(); + bool status = std::static_pointer_cast(collector)->Shutdown(); result_collector = result_reader && status; } if (!result_collector) diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc index 180b33c695..9e2abfb982 100644 --- a/sdk/src/metrics/metric_reader.cc +++ b/sdk/src/metrics/metric_reader.cc @@ -15,9 +15,9 @@ MetricReader::MetricReader(AggregationTemporarily aggregation_temporarily) : aggregation_temporarily_(aggregation_temporarily) {} -void MetricReader::SetMetricProducer(std::shared_ptr &&metric_producer) +void MetricReader::SetMetricProducer(MetricProducer *metric_producer) { - metric_producer_ = std::move(metric_producer); + metric_producer_ = metric_producer; } AggregationTemporarily MetricReader::GetAggregationTemporarily() const noexcept @@ -75,4 +75,4 @@ bool MetricReader::ForceFlush(std::chrono::microseconds timeout) noexcept } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE -#endif \ No newline at end of file +#endif diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index 246e906358..dc7039c2d4 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -4,6 +4,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk/common/global_log_handler.h" +# include "opentelemetry/sdk/metrics/meter.h" # include "opentelemetry/sdk/metrics/meter_context.h" # include "opentelemetry/sdk/metrics/metric_reader.h" # include "opentelemetry/sdk_config.h" @@ -20,12 +21,22 @@ MetricCollector::MetricCollector( std::unique_ptr metric_reader) : meter_context_{std::move(context)}, metric_reader_{std::move(metric_reader)} { - metric_reader_->SetMetricProducer(this->shared_from_this()); + metric_reader_->SetMetricProducer(this); +} + +AggregationTemporarily MetricCollector::GetAggregationTemporarily() noexcept +{ + return metric_reader_->GetAggregationTemporarily(); } bool MetricCollector::Collect(nostd::function_ref callback) noexcept { - return metric_reader_->Collect(callback); + for (auto &meter : meter_context_->GetMeters()) + { + auto collection_ts = std::chrono::system_clock::now(); + meter->collect(this, collection_ts, callback); + } + return true; } bool MetricCollector::ForceFlush(std::chrono::microseconds timeout) noexcept @@ -41,4 +52,4 @@ bool MetricCollector::Shutdown() noexcept } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE -#endif \ No newline at end of file +#endif diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index d49d855d8f..79df854e17 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -16,6 +16,22 @@ cc_test( ], ) +cc_test( + name = "meter_reader_test", + srcs = [ + "meter_reader_test.cc", + ], + tags = [ + "metrics", + "test", + ], + deps = [ + "//sdk/src/metrics", + "//sdk/src/resource", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "view_registry_test", srcs = [ diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index fb823feaca..5581b739e0 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -8,7 +8,8 @@ foreach( sync_metric_storage_test multi_metric_storage_test sync_instruments_test - async_instruments_test) + async_instruments_test + metric_reader_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} diff --git a/sdk/test/metrics/metric_reader_test.cc b/sdk/test/metrics/metric_reader_test.cc new file mode 100644 index 0000000000..0b27d18cc0 --- /dev/null +++ b/sdk/test/metrics/metric_reader_test.cc @@ -0,0 +1,45 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/metric_reader.h" +# include +# include "opentelemetry/sdk/metrics/meter_context.h" +# include "opentelemetry/sdk/metrics/metric_exporter.h" + +using namespace opentelemetry; +using namespace opentelemetry::sdk::instrumentationlibrary; +using namespace opentelemetry::sdk::metrics; + +# include + +class MockMetricReader : public MetricReader +{ +public: + MockMetricReader(AggregationTemporarily aggr_temporarily) : MetricReader(aggr_temporarily) {} + + virtual bool OnForceFlush() noexcept override { return true; } + + virtual bool OnShutDown() noexcept override { return true; } + + virtual void OnInitialized() noexcept override {} +}; + +TEST(MetricReaderTest, BasicTests) +{ + AggregationTemporarily aggr_temporarily = AggregationTemporarily::kDelta; + std::unique_ptr metric_reader1(new MockMetricReader(aggr_temporarily)); + EXPECT_EQ(metric_reader1->GetAggregationTemporarily(), aggr_temporarily); + + std::vector> exporters; + std::shared_ptr meter_context1(new MeterContext(std::move(exporters))); + EXPECT_NO_THROW(meter_context1->AddMetricReader(std::move(metric_reader1))); + + std::unique_ptr metric_reader2(new MockMetricReader(aggr_temporarily)); + std::shared_ptr meter_context2(new MeterContext(std::move(exporters))); + MetricProducer *metric_producer = + new MetricCollector(std::move(meter_context2), std::move(metric_reader2)); + std::cout << metric_producer << "\n"; + EXPECT_NO_THROW(metric_producer->Collect([](MetricData data) { return true; })); +} +#endif From ffb54ee3ba261283682fec959cdfa4ca8b38a5c2 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 2 Mar 2022 22:33:14 -0800 Subject: [PATCH 04/32] fix bazel build --- sdk/test/metrics/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index 79df854e17..69dbbe76d2 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -17,9 +17,9 @@ cc_test( ) cc_test( - name = "meter_reader_test", + name = "metric_reader_test", srcs = [ - "meter_reader_test.cc", + "metric_reader_test.cc", ], tags = [ "metrics", From 8ecf7291b22476ff7d35fda849ab395b3fa2621a Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 2 Mar 2022 23:27:01 -0800 Subject: [PATCH 05/32] fix ostream exporter --- .../include/opentelemetry/exporters/ostream/metric_exporter.h | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h index 5658e3bff7..024462e7ae 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h @@ -8,6 +8,7 @@ # include # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/nostd/span.h" +# include "opentelemetry/sdk/metrics/data/point_data.h" # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/sdk/metrics/recordable.h" From ad471d061585bfd4e9e93ad5e4c97acbd2fbd062 Mon Sep 17 00:00:00 2001 From: Lalit Date: Thu, 3 Mar 2022 10:35:11 -0800 Subject: [PATCH 06/32] build error on windows --- .../include/opentelemetry/exporters/ostream/metric_exporter.h | 2 +- 1 file changed, 1 insertion(+), 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 024462e7ae..5f27db13d2 100644 --- a/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h +++ b/exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h @@ -8,7 +8,7 @@ # include # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/nostd/span.h" -# include "opentelemetry/sdk/metrics/data/point_data.h" +# include "opentelemetry/sdk/metrics/data/metric_data.h" # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/sdk/metrics/metric_exporter.h" # include "opentelemetry/sdk/metrics/recordable.h" From 05c1a8d4970cc46be96c96d44cc63a51b554bb48 Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 13 Mar 2022 21:45:03 -0700 Subject: [PATCH 07/32] initial commit --- .../sdk/metrics/aggregation/aggregation.h | 20 ++-- .../metrics/aggregation/default_aggregation.h | 4 +- .../metrics/aggregation/drop_aggregation.h | 16 +++- .../aggregation/histogram_aggregation.h | 38 +++----- .../aggregation/lastvalue_aggregation.h | 20 ++-- .../sdk/metrics/aggregation/sum_aggregation.h | 41 ++++----- .../sdk/metrics/data/point_data.h | 13 +-- .../sdk/metrics/state/sync_metric_storage.h | 86 ++++++++++++++++- .../aggregation/histogram_aggregation.cc | 87 ++++++++++++------ .../aggregation/lastvalue_aggregation.cc | 92 ++++++++++++++----- .../metrics/aggregation/sum_aggregation.cc | 92 ++++++++++++------- sdk/test/metrics/aggregation_test.cc | 38 ++++---- 12 files changed, 365 insertions(+), 182 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index 3bb133ac15..145ff0aab3 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -11,16 +11,6 @@ namespace sdk { namespace metrics { -class InstrumentMonotonicityAwareAggregation -{ -public: - InstrumentMonotonicityAwareAggregation(bool is_monotonic) : is_monotonic_(is_monotonic) {} - bool IsMonotonic() { return is_monotonic_; } - -private: - bool is_monotonic_; -}; - class Aggregation { public: @@ -28,7 +18,15 @@ class Aggregation virtual void Aggregate(double value, const PointAttributes &attributes = {}) noexcept = 0; - virtual PointType Collect() noexcept = 0; + // Returns the result of the merge of the given delta aggregation + + virtual std::unique_ptr Merge(Aggregation &delta) noexcept = 0; + + // Returns the delta aggregation by comparing with next aggregation + + virtual std::unique_ptr Diff(Aggregation &next) noexcept = 0; + + virtual PointType ToPoint() noexcept = 0; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h index cad43d7aa5..5f98379e6b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h @@ -30,8 +30,8 @@ class DefaultAggregation case InstrumentType::kUpDownCounter: case InstrumentType::kObservableUpDownCounter: return (instrument_descriptor.value_type_ == InstrumentValueType::kLong) - ? std::move(std::unique_ptr(new LongSumAggregation(true))) - : std::move(std::unique_ptr(new DoubleSumAggregation(true))); + ? std::move(std::unique_ptr(new LongSumAggregation())) + : std::move(std::unique_ptr(new DoubleSumAggregation())); break; case InstrumentType::kHistogram: return (instrument_descriptor.value_type_ == InstrumentValueType::kLong) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h index 0af5dd43a5..1cf723d682 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h @@ -23,7 +23,21 @@ class DropAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - PointType Collect() noexcept override { return DropPointData(); } + std::unique_ptr Merge(Aggregation &delta) noexcept override + { + return std::unique_ptr(new DropAggregation()); + } + + std::unique_ptr Diff(Aggregation &next) noexcept override + { + return std::unique_ptr(new DropAggregation()); + } + + PointType ToPoint() noexcept override + { + static DropPointData point_data; + return point_data; + } }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 5a66ecb243..04f925130a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -13,57 +13,47 @@ namespace sdk { namespace metrics { -template -static inline void PopulateHistogramDataPoint(HistogramPointData &histogram, - opentelemetry::common::SystemTimestamp epoch_nanos, - T sum, - uint64_t count, - std::vector &counts, - std::vector boundaries) -{ - histogram.epoch_nanos_ = epoch_nanos; - histogram.boundaries_ = boundaries; - histogram.sum_ = sum; - histogram.counts_ = counts; - histogram.count_ = count; -} class LongHistogramAggregation : public Aggregation { public: LongHistogramAggregation(); + LongHistogramAggregation(HistogramPointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - std::vector boundaries_; - long sum_; - std::vector counts_; - uint64_t count_; + HistogramPointData point_data_; }; class DoubleHistogramAggregation : public Aggregation { public: DoubleHistogramAggregation(); + DoubleHistogramAggregation(HistogramPointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - std::vector boundaries_; - double sum_; - std::vector counts_; - uint64_t count_; + HistogramPointData point_data_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h index 092a0df30e..d488c898c1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h @@ -17,34 +17,42 @@ class LongLastValueAggregation : public Aggregation { public: LongLastValueAggregation(); + LongLastValueAggregation(LastValuePointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - long value_; - bool is_lastvalue_valid_; + LastValuePointData point_data_; }; class DoubleLastValueAggregation : public Aggregation { public: DoubleLastValueAggregation(); + DoubleLastValueAggregation(LastValuePointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - double value_; - bool is_lastvalue_valid_; + LastValuePointData point_data_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index c8b2eff33c..45f7700c1c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -14,52 +14,47 @@ namespace sdk namespace metrics { -template -static inline void PopulateSumPointData(SumPointData &sum, - opentelemetry::common::SystemTimestamp start_ts, - opentelemetry::common::SystemTimestamp end_ts, - T value, - bool is_monotonic) -{ - sum.start_epoch_nanos_ = start_ts; - sum.end_epoch_nanos_ = end_ts; - sum.value_ = value; - sum.is_monotonic_ = is_monotonic; - sum.aggregation_temporarily_ = AggregationTemporarily::kDelta; -} - -class LongSumAggregation : public Aggregation, InstrumentMonotonicityAwareAggregation +class LongSumAggregation : public Aggregation { public: - LongSumAggregation(bool is_monotonic); + LongSumAggregation(); + LongSumAggregation(SumPointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - opentelemetry::common::SystemTimestamp start_epoch_nanos_; + SumPointData point_data_; long sum_; }; -class DoubleSumAggregation : public Aggregation, InstrumentMonotonicityAwareAggregation +class DoubleSumAggregation : public Aggregation { public: - DoubleSumAggregation(bool is_monotonic); + DoubleSumAggregation(); + DoubleSumAggregation(SumPointData &&); void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - PointType Collect() noexcept override; + virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + + virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + + PointType ToPoint() noexcept override; private: opentelemetry::common::SpinLockMutex lock_; - opentelemetry::common::SystemTimestamp start_epoch_nanos_; - double sum_; + SumPointData point_data_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 603bcc30bc..07de374a57 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -8,7 +8,7 @@ # include "opentelemetry/sdk/metrics/instruments.h" # include "opentelemetry/version.h" -# include +# include OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -17,30 +17,25 @@ namespace metrics { using ValueType = nostd::variant; -using ListType = nostd::variant, std::vector>; +using ListType = nostd::variant, std::list>; class SumPointData { public: - opentelemetry::common::SystemTimestamp start_epoch_nanos_; - opentelemetry::common::SystemTimestamp end_epoch_nanos_; ValueType value_; - AggregationTemporarily aggregation_temporarily_; - bool is_monotonic_; }; class LastValuePointData { public: - opentelemetry::common::SystemTimestamp epoch_nanos_; - bool is_lastvalue_valid_; ValueType value_; + bool is_lastvalue_valid_; + opentelemetry::common::SystemTimestamp sample_ts_; }; class HistogramPointData { public: - opentelemetry::common::SystemTimestamp epoch_nanos_; ListType boundaries_; ValueType sum_; std::vector counts_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 3744328270..1672420cbf 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -8,11 +8,14 @@ # include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" # include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" +# include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk/metrics/state/metric_storage.h" + # include "opentelemetry/sdk/metrics/view/attributes_processor.h" # include "opentelemetry/sdk/metrics/view/view.h" # include "opentelemetry/sdk/resource/resource.h" +# include # include OPENTELEMETRY_BEGIN_NAMESPACE @@ -21,6 +24,12 @@ namespace sdk namespace metrics { +struct LastReportedMetrics +{ + std::unique_ptr attributes_map; + opentelemetry::common::SystemTimestamp collection_ts; +}; + class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { @@ -95,6 +104,76 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref callback) noexcept override { + opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; + // add delta metrics to unreported metrics + std::shared_ptr delta_metrics = std::move(attributes_hashmap_); + for (auto &col : collectors) + { + unreported_metrics_[col.get()].push_back(delta_metrics); + } + // Get the unreported_metrics_ data for `collector` since last poll + auto present = unreported_metrics_.find(collector); + if (present == unreported_metrics_.end()) + { + // no unreported metrics for the collector, return. + return true; + } + auto unreported_list = std::move(unreported_metrics_[collector]); + + // Iterate over the unreporter metrics for `collector` and do merge + std::unique_ptr merged_attributes; + for (auto &agg_hashmap : unreported_list) + { + agg_hashmap->GetAllEnteries( + [&merged_attributes, this](const MetricAttributes &attributes, Aggregation &aggregation) { + auto agg = merged_attributes->Get(attributes); + if (agg) + { + merged_attributes->Set(attributes, agg->Merge(aggregation)); + } + else + { + merged_attributes->Set(attributes, create_aggregation()); + } + return true; + }); + } + + auto reported = last_reported_metrics_.find(collector); + if (reported != last_reported_metrics_.end()) + { + last_collection_ts = last_reported_metrics_[collector].collection_ts; + auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); + if (collector->GetAggregationTemporarily() == AggregationTemporarily::kCummulative) + { + // merge current delta to pervious cummulative + last_aggr_hashmap->GetAllEnteries( + [&merged_attributes, this](const MetricAttributes &attributes, + Aggregation &aggregation) { + auto agg = merged_attributes->Get(attributes); + if (agg) + { + merged_attributes->Set(attributes, agg->Merge(aggregation)); + } + else + { + merged_attributes->Set(attributes, create_aggregation()); + } + return true; + }); + } + last_reported_metrics_[collector] = {.attributes_map = std::move(merged_attributes), + .collection_ts = collection_ts}; + } + else + { + last_reported_metrics_.insert(std::make_pair( + collector, LastReportedMetrics{.attributes_map = std::move(merged_attributes), + .collection_ts = collection_ts})); + } + + AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); + MetricData data; if (callback(data)) { @@ -107,6 +186,9 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; std::unique_ptr attributes_hashmap_; + std::unordered_map>> + unreported_metrics_; + std::unordered_map last_reported_metrics_; const AttributesProcessor *attributes_processor_; std::function()> create_default_aggregation_; @@ -140,11 +222,11 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage case AggregationType::kSum: if (instrument_descriptor_.value_type_ == InstrumentValueType::kLong) { - return std::move(std::unique_ptr(new LongSumAggregation(true))); + return std::move(std::unique_ptr(new LongSumAggregation())); } else { - return std::move(std::unique_ptr(new DoubleSumAggregation(true))); + return std::move(std::unique_ptr(new DoubleSumAggregation())); } break; default: diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index 2ebe3715bb..efaec848f6 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -13,65 +13,96 @@ namespace metrics { LongHistogramAggregation::LongHistogramAggregation() - : boundaries_{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l}, - counts_(boundaries_.size() + 1, 0), - sum_(0l), - count_(0) +{ + + point_data_.boundaries_ = std::list{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l}; + point_data_.counts_ = + std::vector(nostd::get>(point_data_.boundaries_).size() + 1, 0); + point_data_.sum_ = 0l; + point_data_.count_ = 0; +} + +LongHistogramAggregation::LongHistogramAggregation(HistogramPointData &&data) + : point_data_{std::move(data)} {} void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - count_ += 1; - sum_ += value; - for (auto it = boundaries_.begin(); it != boundaries_.end(); ++it) + point_data_.count_ += 1; + point_data_.sum_ = nostd::get(point_data_.sum_) + value; + for (auto it = nostd::get>(point_data_.boundaries_).begin(); + it != nostd::get>(point_data_.boundaries_).end(); ++it) { if (value < *it) { - counts_[std::distance(boundaries_.begin(), it)] += 1; + point_data_.counts_[std::distance( + nostd::get>(point_data_.boundaries_).begin(), it)] += 1; return; } } } -PointType LongHistogramAggregation::Collect() noexcept +std::unique_ptr LongHistogramAggregation::Merge(Aggregation &delta) noexcept { - HistogramPointData point_data; - auto epoch_nanos = std::chrono::system_clock::now(); - const std::lock_guard locked(lock_); - PopulateHistogramDataPoint(point_data, epoch_nanos, sum_, count_, counts_, boundaries_); - return point_data; + return nullptr; +} + +std::unique_ptr LongHistogramAggregation::Diff(Aggregation &next) noexcept +{ + return nullptr; +} + +PointType LongHistogramAggregation::ToPoint() noexcept +{ + return point_data_; } DoubleHistogramAggregation::DoubleHistogramAggregation() - : boundaries_{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0}, - counts_(boundaries_.size() + 1, 0), - sum_(0.0), - count_(0) +{ + + point_data_.boundaries_ = + std::list{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0}; + point_data_.counts_ = + std::vector(nostd::get>(point_data_.boundaries_).size() + 1, 0); + point_data_.sum_ = 0l; + point_data_.count_ = 0; +} + +DoubleHistogramAggregation::DoubleHistogramAggregation(HistogramPointData &&data) + : point_data_{std::move(data)} {} void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - count_ += 1; - sum_ += value; - for (auto it = boundaries_.begin(); it != boundaries_.end(); ++it) + point_data_.count_ += 1; + point_data_.sum_ = nostd::get(point_data_.sum_) + value; + for (auto it = nostd::get>(point_data_.boundaries_).begin(); + it != nostd::get>(point_data_.boundaries_).end(); ++it) { if (value < *it) { - counts_[std::distance(boundaries_.begin(), it)] += 1; + point_data_.counts_[std::distance( + nostd::get>(point_data_.boundaries_).begin(), it)] += 1; return; } } } -PointType DoubleHistogramAggregation::Collect() noexcept +std::unique_ptr DoubleHistogramAggregation::Merge(Aggregation &delta) noexcept { - HistogramPointData point_data; - auto epoch_nanos = std::chrono::system_clock::now(); - const std::lock_guard locked(lock_); - PopulateHistogramDataPoint(point_data, epoch_nanos, sum_, count_, counts_, boundaries_); - return point_data; + return nullptr; +} + +std::unique_ptr DoubleHistogramAggregation::Diff(Aggregation &next) noexcept +{ + return nullptr; +} + +PointType DoubleHistogramAggregation::ToPoint() noexcept +{ + return point_data_; } } // namespace metrics diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index 290b526344..abbf1a3588 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -14,46 +14,90 @@ namespace sdk namespace metrics { -LongLastValueAggregation::LongLastValueAggregation() : is_lastvalue_valid_(false) {} +LongLastValueAggregation::LongLastValueAggregation() +{ + point_data_.is_lastvalue_valid_ = false; + point_data_.value_ = 0l; +} +LongLastValueAggregation::LongLastValueAggregation(LastValuePointData &&data) + : point_data_{std::move(data)} +{} void LongLastValueAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - is_lastvalue_valid_ = true; - value_ = value; + point_data_.is_lastvalue_valid_ = true; + point_data_.value_ = value; } -PointType LongLastValueAggregation::Collect() noexcept +std::unique_ptr LongLastValueAggregation::Merge(Aggregation &delta) noexcept { - const std::lock_guard locked(lock_); - if (!is_lastvalue_valid_) - { - return LastValuePointData{ - opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()), false, 0l}; - } - return LastValuePointData{ - opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()), true, value_}; + Aggregation &agg_to_merge = + nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch() + ? *this + : delta; + LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); +} + +std::unique_ptr LongLastValueAggregation::Diff(Aggregation &next) noexcept +{ + Aggregation &agg_to_merge = + nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() + ? *this + : next; + LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); +} + +PointType LongLastValueAggregation::ToPoint() noexcept +{ + return point_data_; } -DoubleLastValueAggregation::DoubleLastValueAggregation() : is_lastvalue_valid_(false) {} +DoubleLastValueAggregation::DoubleLastValueAggregation() +{ + point_data_.is_lastvalue_valid_ = false; + point_data_.value_ = 0.0; +} +DoubleLastValueAggregation::DoubleLastValueAggregation(LastValuePointData &&data) + : point_data_{std::move(data)} +{} void DoubleLastValueAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - is_lastvalue_valid_ = true; - value_ = value; + point_data_.is_lastvalue_valid_ = true; + point_data_.value_ = value; } -PointType DoubleLastValueAggregation::Collect() noexcept +std::unique_ptr DoubleLastValueAggregation::Merge(Aggregation &delta) noexcept { - const std::lock_guard locked(lock_); - if (!is_lastvalue_valid_) - { - return LastValuePointData{ - opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()), false, 0.0}; - } - return LastValuePointData{ - opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now()), true, value_}; + Aggregation &agg_to_merge = + nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch() + ? *this + : delta; + LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); +} + +std::unique_ptr DoubleLastValueAggregation::Diff(Aggregation &next) noexcept +{ + Aggregation &agg_to_merge = + nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() + ? *this + : next; + LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); +} + +PointType DoubleLastValueAggregation::ToPoint() noexcept +{ + return point_data_; } } // namespace metrics } // namespace sdk diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index e0d1330f68..49489a6179 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -3,6 +3,7 @@ #ifndef ENABLE_METRICS_PREVIEW # include "opentelemetry/sdk/metrics/aggregation/sum_aggregation.h" +# include "opentelemetry/sdk/metrics/data/point_data.h" # include "opentelemetry/version.h" # include @@ -13,54 +14,81 @@ namespace sdk namespace metrics { -LongSumAggregation::LongSumAggregation(bool is_monotonic) - : InstrumentMonotonicityAwareAggregation(is_monotonic), - start_epoch_nanos_(opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())), - sum_(0l) -{} +LongSumAggregation::LongSumAggregation() +{ + point_data_.value_ = 0l; +} + +LongSumAggregation::LongSumAggregation(SumPointData &&data) : point_data_{std::move(data)} {} void LongSumAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - sum_ += value; + point_data_.value_ = nostd::get(point_data_.value_) + value; +} + +std::unique_ptr LongSumAggregation::Merge(Aggregation &delta) noexcept +{ + SumPointData merge_data = { + .value_ = nostd::get( + nostd::get((static_cast(delta).ToPoint())) + .value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new LongSumAggregation(std::move(merge_data))); } -PointType LongSumAggregation::Collect() noexcept +std::unique_ptr LongSumAggregation::Diff(Aggregation &next) noexcept { - opentelemetry::common::SystemTimestamp current_ts(std::chrono::system_clock::now()); - SumPointData sum; - { - const std::lock_guard locked(lock_); - PopulateSumPointData(sum, start_epoch_nanos_, current_ts, sum_, IsMonotonic()); - start_epoch_nanos_ = current_ts; - sum_ = 0; - } - return sum; + SumPointData diff_data = { + .value_ = nostd::get( + nostd::get((static_cast(next).ToPoint())) + .value_) - + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new LongSumAggregation(std::move(diff_data))); } -DoubleSumAggregation::DoubleSumAggregation(bool is_monotonic) - : InstrumentMonotonicityAwareAggregation(is_monotonic), - start_epoch_nanos_(opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())), - sum_(0L) -{} +PointType LongSumAggregation::ToPoint() noexcept +{ + return point_data_; +} + +DoubleSumAggregation::DoubleSumAggregation() +{ + point_data_.value_ = 0.0; +} + +DoubleSumAggregation::DoubleSumAggregation(SumPointData &&data) : point_data_(std::move(data)) {} void DoubleSumAggregation::Aggregate(double value, const PointAttributes &attributes) noexcept { const std::lock_guard locked(lock_); - sum_ += value; + point_data_.value_ = nostd::get(point_data_.value_) + value; +} + +std::unique_ptr DoubleSumAggregation::Merge(Aggregation &delta) noexcept +{ + SumPointData merge_data = { + .value_ = nostd::get( + nostd::get((static_cast(delta).ToPoint())) + .value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); } -PointType DoubleSumAggregation::Collect() noexcept +std::unique_ptr DoubleSumAggregation::Diff(Aggregation &next) noexcept { - opentelemetry::common::SystemTimestamp current_ts(std::chrono::system_clock::now()); - SumPointData sum; - { - const std::lock_guard locked(lock_); - PopulateSumPointData(sum, start_epoch_nanos_, current_ts, sum_, IsMonotonic()); - start_epoch_nanos_ = current_ts; - sum_ = 0; - } - return sum; + SumPointData merge_data = { + .value_ = nostd::get( + nostd::get((static_cast(next).ToPoint())) + .value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); +} + +PointType DoubleSumAggregation::ToPoint() noexcept +{ + const std::lock_guard locked(lock_); + return point_data_; } } // namespace metrics diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index ec753111ff..a32826b145 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -13,82 +13,80 @@ using namespace opentelemetry::sdk::metrics; namespace nostd = opentelemetry::nostd; TEST(Aggregation, LongSumAggregation) { - LongSumAggregation aggr(true); - auto data = aggr.Collect(); + LongSumAggregation aggr; + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto sum_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(sum_data.value_)); EXPECT_EQ(nostd::get(sum_data.value_), 0l); - EXPECT_EQ(sum_data.is_monotonic_, true); EXPECT_NO_THROW(aggr.Aggregate(12l, {})); EXPECT_NO_THROW(aggr.Aggregate(0l, {})); - sum_data = nostd::get(aggr.Collect()); + sum_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(sum_data.value_), 12l); } TEST(Aggregation, DoubleSumAggregation) { - DoubleSumAggregation aggr(true); - auto data = aggr.Collect(); + DoubleSumAggregation aggr; + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto sum_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(sum_data.value_)); EXPECT_EQ(nostd::get(sum_data.value_), 0); - EXPECT_EQ(sum_data.is_monotonic_, true); EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); EXPECT_NO_THROW(aggr.Aggregate(1.0, {})); - sum_data = nostd::get(aggr.Collect()); + sum_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(sum_data.value_), 13.0); } TEST(Aggregation, LongLastValueAggregation) { LongLastValueAggregation aggr; - auto data = aggr.Collect(); + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto lastvalue_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(lastvalue_data.value_)); EXPECT_EQ(lastvalue_data.is_lastvalue_valid_, false); EXPECT_NO_THROW(aggr.Aggregate(12l, {})); EXPECT_NO_THROW(aggr.Aggregate(1l, {})); - lastvalue_data = nostd::get(aggr.Collect()); + lastvalue_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(lastvalue_data.value_), 1.0); } TEST(Aggregation, DoubleLastValueAggregation) { DoubleLastValueAggregation aggr; - auto data = aggr.Collect(); + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto lastvalue_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(lastvalue_data.value_)); EXPECT_EQ(lastvalue_data.is_lastvalue_valid_, false); EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); EXPECT_NO_THROW(aggr.Aggregate(1.0, {})); - lastvalue_data = nostd::get(aggr.Collect()); + lastvalue_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(lastvalue_data.value_), 1.0); } TEST(Aggregation, LongHistogramAggregation) { LongHistogramAggregation aggr; - auto data = aggr.Collect(); + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto histogram_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(histogram_data.sum_)); - ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); + ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); EXPECT_EQ(nostd::get(histogram_data.sum_), 0); EXPECT_EQ(histogram_data.count_, 0); EXPECT_NO_THROW(aggr.Aggregate(12l, {})); // lies in fourth bucket EXPECT_NO_THROW(aggr.Aggregate(100l, {})); // lies in eight bucket - histogram_data = nostd::get(aggr.Collect()); + histogram_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(histogram_data.sum_), 112); EXPECT_EQ(histogram_data.count_, 2); EXPECT_EQ(histogram_data.counts_[3], 1); EXPECT_EQ(histogram_data.counts_[7], 1); EXPECT_NO_THROW(aggr.Aggregate(13l, {})); // lies in fourth bucket EXPECT_NO_THROW(aggr.Aggregate(252l, {})); // lies in ninth bucket - histogram_data = nostd::get(aggr.Collect()); + histogram_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(histogram_data.count_, 4); EXPECT_EQ(histogram_data.counts_[3], 2); EXPECT_EQ(histogram_data.counts_[8], 1); @@ -97,23 +95,23 @@ TEST(Aggregation, LongHistogramAggregation) TEST(Aggregation, DoubleHistogramAggregation) { DoubleHistogramAggregation aggr; - auto data = aggr.Collect(); + auto data = aggr.ToPoint(); ASSERT_TRUE(nostd::holds_alternative(data)); auto histogram_data = nostd::get(data); ASSERT_TRUE(nostd::holds_alternative(histogram_data.sum_)); - ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); + ASSERT_TRUE(nostd::holds_alternative>(histogram_data.boundaries_)); EXPECT_EQ(nostd::get(histogram_data.sum_), 0); EXPECT_EQ(histogram_data.count_, 0); EXPECT_NO_THROW(aggr.Aggregate(12.0, {})); // lies in fourth bucket EXPECT_NO_THROW(aggr.Aggregate(100.0, {})); // lies in eight bucket - histogram_data = nostd::get(aggr.Collect()); + histogram_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(nostd::get(histogram_data.sum_), 112); EXPECT_EQ(histogram_data.count_, 2); EXPECT_EQ(histogram_data.counts_[3], 1); EXPECT_EQ(histogram_data.counts_[7], 1); EXPECT_NO_THROW(aggr.Aggregate(13.0, {})); // lies in fourth bucket EXPECT_NO_THROW(aggr.Aggregate(252.0, {})); // lies in ninth bucket - histogram_data = nostd::get(aggr.Collect()); + histogram_data = nostd::get(aggr.ToPoint()); EXPECT_EQ(histogram_data.count_, 4); EXPECT_EQ(histogram_data.counts_[3], 2); EXPECT_EQ(histogram_data.counts_[8], 1); From f0380a9e05e372f2598f3ba407b7c42530e2af41 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 16 Mar 2022 14:02:51 -0700 Subject: [PATCH 08/32] implement collect function --- .../sdk/metrics/data/metric_data.h | 15 ++++++++---- sdk/include/opentelemetry/sdk/metrics/meter.h | 2 +- .../sdk/metrics/state/metric_storage.h | 6 ----- .../sdk/metrics/state/sync_metric_storage.h | 23 +++++++++++++------ .../aggregation/histogram_aggregation.cc | 2 +- sdk/src/metrics/meter.cc | 3 +-- sdk/src/metrics/state/metric_collector.cc | 2 +- sdk/test/metrics/sync_metric_storage_test.cc | 19 +++++++++++++++ 8 files changed, 49 insertions(+), 23 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h index cb7bc4cf80..738d4540f7 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h @@ -17,18 +17,23 @@ namespace sdk namespace metrics { -using PointAttributes = opentelemetry::sdk::common::AttributeMap; +using PointAttributes = opentelemetry::sdk::common::OrderedAttributeMap; using PointType = opentelemetry::nostd:: variant; +struct PointDataAttributes +{ + PointAttributes attributes; + PointType point_data; +}; + class MetricData { public: - opentelemetry::sdk::resource::Resource *resource_; - opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library_; - PointAttributes attributes_; InstrumentDescriptor instrument_descriptor; - PointType point_data_; + opentelemetry::common::SystemTimestamp start_ts; + opentelemetry::common::SystemTimestamp end_ts; + std::vector point_data_attr_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index fc9fb36503..c9e02918a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -103,7 +103,7 @@ class Meter final : public opentelemetry::metrics::Meter const noexcept; /** collect metrics across all the meters **/ - bool collect(CollectorHandle *collector, + bool Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts, nostd::function_ref callback) noexcept; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h index d64aff40e1..69f1f8f5af 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h @@ -22,9 +22,6 @@ class MetricStorage /* collect the metrics from this storage */ virtual bool Collect(CollectorHandle *collector, nostd::span> collectors, - const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary - &instrumentation_library, - const opentelemetry::sdk::resource::Resource &resource, opentelemetry::common::SystemTimestamp sdk_start_ts, opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref callback) noexcept = 0; @@ -49,9 +46,6 @@ class NoopMetricStorage : public MetricStorage public: bool Collect(CollectorHandle *collector, nostd::span> collectors, - const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary - &instrumentation_library, - const opentelemetry::sdk::resource::Resource &resource, opentelemetry::common::SystemTimestamp sdk_start_ts, opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref callback) noexcept override diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 1672420cbf..cf78b17df1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -97,14 +97,12 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage bool Collect(CollectorHandle *collector, nostd::span> collectors, - const opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary - &instrumentation_library, - const opentelemetry::sdk::resource::Resource &resource, opentelemetry::common::SystemTimestamp sdk_start_ts, opentelemetry::common::SystemTimestamp collection_ts, nostd::function_ref callback) noexcept override { opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; + auto aggregation_temporarily = collector->GetAggregationTemporarily(); // add delta metrics to unreported metrics std::shared_ptr delta_metrics = std::move(attributes_hashmap_); for (auto &col : collectors) @@ -144,7 +142,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { last_collection_ts = last_reported_metrics_[collector].collection_ts; auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); - if (collector->GetAggregationTemporarily() == AggregationTemporarily::kCummulative) + if (aggregation_temporarily == AggregationTemporarily::kCummulative) { // merge current delta to pervious cummulative last_aggr_hashmap->GetAllEnteries( @@ -173,9 +171,20 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); - - MetricData data; - if (callback(data)) + MetricData metric_data; + metric_data.instrument_descriptor = instrument_descriptor_; + metric_data.start_ts = last_collection_ts; + metric_data.end_ts = collection_ts; + result_to_export->GetAllEnteries( + [&metric_data](const MetricAttributes &attributes, Aggregation &aggregation) { + PointDataAttributes point_data_attr; + point_data_attr.point_data = aggregation.ToPoint(); + point_data_attr.attributes = attributes; + metric_data.point_data_attr_.push_back(point_data_attr); + return true; + }); + + if (callback(metric_data)) { return true; } diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index efaec848f6..e36ffbbf55 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -65,7 +65,7 @@ DoubleHistogramAggregation::DoubleHistogramAggregation() std::list{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0}; point_data_.counts_ = std::vector(nostd::get>(point_data_.boundaries_).size() + 1, 0); - point_data_.sum_ = 0l; + point_data_.sum_ = 0.0; point_data_.count_ = 0; } diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 0db8ca59af..1aca7dbaed 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -209,7 +209,7 @@ std::unique_ptr Meter::RegisterMetricStorage( } /** collect metrics across all the meters **/ -bool Meter::collect(CollectorHandle *collector, +bool Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts, nostd::function_ref callback) noexcept { @@ -218,7 +218,6 @@ bool Meter::collect(CollectorHandle *collector, { // TBD - this needs to be asynchronous metric_storage.second->Collect(collector, meter_context_->GetCollectors(), - *instrumentation_library_, meter_context_->GetResource(), meter_context_->GetSDKStartTime(), collect_ts, [&callback](MetricData &metric_data) { callback(metric_data); diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index dc7039c2d4..d2c4c6ba39 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -34,7 +34,7 @@ bool MetricCollector::Collect(nostd::function_ref callback) no for (auto &meter : meter_context_->GetMeters()) { auto collection_ts = std::chrono::system_clock::now(); - meter->collect(this, collection_ts, callback); + meter->Collect(this, collection_ts, callback); } return true; } diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index 916425dfa1..1b21a512f3 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -12,6 +12,15 @@ using namespace opentelemetry::sdk::metrics; using M = std::map; +class MockCollectorHandle : public CollectorHandle +{ +public: + AggregationTemporarily GetAggregationTemporarily() noexcept override + { + return AggregationTemporarily::kCummulative; + } +}; + TEST(WritableMetricStorageTest, BasicTests) { InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, @@ -24,5 +33,15 @@ TEST(WritableMetricStorageTest, BasicTests) 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}))); EXPECT_NO_THROW(storage.RecordDouble(10.10, opentelemetry::common::KeyValueIterableView({}))); + opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); + MockCollectorHandle collector; + std::vector> collectors; + + auto sdk_start_ts = std::chrono::system_clock::now(); + // Some computation here + auto collection_ts = std::chrono::system_clock::now() + std::chrono::seconds(5); + + storage.Collect(&collector, collectors, sdk_start_ts, collection_ts, + [](const MetricData data) { return true; }); } #endif From e702e89972d310fc2aa3fbce608077235c38966a Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 16 Mar 2022 15:20:22 -0700 Subject: [PATCH 09/32] fix build --- .../sdk/metrics/aggregation/default_aggregation.h | 4 ++-- .../sdk/metrics/state/async_metric_storage.h | 14 ++++++-------- .../sdk/metrics/state/sync_metric_storage.h | 6 ++++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h index df5f49dfe4..b5a1283d26 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h @@ -79,11 +79,11 @@ class DefaultAggregation case AggregationType::kSum: if (instrument_descriptor.value_type_ == InstrumentValueType::kLong) { - return std::unique_ptr(new LongSumAggregation(true)); + return std::unique_ptr(new LongSumAggregation()); } else { - return std::unique_ptr(new DoubleSumAggregation(true)); + return std::unique_ptr(new DoubleSumAggregation()); } break; default: diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index d5491e91cb..31ab70c5b8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -6,12 +6,11 @@ # include "opentelemetry/sdk/common/attributemap_hash.h" # include "opentelemetry/sdk/metrics/instruments.h" -# include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" # include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" # include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" +# include "opentelemetry/sdk/metrics/state/metric_collector.h" # include "opentelemetry/sdk/metrics/state/metric_storage.h" # include "opentelemetry/sdk/metrics/view/attributes_processor.h" -# include "opentelemetry/sdk/resource/resource.h" # include # include "opentelemetry/sdk/metrics/observer_result.h" @@ -37,12 +36,11 @@ class AsyncMetricStorage : public MetricStorage active_attributes_hashmap_(new AttributesHashMap()) {} - bool Collect( - MetricCollector *collector, - nostd::span collectors, - opentelemetry::sdk::instrumentationlibrary::InstrumentationLibrary *instrumentation_library, - opentelemetry::sdk::resource::Resource *resource, - nostd::function_ref metric_collection_callback) noexcept override + bool Collect(CollectorHandle *collector, + nostd::span> collectors, + opentelemetry::common::SystemTimestamp sdk_start_ts, + opentelemetry::common::SystemTimestamp collection_ts, + nostd::function_ref metric_collection_callback) noexcept override { opentelemetry::sdk::metrics::ObserverResult ob_res(attributes_processor_); diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index e6d67d4b92..68b4720198 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -127,7 +127,8 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } else { - merged_attributes->Set(attributes, create_aggregation()); + merged_attributes->Set(attributes, + DefaultAggregation::CreateAggregation(instrument_descriptor_)); } return true; }); @@ -151,7 +152,8 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } else { - merged_attributes->Set(attributes, create_aggregation()); + merged_attributes->Set( + attributes, DefaultAggregation::CreateAggregation(instrument_descriptor_)); } return true; }); From d12686ebbfee97eb88dac9dadb34dc6ea1a84f3d Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 16 Mar 2022 15:23:07 -0700 Subject: [PATCH 10/32] Fix async test --- sdk/test/metrics/async_metric_storage_test.cc | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index fd2a41d325..cdca9f0d3a 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -17,6 +17,15 @@ using namespace opentelemetry::sdk::metrics; using namespace opentelemetry::sdk::instrumentationlibrary; using namespace opentelemetry::sdk::resource; +class MockCollectorHandle : public CollectorHandle +{ +public: + AggregationTemporarily GetAggregationTemporarily() noexcept override + { + return AggregationTemporarily::kCummulative; + } +}; + void measurement_fetch(opentelemetry::metrics::ObserverResult &observer_result) { observer_result.Observe(20l); @@ -28,14 +37,16 @@ TEST(AsyncMetricStorageTest, BasicTests) auto metric_callback = [](MetricData &metric_data) { return true; }; InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, InstrumentValueType::kLong}; - auto instrumentation_library = InstrumentationLibrary::Create("instr_lib"); - auto resource = Resource::Create({}); - MetricCollector collector; - std::vector collectors; + + MockCollectorHandle collector; + std::vector> collectors; + auto sdk_start_ts = std::chrono::system_clock::now(); + // Some computation here + auto collection_ts = std::chrono::system_clock::now() + std::chrono::seconds(5); opentelemetry::sdk::metrics::AsyncMetricStorage storage( instr_desc, AggregationType::kSum, &measurement_fetch, new DefaultAttributesProcessor()); - EXPECT_NO_THROW(storage.Collect(&collector, collectors, instrumentation_library.get(), &resource, - metric_callback)); + EXPECT_NO_THROW( + storage.Collect(&collector, collectors, sdk_start_ts, collection_ts, metric_callback)); } #endif \ No newline at end of file From 884f636f9a5155528afd21e23bb471c3668da25d Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 16 Mar 2022 15:27:13 -0700 Subject: [PATCH 11/32] fix spell --- .../opentelemetry/sdk/metrics/state/sync_metric_storage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 68b4720198..35aaac7ad6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -141,7 +141,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); if (aggregation_temporarily == AggregationTemporarily::kCummulative) { - // merge current delta to pervious cummulative + // merge current delta to previous cummulative last_aggr_hashmap->GetAllEnteries( [&merged_attributes, this](const MetricAttributes &attributes, Aggregation &aggregation) { From 1874375063e5ef7b832b1d40cc34ae0b1fa51b24 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 16 Mar 2022 20:03:15 -0700 Subject: [PATCH 12/32] fix build --- .../sdk/metrics/state/sync_metric_storage.h | 7 ++-- .../metrics/aggregation/sum_aggregation.cc | 35 ++++++++++--------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 35aaac7ad6..826933459a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -158,14 +158,13 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage return true; }); } - last_reported_metrics_[collector] = {.attributes_map = std::move(merged_attributes), - .collection_ts = collection_ts}; + last_reported_metrics_[collector] = + LastReportedMetrics{std::move(merged_attributes), collection_ts}; } else { last_reported_metrics_.insert(std::make_pair( - collector, LastReportedMetrics{.attributes_map = std::move(merged_attributes), - .collection_ts = collection_ts})); + collector, LastReportedMetrics{std::move(merged_attributes), collection_ts})); } AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 49489a6179..7d26437af2 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -29,21 +29,22 @@ void LongSumAggregation::Aggregate(long value, const PointAttributes &attributes std::unique_ptr LongSumAggregation::Merge(Aggregation &delta) noexcept { - SumPointData merge_data = { - .value_ = nostd::get( - nostd::get((static_cast(delta).ToPoint())) - .value_) + - nostd::get(nostd::get(ToPoint()).value_)}; + + SumPointData merge_data{ + nostd::get( + nostd::get((static_cast(delta).ToPoint())).value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new LongSumAggregation(std::move(merge_data))); } std::unique_ptr LongSumAggregation::Diff(Aggregation &next) noexcept { SumPointData diff_data = { - .value_ = nostd::get( - nostd::get((static_cast(next).ToPoint())) - .value_) - - nostd::get(nostd::get(ToPoint()).value_)}; + nostd::get( + nostd::get((static_cast(next).ToPoint())).value_) - + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new LongSumAggregation(std::move(diff_data))); } @@ -68,20 +69,20 @@ void DoubleSumAggregation::Aggregate(double value, const PointAttributes &attrib std::unique_ptr DoubleSumAggregation::Merge(Aggregation &delta) noexcept { SumPointData merge_data = { - .value_ = nostd::get( - nostd::get((static_cast(delta).ToPoint())) - .value_) + - nostd::get(nostd::get(ToPoint()).value_)}; + nostd::get( + nostd::get((static_cast(delta).ToPoint())).value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); } std::unique_ptr DoubleSumAggregation::Diff(Aggregation &next) noexcept { SumPointData merge_data = { - .value_ = nostd::get( - nostd::get((static_cast(next).ToPoint())) - .value_) + - nostd::get(nostd::get(ToPoint()).value_)}; + nostd::get( + nostd::get((static_cast(next).ToPoint())).value_) + + nostd::get(nostd::get(ToPoint()).value_)}; + return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); } From e011a2baa5d8b93897d879f32a0ee7d9b1a658c4 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 18 Mar 2022 00:01:11 -0700 Subject: [PATCH 13/32] add comments --- .../sdk/metrics/aggregation/aggregation.h | 26 ++++++++++++++++--- .../metrics/aggregation/drop_aggregation.h | 4 +++ .../sdk/metrics/state/sync_metric_storage.h | 15 +++++++---- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index 145ff0aab3..9a909651f2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -18,15 +18,35 @@ class Aggregation virtual void Aggregate(double value, const PointAttributes &attributes = {}) noexcept = 0; - // Returns the result of the merge of the given delta aggregation + /** + * Returns the result of the merge of the two aggregations. + * + * This should always assume that the aggregations do not overlap and merge together for a new + * cumulative report. + * + * @param delta the newly captured (delta) aggregation + * @return the result of the merge of the given aggregation. + */ virtual std::unique_ptr Merge(Aggregation &delta) noexcept = 0; - // Returns the delta aggregation by comparing with next aggregation - + /** + * Returns a new DELTA aggregation by comparing two cumulative measurements. + * + * @param next the newly captured (cumulative) aggregation. + * @return The resulting delta aggregation. + */ virtual std::unique_ptr Diff(Aggregation &next) noexcept = 0; + /** + * Returns the point data that the aggregation will produce. + * + * @return PointType + */ + virtual PointType ToPoint() noexcept = 0; + + virtual ~Aggregation() = default; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h index 1cf723d682..f7ff8e2588 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h @@ -14,6 +14,10 @@ namespace sdk namespace metrics { +/** + * A null Aggregation which denotes no aggregation should occur. + */ + class DropAggregation : public Aggregation { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 826933459a..5c418f85a6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -99,13 +99,18 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; auto aggregation_temporarily = collector->GetAggregationTemporarily(); - // add delta metrics to unreported metrics + + // Add the current delta metrics to unreported metrics for all the collectors, + // this will also empty the delta metrics hashmap, and make it available for + // recordings std::shared_ptr delta_metrics = std::move(attributes_hashmap_); for (auto &col : collectors) { unreported_metrics_[col.get()].push_back(delta_metrics); } - // Get the unreported_metrics_ data for `collector` since last poll + + // Get the unreported metrics for the `collector` since last collection, this will + // also cleanup the unreported metrics. Return early if there is no unreported metrics auto present = unreported_metrics_.find(collector); if (present == unreported_metrics_.end()) { @@ -115,12 +120,12 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage auto unreported_list = std::move(unreported_metrics_[collector]); // Iterate over the unreporter metrics for `collector` and do merge - std::unique_ptr merged_attributes; + std::unique_ptr merged_metrics; for (auto &agg_hashmap : unreported_list) { agg_hashmap->GetAllEnteries( - [&merged_attributes, this](const MetricAttributes &attributes, Aggregation &aggregation) { - auto agg = merged_attributes->Get(attributes); + [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { + auto agg = merged_metrics->Get(attributes); if (agg) { merged_attributes->Set(attributes, agg->Merge(aggregation)); From 5ab4f75d85bd95ae93d730c6b905a78880ce6e19 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 18 Mar 2022 00:05:03 -0700 Subject: [PATCH 14/32] comments --- .../sdk/metrics/state/sync_metric_storage.h | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 5c418f85a6..6bbc5eedd5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -100,7 +100,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; auto aggregation_temporarily = collector->GetAggregationTemporarily(); - // Add the current delta metrics to unreported metrics for all the collectors, + // Add the current delta metrics to `unreported metrics stash` for all the collectors, // this will also empty the delta metrics hashmap, and make it available for // recordings std::shared_ptr delta_metrics = std::move(attributes_hashmap_); @@ -109,8 +109,9 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage unreported_metrics_[col.get()].push_back(delta_metrics); } - // Get the unreported metrics for the `collector` since last collection, this will - // also cleanup the unreported metrics. Return early if there is no unreported metrics + // Get the unreported metrics for the `collector` from `unreported metrics stash` + // since last collection, this will also cleanup the unreported metrics for `collector` + // from the stash. auto present = unreported_metrics_.find(collector); if (present == unreported_metrics_.end()) { @@ -119,7 +120,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } auto unreported_list = std::move(unreported_metrics_[collector]); - // Iterate over the unreporter metrics for `collector` and do merge + // Iterate over the unreporter metrics for `collector` and store result in `merged_metrics` std::unique_ptr merged_metrics; for (auto &agg_hashmap : unreported_list) { @@ -128,17 +129,26 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage auto agg = merged_metrics->Get(attributes); if (agg) { - merged_attributes->Set(attributes, agg->Merge(aggregation)); + merged_metrics->Set(attributes, agg->Merge(aggregation)); } else { - merged_attributes->Set(attributes, - DefaultAggregation::CreateAggregation(instrument_descriptor_)); + merged_metrics->Set(attributes, + DefaultAggregation::CreateAggregation(instrument_descriptor_)); } return true; }); } + // Get the last reported metrics for the `collector` from `last reported metrics` stash + // - If the aggregation_temporarily for the collector is cummulative + // - Merge the last reported metrics with unreported metrics (which is in merged_metrics), + // Final result of merge would be in merged_metrics. + // - Move the final merge to the `last reported metrics` stash. + // - If the aggregation_temporarily is delta + // - Store the unreported metrics for `collector`(which is in merged_mtrics) to + // `last reported metrics` stash. + auto reported = last_reported_metrics_.find(collector); if (reported != last_reported_metrics_.end()) { @@ -148,30 +158,31 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { // merge current delta to previous cummulative last_aggr_hashmap->GetAllEnteries( - [&merged_attributes, this](const MetricAttributes &attributes, - Aggregation &aggregation) { - auto agg = merged_attributes->Get(attributes); + [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { + auto agg = merged_metrics->Get(attributes); if (agg) { - merged_attributes->Set(attributes, agg->Merge(aggregation)); + merged_metrics->Set(attributes, agg->Merge(aggregation)); } else { - merged_attributes->Set( - attributes, DefaultAggregation::CreateAggregation(instrument_descriptor_)); + merged_metrics->Set(attributes, + DefaultAggregation::CreateAggregation(instrument_descriptor_)); } return true; }); } last_reported_metrics_[collector] = - LastReportedMetrics{std::move(merged_attributes), collection_ts}; + LastReportedMetrics{std::move(merged_metrics), collection_ts}; } else { - last_reported_metrics_.insert(std::make_pair( - collector, LastReportedMetrics{std::move(merged_attributes), collection_ts})); + last_reported_metrics_.insert( + std::make_pair(collector, LastReportedMetrics{std::move(merged_metrics), collection_ts})); } + // Generate the MetricData from the final merged_metrics, and invoke callback over it. + AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); MetricData metric_data; metric_data.instrument_descriptor = instrument_descriptor_; @@ -195,9 +206,13 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage private: InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; + + // hashmap to maintain the metrics for delta collection (i.e, collection since last Collect call) std::unique_ptr attributes_hashmap_; + // unreported metrics stash for all the collectors std::unordered_map>> unreported_metrics_; + // last reported metrics stash for all the collectors. std::unordered_map last_reported_metrics_; const AttributesProcessor *attributes_processor_; std::function()> create_default_aggregation_; From c30b2345021b69fb5948f52d04d1609fb330bedc Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 21 Mar 2022 19:50:16 -0700 Subject: [PATCH 15/32] temp add .gitpod.yml --- .gitpod.yml | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitpod.yml diff --git a/.gitpod.yml b/.gitpod.yml new file mode 100644 index 0000000000..e6e6fe66a8 --- /dev/null +++ b/.gitpod.yml @@ -0,0 +1 @@ +ubuntu-18.04 From 618de7380a75860898805b3ff141ae410302f587 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 21 Mar 2022 19:50:57 -0700 Subject: [PATCH 16/32] fix format --- .gitpod.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitpod.yml b/.gitpod.yml index e6e6fe66a8..8588340261 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -1 +1 @@ -ubuntu-18.04 +image: ubuntu-18.04 From 66d7af05fa63a926e803272bdb357119e935d6e8 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 21 Mar 2022 20:05:51 -0700 Subject: [PATCH 17/32] fix --- .gitpod.Dockerfile | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitpod.Dockerfile diff --git a/.gitpod.Dockerfile b/.gitpod.Dockerfile new file mode 100644 index 0000000000..617801170f --- /dev/null +++ b/.gitpod.Dockerfile @@ -0,0 +1 @@ +FROM ubuntu:18.04 From e49e7c4357a153c3a144f01050ee8b462b72e71f Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 21 Mar 2022 20:06:52 -0700 Subject: [PATCH 18/32] remove temp files --- .gitpod.Dockerfile | 1 - .gitpod.yml | 1 - 2 files changed, 2 deletions(-) delete mode 100644 .gitpod.Dockerfile delete mode 100644 .gitpod.yml diff --git a/.gitpod.Dockerfile b/.gitpod.Dockerfile deleted file mode 100644 index 617801170f..0000000000 --- a/.gitpod.Dockerfile +++ /dev/null @@ -1 +0,0 @@ -FROM ubuntu:18.04 diff --git a/.gitpod.yml b/.gitpod.yml deleted file mode 100644 index 8588340261..0000000000 --- a/.gitpod.yml +++ /dev/null @@ -1 +0,0 @@ -image: ubuntu-18.04 From da5eb9351816c39188212faa4fbbd46ea4436a8c Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 22 Mar 2022 05:05:58 +0000 Subject: [PATCH 19/32] fix gcc48 install --- .../sdk/metrics/aggregation/sum_aggregation.h | 1 + .../sdk/metrics/data/point_data.h | 37 +++++++++++++---- .../metrics/aggregation/sum_aggregation.cc | 41 +++++++++++-------- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index 45f7700c1c..8f0cd9ef4f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -55,6 +55,7 @@ class DoubleSumAggregation : public Aggregation private: opentelemetry::common::SpinLockMutex lock_; SumPointData point_data_; + long sum_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 07de374a57..f1192f0309 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -22,28 +22,47 @@ using ListType = nostd::variant, std::list>; class SumPointData { public: - ValueType value_; + SumPointData(SumPointData &&) = default; + SumPointData(const SumPointData &) = default; + SumPointData &operator=(SumPointData &&) = default; + SumPointData() = default; + ValueType value_ = {}; }; class LastValuePointData { public: - ValueType value_; - bool is_lastvalue_valid_; - opentelemetry::common::SystemTimestamp sample_ts_; + LastValuePointData(LastValuePointData &&) = default; + LastValuePointData(const LastValuePointData &) = default; + LastValuePointData &operator=(LastValuePointData &&) = default; + + LastValuePointData() = default; + ValueType value_ = {}; + bool is_lastvalue_valid_ = {}; + opentelemetry::common::SystemTimestamp sample_ts_ = {}; }; class HistogramPointData { public: - ListType boundaries_; - ValueType sum_; - std::vector counts_; - uint64_t count_; + HistogramPointData(HistogramPointData &&) = default; + HistogramPointData &operator=(HistogramPointData &&) = default; + HistogramPointData(const HistogramPointData &) = default; + HistogramPointData() = default; + ListType boundaries_ = {}; + ValueType sum_ = {}; + std::vector counts_ = {}; + uint64_t count_ = {}; }; class DropPointData -{}; +{ +public: + DropPointData(DropPointData &&) = default; + DropPointData(const DropPointData &) = default; + DropPointData() = default; + DropPointData &operator=(DropPointData &&) = default; +}; } // namespace metrics } // namespace sdk diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 7d26437af2..3edf26aef2 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -29,23 +29,25 @@ void LongSumAggregation::Aggregate(long value, const PointAttributes &attributes std::unique_ptr LongSumAggregation::Merge(Aggregation &delta) noexcept { - - SumPointData merge_data{ + long merge_value = nostd::get( nostd::get((static_cast(delta).ToPoint())).value_) + - nostd::get(nostd::get(ToPoint()).value_)}; - - return std::unique_ptr(new LongSumAggregation(std::move(merge_data))); + nostd::get(nostd::get(ToPoint()).value_); + std::unique_ptr aggr(new LongSumAggregation()); + static_cast(aggr.get())->sum_ = merge_value; + return aggr; } std::unique_ptr LongSumAggregation::Diff(Aggregation &next) noexcept { - SumPointData diff_data = { + + long diff_value = nostd::get( nostd::get((static_cast(next).ToPoint())).value_) - - nostd::get(nostd::get(ToPoint()).value_)}; - - return std::unique_ptr(new LongSumAggregation(std::move(diff_data))); + nostd::get(nostd::get(ToPoint()).value_); + std::unique_ptr aggr(new LongSumAggregation()); + static_cast(aggr.get())->sum_ = diff_value; + return aggr; } PointType LongSumAggregation::ToPoint() noexcept @@ -68,22 +70,25 @@ void DoubleSumAggregation::Aggregate(double value, const PointAttributes &attrib std::unique_ptr DoubleSumAggregation::Merge(Aggregation &delta) noexcept { - SumPointData merge_data = { + double merge_value = nostd::get( nostd::get((static_cast(delta).ToPoint())).value_) + - nostd::get(nostd::get(ToPoint()).value_)}; - - return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); + nostd::get(nostd::get(ToPoint()).value_); + std::unique_ptr aggr(new DoubleSumAggregation()); + static_cast(aggr.get())->sum_ = merge_value; + return aggr; } std::unique_ptr DoubleSumAggregation::Diff(Aggregation &next) noexcept { - SumPointData merge_data = { - nostd::get( - nostd::get((static_cast(next).ToPoint())).value_) + - nostd::get(nostd::get(ToPoint()).value_)}; - return std::unique_ptr(new DoubleSumAggregation(std::move(merge_data))); + double diff_value = + nostd::get( + nostd::get((static_cast(next).ToPoint())).value_) - + nostd::get(nostd::get(ToPoint()).value_); + std::unique_ptr aggr(new DoubleSumAggregation()); + static_cast(aggr.get())->sum_ = diff_value; + return aggr; } PointType DoubleSumAggregation::ToPoint() noexcept From 0551aebfa70b56c1fcd1df464e845de703034d41 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 21 Mar 2022 23:57:28 -0700 Subject: [PATCH 20/32] comment metric exporter till MetricData is finalised --- exporters/ostream/BUILD | 57 ++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/exporters/ostream/BUILD b/exporters/ostream/BUILD index cca74d6693..19c0f7914d 100644 --- a/exporters/ostream/BUILD +++ b/exporters/ostream/BUILD @@ -43,36 +43,35 @@ cc_library( ], ) -cc_library( - name = "ostream_metric_exporter", - srcs = [ - "src/metric_exporter.cc", - ], - hdrs = [ - "include/opentelemetry/exporters/ostream/metric_exporter.h", - ], - strip_include_prefix = "include", - tags = [ - "metrics", - "ostream", - ], - deps = [ - "//sdk/src/metrics", - ], -) +#TODO MetricData is still changing, uncomment once it is final +# srcs = [ +# "src/metric_exporter.cc", +# ], +# hdrs = [ +# "include/opentelemetry/exporters/ostream/metric_exporter.h", +# ], +# strip_include_prefix = "include", +# tags = [ +# "metrics", +# "ostream", +# ], +# deps = [ +# "//sdk/src/metrics", +# ], +#) -cc_test( - name = "ostream_metric_test", - srcs = ["test/ostream_metric_test.cc"], - tags = [ - "ostream", - "test", - ], - deps = [ - ":ostream_metric_exporter", - "@com_google_googletest//:gtest_main", - ], -) +#cc_test( +# name = "ostream_metric_test", +# srcs = ["test/ostream_metric_test.cc"], +# tags = [ +# "ostream", +# "test", +# ], +# deps = [ +# ":ostream_metric_exporter", +# "@com_google_googletest//:gtest_main", +#], +#) cc_test( name = "ostream_metrics_test_deprecated", From b5d3bd57d4dc838f94e04fd8d766f36eeb0e193a Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 00:08:45 -0700 Subject: [PATCH 21/32] add gcc4.8 TODO --- sdk/include/opentelemetry/sdk/metrics/data/point_data.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index f1192f0309..870d60cbb3 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -19,6 +19,10 @@ namespace metrics using ValueType = nostd::variant; using ListType = nostd::variant, std::list>; +// TODO - +// Remove default constructor, and list initialization from +// classes below once gcc 48 support is removed. + class SumPointData { public: From b4bac61bf80cfd65f83b1ccf5bf6da4883b69a1d Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 00:15:07 -0700 Subject: [PATCH 22/32] add gcc4.8 TODO --- .../sdk/metrics/data/point_data.h | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 870d60cbb3..f4aa956981 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -19,28 +19,31 @@ namespace metrics using ValueType = nostd::variant; using ListType = nostd::variant, std::list>; -// TODO - -// Remove default constructor, and list initialization from -// classes below once gcc 48 support is removed. +// TODO: remove ctors and initializers from below classes when GCC<5 stops shipping on Ubuntu +// refer error: +// https://github.com/open-telemetry/opentelemetry-cpp/runs/5577740472?check_suite_focus=true#step:6:522 class SumPointData { public: + // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu SumPointData(SumPointData &&) = default; SumPointData(const SumPointData &) = default; SumPointData &operator=(SumPointData &&) = default; SumPointData() = default; - ValueType value_ = {}; + + ValueType value_ = {}; }; class LastValuePointData { public: + // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu LastValuePointData(LastValuePointData &&) = default; LastValuePointData(const LastValuePointData &) = default; LastValuePointData &operator=(LastValuePointData &&) = default; + LastValuePointData() = default; - LastValuePointData() = default; ValueType value_ = {}; bool is_lastvalue_valid_ = {}; opentelemetry::common::SystemTimestamp sample_ts_ = {}; @@ -49,19 +52,22 @@ class LastValuePointData class HistogramPointData { public: + // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu HistogramPointData(HistogramPointData &&) = default; HistogramPointData &operator=(HistogramPointData &&) = default; HistogramPointData(const HistogramPointData &) = default; HistogramPointData() = default; - ListType boundaries_ = {}; - ValueType sum_ = {}; - std::vector counts_ = {}; - uint64_t count_ = {}; + + ListType boundaries_ = {}; + ValueType sum_ = {}; + std::vector counts_ = {}; + uint64_t count_ = {}; }; class DropPointData { public: + // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu DropPointData(DropPointData &&) = default; DropPointData(const DropPointData &) = default; DropPointData() = default; From 015757c93c0b522ffdbfd95d0893d3569481faef Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 00:28:52 -0700 Subject: [PATCH 23/32] fix spelling cumulative --- sdk/include/opentelemetry/sdk/metrics/instruments.h | 2 +- sdk/include/opentelemetry/sdk/metrics/metric_reader.h | 2 +- .../opentelemetry/sdk/metrics/state/sync_metric_storage.h | 6 +++--- sdk/test/metrics/async_metric_storage_test.cc | 2 +- sdk/test/metrics/sync_metric_storage_test.cc | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index ad64ce718b..1e3b6333a4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -40,7 +40,7 @@ enum class AggregationTemporarily { kUnspecified, kDelta, - kCummulative + kCumulative }; struct InstrumentDescriptor diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index b8abf29e90..4ed7e7eeca 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -26,7 +26,7 @@ class MetricReader { public: MetricReader( - AggregationTemporarily aggregation_temporarily = AggregationTemporarily::kCummulative); + AggregationTemporarily aggregation_temporarily = AggregationTemporarily::kCumulative); void SetMetricProducer(MetricProducer *metric_producer); diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 6bbc5eedd5..13d0bd1201 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -141,7 +141,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } // Get the last reported metrics for the `collector` from `last reported metrics` stash - // - If the aggregation_temporarily for the collector is cummulative + // - If the aggregation_temporarily for the collector is cumulative // - Merge the last reported metrics with unreported metrics (which is in merged_metrics), // Final result of merge would be in merged_metrics. // - Move the final merge to the `last reported metrics` stash. @@ -154,9 +154,9 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { last_collection_ts = last_reported_metrics_[collector].collection_ts; auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); - if (aggregation_temporarily == AggregationTemporarily::kCummulative) + if (aggregation_temporarily == AggregationTemporarily::kCumulative) { - // merge current delta to previous cummulative + // merge current delta to previous cumulative last_aggr_hashmap->GetAllEnteries( [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { auto agg = merged_metrics->Get(attributes); diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index fce06ea594..e04fe59d49 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -23,7 +23,7 @@ class MockCollectorHandle : public CollectorHandle public: AggregationTemporarily GetAggregationTemporarily() noexcept override { - return AggregationTemporarily::kCummulative; + return AggregationTemporarily::kCumulative; } }; diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index 59cb79069b..95250d5d22 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -18,7 +18,7 @@ class MockCollectorHandle : public CollectorHandle public: AggregationTemporarily GetAggregationTemporarily() noexcept override { - return AggregationTemporarily::kCummulative; + return AggregationTemporarily::kCumulative; } }; From ae65b1d24df995e015b6164cfd3cf78bf8477507 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 17:22:51 -0700 Subject: [PATCH 24/32] add tests for sum aggregation, and fix issues --- .../sdk/metrics/aggregation/sum_aggregation.h | 2 - .../sdk/metrics/state/attributes_hashmap.h | 19 +- .../sdk/metrics/state/sync_metric_storage.h | 41 ++-- .../metrics/aggregation/sum_aggregation.cc | 9 +- sdk/test/metrics/sync_metric_storage_test.cc | 218 ++++++++++++++++-- 5 files changed, 240 insertions(+), 49 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index 8f0cd9ef4f..ffa3ba1a11 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -33,7 +33,6 @@ class LongSumAggregation : public Aggregation private: opentelemetry::common::SpinLockMutex lock_; SumPointData point_data_; - long sum_; }; class DoubleSumAggregation : public Aggregation @@ -55,7 +54,6 @@ class DoubleSumAggregation : public Aggregation private: opentelemetry::common::SpinLockMutex lock_; SumPointData point_data_; - long sum_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h index 32301f8038..a8527d0f7d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h @@ -37,7 +37,7 @@ class AttributesHashMap public: Aggregation *Get(const MetricAttributes &attributes) const { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); auto it = hash_map_.find(attributes); if (it != hash_map_.end()) { @@ -52,7 +52,7 @@ class AttributesHashMap */ bool Has(const MetricAttributes &attributes) const { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); return (hash_map_.find(attributes) == hash_map_.end()) ? false : true; } @@ -64,7 +64,8 @@ class AttributesHashMap Aggregation *GetOrSetDefault(const MetricAttributes &attributes, std::function()> aggregation_callback) { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); + auto it = hash_map_.find(attributes); if (it != hash_map_.end()) { @@ -80,7 +81,7 @@ class AttributesHashMap */ void Set(const MetricAttributes &attributes, std::unique_ptr value) { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); hash_map_[attributes] = std::move(value); } @@ -90,7 +91,7 @@ class AttributesHashMap bool GetAllEnteries( nostd::function_ref callback) const { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); for (auto &kv : hash_map_) { if (!callback(kv.first, *(kv.second.get()))) @@ -106,7 +107,7 @@ class AttributesHashMap */ size_t Size() { - std::lock_guard guard(GetLock()); + std::lock_guard guard(lock); return hash_map_.size(); } @@ -114,11 +115,7 @@ class AttributesHashMap std::unordered_map, AttributeHashGenerator> hash_map_; - static opentelemetry::common::SpinLockMutex &GetLock() noexcept - { - static opentelemetry::common::SpinLockMutex lock; - return lock; - } + mutable opentelemetry::common::SpinLockMutex lock; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 13d0bd1201..e9f5962a00 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -64,7 +64,6 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { return; } - auto attr = attributes_processor_->process(attributes); attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value); } @@ -75,7 +74,6 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { return; } - attributes_hashmap_->GetOrSetDefault({}, create_default_aggregation_)->Aggregate(value); } @@ -86,7 +84,6 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { return; } - auto attr = attributes_processor_->process(attributes); attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value); } @@ -104,6 +101,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage // this will also empty the delta metrics hashmap, and make it available for // recordings std::shared_ptr delta_metrics = std::move(attributes_hashmap_); + attributes_hashmap_.reset(new AttributesHashMap); for (auto &col : collectors) { unreported_metrics_[col.get()].push_back(delta_metrics); @@ -121,25 +119,27 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage auto unreported_list = std::move(unreported_metrics_[collector]); // Iterate over the unreporter metrics for `collector` and store result in `merged_metrics` - std::unique_ptr merged_metrics; + std::unique_ptr merged_metrics(new AttributesHashMap); for (auto &agg_hashmap : unreported_list) { - agg_hashmap->GetAllEnteries( - [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { - auto agg = merged_metrics->Get(attributes); - if (agg) - { - merged_metrics->Set(attributes, agg->Merge(aggregation)); - } - else - { - merged_metrics->Set(attributes, - DefaultAggregation::CreateAggregation(instrument_descriptor_)); - } - return true; - }); + agg_hashmap->GetAllEnteries([&merged_metrics, this](const MetricAttributes &attributes, + Aggregation &aggregation) { + auto agg = merged_metrics->Get(attributes); + if (agg) + { + merged_metrics->Set(attributes, std::move(agg->Merge(aggregation))); + } + else + { + merged_metrics->Set( + attributes, std::move(DefaultAggregation::CreateAggregation(instrument_descriptor_) + ->Merge(aggregation))); + merged_metrics->GetAllEnteries( + [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); + } + return true; + }); } - // Get the last reported metrics for the `collector` from `last reported metrics` stash // - If the aggregation_temporarily for the collector is cumulative // - Merge the last reported metrics with unreported metrics (which is in merged_metrics), @@ -177,6 +177,8 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage } else { + merged_metrics->GetAllEnteries( + [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); last_reported_metrics_.insert( std::make_pair(collector, LastReportedMetrics{std::move(merged_metrics), collection_ts})); } @@ -196,6 +198,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage metric_data.point_data_attr_.push_back(point_data_attr); return true; }); + if (callback(metric_data)) { return true; diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 3edf26aef2..b4449d06fc 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -6,6 +6,7 @@ # include "opentelemetry/sdk/metrics/data/point_data.h" # include "opentelemetry/version.h" +# include # include OPENTELEMETRY_BEGIN_NAMESPACE @@ -34,7 +35,7 @@ std::unique_ptr LongSumAggregation::Merge(Aggregation &delta) noexc nostd::get((static_cast(delta).ToPoint())).value_) + nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new LongSumAggregation()); - static_cast(aggr.get())->sum_ = merge_value; + static_cast(aggr.get())->point_data_.value_ = merge_value; return aggr; } @@ -46,7 +47,7 @@ std::unique_ptr LongSumAggregation::Diff(Aggregation &next) noexcep nostd::get((static_cast(next).ToPoint())).value_) - nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new LongSumAggregation()); - static_cast(aggr.get())->sum_ = diff_value; + static_cast(aggr.get())->point_data_.value_ = diff_value; return aggr; } @@ -75,7 +76,7 @@ std::unique_ptr DoubleSumAggregation::Merge(Aggregation &delta) noe nostd::get((static_cast(delta).ToPoint())).value_) + nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new DoubleSumAggregation()); - static_cast(aggr.get())->sum_ = merge_value; + static_cast(aggr.get())->point_data_.value_ = merge_value; return aggr; } @@ -87,7 +88,7 @@ std::unique_ptr DoubleSumAggregation::Diff(Aggregation &next) noexc nostd::get((static_cast(next).ToPoint())).value_) - nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new DoubleSumAggregation()); - static_cast(aggr.get())->sum_ = diff_value; + static_cast(aggr.get())->point_data_.value_ = diff_value; return aggr; } diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index 95250d5d22..e02884fc27 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -10,40 +10,232 @@ # include # include +# include + using namespace opentelemetry::sdk::metrics; using M = std::map; class MockCollectorHandle : public CollectorHandle { public: - AggregationTemporarily GetAggregationTemporarily() noexcept override - { - return AggregationTemporarily::kCumulative; - } + MockCollectorHandle(AggregationTemporarily temp) : temporarily(temp) {} + + AggregationTemporarily GetAggregationTemporarily() noexcept override { return temporarily; } + +private: + AggregationTemporarily temporarily; }; -TEST(WritableMetricStorageTest, BasicTests) +TEST(WritableMetricStorageTest, LongSumCummulative) { + auto sdk_start_ts = std::chrono::system_clock::now(); + long expected_sum = 0; InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, InstrumentValueType::kLong}; opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, new DefaultAttributesProcessor()); EXPECT_NO_THROW(storage.RecordLong(10l)); - EXPECT_NO_THROW(storage.RecordDouble(10.10)); - EXPECT_NO_THROW(storage.RecordLong( - 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}))); + expected_sum += 10; + + EXPECT_NO_THROW(storage.RecordLong(30l)); + expected_sum += 30; + /*EXPECT_NO_THROW(storage.RecordLong( + 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ EXPECT_NO_THROW(storage.RecordDouble(10.10, opentelemetry::common::KeyValueIterableView({}))); opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); - MockCollectorHandle collector; + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporarily::kCumulative)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); + + EXPECT_NO_THROW(storage.RecordLong(50l)); + expected_sum += 50; + EXPECT_NO_THROW(storage.RecordLong(20l)); + expected_sum += 20; + + collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); +} + +TEST(WritableMetricStorageTest, LongSumDelta) +{ + auto sdk_start_ts = std::chrono::system_clock::now(); + long expected_sum = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kLong}; + + opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, + new DefaultAttributesProcessor()); + EXPECT_NO_THROW(storage.RecordLong(10l)); + expected_sum += 10; + + EXPECT_NO_THROW(storage.RecordLong(30l)); + expected_sum += 30; + /*EXPECT_NO_THROW(storage.RecordLong( + 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ + + EXPECT_NO_THROW(storage.RecordDouble(10.10, opentelemetry::common::KeyValueIterableView({}))); + opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporarily::kDelta)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); + + expected_sum = 0; + EXPECT_NO_THROW(storage.RecordLong(50l)); + expected_sum += 50; + EXPECT_NO_THROW(storage.RecordLong(20l)); + expected_sum += 20; + + collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); +} + +TEST(WritableMetricStorageTest, DoubleSumCummulative) +{ + auto sdk_start_ts = std::chrono::system_clock::now(); + double expected_sum = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kDouble}; + + opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, + new DefaultAttributesProcessor()); + EXPECT_NO_THROW(storage.RecordDouble(10.0)); + expected_sum += 10; + + EXPECT_NO_THROW(storage.RecordDouble(30.0)); + expected_sum += 30; + /*EXPECT_NO_THROW(storage.RecordLong( + 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ + + opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporarily::kCumulative)); std::vector> collectors; + collectors.push_back(collector); - auto sdk_start_ts = std::chrono::system_clock::now(); // Some computation here - auto collection_ts = std::chrono::system_clock::now() + std::chrono::seconds(5); + auto collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); + + EXPECT_NO_THROW(storage.RecordDouble(50.0)); + expected_sum += 50; + EXPECT_NO_THROW(storage.RecordDouble(20.0)); + expected_sum += 20; - storage.Collect(&collector, collectors, sdk_start_ts, collection_ts, - [](const MetricData data) { return true; }); + collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); } + +TEST(WritableMetricStorageTest, DoubleSumDelta) +{ + auto sdk_start_ts = std::chrono::system_clock::now(); + long expected_sum = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kDouble}; + + opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, + new DefaultAttributesProcessor()); + EXPECT_NO_THROW(storage.RecordDouble(10.0)); + expected_sum += 10; + + EXPECT_NO_THROW(storage.RecordDouble(30.0)); + expected_sum += 30; + /*EXPECT_NO_THROW(storage.RecordLong( + 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ + + opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); + std::shared_ptr collector( + new MockCollectorHandle(AggregationTemporarily::kDelta)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); + + expected_sum = 0; + EXPECT_NO_THROW(storage.RecordDouble(50.0)); + expected_sum += 50; + EXPECT_NO_THROW(storage.RecordDouble(20.0)); + expected_sum += 20; + + collection_ts = std::chrono::system_clock::now(); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, + [expected_sum](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); + } + return true; + }); +} + #endif From 4b31fd33c16ee80ebffccb988577f37602b4a130 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 21:35:05 -0700 Subject: [PATCH 25/32] Fix --- sdk/test/metrics/sync_metric_storage_test.cc | 229 ++++++++++++++++--- 1 file changed, 191 insertions(+), 38 deletions(-) diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index e02884fc27..db6ed5b8d7 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -13,6 +13,7 @@ # include using namespace opentelemetry::sdk::metrics; +using namespace opentelemetry::common; using M = std::map; class MockCollectorHandle : public CollectorHandle @@ -26,59 +27,210 @@ class MockCollectorHandle : public CollectorHandle AggregationTemporarily temporarily; }; -TEST(WritableMetricStorageTest, LongSumCummulative) +class WritableMetricStorageTestFixture : public ::testing::TestWithParam +{}; + +TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) { - auto sdk_start_ts = std::chrono::system_clock::now(); - long expected_sum = 0; - InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, + AggregationTemporarily temporarily = GetParam(); + auto sdk_start_ts = std::chrono::system_clock::now(); + long expected_total_get_requests = 0; + long expected_total_put_requests = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, InstrumentValueType::kLong}; + std::map attributes_get = {{"RequestType", "GET"}}; + std::map attributes_put = {{"RequestType", "PUT"}}; opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, new DefaultAttributesProcessor()); - EXPECT_NO_THROW(storage.RecordLong(10l)); - expected_sum += 10; - EXPECT_NO_THROW(storage.RecordLong(30l)); - expected_sum += 30; - /*EXPECT_NO_THROW(storage.RecordLong( - 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ + storage.RecordLong(10l, KeyValueIterableView>(attributes_get)); + expected_total_get_requests += 10; - EXPECT_NO_THROW(storage.RecordDouble(10.10, opentelemetry::common::KeyValueIterableView({}))); - opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); - std::shared_ptr collector( - new MockCollectorHandle(AggregationTemporarily::kCumulative)); + EXPECT_NO_THROW(storage.RecordLong( + 30l, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 30; + + storage.RecordLong(20l, KeyValueIterableView>(attributes_get)); + expected_total_get_requests += 20; + + EXPECT_NO_THROW(storage.RecordLong( + 40l, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 40; + + std::shared_ptr collector(new MockCollectorHandle(temporarily)); std::vector> collectors; collectors.push_back(collector); // Some computation here - auto collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); + auto collection_ts = std::chrono::system_clock::now(); + size_t count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_put_requests); + count_attributes++; + } + } + return true; + }); + + // In case of delta temporarily, subsequent collection would contain new data points, so resetting + // the counts + if (temporarily == AggregationTemporarily::kDelta) + { + expected_total_get_requests = 0; + expected_total_put_requests = 0; + } + + EXPECT_NO_THROW(storage.RecordLong( + 50l, KeyValueIterableView>(attributes_get))); + expected_total_get_requests += 50; + EXPECT_NO_THROW(storage.RecordLong( + 40l, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 40; + + collection_ts = std::chrono::system_clock::now(); + count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_put_requests); + count_attributes++; + } + } + return true; + }); +} +INSTANTIATE_TEST_CASE_P(WritableMetricStorageTestLong, + WritableMetricStorageTestFixture, + ::testing::Values(AggregationTemporarily::kCumulative, + AggregationTemporarily::kDelta)); - EXPECT_NO_THROW(storage.RecordLong(50l)); - expected_sum += 50; - EXPECT_NO_THROW(storage.RecordLong(20l)); - expected_sum += 20; +TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) +{ + AggregationTemporarily temporarily = GetParam(); + auto sdk_start_ts = std::chrono::system_clock::now(); + double expected_total_get_requests = 0; + double expected_total_put_requests = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, + InstrumentValueType::kDouble}; + std::map attributes_get = {{"RequestType", "GET"}}; + std::map attributes_put = {{"RequestType", "PUT"}}; - collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); + opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, + new DefaultAttributesProcessor()); + + storage.RecordDouble(10.0, + KeyValueIterableView>(attributes_get)); + expected_total_get_requests += 10; + + EXPECT_NO_THROW(storage.RecordDouble( + 30.0, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 30; + + storage.RecordDouble(20.0, + KeyValueIterableView>(attributes_get)); + expected_total_get_requests += 20; + + EXPECT_NO_THROW(storage.RecordDouble( + 40.0, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 40; + + std::shared_ptr collector(new MockCollectorHandle(temporarily)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + size_t count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_put_requests); + count_attributes++; + } + } + return true; + }); + + // In case of delta temporarily, subsequent collection would contain new data points, so resetting + // the counts + if (temporarily == AggregationTemporarily::kDelta) + { + expected_total_get_requests = 0; + expected_total_put_requests = 0; + } + + EXPECT_NO_THROW(storage.RecordDouble( + 50.0, KeyValueIterableView>(attributes_get))); + expected_total_get_requests += 50; + EXPECT_NO_THROW(storage.RecordDouble( + 40.0, KeyValueIterableView>(attributes_put))); + expected_total_put_requests += 40; + + collection_ts = std::chrono::system_clock::now(); + count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { + for (auto data_attr : data.point_data_attr_) + { + auto data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_total_put_requests); + count_attributes++; + } + } + return true; + }); } +INSTANTIATE_TEST_CASE_P(WritableMetricStorageTestDouble, + WritableMetricStorageTestFixture, + ::testing::Values(AggregationTemporarily::kCumulative, + AggregationTemporarily::kDelta)); +# if 0 TEST(WritableMetricStorageTest, LongSumDelta) { auto sdk_start_ts = std::chrono::system_clock::now(); @@ -238,4 +390,5 @@ TEST(WritableMetricStorageTest, DoubleSumDelta) }); } +# endif #endif From 62d0e92b40f61ed2a6ba0c87e42a1852c43e5f63 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 22 Mar 2022 21:58:43 -0700 Subject: [PATCH 26/32] remove commented code --- sdk/test/metrics/sync_metric_storage_test.cc | 161 ------------------- 1 file changed, 161 deletions(-) diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index db6ed5b8d7..4d0a417704 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -230,165 +230,4 @@ INSTANTIATE_TEST_CASE_P(WritableMetricStorageTestDouble, ::testing::Values(AggregationTemporarily::kCumulative, AggregationTemporarily::kDelta)); -# if 0 -TEST(WritableMetricStorageTest, LongSumDelta) -{ - auto sdk_start_ts = std::chrono::system_clock::now(); - long expected_sum = 0; - InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, - InstrumentValueType::kLong}; - - opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor()); - EXPECT_NO_THROW(storage.RecordLong(10l)); - expected_sum += 10; - - EXPECT_NO_THROW(storage.RecordLong(30l)); - expected_sum += 30; - /*EXPECT_NO_THROW(storage.RecordLong( - 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ - - EXPECT_NO_THROW(storage.RecordDouble(10.10, opentelemetry::common::KeyValueIterableView({}))); - opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); - std::shared_ptr collector( - new MockCollectorHandle(AggregationTemporarily::kDelta)); - std::vector> collectors; - collectors.push_back(collector); - - // Some computation here - auto collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); - - expected_sum = 0; - EXPECT_NO_THROW(storage.RecordLong(50l)); - expected_sum += 50; - EXPECT_NO_THROW(storage.RecordLong(20l)); - expected_sum += 20; - - collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); -} - -TEST(WritableMetricStorageTest, DoubleSumCummulative) -{ - auto sdk_start_ts = std::chrono::system_clock::now(); - double expected_sum = 0; - InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, - InstrumentValueType::kDouble}; - - opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor()); - EXPECT_NO_THROW(storage.RecordDouble(10.0)); - expected_sum += 10; - - EXPECT_NO_THROW(storage.RecordDouble(30.0)); - expected_sum += 30; - /*EXPECT_NO_THROW(storage.RecordLong( - 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ - - opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); - std::shared_ptr collector( - new MockCollectorHandle(AggregationTemporarily::kCumulative)); - std::vector> collectors; - collectors.push_back(collector); - - // Some computation here - auto collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); - - EXPECT_NO_THROW(storage.RecordDouble(50.0)); - expected_sum += 50; - EXPECT_NO_THROW(storage.RecordDouble(20.0)); - expected_sum += 20; - - collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); -} - -TEST(WritableMetricStorageTest, DoubleSumDelta) -{ - auto sdk_start_ts = std::chrono::system_clock::now(); - long expected_sum = 0; - InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, - InstrumentValueType::kDouble}; - - opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor()); - EXPECT_NO_THROW(storage.RecordDouble(10.0)); - expected_sum += 10; - - EXPECT_NO_THROW(storage.RecordDouble(30.0)); - expected_sum += 30; - /*EXPECT_NO_THROW(storage.RecordLong( - 10l, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}})));*/ - - opentelemetry::common::SystemTimestamp sdk_start_time = std::chrono::system_clock::now(); - std::shared_ptr collector( - new MockCollectorHandle(AggregationTemporarily::kDelta)); - std::vector> collectors; - collectors.push_back(collector); - - // Some computation here - auto collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); - - expected_sum = 0; - EXPECT_NO_THROW(storage.RecordDouble(50.0)); - expected_sum += 50; - EXPECT_NO_THROW(storage.RecordDouble(20.0)); - expected_sum += 20; - - collection_ts = std::chrono::system_clock::now(); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [expected_sum](const MetricData data) { - for (auto data_attr : data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - EXPECT_EQ(opentelemetry::nostd::get(data.value_), expected_sum); - } - return true; - }); -} - -# endif #endif From 4c2633633daf24044340a97ab37446fd200a93b9 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 28 Mar 2022 17:42:59 -0700 Subject: [PATCH 27/32] fix merge conflict --- .../sdk/metrics/aggregation/aggregation.h | 2 +- .../opentelemetry/sdk/metrics/metric_reader.h | 2 +- .../sdk/metrics/state/metric_collector.h | 1 + .../sdk/metrics/state/sync_metric_storage.h | 4 +- sdk/test/metrics/async_metric_storage_test.cc | 7 +- sdk/test/metrics/sync_metric_storage_test.cc | 75 +++++++++++-------- 6 files changed, 52 insertions(+), 39 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index 9a909651f2..142f20f2a6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -31,7 +31,7 @@ class Aggregation virtual std::unique_ptr Merge(Aggregation &delta) noexcept = 0; /** - * Returns a new DELTA aggregation by comparing two cumulative measurements. + * Returns a new delta aggregation by comparing two cumulative measurements. * * @param next the newly captured (cumulative) aggregation. * @return The resulting delta aggregation. diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index 9390ddeac0..57690ac87c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -26,7 +26,7 @@ class MetricReader { public: MetricReader( - AggregationTemporality aggregation_temporality = AggregationTemporality::kCummulative); + AggregationTemporality aggregation_temporality = AggregationTemporality::kCumulative); void SetMetricProducer(MetricProducer *metric_producer); diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h index 6359baea9e..51c9cf6eff 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -44,6 +44,7 @@ class MetricCollector : public MetricProducer, public CollectorHandle bool Collect(nostd::function_ref callback) noexcept override; bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; + bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; private: diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 4f6a85e25c..9c097c1369 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -109,7 +109,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage nostd::function_ref callback) noexcept override { opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; - auto aggregation_temporarily = collector->GetAggregationTemporarily(); + auto aggregation_temporarily = collector->GetAggregationTemporality(); // Add the current delta metrics to `unreported metrics stash` for all the collectors, // this will also empty the delta metrics hashmap, and make it available for @@ -168,7 +168,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage { last_collection_ts = last_reported_metrics_[collector].collection_ts; auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); - if (aggregation_temporarily == AggregationTemporarily::kCumulative) + if (aggregation_temporarily == AggregationTemporality::kCumulative) { // merge current delta to previous cumulative last_aggr_hashmap->GetAllEnteries( diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 07e2473240..f7c9914e3c 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -18,7 +18,7 @@ using namespace opentelemetry::sdk::metrics; using namespace opentelemetry::sdk::instrumentationlibrary; using namespace opentelemetry::sdk::resource; -class MockCollectorHandle : public CollectorHandle +class MockMetricReader : public MetricReader { public: MockMetricReader(AggregationTemporality aggr_temporality) : MetricReader(aggr_temporality) {} @@ -42,11 +42,10 @@ TEST(AsyncMetricStorageTest, BasicTests) InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter, InstrumentValueType::kLong}; - auto sdk_start_ts = std::chrono::system_clock::now(); // Some computation here auto collection_ts = std::chrono::system_clock::now() + std::chrono::seconds(5); - + std::vector> exporters; std::shared_ptr meter_context(new MeterContext(std::move(exporters))); std::unique_ptr metric_reader(new MockMetricReader(AggregationTemporality::kDelta)); @@ -59,6 +58,6 @@ TEST(AsyncMetricStorageTest, BasicTests) opentelemetry::sdk::metrics::AsyncMetricStorage storage( instr_desc, AggregationType::kSum, &measurement_fetch, new DefaultAttributesProcessor()); EXPECT_NO_THROW( - storage.Collect(&collector, collectors, sdk_start_ts, collection_ts, metric_callback)); + storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, metric_callback)); } #endif \ No newline at end of file diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index 525f5bc814..c9811b2bcb 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -20,20 +20,20 @@ using M = std::map; class MockCollectorHandle : public CollectorHandle { public: - MockCollectorHandle(AggregationTemporarily temp) : temporarily(temp) {} + MockCollectorHandle(AggregationTemporality temp) : temporality(temp) {} - AggregationTemporarily GetAggregationTemporarily() noexcept override { return temporarily; } + AggregationTemporality GetAggregationTemporality() noexcept override { return temporality; } private: - AggregationTemporarily temporarily; + AggregationTemporality temporality; }; -class WritableMetricStorageTestFixture : public ::testing::TestWithParam +class WritableMetricStorageTestFixture : public ::testing::TestWithParam {}; TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) { - AggregationTemporarily temporarily = GetParam(); + AggregationTemporality temporality = GetParam(); auto sdk_start_ts = std::chrono::system_clock::now(); long expected_total_get_requests = 0; long expected_total_put_requests = 0; @@ -42,25 +42,29 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) std::map attributes_get = {{"RequestType", "GET"}}; std::map attributes_put = {{"RequestType", "PUT"}}; - opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor(), - NoExemplarReservoir::GetNoExemplarReservoir()); + opentelemetry::sdk::metrics::SyncMetricStorage storage( + instr_desc, AggregationType::kSum, new DefaultAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir()); - storage.RecordLong(10l, KeyValueIterableView>(attributes_get)); + storage.RecordLong(10l, KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); expected_total_get_requests += 10; EXPECT_NO_THROW(storage.RecordLong( - 30l, KeyValueIterableView>(attributes_put))); + 30l, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 30; - storage.RecordLong(20l, KeyValueIterableView>(attributes_get)); + storage.RecordLong(20l, KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); expected_total_get_requests += 20; EXPECT_NO_THROW(storage.RecordLong( - 40l, KeyValueIterableView>(attributes_put))); + 40l, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 40; - std::shared_ptr collector(new MockCollectorHandle(temporarily)); + std::shared_ptr collector(new MockCollectorHandle(temporality)); std::vector> collectors; collectors.push_back(collector); @@ -90,17 +94,19 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) // In case of delta temporarily, subsequent collection would contain new data points, so resetting // the counts - if (temporarily == AggregationTemporarily::kDelta) + if (temporality == AggregationTemporality::kDelta) { expected_total_get_requests = 0; expected_total_put_requests = 0; } EXPECT_NO_THROW(storage.RecordLong( - 50l, KeyValueIterableView>(attributes_get))); + 50l, KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{})); expected_total_get_requests += 50; EXPECT_NO_THROW(storage.RecordLong( - 40l, KeyValueIterableView>(attributes_put))); + 40l, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 40; collection_ts = std::chrono::system_clock::now(); @@ -128,12 +134,12 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) } INSTANTIATE_TEST_CASE_P(WritableMetricStorageTestLong, WritableMetricStorageTestFixture, - ::testing::Values(AggregationTemporarily::kCumulative, - AggregationTemporarily::kDelta)); + ::testing::Values(AggregationTemporality::kCumulative, + AggregationTemporality::kDelta)); TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) { - AggregationTemporarily temporarily = GetParam(); + AggregationTemporality temporality = GetParam(); auto sdk_start_ts = std::chrono::system_clock::now(); double expected_total_get_requests = 0; double expected_total_put_requests = 0; @@ -142,26 +148,31 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) std::map attributes_get = {{"RequestType", "GET"}}; std::map attributes_put = {{"RequestType", "PUT"}}; - opentelemetry::sdk::metrics::SyncMetricStorage storage(instr_desc, AggregationType::kSum, - new DefaultAttributesProcessor()); + opentelemetry::sdk::metrics::SyncMetricStorage storage( + instr_desc, AggregationType::kSum, new DefaultAttributesProcessor(), + NoExemplarReservoir::GetNoExemplarReservoir()); storage.RecordDouble(10.0, - KeyValueIterableView>(attributes_get)); + KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); expected_total_get_requests += 10; EXPECT_NO_THROW(storage.RecordDouble( - 30.0, KeyValueIterableView>(attributes_put))); + 30.0, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 30; storage.RecordDouble(20.0, - KeyValueIterableView>(attributes_get)); + KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); expected_total_get_requests += 20; EXPECT_NO_THROW(storage.RecordDouble( - 40.0, KeyValueIterableView>(attributes_put))); + 40.0, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 40; - std::shared_ptr collector(new MockCollectorHandle(temporarily)); + std::shared_ptr collector(new MockCollectorHandle(temporality)); std::vector> collectors; collectors.push_back(collector); @@ -191,17 +202,19 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) // In case of delta temporarily, subsequent collection would contain new data points, so resetting // the counts - if (temporarily == AggregationTemporarily::kDelta) + if (temporality == AggregationTemporality::kDelta) { expected_total_get_requests = 0; expected_total_put_requests = 0; } EXPECT_NO_THROW(storage.RecordDouble( - 50.0, KeyValueIterableView>(attributes_get))); + 50.0, KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{})); expected_total_get_requests += 50; EXPECT_NO_THROW(storage.RecordDouble( - 40.0, KeyValueIterableView>(attributes_put))); + 40.0, KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{})); expected_total_put_requests += 40; collection_ts = std::chrono::system_clock::now(); @@ -229,7 +242,7 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) } INSTANTIATE_TEST_CASE_P(WritableMetricStorageTestDouble, WritableMetricStorageTestFixture, - ::testing::Values(AggregationTemporarily::kCumulative, - AggregationTemporarily::kDelta)); + ::testing::Values(AggregationTemporality::kCumulative, + AggregationTemporality::kDelta)); #endif From cb1c5a44651afecad346b5361ee2bf4f7125632c Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 28 Mar 2022 18:15:56 -0700 Subject: [PATCH 28/32] review comments --- .../sdk/metrics/state/sync_metric_storage.h | 114 +-------------- sdk/src/metrics/CMakeLists.txt | 1 + .../aggregation/histogram_aggregation.cc | 13 +- .../aggregation/lastvalue_aggregation.cc | 8 +- sdk/src/metrics/state/sync_metric_storage.cc | 136 ++++++++++++++++++ 5 files changed, 151 insertions(+), 121 deletions(-) create mode 100644 sdk/src/metrics/state/sync_metric_storage.cc diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 9c097c1369..8269d2fd3c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -106,119 +106,7 @@ class SyncMetricStorage : public MetricStorage, public WritableMetricStorage nostd::span> collectors, opentelemetry::common::SystemTimestamp sdk_start_ts, opentelemetry::common::SystemTimestamp collection_ts, - nostd::function_ref callback) noexcept override - { - opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; - auto aggregation_temporarily = collector->GetAggregationTemporality(); - - // Add the current delta metrics to `unreported metrics stash` for all the collectors, - // this will also empty the delta metrics hashmap, and make it available for - // recordings - std::shared_ptr delta_metrics = std::move(attributes_hashmap_); - attributes_hashmap_.reset(new AttributesHashMap); - for (auto &col : collectors) - { - unreported_metrics_[col.get()].push_back(delta_metrics); - } - - // Get the unreported metrics for the `collector` from `unreported metrics stash` - // since last collection, this will also cleanup the unreported metrics for `collector` - // from the stash. - auto present = unreported_metrics_.find(collector); - if (present == unreported_metrics_.end()) - { - // no unreported metrics for the collector, return. - return true; - } - auto unreported_list = std::move(unreported_metrics_[collector]); - - // Iterate over the unreporter metrics for `collector` and store result in `merged_metrics` - std::unique_ptr merged_metrics(new AttributesHashMap); - for (auto &agg_hashmap : unreported_list) - { - agg_hashmap->GetAllEnteries([&merged_metrics, this](const MetricAttributes &attributes, - Aggregation &aggregation) { - auto agg = merged_metrics->Get(attributes); - if (agg) - { - merged_metrics->Set(attributes, std::move(agg->Merge(aggregation))); - } - else - { - merged_metrics->Set( - attributes, std::move(DefaultAggregation::CreateAggregation(instrument_descriptor_) - ->Merge(aggregation))); - merged_metrics->GetAllEnteries( - [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); - } - return true; - }); - } - // Get the last reported metrics for the `collector` from `last reported metrics` stash - // - If the aggregation_temporarily for the collector is cumulative - // - Merge the last reported metrics with unreported metrics (which is in merged_metrics), - // Final result of merge would be in merged_metrics. - // - Move the final merge to the `last reported metrics` stash. - // - If the aggregation_temporarily is delta - // - Store the unreported metrics for `collector`(which is in merged_mtrics) to - // `last reported metrics` stash. - - auto reported = last_reported_metrics_.find(collector); - if (reported != last_reported_metrics_.end()) - { - last_collection_ts = last_reported_metrics_[collector].collection_ts; - auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); - if (aggregation_temporarily == AggregationTemporality::kCumulative) - { - // merge current delta to previous cumulative - last_aggr_hashmap->GetAllEnteries( - [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { - auto agg = merged_metrics->Get(attributes); - if (agg) - { - merged_metrics->Set(attributes, agg->Merge(aggregation)); - } - else - { - merged_metrics->Set(attributes, - DefaultAggregation::CreateAggregation(instrument_descriptor_)); - } - return true; - }); - } - last_reported_metrics_[collector] = - LastReportedMetrics{std::move(merged_metrics), collection_ts}; - } - else - { - merged_metrics->GetAllEnteries( - [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); - last_reported_metrics_.insert( - std::make_pair(collector, LastReportedMetrics{std::move(merged_metrics), collection_ts})); - } - - // Generate the MetricData from the final merged_metrics, and invoke callback over it. - - AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); - MetricData metric_data; - metric_data.instrument_descriptor = instrument_descriptor_; - metric_data.start_ts = last_collection_ts; - metric_data.end_ts = collection_ts; - result_to_export->GetAllEnteries( - [&metric_data](const MetricAttributes &attributes, Aggregation &aggregation) { - PointDataAttributes point_data_attr; - point_data_attr.point_data = aggregation.ToPoint(); - point_data_attr.attributes = attributes; - metric_data.point_data_attr_.push_back(point_data_attr); - return true; - }); - - if (callback(metric_data)) - { - return true; - } - return false; - } + nostd::function_ref callback) noexcept override; private: InstrumentDescriptor instrument_descriptor_; diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index 119477eb06..d8eff48cd5 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -5,6 +5,7 @@ add_library( meter_context.cc metric_reader.cc state/metric_collector.cc + state/sync_metric_storage.cc aggregation/histogram_aggregation.cc aggregation/lastvalue_aggregation.cc aggregation/sum_aggregation.cc diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index e36ffbbf55..c4ac383e71 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -31,15 +31,16 @@ void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attr const std::lock_guard locked(lock_); point_data_.count_ += 1; point_data_.sum_ = nostd::get(point_data_.sum_) + value; + size_t index = 0; for (auto it = nostd::get>(point_data_.boundaries_).begin(); it != nostd::get>(point_data_.boundaries_).end(); ++it) { if (value < *it) { - point_data_.counts_[std::distance( - nostd::get>(point_data_.boundaries_).begin(), it)] += 1; + point_data_.counts_[index] += 1; return; } + index++; } } @@ -78,30 +79,34 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes & const std::lock_guard locked(lock_); point_data_.count_ += 1; point_data_.sum_ = nostd::get(point_data_.sum_) + value; + size_t index = 0; for (auto it = nostd::get>(point_data_.boundaries_).begin(); it != nostd::get>(point_data_.boundaries_).end(); ++it) { if (value < *it) { - point_data_.counts_[std::distance( - nostd::get>(point_data_.boundaries_).begin(), it)] += 1; + point_data_.counts_[index] += 1; return; } + index++; } } std::unique_ptr DoubleHistogramAggregation::Merge(Aggregation &delta) noexcept { + // TODO - Implement me return nullptr; } std::unique_ptr DoubleHistogramAggregation::Diff(Aggregation &next) noexcept { + // TODO - Implement me return nullptr; } PointType DoubleHistogramAggregation::ToPoint() noexcept { + // TODO Implement me return point_data_; } diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index abbf1a3588..3d36325cae 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -48,8 +48,8 @@ std::unique_ptr LongLastValueAggregation::Diff(Aggregation &next) n nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() ? *this : next; - LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + LastValuePointData diff_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); } PointType LongLastValueAggregation::ToPoint() noexcept @@ -91,8 +91,8 @@ std::unique_ptr DoubleLastValueAggregation::Diff(Aggregation &next) nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() ? *this : next; - LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); + LastValuePointData diff_data = std::move(nostd::get(agg_to_merge.ToPoint())); + return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); } PointType DoubleLastValueAggregation::ToPoint() noexcept diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc new file mode 100644 index 0000000000..ec299f2434 --- /dev/null +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -0,0 +1,136 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW + +# include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +bool SyncMetricStorage::Collect(CollectorHandle *collector, + nostd::span> collectors, + opentelemetry::common::SystemTimestamp sdk_start_ts, + opentelemetry::common::SystemTimestamp collection_ts, + nostd::function_ref callback) noexcept +{ + opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts; + auto aggregation_temporarily = collector->GetAggregationTemporality(); + + // Add the current delta metrics to `unreported metrics stash` for all the collectors, + // this will also empty the delta metrics hashmap, and make it available for + // recordings + std::shared_ptr delta_metrics = std::move(attributes_hashmap_); + attributes_hashmap_.reset(new AttributesHashMap); + for (auto &col : collectors) + { + unreported_metrics_[col.get()].push_back(delta_metrics); + } + + // Get the unreported metrics for the `collector` from `unreported metrics stash` + // since last collection, this will also cleanup the unreported metrics for `collector` + // from the stash. + auto present = unreported_metrics_.find(collector); + if (present == unreported_metrics_.end()) + { + // no unreported metrics for the collector, return. + return true; + } + auto unreported_list = std::move(present->second); + + // Iterate over the unreporter metrics for `collector` and store result in `merged_metrics` + std::unique_ptr merged_metrics(new AttributesHashMap); + for (auto &agg_hashmap : unreported_list) + { + agg_hashmap->GetAllEnteries([&merged_metrics, this](const MetricAttributes &attributes, + Aggregation &aggregation) { + auto agg = merged_metrics->Get(attributes); + if (agg) + { + merged_metrics->Set(attributes, std::move(agg->Merge(aggregation))); + } + else + { + merged_metrics->Set( + attributes, + std::move( + DefaultAggregation::CreateAggregation(instrument_descriptor_)->Merge(aggregation))); + merged_metrics->GetAllEnteries( + [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); + } + return true; + }); + } + // Get the last reported metrics for the `collector` from `last reported metrics` stash + // - If the aggregation_temporarily for the collector is cumulative + // - Merge the last reported metrics with unreported metrics (which is in merged_metrics), + // Final result of merge would be in merged_metrics. + // - Move the final merge to the `last reported metrics` stash. + // - If the aggregation_temporarily is delta + // - Store the unreported metrics for `collector`(which is in merged_mtrics) to + // `last reported metrics` stash. + + auto reported = last_reported_metrics_.find(collector); + if (reported != last_reported_metrics_.end()) + { + last_collection_ts = last_reported_metrics_[collector].collection_ts; + auto last_aggr_hashmap = std::move(last_reported_metrics_[collector].attributes_map); + if (aggregation_temporarily == AggregationTemporality::kCumulative) + { + // merge current delta to previous cumulative + last_aggr_hashmap->GetAllEnteries( + [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { + auto agg = merged_metrics->Get(attributes); + if (agg) + { + merged_metrics->Set(attributes, agg->Merge(aggregation)); + } + else + { + merged_metrics->Set(attributes, + DefaultAggregation::CreateAggregation(instrument_descriptor_)); + } + return true; + }); + } + last_reported_metrics_[collector] = + LastReportedMetrics{std::move(merged_metrics), collection_ts}; + } + else + { + merged_metrics->GetAllEnteries( + [](const MetricAttributes &attr, Aggregation &aggr) { return true; }); + last_reported_metrics_.insert( + std::make_pair(collector, LastReportedMetrics{std::move(merged_metrics), collection_ts})); + } + + // Generate the MetricData from the final merged_metrics, and invoke callback over it. + + AttributesHashMap *result_to_export = (last_reported_metrics_[collector]).attributes_map.get(); + MetricData metric_data; + metric_data.instrument_descriptor = instrument_descriptor_; + metric_data.start_ts = last_collection_ts; + metric_data.end_ts = collection_ts; + result_to_export->GetAllEnteries( + [&metric_data](const MetricAttributes &attributes, Aggregation &aggregation) { + PointDataAttributes point_data_attr; + point_data_attr.point_data = aggregation.ToPoint(); + point_data_attr.attributes = attributes; + metric_data.point_data_attr_.push_back(point_data_attr); + return true; + }); + + if (callback(metric_data)) + { + return true; + } + return false; +} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif From 0a626798da55412af799cd0d04608466ccf1c142 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 28 Mar 2022 18:58:43 -0700 Subject: [PATCH 29/32] make Aggregation::Merge/Diff/ToPoint methods as const --- .../sdk/metrics/aggregation/aggregation.h | 6 +- .../metrics/aggregation/drop_aggregation.h | 6 +- .../aggregation/histogram_aggregation.h | 16 ++-- .../aggregation/lastvalue_aggregation.h | 16 ++-- .../sdk/metrics/aggregation/sum_aggregation.h | 14 +-- .../aggregation/histogram_aggregation.cc | 15 ++-- .../aggregation/lastvalue_aggregation.cc | 87 +++++++++++-------- .../metrics/aggregation/sum_aggregation.cc | 29 ++++--- 8 files changed, 107 insertions(+), 82 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index 142f20f2a6..7ec9a6ea2b 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -28,7 +28,7 @@ class Aggregation * @return the result of the merge of the given aggregation. */ - virtual std::unique_ptr Merge(Aggregation &delta) noexcept = 0; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept = 0; /** * Returns a new delta aggregation by comparing two cumulative measurements. @@ -36,7 +36,7 @@ class Aggregation * @param next the newly captured (cumulative) aggregation. * @return The resulting delta aggregation. */ - virtual std::unique_ptr Diff(Aggregation &next) noexcept = 0; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept = 0; /** * Returns the point data that the aggregation will produce. @@ -44,7 +44,7 @@ class Aggregation * @return PointType */ - virtual PointType ToPoint() noexcept = 0; + virtual PointType ToPoint() const noexcept = 0; virtual ~Aggregation() = default; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h index f7ff8e2588..4e29fa2e46 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/drop_aggregation.h @@ -27,17 +27,17 @@ class DropAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - std::unique_ptr Merge(Aggregation &delta) noexcept override + std::unique_ptr Merge(const Aggregation &delta) const noexcept override { return std::unique_ptr(new DropAggregation()); } - std::unique_ptr Diff(Aggregation &next) noexcept override + std::unique_ptr Diff(const Aggregation &next) const noexcept override { return std::unique_ptr(new DropAggregation()); } - PointType ToPoint() noexcept override + PointType ToPoint() const noexcept override { static DropPointData point_data; return point_data; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 04f925130a..8f33fa27b4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -24,11 +24,11 @@ class LongHistogramAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: opentelemetry::common::SpinLockMutex lock_; @@ -45,15 +45,15 @@ class DoubleHistogramAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; - HistogramPointData point_data_; + mutable opentelemetry::common::SpinLockMutex lock_; + mutable HistogramPointData point_data_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h index d488c898c1..7f185d51a1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h @@ -23,11 +23,11 @@ class LongLastValueAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: opentelemetry::common::SpinLockMutex lock_; @@ -44,15 +44,15 @@ class DoubleLastValueAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; - LastValuePointData point_data_; + mutable opentelemetry::common::SpinLockMutex lock_; + mutable LastValuePointData point_data_; }; } // namespace metrics diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index ffa3ba1a11..b0f0169b24 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -24,11 +24,11 @@ class LongSumAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: opentelemetry::common::SpinLockMutex lock_; @@ -45,14 +45,14 @@ class DoubleSumAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; - virtual std::unique_ptr Merge(Aggregation &delta) noexcept override; + virtual std::unique_ptr Merge(const Aggregation &delta) const noexcept override; - virtual std::unique_ptr Diff(Aggregation &next) noexcept override; + virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; - PointType ToPoint() noexcept override; + PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; + mutable opentelemetry::common::SpinLockMutex lock_; SumPointData point_data_; }; diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index c4ac383e71..60f65c51b1 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -44,17 +44,18 @@ void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attr } } -std::unique_ptr LongHistogramAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr LongHistogramAggregation::Merge( + const Aggregation &delta) const noexcept { return nullptr; } -std::unique_ptr LongHistogramAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr LongHistogramAggregation::Diff(const Aggregation &next) const noexcept { return nullptr; } -PointType LongHistogramAggregation::ToPoint() noexcept +PointType LongHistogramAggregation::ToPoint() const noexcept { return point_data_; } @@ -92,19 +93,21 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes & } } -std::unique_ptr DoubleHistogramAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr DoubleHistogramAggregation::Merge( + const Aggregation &delta) const noexcept { // TODO - Implement me return nullptr; } -std::unique_ptr DoubleHistogramAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr DoubleHistogramAggregation::Diff( + const Aggregation &next) const noexcept { // TODO - Implement me return nullptr; } -PointType DoubleHistogramAggregation::ToPoint() noexcept +PointType DoubleHistogramAggregation::ToPoint() const noexcept { // TODO Implement me return point_data_; diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index 3d36325cae..9c0252be31 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -30,29 +30,38 @@ void LongLastValueAggregation::Aggregate(long value, const PointAttributes &attr point_data_.value_ = value; } -std::unique_ptr LongLastValueAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr LongLastValueAggregation::Merge( + const Aggregation &delta) const noexcept { - Aggregation &agg_to_merge = - nostd::get(ToPoint()).sample_ts_.time_since_epoch() > - nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch() - ? *this - : delta; - LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) + { + LastValuePointData merge_data = std::move(nostd::get(ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + } + else + { + LastValuePointData merge_data = std::move(nostd::get(delta.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + } } -std::unique_ptr LongLastValueAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr LongLastValueAggregation::Diff(const Aggregation &next) const noexcept { - Aggregation &agg_to_merge = - nostd::get(ToPoint()).sample_ts_.time_since_epoch() > - nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() - ? *this - : next; - LastValuePointData diff_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) + { + LastValuePointData diff_data = std::move(nostd::get(ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + } + else + { + LastValuePointData diff_data = std::move(nostd::get(next.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + } } -PointType LongLastValueAggregation::ToPoint() noexcept +PointType LongLastValueAggregation::ToPoint() const noexcept { return point_data_; } @@ -73,29 +82,39 @@ void DoubleLastValueAggregation::Aggregate(double value, const PointAttributes & point_data_.value_ = value; } -std::unique_ptr DoubleLastValueAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr DoubleLastValueAggregation::Merge( + const Aggregation &delta) const noexcept { - Aggregation &agg_to_merge = - nostd::get(ToPoint()).sample_ts_.time_since_epoch() > - nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch() - ? *this - : delta; - LastValuePointData merge_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(merge_data))); + if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(delta.ToPoint()).sample_ts_.time_since_epoch()) + { + LastValuePointData merge_data = std::move(nostd::get(ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + } + else + { + LastValuePointData merge_data = std::move(nostd::get(delta.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(merge_data))); + } } -std::unique_ptr DoubleLastValueAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr DoubleLastValueAggregation::Diff( + const Aggregation &next) const noexcept { - Aggregation &agg_to_merge = - nostd::get(ToPoint()).sample_ts_.time_since_epoch() > - nostd::get(next.ToPoint()).sample_ts_.time_since_epoch() - ? *this - : next; - LastValuePointData diff_data = std::move(nostd::get(agg_to_merge.ToPoint())); - return std::unique_ptr(new DoubleLastValueAggregation(std::move(diff_data))); + if (nostd::get(ToPoint()).sample_ts_.time_since_epoch() > + nostd::get(next.ToPoint()).sample_ts_.time_since_epoch()) + { + LastValuePointData diff_data = std::move(nostd::get(ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + } + else + { + LastValuePointData diff_data = std::move(nostd::get(next.ToPoint())); + return std::unique_ptr(new LongLastValueAggregation(std::move(diff_data))); + } } -PointType DoubleLastValueAggregation::ToPoint() noexcept +PointType DoubleLastValueAggregation::ToPoint() const noexcept { return point_data_; } diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index b4449d06fc..94b871cd34 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -28,30 +28,31 @@ void LongSumAggregation::Aggregate(long value, const PointAttributes &attributes point_data_.value_ = nostd::get(point_data_.value_) + value; } -std::unique_ptr LongSumAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr LongSumAggregation::Merge(const Aggregation &delta) const noexcept { long merge_value = nostd::get( - nostd::get((static_cast(delta).ToPoint())).value_) + + nostd::get((static_cast(delta).ToPoint())) + .value_) + nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new LongSumAggregation()); static_cast(aggr.get())->point_data_.value_ = merge_value; return aggr; } -std::unique_ptr LongSumAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr LongSumAggregation::Diff(const Aggregation &next) const noexcept { - long diff_value = - nostd::get( - nostd::get((static_cast(next).ToPoint())).value_) - - nostd::get(nostd::get(ToPoint()).value_); + long diff_value = nostd::get(nostd::get( + (static_cast(next).ToPoint())) + .value_) - + nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new LongSumAggregation()); static_cast(aggr.get())->point_data_.value_ = diff_value; return aggr; } -PointType LongSumAggregation::ToPoint() noexcept +PointType LongSumAggregation::ToPoint() const noexcept { return point_data_; } @@ -69,30 +70,32 @@ void DoubleSumAggregation::Aggregate(double value, const PointAttributes &attrib point_data_.value_ = nostd::get(point_data_.value_) + value; } -std::unique_ptr DoubleSumAggregation::Merge(Aggregation &delta) noexcept +std::unique_ptr DoubleSumAggregation::Merge(const Aggregation &delta) const noexcept { double merge_value = nostd::get( - nostd::get((static_cast(delta).ToPoint())).value_) + + nostd::get((static_cast(delta).ToPoint())) + .value_) + nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new DoubleSumAggregation()); static_cast(aggr.get())->point_data_.value_ = merge_value; return aggr; } -std::unique_ptr DoubleSumAggregation::Diff(Aggregation &next) noexcept +std::unique_ptr DoubleSumAggregation::Diff(const Aggregation &next) const noexcept { double diff_value = nostd::get( - nostd::get((static_cast(next).ToPoint())).value_) - + nostd::get((static_cast(next).ToPoint())) + .value_) - nostd::get(nostd::get(ToPoint()).value_); std::unique_ptr aggr(new DoubleSumAggregation()); static_cast(aggr.get())->point_data_.value_ = diff_value; return aggr; } -PointType DoubleSumAggregation::ToPoint() noexcept +PointType DoubleSumAggregation::ToPoint() const noexcept { const std::lock_guard locked(lock_); return point_data_; From 6919d1dfd249746dbaf3176f004fd64ca00b7459 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 28 Mar 2022 22:23:59 -0700 Subject: [PATCH 30/32] Update sdk/src/metrics/state/sync_metric_storage.cc Co-authored-by: Tom Tan --- sdk/src/metrics/state/sync_metric_storage.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index ec299f2434..a0b4f1704e 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -70,7 +70,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, // Final result of merge would be in merged_metrics. // - Move the final merge to the `last reported metrics` stash. // - If the aggregation_temporarily is delta - // - Store the unreported metrics for `collector`(which is in merged_mtrics) to + // - Store the unreported metrics for `collector` (which is in merged_mtrics) to // `last reported metrics` stash. auto reported = last_reported_metrics_.find(collector); From cf3c2727c35aa14ec2b6ed84ea372c3711e1ca38 Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 28 Mar 2022 22:33:10 -0700 Subject: [PATCH 31/32] more review comments --- sdk/src/metrics/state/sync_metric_storage.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index a0b4f1704e..5c14f8bbc3 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -122,12 +122,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, metric_data.point_data_attr_.push_back(point_data_attr); return true; }); - - if (callback(metric_data)) - { - return true; - } - return false; + return callback(metric_data); } } // namespace metrics From 010d1bcdfb4003a4a0de7026606d9efecf42c93d Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 30 Mar 2022 12:18:27 +1200 Subject: [PATCH 32/32] review comments --- .../opentelemetry/sdk/metrics/data/point_data.h | 4 +--- .../sdk/metrics/state/attributes_hashmap.h | 14 +++++++------- sdk/test/metrics/sync_metric_storage_test.cc | 2 -- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index f4aa956981..714f96c9af 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -20,8 +20,6 @@ using ValueType = nostd::variant; using ListType = nostd::variant, std::list>; // TODO: remove ctors and initializers from below classes when GCC<5 stops shipping on Ubuntu -// refer error: -// https://github.com/open-telemetry/opentelemetry-cpp/runs/5577740472?check_suite_focus=true#step:6:522 class SumPointData { @@ -77,4 +75,4 @@ class DropPointData } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE -#endif \ No newline at end of file +#endif diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h index a8527d0f7d..50d40e0ae6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h @@ -37,7 +37,7 @@ class AttributesHashMap public: Aggregation *Get(const MetricAttributes &attributes) const { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); auto it = hash_map_.find(attributes); if (it != hash_map_.end()) { @@ -52,7 +52,7 @@ class AttributesHashMap */ bool Has(const MetricAttributes &attributes) const { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); return (hash_map_.find(attributes) == hash_map_.end()) ? false : true; } @@ -64,7 +64,7 @@ class AttributesHashMap Aggregation *GetOrSetDefault(const MetricAttributes &attributes, std::function()> aggregation_callback) { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); auto it = hash_map_.find(attributes); if (it != hash_map_.end()) @@ -81,7 +81,7 @@ class AttributesHashMap */ void Set(const MetricAttributes &attributes, std::unique_ptr value) { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); hash_map_[attributes] = std::move(value); } @@ -91,7 +91,7 @@ class AttributesHashMap bool GetAllEnteries( nostd::function_ref callback) const { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); for (auto &kv : hash_map_) { if (!callback(kv.first, *(kv.second.get()))) @@ -107,7 +107,7 @@ class AttributesHashMap */ size_t Size() { - std::lock_guard guard(lock); + std::lock_guard guard(lock_); return hash_map_.size(); } @@ -115,7 +115,7 @@ class AttributesHashMap std::unordered_map, AttributeHashGenerator> hash_map_; - mutable opentelemetry::common::SpinLockMutex lock; + mutable opentelemetry::common::SpinLockMutex lock_; }; } // namespace metrics diff --git a/sdk/test/metrics/sync_metric_storage_test.cc b/sdk/test/metrics/sync_metric_storage_test.cc index c9811b2bcb..b0440fdbb3 100644 --- a/sdk/test/metrics/sync_metric_storage_test.cc +++ b/sdk/test/metrics/sync_metric_storage_test.cc @@ -11,8 +11,6 @@ # include # include -# include - using namespace opentelemetry::sdk::metrics; using namespace opentelemetry::common; using M = std::map;