From 1f331f995cec0ddfc58060e9be992b13093601bd Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:16:56 -0400 Subject: [PATCH 01/32] Implement Meter SDK class --- sdk/include/opentelemetry/sdk/metrics/meter.h | 356 +++++++++++++++++- 1 file changed, 349 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 2acbc76d3c..4b7d9ec35c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -1,20 +1,362 @@ -#pragma once - #include "opentelemetry/metrics/meter.h" -#include "opentelemetry/version.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/metrics/record.h" +#include "opentelemetry/sdk/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/async_instruments.h" +#include "opentelemetry/sdk/metrics/sync_instruments.h" -#include +#include +#include OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { -class Meter final : public opentelemetry::metrics::Meter, public std::enable_shared_from_this +namespace metrics_api = opentelemetry::metrics; +class Meter : public metrics_api::Meter { public: + explicit Meter(std::string library_name, std::string library_version = "") + { + library_name_ = library_name; + library_version_ = library_version; + } + + /** + * Creates a Counter with the passed characteristics and returns a shared_ptr to that Counter. + * + * @param name the name of the new Counter. + * @param description a brief description of what the Counter is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @return a shared pointer to the created Counter. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + /** + * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that + * UpDownCounter. + * + * @param name the name of the new UpDownCounter. + * @param description a brief description of what the UpDownCounter is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @return a shared pointer to the created UpDownCounter. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + /** + * Creates a ValueRecorder with the passed characteristics and returns a shared_ptr to that + * ValueRecorder. + * + * @param name the name of the new ValueRecorder. + * @param description a brief description of what the ValueRecorder is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @return a shared pointer to the created DoubleValueRecorder. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + /** + * Creates a SumObserver with the passed characteristics and returns a shared_ptr to that + * SumObserver. + * + * @param name the name of the new SumObserver. + * @param description a brief description of what the SumObserver is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @param callback the function to be observed by the instrument. + * @return a shared pointer to the created SumObserver. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewIntSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewFloatSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewDoubleSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + /** + * Creates an UpDownSumObserver with the passed characteristics and returns a shared_ptr to + * that UpDowNSumObserver. + * + * @param name the name of the new UpDownSumObserver. + * @param description a brief description of what the UpDownSumObserver is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @param callback the function to be observed by the instrument. + * @return a shared pointer to the created UpDownSumObserver. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewIntUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewFloatUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + nostd::shared_ptr> NewDoubleUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + /** + * Creates a ValueObserver with the passed characteristics and returns a shared_ptr to that + * ValueObserver. + * + * @param name the name of the new ValueObserver. + * @param description a brief description of what the ValueObserver is used for. + * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. + * @param enabled a boolean value that turns on or off the metric instrument. + * @param callback the function to be observed by the instrument. + * @return a shared pointer to the created ValueObserver. + * @throws IllegalArgumentException if {@code name} is null + * @throws IllegalArgumentException if a different metric by the same name exists in this meter. + * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + */ + nostd::shared_ptr> NewShortValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewIntValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewFloatValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + nostd::shared_ptr> NewDoubleValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) override; + + /** + * Utility method that allows users to atomically record measurements to a set of + * synchronous metric instruments with a common set of labels. + * + * @param labels the set of labels to associate with this recorder. + * @param values a span of pairs where the first element of the pair is a metric instrument + * to record to, and the second element is the value to update that instrument with. + */ + void RecordShortBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept override; + + void RecordIntBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept override; + + void RecordFloatBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept override; + + void RecordDoubleBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept override; + + /** + * An SDK-only function that checkpoints the aggregators of all instruments created from + * this meter, creates a {@code Record} out of them, and sends them for export. + * + * @return A vector of {@code Records} to be sent to the processor. + */ + std::vector Collect() noexcept; + +private: + + /** + * A private function that creates records from all synchronous instruments created from + * this meter. + * + * @param records A reference to the vector to push the new records to. + */ + void CollectMetrics(std::vector &records); + + /** + * A private function that creates records from all asynchronous instruments created from + * this meter. + * + * @param records A reference to the vector to push the new records to. + */ + void CollectObservers(std::vector &records); + + /** + * Utility function used by the meter that checks if a user-passed name abides by OpenTelemetry + * naming rules. The rules are as follows: + * 1. The name must not be empty. + * 2. The name must not start with a digit, a space, or any punctuation. + * 3. The name must only have the following chaacters: + * All alphanumeric characters, '.', '_' and '-'. + * + * @param name The name to be examined for legality. + * @return A bool representing whether the name is valid by the OpenTelemetry syntax rules. + */ + bool IsValidName(nostd::string_view name); + + /** + * A utility function used by the meter to determine whether an instrument of a specified + * name already exists in this meter. + * + * @param name The name to examine. + * @return A boolean representing whether the name has already been used by this meter. + */ + bool NameAlreadyUsed(nostd::string_view name); + + /* + * All instruments must be stored in a map so the meter can collect on these instruments. + * Additionally, when creating a new instrument, the meter must check if an instrument of the same + * name already exists. + */ + std::map>> short_metrics_; + std::map>> int_metrics_; + std::map>> float_metrics_; + std::map>> double_metrics_; + + std::map>> short_observers_; + std::map>> int_observers_; + std::map>> float_observers_; + std::map>> double_observers_; + + std::string library_name_; + std::string library_version_; + + std::mutex metrics_lock_; + std::mutex observers_lock_; }; -} // namespace trace -} // namespace sdk + +} +} OPENTELEMETRY_END_NAMESPACE From d40df5183247f6d455015829388d08a7cf5517e5 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:17:23 -0400 Subject: [PATCH 02/32] Create meter.cc --- sdk/src/metrics/meter.cc | 606 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 606 insertions(+) create mode 100644 sdk/src/metrics/meter.cc diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc new file mode 100644 index 0000000000..e1c1a731a5 --- /dev/null +++ b/sdk/src/metrics/meter.cc @@ -0,0 +1,606 @@ +#include "opentelemetry/sdk/metrics/meter.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +nostd::shared_ptr> Meter::NewShortCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto counter = new Counter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(counter); + metrics_lock_.lock(); + short_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto counter = new Counter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(counter); + metrics_lock_.lock(); + int_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto counter = new Counter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(counter); + metrics_lock_.lock(); + float_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto counter = new Counter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(counter); + metrics_lock_.lock(); + double_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewShortUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto udcounter = new UpDownCounter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(udcounter); + metrics_lock_.lock(); + short_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto udcounter = new UpDownCounter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(udcounter); + metrics_lock_.lock(); + int_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto udcounter = new UpDownCounter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(udcounter); + metrics_lock_.lock(); + float_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleUpDownCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto udcounter = new UpDownCounter(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(udcounter); + metrics_lock_.lock(); + double_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewShortValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto recorder = new ValueRecorder(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(recorder); + metrics_lock_.lock(); + short_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto recorder = new ValueRecorder(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(recorder); + metrics_lock_.lock(); + int_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto recorder = new ValueRecorder(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(recorder); + metrics_lock_.lock(); + float_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleValueRecorder( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto recorder = new ValueRecorder(name, description, unit, enabled); + auto ptr = nostd::shared_ptr>(recorder); + metrics_lock_.lock(); + double_metrics_.insert(std::make_pair(std::string(name), ptr)); + metrics_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewShortSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new SumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + short_observers_.insert(std::make_pair(std::string(name),ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new SumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + int_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new SumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + float_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new SumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + double_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewShortUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + short_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + int_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + float_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleUpDownSumObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + double_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewShortValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new ValueObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + short_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewIntValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new ValueObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + int_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewFloatValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new ValueObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + float_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +nostd::shared_ptr> Meter::NewDoubleValueObserver( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled, + void (*callback)(metrics_api::ObserverResult)) +{ + if (!IsValidName(name) || NameAlreadyUsed(name)) + { + throw std::invalid_argument("Invalid Name"); + } + auto sumobs = new ValueObserver(name, description, unit, enabled, callback); + auto ptr = nostd::shared_ptr>(sumobs); + observers_lock_.lock(); + double_observers_.insert(std::make_pair(std::string(name), ptr)); + observers_lock_.unlock(); + return ptr; +} + +void Meter::RecordShortBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept +{ + for (int i = 0; i < instruments.size(); ++i) + { + instruments[i]->update(values[i], labels); + } +} + +void Meter::RecordIntBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept +{ + for (int i = 0; i < instruments.size(); ++i) + { + instruments[i]->update(values[i], labels); + } +} + +void Meter::RecordFloatBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept +{ + for (int i = 0; i < instruments.size(); ++i) + { + instruments[i]->update(values[i], labels); + } +} + +void Meter::RecordDoubleBatch( + const trace::KeyValueIterable &labels, + nostd::span>> instruments, + nostd::span values) noexcept +{ + for (int i = 0; i < instruments.size(); ++i) + { + instruments[i]->update(values[i], labels); + } +} + +std::vector Meter::Collect() noexcept +{ + std::vector records; + CollectMetrics(records); + CollectObservers(records); + return records; +} + +// Must cast to sdk::SynchronousInstrument to have access to GetRecords() function +void Meter::CollectMetrics(std::vector &records) +{ + metrics_lock_.lock(); + for (const auto &pair : short_metrics_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : int_metrics_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : float_metrics_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : double_metrics_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + metrics_lock_.unlock(); +} + +void Meter::CollectObservers(std::vector &records) +{ + observers_lock_.lock(); + for (const auto &pair : short_observers_) + { + // Must cast to sdk::SynchronousInstrument to have access to GetRecords() function + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : int_observers_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : float_observers_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + for (const auto &pair : double_observers_) + { + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); + } + observers_lock_.unlock(); +} + +bool Meter::IsValidName(nostd::string_view name) +{ + if (name.empty() || isdigit(name[0]) || isspace(name[0]) || ispunct(name[0])) + return false; + else + { + for (int i = 0; i < name.size(); ++i) + { + if (!isalnum(name[i]) && name[i] != '_' && name[i] != '.' && name[i] != '-') + return false; + } + } + return true; +} + +bool Meter::NameAlreadyUsed(nostd::string_view name) +{ + if (short_metrics_.find(std::string(name)) != short_metrics_.end()) + return true; + else if (int_metrics_.find(std::string(name)) != int_metrics_.end()) + return true; + else if (float_metrics_.find(std::string(name)) != float_metrics_.end()) + return true; + else if (double_metrics_.find(std::string(name)) != double_metrics_.end()) + return true; + + else if (short_observers_.find(std::string(name)) != short_observers_.end()) + return true; + else if (int_observers_.find(std::string(name)) != int_observers_.end()) + return true; + else if (float_observers_.find(std::string(name)) != float_observers_.end()) + return true; + else if (double_observers_.find(std::string(name)) != double_observers_.end()) + return true; + + else + return false; +} +} +} +OPENTELEMETRY_END_NAMESPACE From 765d63ae352ae9ad3faf65aad1d8f0a1a9318d8d Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:17:37 -0400 Subject: [PATCH 03/32] Add meter.cc --- sdk/src/metrics/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index 6e5fa0284a..e392745300 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -1 +1 @@ -add_library(opentelemetry_metrics meter_provider.cc) +add_library(opentelemetry_metrics meter_provider.cc meter.cc) From c6e1359cb94c687cc343604852d0cf82a37a6702 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:19:49 -0400 Subject: [PATCH 04/32] Add meter tests -Create Synchronous and Asynchronous Instruments -Collect Synchronous and Asynchronous Insturments -Collect Synchronous and Asynchronous Instruments after their returned shared_ptr is deleted -RecordBatch for all 4 primitive types -String utility functions, IsValid() and NameAlreadyExists() --- sdk/test/metrics/meter_test.cc | 261 +++++++++++++++++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 sdk/test/metrics/meter_test.cc diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc new file mode 100644 index 0000000000..3686e657f7 --- /dev/null +++ b/sdk/test/metrics/meter_test.cc @@ -0,0 +1,261 @@ +#include "opentelemetry/sdk/metrics/meter.h" +#include + +using namespace opentelemetry::sdk::metrics; +namespace metrics_api = opentelemetry::metrics; + +OPENTELEMETRY_BEGIN_NAMESPACE + +TEST(Meter, CreateSyncInstruments) +{ + // Test that there are no errors creating synchronous instruments. + auto m = new Meter("Test"); + + m->NewShortCounter("Test-short-counter", "For testing", "Unitless", true); + m->NewIntCounter("Test-int-counter", "For testing", "Unitless", true); + m->NewFloatCounter("Test-float-counter", "For testing", "Unitless", true); + m->NewDoubleCounter("Test-double-counter", "For testing", "Unitless", true); + + m->NewShortUpDownCounter("Test-short-ud-counter", "For testing", "Unitless", true); + m->NewIntUpDownCounter("Test-int-ud-counter", "For testing", "Unitless", true); + m->NewFloatUpDownCounter("Test-float-ud-counter", "For testing", "Unitless", true); + m->NewDoubleUpDownCounter("Test-double-ud-counter", "For testing", "Unitless", true); + + m->NewShortValueRecorder("Test-short-recorder", "For testing", "Unitless", true); + m->NewIntValueRecorder("Test-int-recorder", "For testing", "Unitless", true); + m->NewFloatValueRecorder("Test-float-recorder", "For testing", "Unitless", true); + m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); +} + +//Dummy functions for asynchronous instrument constructors +void ShortCallback(metrics_api::ObserverResult) {} +void IntCallback(metrics_api::ObserverResult) {} +void FloatCallback(metrics_api::ObserverResult) {} +void DoubleCallback(metrics_api::ObserverResult) {} + +TEST(Meter, CreateAsyncInstruments) +{ + // Test that there are no errors when creating asynchronous instruments. + auto m = new Meter("Test"); + + m->NewShortSumObserver("Test-short-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewIntSumObserver("Test-int-sum-obs", "For testing", "Unitless", true, &IntCallback); + m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + + m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); + m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); +} + +TEST(Meter, CollectSyncInstruments) +{ + // Verify that the records returned on a call to Collect() are correct for synchronous + // instruments. + auto m = new Meter("Test"); + + ASSERT_EQ(m->Collect().size(), 0); + + auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); + + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + + counter->add(1, labelkv); + + std::vector res = m->Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 1); + + // Now call add() and Collect() again to ensure that the value in the underlying + // aggregator was reset to the default. + + counter->add(10, labelkv); + + res = m->Collect(); + agg_var = res[0].GetAggregator(); + agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 10); +} + +TEST(Meter, CollectDeletedSync) +{ + // Verify that calling Collect() after creating a synchronous instrument and destroying + // the return pointer does not result in a segfault. + + auto m = new Meter("Test"); + + ASSERT_EQ(m->Collect().size(), 0); + + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); + counter->add(1, labelkv); + } // counter shared_ptr deleted here + + std::vector res = m->Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 1); +} + +// Dummy function for asynchronous instrument constructors. +void Callback(opentelemetry::metrics::ObserverResult result) +{ + std::map labels = {{"key", "value"}}; + auto labelkv = trace::KeyValueIterableView{labels}; + result.observe(1, labelkv); +} + +TEST(Meter, CollectAsyncInstruments) +{ + // Verify that the records returned on a call to Collect() are correct for asynchronous + // instruments. + auto m = new Meter("Test"); + + ASSERT_EQ(m->Collect().size(), 0); + + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + + sumobs->observe(1, labelkv); + + std::vector res = m->Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 1); + + // Now call observe() and Collect() again to ensure that the value in the underlying + // aggregator was reset to the default. + + sumobs->observe(10, labelkv); + + res = m->Collect(); + agg_var = res[0].GetAggregator(); + agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 10); +} + +TEST(Meter, CollectDeletedAsync) +{ + // Verify that calling Collect() after creating an asynchronous instrument and destroying + // the return pointer does not result in a segfault. + + auto m = new Meter("Test"); + + ASSERT_EQ(m->Collect().size(), 0); + + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + sumobs->observe(1, labelkv); + } // sumobs shared_ptr deleted here + + std::vector res = m->Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); + + ASSERT_EQ(agg->get_checkpoint()[0], 1); +} + +TEST(Meter, RecordBatch) +{ + // This tests that RecordBatch appropriately updates the aggregators of the instruments + // passed to the function. Short, int, float, and double data types are tested. + auto m = new Meter("Test"); + + auto scounter = m->NewShortCounter("Test-scounter", "For testing", "Unitless", true); + auto icounter = m->NewIntCounter("Test-icounter", "For testing", "Unitless", true); + auto fcounter = m->NewFloatCounter("Test-fcounter", "For testing", "Unitless", true); + auto dcounter = m->NewDoubleCounter("Test-dcounter", "For testing", "Unitless", true); + + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + + nostd::shared_ptr> sinstr_arr[] = {scounter}; + short svalues_arr[] = {1}; + + nostd::span>> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; + + m->RecordShortBatch(labelkv, sinstrs, svalues); + std::vector res = m->Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); + ASSERT_EQ(agg->get_values()[0], 1); + + nostd::shared_ptr> iinstr_arr[] = {icounter}; + int ivalues_arr[] = {1}; + + nostd::span>> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; + + m->RecordIntBatch(labelkv, iinstrs, ivalues); + res = m->Collect(); + agg_var = res[1].GetAggregator(); + agg = opentelemetry::nostd::get<0>(agg_var); + ASSERT_EQ(agg->get_values()[0], 1); + + nostd::shared_ptr> finstr_arr[] = {fcounter}; + float fvalues_arr[] = {1.0}; + + nostd::span>> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; + + m->RecordFloatBatch(labelkv, finstrs, fvalues); + res = m->Collect(); + agg_var = res[2].GetAggregator(); + agg = opentelemetry::nostd::get<0>(agg_var); + ASSERT_EQ(agg->get_values()[0], 1.0); + + nostd::shared_ptr> dinstr_arr[] = {dcounter}; + double dvalues_arr[] = {1.0}; + + nostd::span>> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; + + m->RecordDoubleBatch(labelkv, dinstrs, dvalues); + res = m->Collect(); + agg_var = res[3].GetAggregator(); + agg = opentelemetry::nostd::get<0>(agg_var); + ASSERT_EQ(agg->get_values()[0], 1.0); +} + +TEST(MeterStringUtil, IsValid) +{ + auto m = new Meter("Test"); + ASSERT_ANY_THROW(m->NewShortCounter("", "Empty name is invalid", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); +} + +TEST(MeterStringUtil, AlreadyExists) +{ + auto m = new Meter("Test"); + + m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); +} +OPENTELEMETRY_END_NAMESPACE From 0b1b786a546f83fc5fed7573b1e5c2dbb2dd0285 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:20:14 -0400 Subject: [PATCH 05/32] Add meter_test to CMake targets --- sdk/test/metrics/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 800b57557a..73bb8faf4b 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach(testname meter_provider_sdk_test) +foreach(testname meter_provider_sdk_test meter_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common opentelemetry_metrics) From c387d2ca966e899bb697d731ea632370640f142b Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sat, 25 Jul 2020 19:20:45 -0400 Subject: [PATCH 06/32] Add meter_test to Bazel targets --- sdk/test/metrics/BUILD | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index e2d5c5cb71..8a9cfeb724 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -8,3 +8,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "meter_test", + srcs = [ + "meter_test.cc", + ], + deps = [ + "//sdk/src/metrics", + "@com_google_googletest//:gtest_main", + ], +) From 0a46ee2568563a2180e5d17f880acbd77c7b2f47 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 16:05:31 -0400 Subject: [PATCH 07/32] Fix RecordBatch assert checks --- sdk/test/metrics/meter_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 3686e657f7..90cede4dd6 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -198,7 +198,7 @@ TEST(Meter, RecordBatch) std::vector res = m->Collect(); auto agg_var = res[0].GetAggregator(); auto agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_values()[0], 1); + ASSERT_EQ(agg->get_checkpoint()[0], 1); nostd::shared_ptr> iinstr_arr[] = {icounter}; int ivalues_arr[] = {1}; @@ -210,7 +210,7 @@ TEST(Meter, RecordBatch) res = m->Collect(); agg_var = res[1].GetAggregator(); agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_values()[0], 1); + ASSERT_EQ(agg->get_checkpoint()[0], 1); nostd::shared_ptr> finstr_arr[] = {fcounter}; float fvalues_arr[] = {1.0}; @@ -222,7 +222,7 @@ TEST(Meter, RecordBatch) res = m->Collect(); agg_var = res[2].GetAggregator(); agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_values()[0], 1.0); + ASSERT_EQ(agg->get_checkpoint()[0], 1.0); nostd::shared_ptr> dinstr_arr[] = {dcounter}; double dvalues_arr[] = {1.0}; @@ -234,7 +234,7 @@ TEST(Meter, RecordBatch) res = m->Collect(); agg_var = res[3].GetAggregator(); agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_values()[0], 1.0); + ASSERT_EQ(agg->get_checkpoint()[0], 1.0); } TEST(MeterStringUtil, IsValid) From 37c6b52ff8757a87b8c001e88d7a996200fe781d Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 16:10:43 -0400 Subject: [PATCH 08/32] Fix RecordBatch variant unpacking --- sdk/test/metrics/meter_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 90cede4dd6..40aa980e62 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -197,8 +197,8 @@ TEST(Meter, RecordBatch) m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_checkpoint()[0], 1); + auto short_agg = opentelemetry::nostd::get<0>(agg_var); + ASSERT_EQ(short_agg->get_checkpoint()[0], 1); nostd::shared_ptr> iinstr_arr[] = {icounter}; int ivalues_arr[] = {1}; @@ -209,8 +209,8 @@ TEST(Meter, RecordBatch) m->RecordIntBatch(labelkv, iinstrs, ivalues); res = m->Collect(); agg_var = res[1].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_checkpoint()[0], 1); + auto int_agg = opentelemetry::nostd::get<1>(agg_var); + ASSERT_EQ(int_agg->get_checkpoint()[0], 1); nostd::shared_ptr> finstr_arr[] = {fcounter}; float fvalues_arr[] = {1.0}; @@ -221,8 +221,8 @@ TEST(Meter, RecordBatch) m->RecordFloatBatch(labelkv, finstrs, fvalues); res = m->Collect(); agg_var = res[2].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_checkpoint()[0], 1.0); + auto float_agg = opentelemetry::nostd::get<2>(agg_var); + ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); nostd::shared_ptr> dinstr_arr[] = {dcounter}; double dvalues_arr[] = {1.0}; @@ -233,8 +233,8 @@ TEST(Meter, RecordBatch) m->RecordDoubleBatch(labelkv, dinstrs, dvalues); res = m->Collect(); agg_var = res[3].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); - ASSERT_EQ(agg->get_checkpoint()[0], 1.0); + auto double_agg = opentelemetry::nostd::get<3>(agg_var); + ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(MeterStringUtil, IsValid) From cda3bb8120c39b405386aaf8f4af766c40f02bfb Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 16:23:42 -0400 Subject: [PATCH 09/32] Fix RecordBatch test Metric instruments are collected from in the reverse order that they were created. The test assumed that they were collected in-order. This has been rectified by making all ASSERTs in the RecordBatch test examine the first aggregator in the vector of records returned from m->collect() --- sdk/test/metrics/meter_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 40aa980e62..3b35916954 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -196,8 +196,8 @@ TEST(Meter, RecordBatch) m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); nostd::shared_ptr> iinstr_arr[] = {icounter}; @@ -208,8 +208,8 @@ TEST(Meter, RecordBatch) m->RecordIntBatch(labelkv, iinstrs, ivalues); res = m->Collect(); - agg_var = res[1].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(agg_var); + auto int_agg_var = res[0].GetAggregator(); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); nostd::shared_ptr> finstr_arr[] = {fcounter}; @@ -220,8 +220,8 @@ TEST(Meter, RecordBatch) m->RecordFloatBatch(labelkv, finstrs, fvalues); res = m->Collect(); - agg_var = res[2].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(agg_var); + auto float_agg_var = res[0].GetAggregator(); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); nostd::shared_ptr> dinstr_arr[] = {dcounter}; @@ -232,8 +232,8 @@ TEST(Meter, RecordBatch) m->RecordDoubleBatch(labelkv, dinstrs, dvalues); res = m->Collect(); - agg_var = res[3].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(agg_var); + auto double_agg_var = res[0].GetAggregator(); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } From 7199eaae24bcc2a9a84830b4dadfb8c94a654b2e Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 17:50:40 -0400 Subject: [PATCH 10/32] Test collection of disabled instruments --- sdk/test/metrics/meter_test.cc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 3b35916954..c6a68ac788 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -237,6 +237,26 @@ TEST(Meter, RecordBatch) ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } +TEST(Meter, DisableCollectSync) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + auto c = m->NewShortCounter("c", "", "", false); + c->add(1, labelkv); + ASSERT_EQ(m->Collect().size(), 0); +} + +TEST(Meter, DisableCollectAsync) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + c->observe(1, labelkv); + ASSERT_EQ(m->Collect().size(), 0); +} + TEST(MeterStringUtil, IsValid) { auto m = new Meter("Test"); From 194e7602637bb6ebc862db341fbd4b22668caee7 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Sun, 26 Jul 2020 17:51:10 -0400 Subject: [PATCH 11/32] Add logic to skip collection of disabled instruments --- sdk/src/metrics/meter.cc | 41 +++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index e1c1a731a5..8aa8fe4a13 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -507,24 +507,40 @@ void Meter::CollectMetrics(std::vector &records) metrics_lock_.lock(); for (const auto &pair : short_metrics_) { + if (!pair.second->IsEnabled()) + { + continue; + } auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : int_metrics_) { + if (!pair.second->IsEnabled()) + { + continue; + } auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : float_metrics_) { + if (!pair.second->IsEnabled()) + { + continue; + } auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : double_metrics_) { + if (!pair.second->IsEnabled()) + { + continue; + } auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); @@ -537,26 +553,41 @@ void Meter::CollectObservers(std::vector &records) observers_lock_.lock(); for (const auto &pair : short_observers_) { - // Must cast to sdk::SynchronousInstrument to have access to GetRecords() function - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + if (!pair.second->IsEnabled()) + { + continue; + } + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : int_observers_) { - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + if (!pair.second->IsEnabled()) + { + continue; + } + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : float_observers_) { - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + if (!pair.second->IsEnabled()) + { + continue; + } + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } for (const auto &pair : double_observers_) { - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + if (!pair.second->IsEnabled()) + { + continue; + } + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } From 180df0714c92f50ff30df89ff1e50babb479e1c3 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Mon, 27 Jul 2020 18:26:59 -0400 Subject: [PATCH 12/32] Change to nostd pointer cast in CollectObservers --- sdk/src/metrics/meter.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 8aa8fe4a13..46b92dd561 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -557,7 +557,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -567,7 +567,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -577,7 +577,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -587,7 +587,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); + auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } From cb9ac9f7f7d5de34a566c375177970da59f9d33a Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 29 Jul 2020 16:35:46 -0400 Subject: [PATCH 13/32] Format --- sdk/include/opentelemetry/sdk/metrics/meter.h | 71 +++++------ sdk/src/metrics/meter.cc | 17 ++- sdk/test/metrics/CMakeLists.txt | 5 +- sdk/test/metrics/meter_test.cc | 110 ++++++++++-------- 4 files changed, 106 insertions(+), 97 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 4b7d9ec35c..e6710312f7 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -1,8 +1,8 @@ #include "opentelemetry/metrics/meter.h" #include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/sdk/metrics/record.h" -#include "opentelemetry/sdk/metrics/instrument.h" #include "opentelemetry/sdk/metrics/async_instruments.h" +#include "opentelemetry/sdk/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" #include @@ -19,7 +19,7 @@ class Meter : public metrics_api::Meter public: explicit Meter(std::string library_name, std::string library_version = "") { - library_name_ = library_name; + library_name_ = library_name; library_version_ = library_version; } @@ -35,29 +35,25 @@ class Meter : public metrics_api::Meter * @throws IllegalArgumentException if a different metric by the same name exists in this meter. * @throws IllegalArgumentException if the {@code name} does not match spec requirements. */ - nostd::shared_ptr> NewShortCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewFloatCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewDoubleCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; + nostd::shared_ptr> NewShortCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -296,7 +292,6 @@ class Meter : public metrics_api::Meter std::vector Collect() noexcept; private: - /** * A private function that creates records from all synchronous instruments created from * this meter. @@ -340,15 +335,21 @@ class Meter : public metrics_api::Meter * Additionally, when creating a new instrument, the meter must check if an instrument of the same * name already exists. */ - std::map>> short_metrics_; + std::map>> + short_metrics_; std::map>> int_metrics_; - std::map>> float_metrics_; - std::map>> double_metrics_; + std::map>> + float_metrics_; + std::map>> + double_metrics_; - std::map>> short_observers_; + std::map>> + short_observers_; std::map>> int_observers_; - std::map>> float_observers_; - std::map>> double_observers_; + std::map>> + float_observers_; + std::map>> + double_observers_; std::string library_name_; std::string library_version_; @@ -357,6 +358,6 @@ class Meter : public metrics_api::Meter std::mutex observers_lock_; }; -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 46b92dd561..e7eaea9205 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -23,18 +23,17 @@ nostd::shared_ptr> Meter::NewShortCounter( return ptr; } -nostd::shared_ptr> Meter::NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) +nostd::shared_ptr> Meter::NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) { if (!IsValidName(name) || NameAlreadyUsed(name)) { throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(counter); + auto ptr = nostd::shared_ptr>(counter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); @@ -235,7 +234,7 @@ nostd::shared_ptr> Meter::NewShortSumObserver( auto sumobs = new SumObserver(name, description, unit, enabled, callback); auto ptr = nostd::shared_ptr>(sumobs); observers_lock_.lock(); - short_observers_.insert(std::make_pair(std::string(name),ptr)); + short_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); return ptr; } @@ -632,6 +631,6 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) else return false; } -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 73bb8faf4b..2cf4d4d49b 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -1,6 +1,7 @@ foreach(testname meter_provider_sdk_test meter_test) add_executable(${testname} "${testname}.cc") - target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common opentelemetry_metrics) + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_common opentelemetry_metrics) gtest_add_tests(TARGET ${testname} TEST_PREFIX metrics. TEST_LIST ${testname}) endforeach() diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index c6a68ac788..0621b4612a 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -27,7 +27,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -43,15 +43,19 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, + &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -70,8 +74,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -80,9 +84,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -101,11 +105,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -126,7 +130,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -134,8 +139,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -144,9 +149,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -163,13 +168,14 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -189,70 +195,73 @@ TEST(Meter, RecordBatch) auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; nostd::shared_ptr> sinstr_arr[] = {scounter}; - short svalues_arr[] = {1}; + short svalues_arr[] = {1}; - nostd::span>> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span>> sinstrs{ + sinstr_arr}; + nostd::span svalues{svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); nostd::shared_ptr> iinstr_arr[] = {icounter}; - int ivalues_arr[] = {1}; + int ivalues_arr[] = {1}; - nostd::span>> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span>> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); nostd::shared_ptr> finstr_arr[] = {fcounter}; - float fvalues_arr[] = {1.0}; + float fvalues_arr[] = {1.0}; - nostd::span>> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span>> finstrs{ + finstr_arr}; + nostd::span fvalues{fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); nostd::shared_ptr> dinstr_arr[] = {dcounter}; - double dvalues_arr[] = {1.0}; + double dvalues_arr[] = {1.0}; - nostd::span>> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span>> dinstrs{ + dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } @@ -264,7 +273,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -272,10 +282,8 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver( + "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From 223d8a0c0f5dc22a620ea463eddbd13538dc3689 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 29 Jul 2020 20:29:21 -0400 Subject: [PATCH 14/32] Remove dynamic pointer casting --- sdk/src/metrics/meter.cc | 126 +++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index e7eaea9205..7e7bb32ee8 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/metrics/meter.h" - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -16,28 +15,29 @@ nostd::shared_ptr> Meter::NewShortCounter( throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); short_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } -nostd::shared_ptr> Meter::NewIntCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) +nostd::shared_ptr> Meter::NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) { if (!IsValidName(name) || NameAlreadyUsed(name)) { throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatCounter( @@ -51,11 +51,11 @@ nostd::shared_ptr> Meter::NewFloatCounter( throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); float_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleCounter( @@ -69,11 +69,11 @@ nostd::shared_ptr> Meter::NewDoubleCounter( throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); double_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewShortUpDownCounter( @@ -87,11 +87,11 @@ nostd::shared_ptr> Meter::NewShortUpDownCounte throw std::invalid_argument("Invalid Name"); } auto udcounter = new UpDownCounter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(udcounter); + auto ptr = std::shared_ptr>(udcounter); metrics_lock_.lock(); short_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewIntUpDownCounter( @@ -105,11 +105,11 @@ nostd::shared_ptr> Meter::NewIntUpDownCounter( throw std::invalid_argument("Invalid Name"); } auto udcounter = new UpDownCounter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(udcounter); + auto ptr = std::shared_ptr>(udcounter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatUpDownCounter( @@ -123,11 +123,11 @@ nostd::shared_ptr> Meter::NewFloatUpDownCounte throw std::invalid_argument("Invalid Name"); } auto udcounter = new UpDownCounter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(udcounter); + auto ptr = std::shared_ptr>(udcounter); metrics_lock_.lock(); float_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleUpDownCounter( @@ -141,11 +141,11 @@ nostd::shared_ptr> Meter::NewDoubleUpDownCoun throw std::invalid_argument("Invalid Name"); } auto udcounter = new UpDownCounter(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(udcounter); + auto ptr = std::shared_ptr>(udcounter); metrics_lock_.lock(); double_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewShortValueRecorder( @@ -159,11 +159,11 @@ nostd::shared_ptr> Meter::NewShortValueRecorde throw std::invalid_argument("Invalid Name"); } auto recorder = new ValueRecorder(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(recorder); + auto ptr = std::shared_ptr>(recorder); metrics_lock_.lock(); short_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewIntValueRecorder( @@ -177,11 +177,11 @@ nostd::shared_ptr> Meter::NewIntValueRecorder( throw std::invalid_argument("Invalid Name"); } auto recorder = new ValueRecorder(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(recorder); + auto ptr = std::shared_ptr>(recorder); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatValueRecorder( @@ -195,11 +195,11 @@ nostd::shared_ptr> Meter::NewFloatValueRecorde throw std::invalid_argument("Invalid Name"); } auto recorder = new ValueRecorder(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(recorder); + auto ptr = std::shared_ptr>(recorder); metrics_lock_.lock(); float_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleValueRecorder( @@ -213,11 +213,11 @@ nostd::shared_ptr> Meter::NewDoubleValueRecor throw std::invalid_argument("Invalid Name"); } auto recorder = new ValueRecorder(name, description, unit, enabled); - auto ptr = nostd::shared_ptr>(recorder); + auto ptr = std::shared_ptr>(recorder); metrics_lock_.lock(); double_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewShortSumObserver( @@ -232,11 +232,11 @@ nostd::shared_ptr> Meter::NewShortSumObserver( throw std::invalid_argument("Invalid Name"); } auto sumobs = new SumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); short_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewIntSumObserver( @@ -251,11 +251,11 @@ nostd::shared_ptr> Meter::NewIntSumObserver( throw std::invalid_argument("Invalid Name"); } auto sumobs = new SumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); int_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatSumObserver( @@ -270,11 +270,11 @@ nostd::shared_ptr> Meter::NewFloatSumObserver( throw std::invalid_argument("Invalid Name"); } auto sumobs = new SumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); float_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleSumObserver( @@ -289,11 +289,11 @@ nostd::shared_ptr> Meter::NewDoubleSumObserver( throw std::invalid_argument("Invalid Name"); } auto sumobs = new SumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); double_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewShortUpDownSumObserver( @@ -308,11 +308,11 @@ nostd::shared_ptr> Meter::NewShortUpDownSu throw std::invalid_argument("Invalid Name"); } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); short_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewIntUpDownSumObserver( @@ -327,11 +327,11 @@ nostd::shared_ptr> Meter::NewIntUpDownSumObs throw std::invalid_argument("Invalid Name"); } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); int_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatUpDownSumObserver( @@ -346,11 +346,11 @@ nostd::shared_ptr> Meter::NewFloatUpDownSu throw std::invalid_argument("Invalid Name"); } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); float_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleUpDownSumObserver( @@ -365,11 +365,11 @@ nostd::shared_ptr> Meter::NewDoubleUpDown throw std::invalid_argument("Invalid Name"); } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); double_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewShortValueObserver( @@ -384,11 +384,11 @@ nostd::shared_ptr> Meter::NewShortValueObserve throw std::invalid_argument("Invalid Name"); } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); short_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewIntValueObserver( @@ -403,11 +403,11 @@ nostd::shared_ptr> Meter::NewIntValueObserver( throw std::invalid_argument("Invalid Name"); } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); int_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewFloatValueObserver( @@ -422,11 +422,11 @@ nostd::shared_ptr> Meter::NewFloatValueObserve throw std::invalid_argument("Invalid Name"); } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); float_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } nostd::shared_ptr> Meter::NewDoubleValueObserver( @@ -441,11 +441,11 @@ nostd::shared_ptr> Meter::NewDoubleValueObser throw std::invalid_argument("Invalid Name"); } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); - auto ptr = nostd::shared_ptr>(sumobs); + auto ptr = std::shared_ptr>(sumobs); observers_lock_.lock(); double_observers_.insert(std::make_pair(std::string(name), ptr)); observers_lock_.unlock(); - return ptr; + return nostd::shared_ptr>(ptr); } void Meter::RecordShortBatch( @@ -510,7 +510,7 @@ void Meter::CollectMetrics(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -520,7 +520,7 @@ void Meter::CollectMetrics(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -530,7 +530,7 @@ void Meter::CollectMetrics(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -540,7 +540,7 @@ void Meter::CollectMetrics(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -556,7 +556,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -566,7 +566,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -576,7 +576,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -586,7 +586,7 @@ void Meter::CollectObservers(std::vector &records) { continue; } - auto cast_ptr = nostd::dynamic_pointer_cast>(pair.second); + auto cast_ptr = std::dynamic_pointer_cast>(pair.second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -631,6 +631,6 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) else return false; } -} // namespace metrics -} // namespace sdk +} +} OPENTELEMETRY_END_NAMESPACE From 3c0ef456053e3cc97cbd99cf90be529a5958aafd Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 29 Jul 2020 21:57:32 -0400 Subject: [PATCH 15/32] Update RecordBatch function to stay in line with API --- sdk/include/opentelemetry/sdk/metrics/meter.h | 87 ++++++------- sdk/src/metrics/meter.cc | 10 +- sdk/test/metrics/meter_test.cc | 122 +++++++++--------- 3 files changed, 107 insertions(+), 112 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index e6710312f7..93c3bfdbed 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -1,8 +1,8 @@ #include "opentelemetry/metrics/meter.h" #include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/sdk/metrics/async_instruments.h" -#include "opentelemetry/sdk/metrics/instrument.h" #include "opentelemetry/sdk/metrics/record.h" +#include "opentelemetry/sdk/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/async_instruments.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" #include @@ -19,7 +19,7 @@ class Meter : public metrics_api::Meter public: explicit Meter(std::string library_name, std::string library_version = "") { - library_name_ = library_name; + library_name_ = library_name; library_version_ = library_version; } @@ -35,25 +35,29 @@ class Meter : public metrics_api::Meter * @throws IllegalArgumentException if a different metric by the same name exists in this meter. * @throws IllegalArgumentException if the {@code name} does not match spec requirements. */ - nostd::shared_ptr> NewShortCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewIntCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewFloatCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewDoubleCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; + nostd::shared_ptr> NewShortCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -265,22 +269,22 @@ class Meter : public metrics_api::Meter */ void RecordShortBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept override; void RecordIntBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept override; void RecordFloatBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept override; void RecordDoubleBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept override; /** @@ -292,6 +296,7 @@ class Meter : public metrics_api::Meter std::vector Collect() noexcept; private: + /** * A private function that creates records from all synchronous instruments created from * this meter. @@ -335,21 +340,15 @@ class Meter : public metrics_api::Meter * Additionally, when creating a new instrument, the meter must check if an instrument of the same * name already exists. */ - std::map>> - short_metrics_; - std::map>> int_metrics_; - std::map>> - float_metrics_; - std::map>> - double_metrics_; - - std::map>> - short_observers_; - std::map>> int_observers_; - std::map>> - float_observers_; - std::map>> - double_observers_; + std::map>> short_metrics_; + std::map>> int_metrics_; + std::map>> float_metrics_; + std::map>> double_metrics_; + + std::map>> short_observers_; + std::map>> int_observers_; + std::map>> float_observers_; + std::map>> double_observers_; std::string library_name_; std::string library_version_; @@ -358,6 +357,6 @@ class Meter : public metrics_api::Meter std::mutex observers_lock_; }; -} // namespace metrics -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE +} +} +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 7e7bb32ee8..45fbb4823d 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -450,7 +450,7 @@ nostd::shared_ptr> Meter::NewDoubleValueObser void Meter::RecordShortBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) @@ -461,7 +461,7 @@ void Meter::RecordShortBatch( void Meter::RecordIntBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) @@ -472,7 +472,7 @@ void Meter::RecordIntBatch( void Meter::RecordFloatBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) @@ -483,7 +483,7 @@ void Meter::RecordFloatBatch( void Meter::RecordDoubleBatch( const trace::KeyValueIterable &labels, - nostd::span>> instruments, + nostd::span*> instruments, nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) @@ -633,4 +633,4 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) } } } -OPENTELEMETRY_END_NAMESPACE +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 0621b4612a..a501a5eef8 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -27,7 +27,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -// Dummy functions for asynchronous instrument constructors +//Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -43,19 +43,15 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, - &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, - &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -74,8 +70,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -84,9 +80,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -105,11 +101,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -130,8 +126,7 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -139,8 +134,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -149,9 +144,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -168,14 +163,13 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -194,74 +188,75 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - nostd::shared_ptr> sinstr_arr[] = {scounter}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument* sinstr_arr[] = + {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span>> sinstrs{ - sinstr_arr}; - nostd::span svalues{svalues_arr}; + nostd::span*> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - nostd::shared_ptr> iinstr_arr[] = {icounter}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument* iinstr_arr[] = + {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span>> iinstrs{iinstr_arr}; - nostd::span ivalues{ivalues_arr}; + nostd::span*> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - nostd::shared_ptr> finstr_arr[] = {fcounter}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* finstr_arr[] = + {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span>> finstrs{ - finstr_arr}; - nostd::span fvalues{fvalues_arr}; + nostd::span*> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - nostd::shared_ptr> dinstr_arr[] = {dcounter}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* dinstr_arr[] = + {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span>> dinstrs{ - dinstr_arr}; - nostd::span dvalues{dvalues_arr}; + nostd::span*> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } @@ -273,8 +268,7 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter( - "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -282,8 +276,10 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver( - "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From 5f4b1307acd39e0fc8a2230f27019588c3526a2d Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 29 Jul 2020 22:01:31 -0400 Subject: [PATCH 16/32] Format --- sdk/include/opentelemetry/sdk/metrics/meter.h | 91 +++++++------- sdk/src/metrics/meter.cc | 43 +++---- sdk/test/metrics/meter_test.cc | 119 +++++++++--------- 3 files changed, 122 insertions(+), 131 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index f696f2677c..67d44a9ad0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -19,7 +19,7 @@ class Meter : public metrics_api::Meter public: explicit Meter(std::string library_name, std::string library_version = "") { - library_name_ = library_name; + library_name_ = library_name; library_version_ = library_version; } @@ -35,29 +35,25 @@ class Meter : public metrics_api::Meter * @throws IllegalArgumentException if a different metric by the same name exists in this meter. * @throws IllegalArgumentException if the {@code name} does not match spec requirements. */ - nostd::shared_ptr> NewShortCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewFloatCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewDoubleCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; + nostd::shared_ptr> NewShortCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -267,25 +263,21 @@ class Meter : public metrics_api::Meter * @param values a span of pairs where the first element of the pair is a metric instrument * to record to, and the second element is the value to update that instrument with. */ - void RecordShortBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordIntBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordFloatBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordDoubleBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; + void RecordShortBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordIntBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordFloatBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordDoubleBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; /** * An SDK-only function that checkpoints the aggregators of all instruments created from @@ -296,7 +288,6 @@ class Meter : public metrics_api::Meter std::vector Collect() noexcept; private: - /** * A private function that creates records from all synchronous instruments created from * this meter. @@ -343,12 +334,16 @@ class Meter : public metrics_api::Meter std::map>> short_metrics_; std::map>> int_metrics_; std::map>> float_metrics_; - std::map>> double_metrics_; + std::map>> + double_metrics_; - std::map>> short_observers_; + std::map>> + short_observers_; std::map>> int_observers_; - std::map>> float_observers_; - std::map>> double_observers_; + std::map>> + float_observers_; + std::map>> + double_observers_; std::string library_name_; std::string library_version_; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 45fbb4823d..bc919fb9da 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -22,18 +22,17 @@ nostd::shared_ptr> Meter::NewShortCounter( return nostd::shared_ptr>(ptr); } -nostd::shared_ptr> Meter::NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) +nostd::shared_ptr> Meter::NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) { if (!IsValidName(name) || NameAlreadyUsed(name)) { throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = std::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); @@ -448,10 +447,9 @@ nostd::shared_ptr> Meter::NewDoubleValueObser return nostd::shared_ptr>(ptr); } -void Meter::RecordShortBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordShortBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -459,10 +457,9 @@ void Meter::RecordShortBatch( } } -void Meter::RecordIntBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordIntBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -470,10 +467,9 @@ void Meter::RecordIntBatch( } } -void Meter::RecordFloatBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordFloatBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -481,10 +477,9 @@ void Meter::RecordFloatBatch( } } -void Meter::RecordDoubleBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordDoubleBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -631,6 +626,6 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) else return false; } -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index a501a5eef8..072b02fd4b 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -27,7 +27,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -43,15 +43,19 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, + &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -70,8 +74,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -80,9 +84,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -101,11 +105,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -126,7 +130,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -134,8 +139,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -144,9 +149,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -163,13 +168,14 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -188,75 +194,71 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument* sinstr_arr[] = - {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span*> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span *> sinstrs{sinstr_arr}; + nostd::span svalues{svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* iinstr_arr[] = - {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span*> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span *> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* finstr_arr[] = - {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span*> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span *> finstrs{finstr_arr}; + nostd::span fvalues{fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument* dinstr_arr[] = - {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span*> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span *> dinstrs{dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } @@ -268,7 +270,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -276,10 +279,8 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver( + "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From d2981ca5978b6918607fcbf93a35fd15a103f5d7 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Thu, 30 Jul 2020 11:24:01 -0400 Subject: [PATCH 17/32] Add pragma once --- sdk/include/opentelemetry/sdk/metrics/meter.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 67d44a9ad0..4eaac09582 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -1,3 +1,5 @@ +#pragma once + #include "opentelemetry/metrics/meter.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/sdk/metrics/async_instruments.h" From 6a759e836f4e53dcfb4a0a17c80c77fa01104bd9 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Mon, 3 Aug 2020 10:38:11 -0400 Subject: [PATCH 18/32] Add more helper function signatures --- sdk/include/opentelemetry/sdk/metrics/meter.h | 120 +++++++++++------- 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 4eaac09582..c3a729b58f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -2,9 +2,9 @@ #include "opentelemetry/metrics/meter.h" #include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/sdk/metrics/async_instruments.h" -#include "opentelemetry/sdk/metrics/instrument.h" #include "opentelemetry/sdk/metrics/record.h" +#include "opentelemetry/sdk/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/async_instruments.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" #include @@ -21,7 +21,7 @@ class Meter : public metrics_api::Meter public: explicit Meter(std::string library_name, std::string library_version = "") { - library_name_ = library_name; + library_name_ = library_name; library_version_ = library_version; } @@ -37,25 +37,29 @@ class Meter : public metrics_api::Meter * @throws IllegalArgumentException if a different metric by the same name exists in this meter. * @throws IllegalArgumentException if the {@code name} does not match spec requirements. */ - nostd::shared_ptr> NewShortCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewIntCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewFloatCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewDoubleCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; + nostd::shared_ptr> NewShortCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -265,21 +269,25 @@ class Meter : public metrics_api::Meter * @param values a span of pairs where the first element of the pair is a metric instrument * to record to, and the second element is the value to update that instrument with. */ - void RecordShortBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept override; - - void RecordIntBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept override; - - void RecordFloatBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept override; - - void RecordDoubleBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept override; + void RecordShortBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept override; + + void RecordIntBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept override; + + void RecordFloatBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept override; + + void RecordDoubleBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept override; /** * An SDK-only function that checkpoints the aggregators of all instruments created from @@ -290,6 +298,7 @@ class Meter : public metrics_api::Meter std::vector Collect() noexcept; private: + /** * A private function that creates records from all synchronous instruments created from * this meter. @@ -298,6 +307,16 @@ class Meter : public metrics_api::Meter */ void CollectMetrics(std::vector &records); + /** + * Helper function to collect Records from a single synchronous instrument + * + * @tparam T The integral type of the instrument to collect from. + * @param i A map iterator pointing to the instrument to collect from + * @param records The vector to add the new records to. + */ + template + void CollectSingleSyncInstrument(typename std::map>>::iterator i, std::vector &records); + /** * A private function that creates records from all asynchronous instruments created from * this meter. @@ -306,6 +325,16 @@ class Meter : public metrics_api::Meter */ void CollectObservers(std::vector &records); + /** + * Helper function to collect Records from a single asynchronous instrument + * + * @tparam T The integral type of the instrument to collect from. + * @param i A map iterator pointing to the instrument to collect from + * @param records The vector to add the new records to. + */ + template + void CollectSingleAsyncInstrument(typename std::map>>::iterator i, std::vector &records); + /** * Utility function used by the meter that checks if a user-passed name abides by OpenTelemetry * naming rules. The rules are as follows: @@ -336,16 +365,12 @@ class Meter : public metrics_api::Meter std::map>> short_metrics_; std::map>> int_metrics_; std::map>> float_metrics_; - std::map>> - double_metrics_; + std::map>> double_metrics_; - std::map>> - short_observers_; + std::map>> short_observers_; std::map>> int_observers_; - std::map>> - float_observers_; - std::map>> - double_observers_; + std::map>> float_observers_; + std::map>> double_observers_; std::string library_name_; std::string library_version_; @@ -353,6 +378,7 @@ class Meter : public metrics_api::Meter std::mutex metrics_lock_; std::mutex observers_lock_; }; -} // namespace metrics -} // namespace sdk + +} +} OPENTELEMETRY_END_NAMESPACE From f2ec0f512285b5a3fd984f83336b98cd029e9151 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Mon, 3 Aug 2020 10:39:20 -0400 Subject: [PATCH 19/32] Add logic to delete instruments with no user refs --- sdk/src/metrics/meter.cc | 183 ++++++++++++++++++++++++--------------- 1 file changed, 115 insertions(+), 68 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index bc919fb9da..0be99f4ff0 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -22,17 +22,18 @@ nostd::shared_ptr> Meter::NewShortCounter( return nostd::shared_ptr>(ptr); } -nostd::shared_ptr> Meter::NewIntCounter(nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) +nostd::shared_ptr> Meter::NewIntCounter( + nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) { if (!IsValidName(name) || NameAlreadyUsed(name)) { throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = std::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); @@ -447,9 +448,10 @@ nostd::shared_ptr> Meter::NewDoubleValueObser return nostd::shared_ptr>(ptr); } -void Meter::RecordShortBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept +void Meter::RecordShortBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -457,9 +459,10 @@ void Meter::RecordShortBatch(const trace::KeyValueIterable &labels, } } -void Meter::RecordIntBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept +void Meter::RecordIntBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -467,9 +470,10 @@ void Meter::RecordIntBatch(const trace::KeyValueIterable &labels, } } -void Meter::RecordFloatBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept +void Meter::RecordFloatBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -477,9 +481,10 @@ void Meter::RecordFloatBatch(const trace::KeyValueIterable &labels, } } -void Meter::RecordDoubleBatch(const trace::KeyValueIterable &labels, - nostd::span *> instruments, - nostd::span values) noexcept +void Meter::RecordDoubleBatch( + const trace::KeyValueIterable &labels, + nostd::span*> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -499,95 +504,137 @@ std::vector Meter::Collect() noexcept void Meter::CollectMetrics(std::vector &records) { metrics_lock_.lock(); - for (const auto &pair : short_metrics_) + for (auto i = short_metrics_.begin(); i != short_metrics_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleSyncInstrument(i, records); + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - continue; + i = short_metrics_.erase(i); // Remove instrument that is no longer accessible + } + else + { + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : int_metrics_) + for (auto i = int_metrics_.begin(); i != int_metrics_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleSyncInstrument(i, records); + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + { + i = int_metrics_.erase(i); // Remove instrument that is no longer accessible + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : float_metrics_) + for (auto i = float_metrics_.begin(); i != float_metrics_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleSyncInstrument(i, records); + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - continue; + i = float_metrics_.erase(i); // Remove instrument that is no longer accessible + } + else + { + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : double_metrics_) + for (auto i = double_metrics_.begin(); i != double_metrics_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleSyncInstrument(i, records); + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + { + i = double_metrics_.erase(i); // Remove instrument that is no longer accessible + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } metrics_lock_.unlock(); } +template +void Meter::CollectSingleSyncInstrument(typename std::map>>::iterator i, std::vector &records) +{ + if (!i->second->IsEnabled()) + { + i++; + return; + } + auto cast_ptr = std::dynamic_pointer_cast>(i->second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); +} + void Meter::CollectObservers(std::vector &records) { observers_lock_.lock(); - for (const auto &pair : short_observers_) + for (auto i = short_observers_.begin(); i != short_observers_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleAsyncInstrument(i, records); + if (i->second.use_count() == 1) + { + i = short_observers_.erase(i); + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : int_observers_) + for (auto i = int_observers_.begin(); i != int_observers_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleAsyncInstrument(i, records); + if (i->second.use_count() == 1) + { + i = int_observers_.erase(i); + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : float_observers_) + for (auto i = float_observers_.begin(); i != float_observers_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleAsyncInstrument(i, records); + if (i->second.use_count() == 1) + { + i = float_observers_.erase(i); + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } - for (const auto &pair : double_observers_) + for (auto i = double_observers_.begin(); i != double_observers_.end();) { - if (!pair.second->IsEnabled()) + CollectSingleAsyncInstrument(i, records); + if (i->second.use_count() == 1) + { + i = double_observers_.erase(i); + } + else { - continue; + i++; } - auto cast_ptr = std::dynamic_pointer_cast>(pair.second); - std::vector new_records = cast_ptr->GetRecords(); - records.insert(records.begin(), new_records.begin(), new_records.end()); } observers_lock_.unlock(); } +template +void Meter::CollectSingleAsyncInstrument(typename std::map>>::iterator i, std::vector &records) +{ + if (!i->second->IsEnabled()) + { + i++; + return; + } + auto cast_ptr = std::dynamic_pointer_cast>(i->second); + std::vector new_records = cast_ptr->GetRecords(); + records.insert(records.begin(), new_records.begin(), new_records.end()); +} + bool Meter::IsValidName(nostd::string_view name) { if (name.empty() || isdigit(name[0]) || isspace(name[0]) || ispunct(name[0])) @@ -626,6 +673,6 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) else return false; } -} // namespace metrics -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +} +} +OPENTELEMETRY_END_NAMESPACE From 36e6e4725fd6d3055b96a107f1269c214f041e2b Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Mon, 3 Aug 2020 10:40:08 -0400 Subject: [PATCH 20/32] Add tests to ensure instruments with no refs are deleted --- sdk/test/metrics/meter_test.cc | 208 +++++++++++++++++++++++---------- 1 file changed, 148 insertions(+), 60 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 072b02fd4b..6e97190423 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,4 +1,5 @@ #include "opentelemetry/sdk/metrics/meter.h" +#include #include using namespace opentelemetry::sdk::metrics; @@ -27,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -// Dummy functions for asynchronous instrument constructors +//Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -43,19 +44,15 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, - &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, - &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -74,8 +71,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -84,9 +81,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -105,11 +102,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -130,8 +127,7 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -139,8 +135,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -149,9 +145,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -168,14 +164,13 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -194,75 +189,167 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument* sinstr_arr[] = + {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span *> sinstrs{sinstr_arr}; - nostd::span svalues{svalues_arr}; + nostd::span*> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument* iinstr_arr[] = + {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span *> iinstrs{iinstr_arr}; - nostd::span ivalues{ivalues_arr}; + nostd::span*> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* finstr_arr[] = + {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span *> finstrs{finstr_arr}; - nostd::span fvalues{fvalues_arr}; + nostd::span*> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* dinstr_arr[] = + {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span *> dinstrs{dinstr_arr}; - nostd::span dvalues{dvalues_arr}; + nostd::span*> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } +TEST(Meter, DeleteOneSyncInstrument) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto c = m->NewShortCounter("c", "", "", true); + c->add(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); +} + +TEST(Meter, DeleteManySyncInstruments) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto c = m->NewShortCounter("c", "", "", true); + c->add(1, labelkv); + { + auto d = m->NewIntCounter("d", "", "", true); + d->add(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 2); + ASSERT_EQ(m->Collect().size(), 1); + { + auto e = m->NewFloatCounter("e", "", "", true); + e->add(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 2); + ASSERT_EQ(m->Collect().size(), 1); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); + { + auto f = m->NewDoubleCounter("f", "", "", true); + f->add(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); +} + +TEST(Meter, DeleteOneAsyncInstrument) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto c = m->NewShortSumObserver("c", "", "", true, &ShortCallback); + c->observe(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); +} + +TEST(Meter, DeleteManyAsyncInstruments) +{ + auto m = new Meter("Test"); + std::map labels = {{"Key", "Value"}}; + auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; + { + auto c = m->NewShortSumObserver("c", "", "", true, &ShortCallback); + c->observe(1, labelkv); + { + auto d = m->NewIntSumObserver("d", "", "", true, &IntCallback); + d->observe(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 2); + ASSERT_EQ(m->Collect().size(), 1); + { + auto e = m->NewFloatSumObserver("e", "", "", true, &FloatCallback); + e->observe(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 2); + ASSERT_EQ(m->Collect().size(), 1); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); + { + auto f = m->NewDoubleSumObserver("f", "", "", true, &DoubleCallback); + f->observe(1, labelkv); + } + ASSERT_EQ(m->Collect().size(), 1); + ASSERT_EQ(m->Collect().size(), 0); +} + TEST(MeterStringUtil, IsValid) { auto m = new Meter("Test"); @@ -270,8 +357,7 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter( - "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -279,8 +365,10 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver( - "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From 73aaf38fcd8a94d3e35a4739831abc74504d2d89 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:09:48 -0400 Subject: [PATCH 21/32] Format --- sdk/include/opentelemetry/sdk/metrics/meter.h | 113 +++++++-------- sdk/src/metrics/CMakeLists.txt | 3 +- sdk/src/metrics/meter.cc | 77 +++++------ sdk/test/metrics/meter_test.cc | 129 +++++++++--------- 4 files changed, 163 insertions(+), 159 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index b20114c356..36d868fce8 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -2,9 +2,9 @@ #include "opentelemetry/metrics/meter.h" #include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/sdk/metrics/record.h" -#include "opentelemetry/sdk/metrics/instrument.h" #include "opentelemetry/sdk/metrics/async_instruments.h" +#include "opentelemetry/sdk/metrics/instrument.h" +#include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" #include @@ -21,7 +21,7 @@ class Meter : public metrics_api::Meter public: explicit Meter(std::string library_name, std::string library_version = "") { - library_name_ = library_name; + library_name_ = library_name; library_version_ = library_version; } @@ -37,29 +37,25 @@ class Meter : public metrics_api::Meter * @throws IllegalArgumentException if a different metric by the same name exists in this meter. * @throws IllegalArgumentException if the {@code name} does not match spec requirements. */ - nostd::shared_ptr> NewShortCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewFloatCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; - - nostd::shared_ptr> NewDoubleCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) override; + nostd::shared_ptr> NewShortCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewFloatCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; + + nostd::shared_ptr> NewDoubleCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) override; /** * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that @@ -269,25 +265,21 @@ class Meter : public metrics_api::Meter * @param values a span of pairs where the first element of the pair is a metric instrument * to record to, and the second element is the value to update that instrument with. */ - void RecordShortBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordIntBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordFloatBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; - - void RecordDoubleBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept override; + void RecordShortBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordIntBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordFloatBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; + + void RecordDoubleBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept override; /** * An SDK-only function that checkpoints the aggregators of all instruments created from @@ -298,7 +290,6 @@ class Meter : public metrics_api::Meter std::vector Collect() noexcept; private: - /** * A private function that creates records from all synchronous instruments created from * this meter. @@ -314,8 +305,11 @@ class Meter : public metrics_api::Meter * @param i A map iterator pointing to the instrument to collect from * @param records The vector to add the new records to. */ - template - void CollectSingleSyncInstrument(typename std::map>>::iterator i, std::vector &records); + template + void CollectSingleSyncInstrument( + typename std::map>>::iterator i, + std::vector &records); /** * A private function that creates records from all asynchronous instruments created from @@ -332,8 +326,11 @@ class Meter : public metrics_api::Meter * @param i A map iterator pointing to the instrument to collect from * @param records The vector to add the new records to. */ - template - void CollectSingleAsyncInstrument(typename std::map>>::iterator i, std::vector &records); + template + void CollectSingleAsyncInstrument( + typename std::map>>::iterator i, + std::vector &records); /** * Utility function used by the meter that checks if a user-passed name abides by OpenTelemetry @@ -365,12 +362,16 @@ class Meter : public metrics_api::Meter std::map>> short_metrics_; std::map>> int_metrics_; std::map>> float_metrics_; - std::map>> double_metrics_; + std::map>> + double_metrics_; - std::map>> short_observers_; + std::map>> + short_observers_; std::map>> int_observers_; - std::map>> float_observers_; - std::map>> double_observers_; + std::map>> + float_observers_; + std::map>> + double_observers_; std::string library_name_; std::string library_version_; @@ -378,6 +379,6 @@ class Meter : public metrics_api::Meter std::mutex metrics_lock_; std::mutex observers_lock_; }; -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index 3993964a5d..c147a3e3c2 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -1 +1,2 @@ -add_library(opentelemetry_metrics meter_provider.cc meter.cc ungrouped_processor.cc) +add_library(opentelemetry_metrics meter_provider.cc meter.cc + ungrouped_processor.cc) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 0be99f4ff0..3f0c930354 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -22,18 +22,17 @@ nostd::shared_ptr> Meter::NewShortCounter( return nostd::shared_ptr>(ptr); } -nostd::shared_ptr> Meter::NewIntCounter( - nostd::string_view name, - nostd::string_view description, - nostd::string_view unit, - const bool enabled) +nostd::shared_ptr> Meter::NewIntCounter(nostd::string_view name, + nostd::string_view description, + nostd::string_view unit, + const bool enabled) { if (!IsValidName(name) || NameAlreadyUsed(name)) { throw std::invalid_argument("Invalid Name"); } auto counter = new Counter(name, description, unit, enabled); - auto ptr = std::shared_ptr>(counter); + auto ptr = std::shared_ptr>(counter); metrics_lock_.lock(); int_metrics_.insert(std::make_pair(std::string(name), ptr)); metrics_lock_.unlock(); @@ -448,10 +447,9 @@ nostd::shared_ptr> Meter::NewDoubleValueObser return nostd::shared_ptr>(ptr); } -void Meter::RecordShortBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordShortBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -459,10 +457,9 @@ void Meter::RecordShortBatch( } } -void Meter::RecordIntBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordIntBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -470,10 +467,9 @@ void Meter::RecordIntBatch( } } -void Meter::RecordFloatBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordFloatBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -481,10 +477,9 @@ void Meter::RecordFloatBatch( } } -void Meter::RecordDoubleBatch( - const trace::KeyValueIterable &labels, - nostd::span*> instruments, - nostd::span values) noexcept +void Meter::RecordDoubleBatch(const trace::KeyValueIterable &labels, + nostd::span *> instruments, + nostd::span values) noexcept { for (int i = 0; i < instruments.size(); ++i) { @@ -507,9 +502,9 @@ void Meter::CollectMetrics(std::vector &records) for (auto i = short_metrics_.begin(); i != short_metrics_.end();) { CollectSingleSyncInstrument(i, records); - if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - i = short_metrics_.erase(i); // Remove instrument that is no longer accessible + i = short_metrics_.erase(i); // Remove instrument that is no longer accessible } else { @@ -519,9 +514,9 @@ void Meter::CollectMetrics(std::vector &records) for (auto i = int_metrics_.begin(); i != int_metrics_.end();) { CollectSingleSyncInstrument(i, records); - if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - i = int_metrics_.erase(i); // Remove instrument that is no longer accessible + i = int_metrics_.erase(i); // Remove instrument that is no longer accessible } else { @@ -531,9 +526,9 @@ void Meter::CollectMetrics(std::vector &records) for (auto i = float_metrics_.begin(); i != float_metrics_.end();) { CollectSingleSyncInstrument(i, records); - if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - i = float_metrics_.erase(i); // Remove instrument that is no longer accessible + i = float_metrics_.erase(i); // Remove instrument that is no longer accessible } else { @@ -543,9 +538,9 @@ void Meter::CollectMetrics(std::vector &records) for (auto i = double_metrics_.begin(); i != double_metrics_.end();) { CollectSingleSyncInstrument(i, records); - if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted + if (i->second.use_count() == 1) // Evaluates to true if user's shared_ptr has been deleted { - i = double_metrics_.erase(i); // Remove instrument that is no longer accessible + i = double_metrics_.erase(i); // Remove instrument that is no longer accessible } else { @@ -555,15 +550,18 @@ void Meter::CollectMetrics(std::vector &records) metrics_lock_.unlock(); } -template -void Meter::CollectSingleSyncInstrument(typename std::map>>::iterator i, std::vector &records) +template +void Meter::CollectSingleSyncInstrument( + typename std::map>>::iterator + i, + std::vector &records) { if (!i->second->IsEnabled()) { i++; return; } - auto cast_ptr = std::dynamic_pointer_cast>(i->second); + auto cast_ptr = std::dynamic_pointer_cast>(i->second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -622,15 +620,18 @@ void Meter::CollectObservers(std::vector &records) observers_lock_.unlock(); } -template -void Meter::CollectSingleAsyncInstrument(typename std::map>>::iterator i, std::vector &records) +template +void Meter::CollectSingleAsyncInstrument( + typename std::map>>::iterator i, + std::vector &records) { if (!i->second->IsEnabled()) { i++; return; } - auto cast_ptr = std::dynamic_pointer_cast>(i->second); + auto cast_ptr = std::dynamic_pointer_cast>(i->second); std::vector new_records = cast_ptr->GetRecords(); records.insert(records.begin(), new_records.begin(), new_records.end()); } @@ -673,6 +674,6 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) else return false; } -} -} +} // namespace metrics +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 6e97190423..f8180a9cbf 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,15 +44,19 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, + &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -71,8 +75,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -81,9 +85,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -102,11 +106,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -127,7 +131,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -135,8 +140,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -145,9 +150,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -164,13 +169,14 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -189,82 +195,78 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument* sinstr_arr[] = - {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span*> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span *> sinstrs{sinstr_arr}; + nostd::span svalues{svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* iinstr_arr[] = - {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span*> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span *> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* finstr_arr[] = - {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span*> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span *> finstrs{finstr_arr}; + nostd::span fvalues{fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument* dinstr_arr[] = - {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span*> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span *> dinstrs{dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DeleteOneSyncInstrument) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { @@ -277,7 +279,7 @@ TEST(Meter, DeleteOneSyncInstrument) TEST(Meter, DeleteManySyncInstruments) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { @@ -308,7 +310,7 @@ TEST(Meter, DeleteManySyncInstruments) TEST(Meter, DeleteOneAsyncInstrument) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { @@ -321,7 +323,7 @@ TEST(Meter, DeleteOneAsyncInstrument) TEST(Meter, DeleteManyAsyncInstruments) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { @@ -357,7 +359,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -365,10 +368,8 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver( + "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From fe2c65459d2202c6942ee29097f5235983b9ab1c Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:25:38 -0400 Subject: [PATCH 22/32] Add exception checks --- sdk/src/metrics/meter.cc | 96 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 3f0c930354..60c98c5163 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -12,7 +12,11 @@ nostd::shared_ptr> Meter::NewShortCounter( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto counter = new Counter(name, description, unit, enabled); auto ptr = std::shared_ptr>(counter); @@ -29,7 +33,11 @@ nostd::shared_ptr> Meter::NewIntCounter(nostd::string_ { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto counter = new Counter(name, description, unit, enabled); auto ptr = std::shared_ptr>(counter); @@ -47,7 +55,11 @@ nostd::shared_ptr> Meter::NewFloatCounter( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto counter = new Counter(name, description, unit, enabled); auto ptr = std::shared_ptr>(counter); @@ -65,7 +77,11 @@ nostd::shared_ptr> Meter::NewDoubleCounter( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto counter = new Counter(name, description, unit, enabled); auto ptr = std::shared_ptr>(counter); @@ -83,7 +99,11 @@ nostd::shared_ptr> Meter::NewShortUpDownCounte { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto udcounter = new UpDownCounter(name, description, unit, enabled); auto ptr = std::shared_ptr>(udcounter); @@ -101,7 +121,11 @@ nostd::shared_ptr> Meter::NewIntUpDownCounter( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto udcounter = new UpDownCounter(name, description, unit, enabled); auto ptr = std::shared_ptr>(udcounter); @@ -119,7 +143,11 @@ nostd::shared_ptr> Meter::NewFloatUpDownCounte { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto udcounter = new UpDownCounter(name, description, unit, enabled); auto ptr = std::shared_ptr>(udcounter); @@ -137,7 +165,11 @@ nostd::shared_ptr> Meter::NewDoubleUpDownCoun { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto udcounter = new UpDownCounter(name, description, unit, enabled); auto ptr = std::shared_ptr>(udcounter); @@ -155,7 +187,11 @@ nostd::shared_ptr> Meter::NewShortValueRecorde { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto recorder = new ValueRecorder(name, description, unit, enabled); auto ptr = std::shared_ptr>(recorder); @@ -173,7 +209,11 @@ nostd::shared_ptr> Meter::NewIntValueRecorder( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto recorder = new ValueRecorder(name, description, unit, enabled); auto ptr = std::shared_ptr>(recorder); @@ -191,7 +231,11 @@ nostd::shared_ptr> Meter::NewFloatValueRecorde { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto recorder = new ValueRecorder(name, description, unit, enabled); auto ptr = std::shared_ptr>(recorder); @@ -209,7 +253,11 @@ nostd::shared_ptr> Meter::NewDoubleValueRecor { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto recorder = new ValueRecorder(name, description, unit, enabled); auto ptr = std::shared_ptr>(recorder); @@ -228,7 +276,11 @@ nostd::shared_ptr> Meter::NewShortSumObserver( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new SumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -247,7 +299,11 @@ nostd::shared_ptr> Meter::NewIntSumObserver( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new SumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -266,7 +322,11 @@ nostd::shared_ptr> Meter::NewFloatSumObserver( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new SumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -285,7 +345,11 @@ nostd::shared_ptr> Meter::NewDoubleSumObserver( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new SumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -304,7 +368,11 @@ nostd::shared_ptr> Meter::NewShortUpDownSu { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -323,7 +391,11 @@ nostd::shared_ptr> Meter::NewIntUpDownSumObs { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -342,7 +414,11 @@ nostd::shared_ptr> Meter::NewFloatUpDownSu { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -361,7 +437,11 @@ nostd::shared_ptr> Meter::NewDoubleUpDown { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new UpDownSumObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -380,7 +460,11 @@ nostd::shared_ptr> Meter::NewShortValueObserve { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -399,7 +483,11 @@ nostd::shared_ptr> Meter::NewIntValueObserver( { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -418,7 +506,11 @@ nostd::shared_ptr> Meter::NewFloatValueObserve { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); @@ -437,7 +529,11 @@ nostd::shared_ptr> Meter::NewDoubleValueObser { if (!IsValidName(name) || NameAlreadyUsed(name)) { +#if __EXCEPTIONS throw std::invalid_argument("Invalid Name"); +#else + std::terminate(); +#endif } auto sumobs = new ValueObserver(name, description, unit, enabled, callback); auto ptr = std::shared_ptr>(sumobs); From 6d522efadad85d6c6f8fff694a4797d03f191a8b Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:36:48 -0400 Subject: [PATCH 23/32] Delete Delete Tests These tests are no longer necessary as additional functionality that covers these cases has been added to the Synchronous and Asynchronous SDK instruments merged in [PR #191](https://github.com/open-telemetry/opentelemetry-cpp/pull/191). --- sdk/test/metrics/meter_test.cc | 209 ++++++++++----------------------- 1 file changed, 60 insertions(+), 149 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index f8180a9cbf..9b7af69cce 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -// Dummy functions for asynchronous instrument constructors +//Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,19 +44,15 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, - &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, - &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -75,8 +71,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -85,9 +81,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -106,11 +102,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -131,8 +127,7 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -140,8 +135,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -150,9 +145,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -169,14 +164,13 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -195,163 +189,79 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument* sinstr_arr[] = + {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span *> sinstrs{sinstr_arr}; - nostd::span svalues{svalues_arr}; + nostd::span*> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument* iinstr_arr[] = + {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span *> iinstrs{iinstr_arr}; - nostd::span ivalues{ivalues_arr}; + nostd::span*> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* finstr_arr[] = + {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span *> finstrs{finstr_arr}; - nostd::span fvalues{fvalues_arr}; + nostd::span*> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* dinstr_arr[] = + {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span *> dinstrs{dinstr_arr}; - nostd::span dvalues{dvalues_arr}; + nostd::span*> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } -TEST(Meter, DeleteOneSyncInstrument) -{ - auto m = new Meter("Test"); - std::map labels = {{"Key", "Value"}}; - auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - { - auto c = m->NewShortCounter("c", "", "", true); - c->add(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); -} - -TEST(Meter, DeleteManySyncInstruments) -{ - auto m = new Meter("Test"); - std::map labels = {{"Key", "Value"}}; - auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - { - auto c = m->NewShortCounter("c", "", "", true); - c->add(1, labelkv); - { - auto d = m->NewIntCounter("d", "", "", true); - d->add(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 2); - ASSERT_EQ(m->Collect().size(), 1); - { - auto e = m->NewFloatCounter("e", "", "", true); - e->add(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 2); - ASSERT_EQ(m->Collect().size(), 1); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); - { - auto f = m->NewDoubleCounter("f", "", "", true); - f->add(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); -} - -TEST(Meter, DeleteOneAsyncInstrument) -{ - auto m = new Meter("Test"); - std::map labels = {{"Key", "Value"}}; - auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - { - auto c = m->NewShortSumObserver("c", "", "", true, &ShortCallback); - c->observe(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); -} - -TEST(Meter, DeleteManyAsyncInstruments) -{ - auto m = new Meter("Test"); - std::map labels = {{"Key", "Value"}}; - auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - { - auto c = m->NewShortSumObserver("c", "", "", true, &ShortCallback); - c->observe(1, labelkv); - { - auto d = m->NewIntSumObserver("d", "", "", true, &IntCallback); - d->observe(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 2); - ASSERT_EQ(m->Collect().size(), 1); - { - auto e = m->NewFloatSumObserver("e", "", "", true, &FloatCallback); - e->observe(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 2); - ASSERT_EQ(m->Collect().size(), 1); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); - { - auto f = m->NewDoubleSumObserver("f", "", "", true, &DoubleCallback); - f->observe(1, labelkv); - } - ASSERT_EQ(m->Collect().size(), 1); - ASSERT_EQ(m->Collect().size(), 0); -} - TEST(MeterStringUtil, IsValid) { auto m = new Meter("Test"); @@ -359,8 +269,7 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter( - "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -368,8 +277,10 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver( - "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From 2f04b75f49848d4398bdaa6fc10c3daf3f140d1a Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:39:43 -0400 Subject: [PATCH 24/32] Format --- sdk/test/metrics/meter_test.cc | 121 +++++++++++++++++---------------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 9b7af69cce..c4a6f14560 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,15 +44,19 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, + &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -71,8 +75,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -81,9 +85,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -102,11 +106,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -127,7 +131,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -135,8 +140,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -145,9 +150,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -164,13 +169,14 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -189,75 +195,71 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument* sinstr_arr[] = - {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span*> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span *> sinstrs{sinstr_arr}; + nostd::span svalues{svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* iinstr_arr[] = - {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span*> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span *> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* finstr_arr[] = - {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span*> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span *> finstrs{finstr_arr}; + nostd::span fvalues{fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument* dinstr_arr[] = - {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span*> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span *> dinstrs{dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } @@ -269,7 +271,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); } TEST(MeterStringUtil, AlreadyExists) @@ -277,10 +280,8 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver( + "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); } OPENTELEMETRY_END_NAMESPACE From b4cbb8baa5ae41259685d993499dc4dc73e4a889 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:50:31 -0400 Subject: [PATCH 25/32] Add exception checks for CI --- sdk/test/metrics/meter_test.cc | 125 +++++++++++++++++---------------- 1 file changed, 64 insertions(+), 61 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index c4a6f14560..ae636bf4f1 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -// Dummy functions for asynchronous instrument constructors +//Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,19 +44,15 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, - &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, - &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, - &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -75,8 +71,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -85,9 +81,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -106,11 +102,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -131,8 +127,7 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -140,8 +135,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -150,9 +145,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -169,14 +164,13 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -195,93 +189,102 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument* sinstr_arr[] = + {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span *> sinstrs{sinstr_arr}; - nostd::span svalues{svalues_arr}; + nostd::span*> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument* iinstr_arr[] = + {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span *> iinstrs{iinstr_arr}; - nostd::span ivalues{ivalues_arr}; + nostd::span*> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* finstr_arr[] = + {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span *> finstrs{finstr_arr}; - nostd::span fvalues{fvalues_arr}; + nostd::span*> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* dinstr_arr[] = + {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span *> dinstrs{dinstr_arr}; - nostd::span dvalues{dvalues_arr}; + nostd::span*> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(MeterStringUtil, IsValid) { +#if __EXCEPTIONS auto m = new Meter("Test"); ASSERT_ANY_THROW(m->NewShortCounter("", "Empty name is invalid", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter( - "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); +#endif } TEST(MeterStringUtil, AlreadyExists) { +#if __EXCEPTIONS auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver( - "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); +#endif } OPENTELEMETRY_END_NAMESPACE From d140f83143ebb5878c311a12c41bed55491ddebf Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 15:53:27 -0400 Subject: [PATCH 26/32] Format --- sdk/test/metrics/meter_test.cc | 121 +++++++++++++++++---------------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index ae636bf4f1..3345d4b9c2 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,15 +44,19 @@ TEST(Meter, CreateAsyncInstruments) m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); + m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, + &DoubleCallback); } TEST(Meter, CollectSyncInstruments) @@ -71,8 +75,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -81,9 +85,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -102,11 +106,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -127,7 +131,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m->Collect().size(), 0); - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -135,8 +140,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -145,9 +150,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m->Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -164,13 +169,14 @@ TEST(Meter, CollectDeletedAsync) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = + m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -189,75 +195,71 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument* sinstr_arr[] = - {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span*> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span *> sinstrs{sinstr_arr}; + nostd::span svalues{svalues_arr}; m->RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* iinstr_arr[] = - {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span*> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span *> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + res = m->Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* finstr_arr[] = - {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span*> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span *> finstrs{finstr_arr}; + nostd::span fvalues{fvalues_arr}; m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + res = m->Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument* dinstr_arr[] = - {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span*> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span *> dinstrs{dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + res = m->Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m->NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + auto m = new Meter("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m->Collect().size(), 0); } @@ -270,7 +272,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m->NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); #endif } @@ -280,11 +283,9 @@ TEST(MeterStringUtil, AlreadyExists) auto m = new Meter("Test"); m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m->NewShortSumObserver( + "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); #endif } OPENTELEMETRY_END_NAMESPACE From ca5bc47e1b69e50f1d270b0eeb6fbd3375464ac5 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 16:03:43 -0400 Subject: [PATCH 27/32] Allocate meters on the stack --- sdk/test/metrics/meter_test.cc | 245 ++++++++++++++++----------------- 1 file changed, 122 insertions(+), 123 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 3345d4b9c2..f3c962f59d 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -10,25 +10,25 @@ OPENTELEMETRY_BEGIN_NAMESPACE TEST(Meter, CreateSyncInstruments) { // Test that there are no errors creating synchronous instruments. - auto m = new Meter("Test"); - - m->NewShortCounter("Test-short-counter", "For testing", "Unitless", true); - m->NewIntCounter("Test-int-counter", "For testing", "Unitless", true); - m->NewFloatCounter("Test-float-counter", "For testing", "Unitless", true); - m->NewDoubleCounter("Test-double-counter", "For testing", "Unitless", true); - - m->NewShortUpDownCounter("Test-short-ud-counter", "For testing", "Unitless", true); - m->NewIntUpDownCounter("Test-int-ud-counter", "For testing", "Unitless", true); - m->NewFloatUpDownCounter("Test-float-ud-counter", "For testing", "Unitless", true); - m->NewDoubleUpDownCounter("Test-double-ud-counter", "For testing", "Unitless", true); - - m->NewShortValueRecorder("Test-short-recorder", "For testing", "Unitless", true); - m->NewIntValueRecorder("Test-int-recorder", "For testing", "Unitless", true); - m->NewFloatValueRecorder("Test-float-recorder", "For testing", "Unitless", true); - m->NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); + Meter m("Test"); + + m.NewShortCounter("Test-short-counter", "For testing", "Unitless", true); + m.NewIntCounter("Test-int-counter", "For testing", "Unitless", true); + m.NewFloatCounter("Test-float-counter", "For testing", "Unitless", true); + m.NewDoubleCounter("Test-double-counter", "For testing", "Unitless", true); + + m.NewShortUpDownCounter("Test-short-ud-counter", "For testing", "Unitless", true); + m.NewIntUpDownCounter("Test-int-ud-counter", "For testing", "Unitless", true); + m.NewFloatUpDownCounter("Test-float-ud-counter", "For testing", "Unitless", true); + m.NewDoubleUpDownCounter("Test-double-ud-counter", "For testing", "Unitless", true); + + m.NewShortValueRecorder("Test-short-recorder", "For testing", "Unitless", true); + m.NewIntValueRecorder("Test-int-recorder", "For testing", "Unitless", true); + m.NewFloatValueRecorder("Test-float-recorder", "For testing", "Unitless", true); + m.NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -// Dummy functions for asynchronous instrument constructors +//Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -37,46 +37,42 @@ void DoubleCallback(metrics_api::ObserverResult) {} TEST(Meter, CreateAsyncInstruments) { // Test that there are no errors when creating asynchronous instruments. - auto m = new Meter("Test"); - - m->NewShortSumObserver("Test-short-sum-obs", "For testing", "Unitless", true, &ShortCallback); - m->NewIntSumObserver("Test-int-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - - m->NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, - &ShortCallback); - m->NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, - &FloatCallback); - m->NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, - &DoubleCallback); - - m->NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); - m->NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); - m->NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); - m->NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, - &DoubleCallback); + Meter m("Test"); + + m.NewShortSumObserver("Test-short-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m.NewIntSumObserver("Test-int-sum-obs", "For testing", "Unitless", true, &IntCallback); + m.NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m.NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + + m.NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m.NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); + m.NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); + m.NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + + m.NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); + m.NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); + m.NewFloatValueObserver("Test-float-val-obs", "For testing", "Unitless", true, &FloatCallback); + m.NewDoubleValueObserver("Test-double-val-obs", "For testing", "Unitless", true, &DoubleCallback); } TEST(Meter, CollectSyncInstruments) { // Verify that the records returned on a call to Collect() are correct for synchronous // instruments. - auto m = new Meter("Test"); + Meter m("Test"); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); - auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); + auto counter = m.NewShortCounter("Test-counter", "For testing", "Unitless", true); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; counter->add(1, labelkv); - std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + std::vector res = m.Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -85,9 +81,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m->Collect(); + res = m.Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -97,20 +93,20 @@ TEST(Meter, CollectDeletedSync) // Verify that calling Collect() after creating a synchronous instrument and destroying // the return pointer does not result in a segfault. - auto m = new Meter("Test"); + Meter m("Test"); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto counter = m->NewShortCounter("Test-counter", "For testing", "Unitless", true); + auto counter = m.NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here - std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + std::vector res = m.Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -127,21 +123,20 @@ TEST(Meter, CollectAsyncInstruments) { // Verify that the records returned on a call to Collect() are correct for asynchronous // instruments. - auto m = new Meter("Test"); + Meter m("Test"); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = m.NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; sumobs->observe(1, labelkv); - std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + std::vector res = m.Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -150,9 +145,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m->Collect(); + res = m.Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -162,21 +157,20 @@ TEST(Meter, CollectDeletedAsync) // Verify that calling Collect() after creating an asynchronous instrument and destroying // the return pointer does not result in a segfault. - auto m = new Meter("Test"); + Meter m("Test"); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; { - auto sumobs = - m->NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); + auto sumobs = m.NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here - std::vector res = m->Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + std::vector res = m.Collect(); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -185,107 +179,112 @@ TEST(Meter, RecordBatch) { // This tests that RecordBatch appropriately updates the aggregators of the instruments // passed to the function. Short, int, float, and double data types are tested. - auto m = new Meter("Test"); + Meter m("Test"); - auto scounter = m->NewShortCounter("Test-scounter", "For testing", "Unitless", true); - auto icounter = m->NewIntCounter("Test-icounter", "For testing", "Unitless", true); - auto fcounter = m->NewFloatCounter("Test-fcounter", "For testing", "Unitless", true); - auto dcounter = m->NewDoubleCounter("Test-dcounter", "For testing", "Unitless", true); + auto scounter = m.NewShortCounter("Test-scounter", "For testing", "Unitless", true); + auto icounter = m.NewIntCounter("Test-icounter", "For testing", "Unitless", true); + auto fcounter = m.NewFloatCounter("Test-fcounter", "For testing", "Unitless", true); + auto dcounter = m.NewDoubleCounter("Test-dcounter", "For testing", "Unitless", true); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument* sinstr_arr[] = + {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span *> sinstrs{sinstr_arr}; - nostd::span svalues{svalues_arr}; + nostd::span*> sinstrs {sinstr_arr}; + nostd::span svalues {svalues_arr}; - m->RecordShortBatch(labelkv, sinstrs, svalues); - std::vector res = m->Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + m.RecordShortBatch(labelkv, sinstrs, svalues); + std::vector res = m.Collect(); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument* iinstr_arr[] = + {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span *> iinstrs{iinstr_arr}; - nostd::span ivalues{ivalues_arr}; + nostd::span*> iinstrs {iinstr_arr}; + nostd::span ivalues {ivalues_arr}; - m->RecordIntBatch(labelkv, iinstrs, ivalues); - res = m->Collect(); + m.RecordIntBatch(labelkv, iinstrs, ivalues); + res = m.Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* finstr_arr[] = + {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span *> finstrs{finstr_arr}; - nostd::span fvalues{fvalues_arr}; + nostd::span*> finstrs {finstr_arr}; + nostd::span fvalues {fvalues_arr}; - m->RecordFloatBatch(labelkv, finstrs, fvalues); - res = m->Collect(); + m.RecordFloatBatch(labelkv, finstrs, fvalues); + res = m.Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument* dinstr_arr[] = + {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span *> dinstrs{dinstr_arr}; - nostd::span dvalues{dvalues_arr}; + nostd::span*> dinstrs {dinstr_arr}; + nostd::span dvalues {dvalues_arr}; - m->RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m->Collect(); + m.RecordDoubleBatch(labelkv, dinstrs, dvalues); + res = m.Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } TEST(Meter, DisableCollectSync) { - auto m = new Meter("Test"); + Meter m("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortCounter("c", "", "", false); + auto c = m.NewShortCounter("c", "", "", false); c->add(1, labelkv); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); } TEST(Meter, DisableCollectAsync) { - auto m = new Meter("Test"); + Meter m("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m->NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m.NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); - ASSERT_EQ(m->Collect().size(), 0); + ASSERT_EQ(m.Collect().size(), 0); } TEST(MeterStringUtil, IsValid) { #if __EXCEPTIONS - auto m = new Meter("Test"); - ASSERT_ANY_THROW(m->NewShortCounter("", "Empty name is invalid", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter("1a", "Can't begin with a number", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter(".a", "Can't begin with punctuation", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m->NewShortCounter( - "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + Meter m("Test"); + ASSERT_ANY_THROW(m.NewShortCounter("", "Empty name is invalid", " ", true)); + ASSERT_ANY_THROW(m.NewShortCounter("1a", "Can't begin with a number", " ", true)); + ASSERT_ANY_THROW(m.NewShortCounter(".a", "Can't begin with punctuation", " ", true)); + ASSERT_ANY_THROW(m.NewShortCounter(" a", "Can't begin with space", " ", true)); + ASSERT_ANY_THROW(m.NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); #endif } TEST(MeterStringUtil, AlreadyExists) { #if __EXCEPTIONS - auto m = new Meter("Test"); - - m->NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m->NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); - ASSERT_ANY_THROW(m->NewShortSumObserver( - "a", "Still illegal even though it is not a short counter", "", true, &ShortCallback)); + Meter m("Test"); + + m.NewShortCounter("a", "First instance of instrument named 'a'", "", true); + ASSERT_ANY_THROW(m.NewShortCounter("a", "Second (illegal) instrument named 'a'", + "", true)); + ASSERT_ANY_THROW(m.NewShortSumObserver("a", + "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); #endif } OPENTELEMETRY_END_NAMESPACE From 82f0b8a7a04612a92565a6c8353dd8a0658991f5 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 16:05:14 -0400 Subject: [PATCH 28/32] Format --- sdk/test/metrics/meter_test.cc | 111 ++++++++++++++++----------------- 1 file changed, 55 insertions(+), 56 deletions(-) diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index f3c962f59d..b3cdc0ce39 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -1,6 +1,6 @@ #include "opentelemetry/sdk/metrics/meter.h" -#include #include +#include using namespace opentelemetry::sdk::metrics; namespace metrics_api = opentelemetry::metrics; @@ -28,7 +28,7 @@ TEST(Meter, CreateSyncInstruments) m.NewDoubleValueRecorder("Test-double-recorder", "For testing", "Unitless", true); } -//Dummy functions for asynchronous instrument constructors +// Dummy functions for asynchronous instrument constructors void ShortCallback(metrics_api::ObserverResult) {} void IntCallback(metrics_api::ObserverResult) {} void FloatCallback(metrics_api::ObserverResult) {} @@ -44,10 +44,13 @@ TEST(Meter, CreateAsyncInstruments) m.NewFloatSumObserver("Test-float-sum-obs", "For testing", "Unitless", true, &FloatCallback); m.NewDoubleSumObserver("Test-double-sum-obs", "For testing", "Unitless", true, &DoubleCallback); - m.NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, &ShortCallback); + m.NewShortUpDownSumObserver("Test-short-ud-sum-obs", "For testing", "Unitless", true, + &ShortCallback); m.NewIntUpDownSumObserver("Test-int-ud-sum-obs", "For testing", "Unitless", true, &IntCallback); - m.NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, &FloatCallback); - m.NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, &DoubleCallback); + m.NewFloatUpDownSumObserver("Test-float-ud-sum-obs", "For testing", "Unitless", true, + &FloatCallback); + m.NewDoubleUpDownSumObserver("Test-double-ud-sum-obs", "For testing", "Unitless", true, + &DoubleCallback); m.NewShortValueObserver("Test-short-val-obs", "For testing", "Unitless", true, &ShortCallback); m.NewIntValueObserver("Test-int-val-obs", "For testing", "Unitless", true, &IntCallback); @@ -71,8 +74,8 @@ TEST(Meter, CollectSyncInstruments) counter->add(1, labelkv); std::vector res = m.Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -81,9 +84,9 @@ TEST(Meter, CollectSyncInstruments) counter->add(10, labelkv); - res = m.Collect(); + res = m.Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -102,11 +105,11 @@ TEST(Meter, CollectDeletedSync) { auto counter = m.NewShortCounter("Test-counter", "For testing", "Unitless", true); counter->add(1, labelkv); - } // counter shared_ptr deleted here + } // counter shared_ptr deleted here std::vector res = m.Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -127,7 +130,8 @@ TEST(Meter, CollectAsyncInstruments) ASSERT_EQ(m.Collect().size(), 0); - auto sumobs = m.NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); + auto sumobs = + m.NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &ShortCallback); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; @@ -135,8 +139,8 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(1, labelkv); std::vector res = m.Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); @@ -145,9 +149,9 @@ TEST(Meter, CollectAsyncInstruments) sumobs->observe(10, labelkv); - res = m.Collect(); + res = m.Collect(); agg_var = res[0].GetAggregator(); - agg = opentelemetry::nostd::get<0>(agg_var); + agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 10); } @@ -166,11 +170,11 @@ TEST(Meter, CollectDeletedAsync) { auto sumobs = m.NewShortSumObserver("Test-counter", "For testing", "Unitless", true, &Callback); sumobs->observe(1, labelkv); - } // sumobs shared_ptr deleted here + } // sumobs shared_ptr deleted here std::vector res = m.Collect(); - auto agg_var = res[0].GetAggregator(); - auto agg = opentelemetry::nostd::get<0>(agg_var); + auto agg_var = res[0].GetAggregator(); + auto agg = opentelemetry::nostd::get<0>(agg_var); ASSERT_EQ(agg->get_checkpoint()[0], 1); } @@ -189,56 +193,52 @@ TEST(Meter, RecordBatch) std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - metrics_api::SynchronousInstrument* sinstr_arr[] = - {scounter.get()}; - short svalues_arr[] = {1}; + metrics_api::SynchronousInstrument *sinstr_arr[] = {scounter.get()}; + short svalues_arr[] = {1}; - nostd::span*> sinstrs {sinstr_arr}; - nostd::span svalues {svalues_arr}; + nostd::span *> sinstrs{sinstr_arr}; + nostd::span svalues{svalues_arr}; m.RecordShortBatch(labelkv, sinstrs, svalues); std::vector res = m.Collect(); - auto short_agg_var = res[0].GetAggregator(); - auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); + auto short_agg_var = res[0].GetAggregator(); + auto short_agg = opentelemetry::nostd::get<0>(short_agg_var); ASSERT_EQ(short_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* iinstr_arr[] = - {icounter.get()}; - int ivalues_arr[] = {1}; + metrics_api::SynchronousInstrument *iinstr_arr[] = {icounter.get()}; + int ivalues_arr[] = {1}; - nostd::span*> iinstrs {iinstr_arr}; - nostd::span ivalues {ivalues_arr}; + nostd::span *> iinstrs{iinstr_arr}; + nostd::span ivalues{ivalues_arr}; m.RecordIntBatch(labelkv, iinstrs, ivalues); - res = m.Collect(); + res = m.Collect(); auto int_agg_var = res[0].GetAggregator(); - auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); + auto int_agg = opentelemetry::nostd::get<1>(int_agg_var); ASSERT_EQ(int_agg->get_checkpoint()[0], 1); - metrics_api::SynchronousInstrument* finstr_arr[] = - {fcounter.get()}; - float fvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *finstr_arr[] = {fcounter.get()}; + float fvalues_arr[] = {1.0}; - nostd::span*> finstrs {finstr_arr}; - nostd::span fvalues {fvalues_arr}; + nostd::span *> finstrs{finstr_arr}; + nostd::span fvalues{fvalues_arr}; m.RecordFloatBatch(labelkv, finstrs, fvalues); - res = m.Collect(); + res = m.Collect(); auto float_agg_var = res[0].GetAggregator(); - auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); + auto float_agg = opentelemetry::nostd::get<2>(float_agg_var); ASSERT_EQ(float_agg->get_checkpoint()[0], 1.0); - metrics_api::SynchronousInstrument* dinstr_arr[] = - {dcounter.get()}; - double dvalues_arr[] = {1.0}; + metrics_api::SynchronousInstrument *dinstr_arr[] = {dcounter.get()}; + double dvalues_arr[] = {1.0}; - nostd::span*> dinstrs {dinstr_arr}; - nostd::span dvalues {dvalues_arr}; + nostd::span *> dinstrs{dinstr_arr}; + nostd::span dvalues{dvalues_arr}; m.RecordDoubleBatch(labelkv, dinstrs, dvalues); - res = m.Collect(); + res = m.Collect(); auto double_agg_var = res[0].GetAggregator(); - auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); + auto double_agg = opentelemetry::nostd::get<3>(double_agg_var); ASSERT_EQ(double_agg->get_checkpoint()[0], 1.0); } @@ -247,7 +247,7 @@ TEST(Meter, DisableCollectSync) Meter m("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m.NewShortCounter("c", "", "", false); + auto c = m.NewShortCounter("c", "", "", false); c->add(1, labelkv); ASSERT_EQ(m.Collect().size(), 0); } @@ -257,7 +257,7 @@ TEST(Meter, DisableCollectAsync) Meter m("Test"); std::map labels = {{"Key", "Value"}}; auto labelkv = opentelemetry::trace::KeyValueIterableView{labels}; - auto c = m.NewShortValueObserver("c", "", "", false, &ShortCallback); + auto c = m.NewShortValueObserver("c", "", "", false, &ShortCallback); c->observe(1, labelkv); ASSERT_EQ(m.Collect().size(), 0); } @@ -270,7 +270,8 @@ TEST(MeterStringUtil, IsValid) ASSERT_ANY_THROW(m.NewShortCounter("1a", "Can't begin with a number", " ", true)); ASSERT_ANY_THROW(m.NewShortCounter(".a", "Can't begin with punctuation", " ", true)); ASSERT_ANY_THROW(m.NewShortCounter(" a", "Can't begin with space", " ", true)); - ASSERT_ANY_THROW(m.NewShortCounter("te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); + ASSERT_ANY_THROW(m.NewShortCounter( + "te^ s=%t", "Only alphanumeric ., -, and _ characters are allowed", " ", true)); #endif } @@ -280,11 +281,9 @@ TEST(MeterStringUtil, AlreadyExists) Meter m("Test"); m.NewShortCounter("a", "First instance of instrument named 'a'", "", true); - ASSERT_ANY_THROW(m.NewShortCounter("a", "Second (illegal) instrument named 'a'", - "", true)); - ASSERT_ANY_THROW(m.NewShortSumObserver("a", - "Still illegal even though it is not a short counter", - "", true, &ShortCallback)); + ASSERT_ANY_THROW(m.NewShortCounter("a", "Second (illegal) instrument named 'a'", "", true)); + ASSERT_ANY_THROW(m.NewShortSumObserver("a", "Still illegal even though it is not a short counter", + "", true, &ShortCallback)); #endif } OPENTELEMETRY_END_NAMESPACE From 8bb2c2bd345ed9f7517eb86c27be0df1dca71c6c Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 17:01:30 -0400 Subject: [PATCH 29/32] Update comments --- sdk/include/opentelemetry/sdk/metrics/meter.h | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 36d868fce8..18891b7668 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -33,9 +33,7 @@ class Meter : public metrics_api::Meter * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. * @param enabled a boolean value that turns on or off the metric instrument. * @return a shared pointer to the created Counter. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortCounter(nostd::string_view name, nostd::string_view description, @@ -66,9 +64,7 @@ class Meter : public metrics_api::Meter * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. * @param enabled a boolean value that turns on or off the metric instrument. * @return a shared pointer to the created UpDownCounter. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortUpDownCounter( nostd::string_view name, @@ -103,9 +99,7 @@ class Meter : public metrics_api::Meter * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html. * @param enabled a boolean value that turns on or off the metric instrument. * @return a shared pointer to the created DoubleValueRecorder. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortValueRecorder( nostd::string_view name, @@ -141,9 +135,7 @@ class Meter : public metrics_api::Meter * @param enabled a boolean value that turns on or off the metric instrument. * @param callback the function to be observed by the instrument. * @return a shared pointer to the created SumObserver. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortSumObserver( nostd::string_view name, @@ -183,9 +175,7 @@ class Meter : public metrics_api::Meter * @param enabled a boolean value that turns on or off the metric instrument. * @param callback the function to be observed by the instrument. * @return a shared pointer to the created UpDownSumObserver. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortUpDownSumObserver( nostd::string_view name, @@ -225,9 +215,7 @@ class Meter : public metrics_api::Meter * @param enabled a boolean value that turns on or off the metric instrument. * @param callback the function to be observed by the instrument. * @return a shared pointer to the created ValueObserver. - * @throws IllegalArgumentException if {@code name} is null - * @throws IllegalArgumentException if a different metric by the same name exists in this meter. - * @throws IllegalArgumentException if the {@code name} does not match spec requirements. + * @throws invalid_argument exception if name is null or does not conform to OTel syntax. */ nostd::shared_ptr> NewShortValueObserver( nostd::string_view name, @@ -379,6 +367,7 @@ class Meter : public metrics_api::Meter std::mutex metrics_lock_; std::mutex observers_lock_; }; + } // namespace metrics } // namespace sdk -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE From be68945ce2f3b00a0977ef4727a34550ce19b08b Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 17:50:58 -0400 Subject: [PATCH 30/32] Add mutexes to NameAlreadyUsed function --- sdk/src/metrics/meter.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 60c98c5163..1f76963e1a 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -749,6 +749,8 @@ bool Meter::IsValidName(nostd::string_view name) bool Meter::NameAlreadyUsed(nostd::string_view name) { + std::lock_guard lg_metrics(metrics_lock_); + std::lock_guard lg_obsevers(observers_lock_); if (short_metrics_.find(std::string(name)) != short_metrics_.end()) return true; else if (int_metrics_.find(std::string(name)) != int_metrics_.end()) @@ -772,4 +774,4 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) } } // namespace metrics } // namespace sdk -OPENTELEMETRY_END_NAMESPACE +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file From 19befe93aa46d084d153ccb5226c93e785b631a2 Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 21:04:58 -0400 Subject: [PATCH 31/32] Remove NameAlreadyUsed data race --- sdk/include/opentelemetry/sdk/metrics/meter.h | 5 ++++- sdk/src/metrics/meter.cc | 21 ++++--------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 18891b7668..88897b77ae 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -7,7 +7,8 @@ #include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" -#include +#include +#include #include OPENTELEMETRY_BEGIN_NAMESPACE @@ -361,6 +362,8 @@ class Meter : public metrics_api::Meter std::map>> double_observers_; + std::unordered_set names_; + std::string library_name_; std::string library_version_; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 1f76963e1a..f856481a5e 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -751,26 +751,13 @@ bool Meter::NameAlreadyUsed(nostd::string_view name) { std::lock_guard lg_metrics(metrics_lock_); std::lock_guard lg_obsevers(observers_lock_); - if (short_metrics_.find(std::string(name)) != short_metrics_.end()) + if (names_.find(std::string(name)) != names_.end()) return true; - else if (int_metrics_.find(std::string(name)) != int_metrics_.end()) - return true; - else if (float_metrics_.find(std::string(name)) != float_metrics_.end()) - return true; - else if (double_metrics_.find(std::string(name)) != double_metrics_.end()) - return true; - - else if (short_observers_.find(std::string(name)) != short_observers_.end()) - return true; - else if (int_observers_.find(std::string(name)) != int_observers_.end()) - return true; - else if (float_observers_.find(std::string(name)) != float_observers_.end()) - return true; - else if (double_observers_.find(std::string(name)) != double_observers_.end()) - return true; - else + { + names_.insert(std::string(name)); return false; + } } } // namespace metrics } // namespace sdk From 07e154d62ae010d9c568d3067b86d211b8997dbc Mon Sep 17 00:00:00 2001 From: Brandon Kimberly Date: Wed, 5 Aug 2020 21:26:33 -0400 Subject: [PATCH 32/32] Remove extraneous include --- sdk/include/opentelemetry/sdk/metrics/meter.h | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 88897b77ae..001866dd99 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -7,7 +7,6 @@ #include "opentelemetry/sdk/metrics/record.h" #include "opentelemetry/sdk/metrics/sync_instruments.h" -#include #include #include