From aad198119537bc77992245ba91ca0d9e8f1fd321 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Sat, 30 Jul 2022 21:44:02 +0000 Subject: [PATCH 01/10] Fix metrics context circular reference --- sdk/include/opentelemetry/sdk/metrics/meter.h | 7 ++++--- .../sdk/metrics/state/metric_collector.h | 2 +- sdk/src/metrics/meter.cc | 11 ++++++----- sdk/src/metrics/state/metric_collector.cc | 5 +++-- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 22608e2b05..feb676582a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -29,7 +29,7 @@ class Meter final : public opentelemetry::metrics::Meter public: /** Construct a new Meter with the given pipeline. */ explicit Meter( - std::shared_ptr meter_context, + std::weak_ptr meter_context, std::unique_ptr scope = opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create("")) noexcept; @@ -122,7 +122,7 @@ class Meter final : public opentelemetry::metrics::Meter // order of declaration is important here - instrumentation scope should destroy after // meter-context. std::unique_ptr scope_; - std::shared_ptr meter_context_; + std::weak_ptr meter_context_; // Mapping between instrument-name and Aggregation Storage. std::unordered_map> storage_registry_; @@ -135,7 +135,8 @@ class Meter final : public opentelemetry::metrics::Meter void *), void *state = nullptr) { - auto view_registry = meter_context_->GetViewRegistry(); + auto ctx = meter_context_.lock(); + auto view_registry = ctx->GetViewRegistry(); auto success = view_registry->FindViews( instrument_descriptor, *scope_, [this, &instrument_descriptor, callback, state](const View &view) { diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h index 20372f5209..15560e14cb 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -48,7 +48,7 @@ class MetricCollector : public MetricProducer, public CollectorHandle bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; private: - std::shared_ptr meter_context_; + std::weak_ptr meter_context_; std::shared_ptr metric_reader_; }; } // namespace metrics diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index d17b61f120..55bb809e84 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -24,7 +24,7 @@ namespace metrics = opentelemetry::metrics; namespace nostd = opentelemetry::nostd; Meter::Meter( - std::shared_ptr meter_context, + std::weak_ptr meter_context, std::unique_ptr instrumentation_scope) noexcept : scope_{std::move(instrumentation_scope)}, meter_context_{meter_context} {} @@ -201,7 +201,8 @@ const sdk::instrumentationscope::InstrumentationScope *Meter::GetInstrumentation std::unique_ptr Meter::RegisterMetricStorage( InstrumentDescriptor &instrument_descriptor) { - auto view_registry = meter_context_->GetViewRegistry(); + auto ctx = meter_context_.lock(); + auto view_registry = ctx->GetViewRegistry(); std::unique_ptr storages(new MultiMetricStorage()); auto success = view_registry->FindViews( @@ -238,11 +239,11 @@ std::vector Meter::Collect(CollectorHandle *collector, opentelemetry::common::SystemTimestamp collect_ts) noexcept { std::vector metric_data_list; + auto ctx = meter_context_.lock(); for (auto &metric_storage : storage_registry_) { - metric_storage.second->Collect(collector, meter_context_->GetCollectors(), - meter_context_->GetSDKStartTime(), collect_ts, - [&metric_data_list](MetricData metric_data) { + metric_storage.second->Collect(collector, ctx->GetCollectors(), ctx->GetSDKStartTime(), + collect_ts, [&metric_data_list](MetricData metric_data) { metric_data_list.push_back(metric_data); return true; }); diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index 16012ca0f9..57dd3b7f51 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -33,7 +33,8 @@ bool MetricCollector::Collect( nostd::function_ref callback) noexcept { ResourceMetrics resource_metrics; - for (auto &meter : meter_context_->GetMeters()) + auto ctx = meter_context_.lock(); + for (auto &meter : ctx->GetMeters()) { auto collection_ts = std::chrono::system_clock::now(); ScopeMetrics scope_metrics; @@ -41,7 +42,7 @@ bool MetricCollector::Collect( scope_metrics.scope_ = meter->GetInstrumentationScope(); resource_metrics.scope_metric_data_.push_back(scope_metrics); } - resource_metrics.resource_ = &meter_context_->GetResource(); + resource_metrics.resource_ = &ctx->GetResource(); callback(resource_metrics); return true; } From 61795b2cfc4cb293dfd80a3c5ff0713fdc8e751a Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Tue, 9 Aug 2022 17:20:15 +0200 Subject: [PATCH 02/10] revert changes from main merge --- sdk/include/opentelemetry/sdk/metrics/meter.h | 39 ++----------------- sdk/src/metrics/meter.cc | 3 +- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index 1f0ac3821b..c6bce29205 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -114,43 +114,10 @@ class Meter final : public opentelemetry::metrics::Meter std::weak_ptr meter_context_; // Mapping between instrument-name and Aggregation Storage. std::unordered_map> storage_registry_; - - std::unique_ptr RegisterMetricStorage( + std::unique_ptr RegisterSyncMetricStorage( + InstrumentDescriptor &instrument_descriptor); + std::unique_ptr RegisterAsyncMetricStorage( InstrumentDescriptor &instrument_descriptor); - - template - void RegisterAsyncMetricStorage(InstrumentDescriptor &instrument_descriptor, - void (*callback)(opentelemetry::metrics::ObserverResult &, - void *), - void *state = nullptr) - { - auto ctx = meter_context_.lock(); - auto view_registry = ctx->GetViewRegistry(); - auto success = view_registry->FindViews( - instrument_descriptor, *scope_, - [this, &instrument_descriptor, callback, state](const View &view) { - auto view_instr_desc = instrument_descriptor; - if (!view.GetName().empty()) - { - view_instr_desc.name_ = view.GetName(); - } - if (!view.GetDescription().empty()) - { - view_instr_desc.description_ = view.GetDescription(); - } - auto storage = std::shared_ptr>( - new AsyncMetricStorage(view_instr_desc, view.GetAggregationType(), callback, - &view.GetAttributesProcessor(), state)); - storage_registry_[instrument_descriptor.name_] = storage; - return true; - }); - if (!success) - { - OTEL_INTERNAL_LOG_ERROR( - "[Meter::RegisterAsyncMetricStorage] - Error during finding matching views." - << "Some of the matching view configurations may not be used for metric collection"); - } - } }; } // namespace metrics } // namespace sdk diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 09d2d5cd23..39cac71060 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -241,7 +241,8 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( std::unique_ptr Meter::RegisterAsyncMetricStorage( InstrumentDescriptor &instrument_descriptor) { - auto view_registry = meter_context_->GetViewRegistry(); + auto ctx = meter_context_.lock(); + auto view_registry = ctx->GetViewRegistry(); std::unique_ptr storages(new AsyncMultiMetricStorage()); auto success = view_registry->FindViews( instrument_descriptor, *GetInstrumentationScope(), From d1aa868cc05a304dbc0ac895a02bf132bec2ea7b Mon Sep 17 00:00:00 2001 From: Oblivion Date: Tue, 9 Aug 2022 16:09:49 +0000 Subject: [PATCH 03/10] collector cannot have smart pointer to the context --- .../sdk/metrics/state/metric_collector.h | 5 ++--- sdk/src/metrics/state/metric_collector.cc | 12 +++++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h index dcb3fa388f..8134edea8a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -31,8 +31,7 @@ class CollectorHandle class MetricCollector : public MetricProducer, public CollectorHandle { public: - MetricCollector(std::shared_ptr &&context, - std::unique_ptr metric_reader); + MetricCollector(MeterContext *context, std::unique_ptr metric_reader); AggregationTemporality GetAggregationTemporality( InstrumentType instrument_type) noexcept override; @@ -50,7 +49,7 @@ class MetricCollector : public MetricProducer, public CollectorHandle bool Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept; private: - std::weak_ptr meter_context_; + MeterContext *meter_context_; std::shared_ptr metric_reader_; }; } // namespace metrics diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index a76ae19a04..dbc2b87767 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -16,10 +16,9 @@ namespace sdk namespace metrics { -MetricCollector::MetricCollector( - std::shared_ptr &&context, - std::unique_ptr metric_reader) - : meter_context_{std::move(context)}, metric_reader_{std::move(metric_reader)} +MetricCollector::MetricCollector(opentelemetry::sdk::metrics::MeterContext *context, + std::unique_ptr metric_reader) + : meter_context_{context}, metric_reader_{std::move(metric_reader)} { metric_reader_->SetMetricProducer(this); } @@ -34,8 +33,7 @@ bool MetricCollector::Collect( nostd::function_ref callback) noexcept { ResourceMetrics resource_metrics; - auto ctx = meter_context_.lock(); - for (auto &meter : ctx->GetMeters()) + for (auto &meter : meter_context_->GetMeters()) { auto collection_ts = std::chrono::system_clock::now(); ScopeMetrics scope_metrics; @@ -43,7 +41,7 @@ bool MetricCollector::Collect( scope_metrics.scope_ = meter->GetInstrumentationScope(); resource_metrics.scope_metric_data_.push_back(scope_metrics); } - resource_metrics.resource_ = &ctx->GetResource(); + resource_metrics.resource_ = &meter_context_->GetResource(); callback(resource_metrics); return true; } From 4b8b3f10dafcbdd471cf4933de040955629393ab Mon Sep 17 00:00:00 2001 From: Oblivion Date: Tue, 9 Aug 2022 16:21:32 +0000 Subject: [PATCH 04/10] fix CI --- sdk/src/metrics/meter_context.cc | 3 +-- sdk/test/metrics/metric_reader_test.cc | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 73721e324d..346b1dce29 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -46,8 +46,7 @@ opentelemetry::common::SystemTimestamp MeterContext::GetSDKStartTime() noexcept void MeterContext::AddMetricReader(std::unique_ptr reader) noexcept { - auto collector = - std::shared_ptr{new MetricCollector(shared_from_this(), std::move(reader))}; + auto collector = std::shared_ptr{new MetricCollector(this, std::move(reader))}; collectors_.push_back(collector); } diff --git a/sdk/test/metrics/metric_reader_test.cc b/sdk/test/metrics/metric_reader_test.cc index 82fca653f6..9f15aa1cbe 100644 --- a/sdk/test/metrics/metric_reader_test.cc +++ b/sdk/test/metrics/metric_reader_test.cc @@ -38,7 +38,7 @@ TEST(MetricReaderTest, BasicTests) std::unique_ptr metric_reader2(new MockMetricReader()); std::shared_ptr meter_context2(new MeterContext()); std::shared_ptr metric_producer{ - new MetricCollector(std::move(meter_context2), std::move(metric_reader2))}; + new MetricCollector(meter_context2.get(), std::move(metric_reader2))}; EXPECT_NO_THROW(metric_producer->Collect([](ResourceMetrics &metric_data) { return true; })); } #endif From 37fe3d1dab96ceb981e3e037a9d84720ff10cdfa Mon Sep 17 00:00:00 2001 From: Oblivion Date: Tue, 9 Aug 2022 19:33:00 +0000 Subject: [PATCH 05/10] context and storage null check --- sdk/src/metrics/meter.cc | 23 +++- sdk/src/metrics/state/metric_collector.cc | 6 + sdk/src/metrics/state/observable_registry.cc | 7 + sdk/src/metrics/sync_instruments.cc | 127 +++++++++++++++++- .../histogram_exemplar_reservoir_test.cc | 23 ++++ 5 files changed, 178 insertions(+), 8 deletions(-) create mode 100644 sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 39cac71060..3f6b1aa4a1 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -204,7 +204,13 @@ const sdk::instrumentationscope::InstrumentationScope *Meter::GetInstrumentation std::unique_ptr Meter::RegisterSyncMetricStorage( InstrumentDescriptor &instrument_descriptor) { - auto ctx = meter_context_.lock(); + auto ctx = meter_context_.lock(); + if (!ctx) + { + OTEL_INTERNAL_LOG_ERROR("[Meter::RegisterMetricStorage] - Error during finding matching views." + << "The metric context is invalid"); + return nullptr; + } auto view_registry = ctx->GetViewRegistry(); std::unique_ptr storages(new SyncMultiMetricStorage()); @@ -241,7 +247,14 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( std::unique_ptr Meter::RegisterAsyncMetricStorage( InstrumentDescriptor &instrument_descriptor) { - auto ctx = meter_context_.lock(); + auto ctx = meter_context_.lock(); + if (!ctx) + { + OTEL_INTERNAL_LOG_ERROR( + "[Meter::RegisterAsyncMetricStorage] - Error during finding matching views." + << "The metric context is invalid"); + return nullptr; + } auto view_registry = ctx->GetViewRegistry(); std::unique_ptr storages(new AsyncMultiMetricStorage()); auto success = view_registry->FindViews( @@ -279,6 +292,12 @@ std::vector Meter::Collect(CollectorHandle *collector, std::vector metric_data_list; auto ctx = meter_context_.lock(); + if (!ctx) + { + OTEL_INTERNAL_LOG_ERROR("[Meter::Collect] - Error during collection." + << "The metric context is invalid"); + return std::vector{}; + } for (auto &metric_storage : storage_registry_) { metric_storage.second->Collect(collector, ctx->GetCollectors(), ctx->GetSDKStartTime(), diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index dbc2b87767..97f29a8fbe 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -32,6 +32,12 @@ AggregationTemporality MetricCollector::GetAggregationTemporality( bool MetricCollector::Collect( nostd::function_ref callback) noexcept { + if (!meter_context_) + { + OTEL_INTERNAL_LOG_ERROR("[MetricCollector::Collect] - Error during collecting." + << "The metric context is invalid"); + return false; + } ResourceMetrics resource_metrics; for (auto &meter : meter_context_->GetMeters()) { diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc index 7e12d4a190..0d6d770f87 100644 --- a/sdk/src/metrics/state/observable_registry.cc +++ b/sdk/src/metrics/state/observable_registry.cc @@ -7,6 +7,7 @@ # include "opentelemetry/sdk/metrics/async_instruments.h" # include "opentelemetry/sdk/metrics/observer_result.h" # include "opentelemetry/sdk/metrics/state/metric_storage.h" +# include "opentelemetry/sdk_config.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -51,6 +52,12 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti auto storage = static_cast(callback_wrap->instrument) ->GetMetricStorage(); + if (!storage) + { + OTEL_INTERNAL_LOG_ERROR("[ObservableRegistry::Observe] - Error during observe." + << "The metric storage is invalid"); + return; + } if (value_type == InstrumentValueType::kDouble) { nostd::shared_ptr> ob_res( diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 61e30248c3..38bce92a12 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -16,11 +16,22 @@ namespace metrics LongCounter::LongCounter(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR("[LongCounter::LongCounter] - Error during constructing LongCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void LongCounter::Add(long value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { + if (!storage_) + { + return; + } auto context = opentelemetry::context::Context{}; return storage_->RecordLong(value, attributes, context); } @@ -29,29 +40,53 @@ void LongCounter::Add(long value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordLong(value, attributes, context); } void LongCounter::Add(long value) noexcept { auto context = opentelemetry::context::Context{}; + if (!storage_) + { + return; + } return storage_->RecordLong(value, context); } void LongCounter::Add(long value, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordLong(value, context); } DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { auto context = opentelemetry::context::Context{}; + if (!storage_) + { + return; + } return storage_->RecordDouble(value, attributes, context); } @@ -59,29 +94,53 @@ void DoubleCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordDouble(value, attributes, context); } void DoubleCounter::Add(double value) noexcept { auto context = opentelemetry::context::Context{}; + if (!storage_) + { + return; + } return storage_->RecordDouble(value, context); } void DoubleCounter::Add(double value, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordDouble(value, context); } LongUpDownCounter::LongUpDownCounter(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void LongUpDownCounter::Add(long value, const opentelemetry::common::KeyValueIterable &attributes) noexcept { auto context = opentelemetry::context::Context{}; + if (!storage_) + { + return; + } return storage_->RecordLong(value, attributes, context); } @@ -89,24 +148,44 @@ void LongUpDownCounter::Add(long value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordLong(value, attributes, context); } void LongUpDownCounter::Add(long value) noexcept { auto context = opentelemetry::context::Context{}; + if (!storage_) + { + return; + } return storage_->RecordLong(value, context); } void LongUpDownCounter::Add(long value, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordLong(value, context); } DoubleUpDownCounter::DoubleUpDownCounter(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void DoubleUpDownCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes) noexcept @@ -119,24 +198,44 @@ void DoubleUpDownCounter::Add(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordDouble(value, attributes, context); } void DoubleUpDownCounter::Add(double value) noexcept { + if (!storage_) + { + return; + } auto context = opentelemetry::context::Context{}; return storage_->RecordDouble(value, context); } void DoubleUpDownCounter::Add(double value, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } return storage_->RecordDouble(value, context); } LongHistogram::LongHistogram(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void LongHistogram::Record(long value, const opentelemetry::common::KeyValueIterable &attributes, @@ -167,12 +266,24 @@ void LongHistogram::Record(long value, const opentelemetry::context::Context &co DoubleHistogram::DoubleHistogram(InstrumentDescriptor instrument_descriptor, std::unique_ptr storage) : Synchronous(instrument_descriptor, std::move(storage)) -{} +{ + if (!storage_) + { + OTEL_INTERNAL_LOG_ERROR( + "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + << "The metric storage is invalid" + << "No value will be added"); + } +} void DoubleHistogram::Record(double value, const opentelemetry::common::KeyValueIterable &attributes, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } if (value < 0 || std::isnan(value) || std::isinf(value)) { OTEL_INTERNAL_LOG_WARN( @@ -186,6 +297,10 @@ void DoubleHistogram::Record(double value, void DoubleHistogram::Record(double value, const opentelemetry::context::Context &context) noexcept { + if (!storage_) + { + return; + } if (value < 0 || std::isnan(value) || std::isinf(value)) { OTEL_INTERNAL_LOG_WARN( diff --git a/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc b/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc new file mode 100644 index 0000000000..3c5b224051 --- /dev/null +++ b/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc @@ -0,0 +1,23 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include "opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h" +# include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +class HistogramExemplarReservoirTestPeer : public ::testing::Test +{ +public: +}; + +TEST_F(HistogramExemplarReservoirTestPeer, OfferMeasurement) {} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif From 42473ab1f2356d0fdcd801e99a861f89885781df Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Tue, 9 Aug 2022 21:35:55 +0200 Subject: [PATCH 06/10] revert unrelated change --- .../histogram_exemplar_reservoir_test.cc | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc diff --git a/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc b/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc deleted file mode 100644 index 3c5b224051..0000000000 --- a/sdk/test/metrics/exemplar/histogram_exemplar_reservoir_test.cc +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#ifndef ENABLE_METRICS_PREVIEW -# include "opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h" -# include - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace metrics -{ -class HistogramExemplarReservoirTestPeer : public ::testing::Test -{ -public: -}; - -TEST_F(HistogramExemplarReservoirTestPeer, OfferMeasurement) {} - -} // namespace metrics -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE -#endif From c7486c9a11cb1bf8ff9d9b85dbf3e37e0fca0bf7 Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Tue, 9 Aug 2022 21:40:33 +0200 Subject: [PATCH 07/10] fix log messages --- sdk/src/metrics/sync_instruments.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 38bce92a12..e94ff0f126 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -73,7 +73,7 @@ DoubleCounter::DoubleCounter(InstrumentDescriptor instrument_descriptor, if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + "[DoubleCounter::DoubleCounter] - Error during constructing DoubleCounter." << "The metric storage is invalid" << "No value will be added"); } @@ -181,7 +181,7 @@ DoubleUpDownCounter::DoubleUpDownCounter(InstrumentDescriptor instrument_descrip if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + "[DoubleUpDownCounter::DoubleUpDownCounter] - Error during constructing DoubleUpDownCounter." << "The metric storage is invalid" << "No value will be added"); } @@ -231,7 +231,7 @@ LongHistogram::LongHistogram(InstrumentDescriptor instrument_descriptor, if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + "[LongHistogram::LongHistogram] - Error during constructing LongHistogram." << "The metric storage is invalid" << "No value will be added"); } @@ -270,7 +270,7 @@ DoubleHistogram::DoubleHistogram(InstrumentDescriptor instrument_descriptor, if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[LongUpDownCounter::LongUpDownCounter] - Error during constructing LongUpDownCounter." + "[DoubleHistogram::DoubleHistogram] - Error during constructing DoubleHistogram." << "The metric storage is invalid" << "No value will be added"); } From f6afdb2a2d75f597213825239db0bceecdbe5224 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Tue, 9 Aug 2022 19:41:42 +0000 Subject: [PATCH 08/10] format --- .../Histogram_exemplar_resrvoid_test.cc | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc diff --git a/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc b/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc new file mode 100644 index 0000000000..bfbfe66a89 --- /dev/null +++ b/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc @@ -0,0 +1,23 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef ENABLE_METRICS_PREVIEW +# include +# include "opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ +class HistogramExemplarReservoirTestPeer : public ::testing::Test +{ +public: +}; + +TEST_F(HistogramExemplarReservoirTestPeer, OfferMeasurement) {} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE +#endif From 10c8f81fc367df6072f206c685bf985e4f0360e8 Mon Sep 17 00:00:00 2001 From: Oblivion Date: Tue, 9 Aug 2022 19:42:52 +0000 Subject: [PATCH 09/10] format --- sdk/src/metrics/sync_instruments.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index e94ff0f126..5db5d02251 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/sdk/src/metrics/sync_instruments.cc @@ -181,7 +181,8 @@ DoubleUpDownCounter::DoubleUpDownCounter(InstrumentDescriptor instrument_descrip if (!storage_) { OTEL_INTERNAL_LOG_ERROR( - "[DoubleUpDownCounter::DoubleUpDownCounter] - Error during constructing DoubleUpDownCounter." + "[DoubleUpDownCounter::DoubleUpDownCounter] - Error during constructing " + "DoubleUpDownCounter." << "The metric storage is invalid" << "No value will be added"); } From 7ac22d709899f62f00196625e86e59cca349c40c Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Tue, 9 Aug 2022 21:44:01 +0200 Subject: [PATCH 10/10] revert unrelated change --- .../Histogram_exemplar_resrvoid_test.cc | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc diff --git a/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc b/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc deleted file mode 100644 index bfbfe66a89..0000000000 --- a/sdk/test/metrics/exemplar/Histogram_exemplar_resrvoid_test.cc +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#ifndef ENABLE_METRICS_PREVIEW -# include -# include "opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace metrics -{ -class HistogramExemplarReservoirTestPeer : public ::testing::Test -{ -public: -}; - -TEST_F(HistogramExemplarReservoirTestPeer, OfferMeasurement) {} - -} // namespace metrics -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE -#endif