diff --git a/CHANGELOG.md b/CHANGELOG.md index 644edfe2c2..b7180b4aec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,9 @@ Increment the: * [SDK] Optimize PeriodicExportingMetricReader thread usage [#3383](https://github.com/open-telemetry/opentelemetry-cpp/pull/3383) +* [SDK] Aggregate identical metrics instruments and detect duplicates + [#3358](https://github.com/open-telemetry/opentelemetry-cpp/pull/3358) + ## [1.20 2025-04-01] * [BUILD] Update opentelemetry-proto version diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 9e3c843f5e..620fddedb5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -3,6 +3,8 @@ #pragma once +#include +#include #include #include @@ -65,6 +67,152 @@ struct InstrumentDescriptor InstrumentValueType value_type_; }; +struct InstrumentDescriptorUtil +{ + // Case-insensitive comparison of two ASCII strings used to evaluate equality of instrument names + static bool CaseInsensitiveAsciiEquals(const std::string &lhs, const std::string &rhs) noexcept + { + return lhs.size() == rhs.size() && + std::equal(lhs.begin(), lhs.end(), rhs.begin(), [](char a, char b) { + return std::tolower(static_cast(a)) == + std::tolower(static_cast(b)); + }); + } + + // Implementation of the specification requirements on duplicate instruments + // An instrument is a duplicate if it has the same name (case-insensitive) as another instrument, + // but different instrument kind, unit, or description. + // https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L869 + static bool IsDuplicate(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) noexcept + { + // Not a duplicate if case-insensitive names are not equal + if (!InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_)) + { + return false; + } + + // Duplicate if names equal and kinds (Type and ValueType) are not equal + if (lhs.type_ != rhs.type_ || lhs.value_type_ != rhs.value_type_) + { + return true; + } + + // Duplicate if names equal and units (case-sensitive) are not equal + if (lhs.unit_ != rhs.unit_) + { + return true; + } + + // Duplicate if names equal and descriptions (case-sensitive) are not equal + if (lhs.description_ != rhs.description_) + { + return true; + } + + // All identifying fields are equal + // These are identical instruments or only have a name case conflict + return false; + } + + static opentelemetry::nostd::string_view GetInstrumentTypeString(InstrumentType type) noexcept + { + switch (type) + { + case InstrumentType::kCounter: + return "Counter"; + case InstrumentType::kUpDownCounter: + return "UpDownCounter"; + case InstrumentType::kHistogram: + return "Histogram"; + case InstrumentType::kObservableCounter: + return "ObservableCounter"; + case InstrumentType::kObservableUpDownCounter: + return "ObservableUpDownCounter"; + case InstrumentType::kObservableGauge: + return "ObservableGauge"; + case InstrumentType::kGauge: + return "Gauge"; + default: + return "Unknown"; + } + } + + static opentelemetry::nostd::string_view GetInstrumentValueTypeString( + InstrumentValueType value_type) noexcept + { + switch (value_type) + { + case InstrumentValueType::kInt: + return "Int"; + case InstrumentValueType::kLong: + return "Long"; + case InstrumentValueType::kFloat: + return "Float"; + case InstrumentValueType::kDouble: + return "Double"; + default: + return "Unknown"; + } + } +}; + +struct InstrumentEqualNameCaseInsensitive +{ + bool operator()(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) const noexcept + { + // Names (case-insensitive) + if (!InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_)) + { + return false; + } + + // Kinds (Type and ValueType) + if (lhs.type_ != rhs.type_ || lhs.value_type_ != rhs.value_type_) + { + return false; + } + + // Units (case-sensitive) + if (lhs.unit_ != rhs.unit_) + { + return false; + } + + // Descriptions (case-sensitive) + if (lhs.description_ != rhs.description_) + { + return false; + } + + // All identifying fields are equal + return true; + } +}; + +// Hash for InstrumentDescriptor +// Identical instruments must have the same hash value +// Two instruments are identical when all identifying fields (case-insensitive name , kind, +// description, unit) are equal. +struct InstrumentDescriptorHash +{ + std::size_t operator()(const InstrumentDescriptor &instrument_descriptor) const noexcept + { + std::size_t hashcode{}; + + for (char c : instrument_descriptor.name_) + { + sdk::common::GetHash(hashcode, + static_cast(std::tolower(static_cast(c)))); + } + + sdk::common::GetHash(hashcode, instrument_descriptor.description_); + sdk::common::GetHash(hashcode, instrument_descriptor.unit_); + sdk::common::GetHash(hashcode, static_cast(instrument_descriptor.type_)); + sdk::common::GetHash(hashcode, static_cast(instrument_descriptor.value_type_)); + return hashcode; + } +}; + using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap; using MetricAttributesHash = opentelemetry::sdk::metrics::FilteredOrderedAttributeMapHash; using AggregationTemporalitySelector = std::function; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h index bf1b0e6c37..9e2107fcd1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter.h @@ -136,8 +136,12 @@ class Meter final : public opentelemetry::metrics::Meter // meter-context. std::unique_ptr scope_; std::weak_ptr meter_context_; - // Mapping between instrument-name and Aggregation Storage. - std::unordered_map> storage_registry_; + // Mapping between instrument descriptor and Aggregation Storage. + using MetricStorageMap = std::unordered_map, + InstrumentDescriptorHash, + InstrumentEqualNameCaseInsensitive>; + MetricStorageMap storage_registry_; std::shared_ptr observable_registry_; MeterConfig meter_config_; std::unique_ptr RegisterSyncMetricStorage( @@ -164,6 +168,19 @@ class Meter final : public opentelemetry::metrics::Meter return instrument_validator.ValidateName(name) && instrument_validator.ValidateUnit(unit) && instrument_validator.ValidateDescription(description); } + + // This function checks if the instrument is a duplicate of an existing one + // and emits a warning through the internal logger. + static void WarnOnDuplicateInstrument( + const sdk::instrumentationscope::InstrumentationScope *scope, + const MetricStorageMap &storage_registry, + const InstrumentDescriptor &new_instrument); + + // This function checks if the instrument has a name case conflict with an existing one + // and emits a warning through the internal logger. + static void WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope, + const InstrumentDescriptor &existing_instrument, + const InstrumentDescriptor &new_instrument); }; } // namespace metrics } // namespace sdk diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index f89de3b6b0..4424bbb4c9 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -43,6 +43,44 @@ # include "opentelemetry/sdk/metrics/exemplar/reservoir_utils.h" #endif +namespace +{ + +struct InstrumentationScopeLogStreamable +{ + const opentelemetry::sdk::instrumentationscope::InstrumentationScope &scope; +}; + +struct InstrumentDescriptorLogStreamable +{ + const opentelemetry::sdk::metrics::InstrumentDescriptor &instrument; +}; + +std::ostream &operator<<(std::ostream &os, + const InstrumentationScopeLogStreamable &streamable) noexcept +{ + os << "\n name=\"" << streamable.scope.GetName() << "\"" << "\n schema_url=\"" + << streamable.scope.GetSchemaURL() << "\"" << "\n version=\"" << streamable.scope.GetVersion() + << "\""; + return os; +} + +std::ostream &operator<<(std::ostream &os, + const InstrumentDescriptorLogStreamable &streamable) noexcept +{ + os << "\n name=\"" << streamable.instrument.name_ << "\"" << "\n description=\"" + << streamable.instrument.description_ << "\"" << "\n unit=\"" << streamable.instrument.unit_ + << "\"" << "\n kind=\"" + << opentelemetry::sdk::metrics::InstrumentDescriptorUtil::GetInstrumentValueTypeString( + streamable.instrument.value_type_) + << opentelemetry::sdk::metrics::InstrumentDescriptorUtil::GetInstrumentTypeString( + streamable.instrument.type_) + << "\""; + return os; +} + +} // namespace + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -486,18 +524,31 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto multi_storage = static_cast(storages.get()); - - auto storage = std::shared_ptr(new SyncMetricStorage( - view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), + std::shared_ptr sync_storage{}; + auto storage_iter = storage_registry_.find(view_instr_desc); + if (storage_iter != storage_registry_.end()) + { + WarnOnNameCaseConflict(GetInstrumentationScope(), storage_iter->first, view_instr_desc); + // static_pointer_cast is okay here. If storage_registry_.find is successful + // InstrumentEqualNameCaseInsensitive ensures that the + // instrument type and value type are the same for the existing and new instrument. + sync_storage = std::static_pointer_cast(storage_iter->second); + } + else + { + WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + sync_storage = std::shared_ptr(new SyncMetricStorage( + view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW - exemplar_filter_type, - GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), - instrument_descriptor), + exemplar_filter_type, + GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), + view_instr_desc), #endif - view.GetAggregationConfig())); - storage_registry_[instrument_descriptor.name_] = storage; - multi_storage->AddStorage(storage); + view.GetAggregationConfig())); + storage_registry_.insert({view_instr_desc, sync_storage}); + } + auto sync_multi_storage = static_cast(storages.get()); + sync_multi_storage->AddStorage(sync_storage); return true; }); @@ -546,16 +597,31 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( { view_instr_desc.description_ = view.GetDescription(); } - auto storage = std::shared_ptr(new AsyncMetricStorage( - view_instr_desc, view.GetAggregationType(), + std::shared_ptr async_storage{}; + auto storage_iter = storage_registry_.find(view_instr_desc); + if (storage_iter != storage_registry_.end()) + { + WarnOnNameCaseConflict(GetInstrumentationScope(), storage_iter->first, view_instr_desc); + // static_pointer_cast is okay here. If storage_registry_.find is successful + // InstrumentEqualNameCaseInsensitive ensures that the + // instrument type and value type are the same for the existing and new instrument. + async_storage = std::static_pointer_cast(storage_iter->second); + } + else + { + WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + async_storage = std::shared_ptr(new AsyncMetricStorage( + view_instr_desc, view.GetAggregationType(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW - exemplar_filter_type, - GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), - instrument_descriptor), + exemplar_filter_type, + GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), + view_instr_desc), #endif - view.GetAggregationConfig())); - storage_registry_[instrument_descriptor.name_] = storage; - static_cast(storages.get())->AddStorage(storage); + view.GetAggregationConfig())); + storage_registry_.insert({view_instr_desc, async_storage}); + } + auto async_multi_storage = static_cast(storages.get()); + async_multi_storage->AddStorage(async_storage); return true; }); if (!success) @@ -596,6 +662,73 @@ std::vector Meter::Collect(CollectorHandle *collector, return metric_data_list; } +// Implementation of the log message recommended by the SDK specification for duplicate instruments. +// See +// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L882 +void Meter::WarnOnDuplicateInstrument(const sdk::instrumentationscope::InstrumentationScope *scope, + const MetricStorageMap &storage_registry, + const InstrumentDescriptor &new_instrument) +{ + for (const auto &element : storage_registry) + { + const auto &existing_instrument = element.first; + if (InstrumentDescriptorUtil::IsDuplicate(existing_instrument, new_instrument)) + { + std::string resolution_info{""}; + + if (existing_instrument.type_ != new_instrument.type_ || + existing_instrument.value_type_ != new_instrument.value_type_) + { + resolution_info += + "\nDifferent instrument kinds found. Consider configuring a View to change the name of " + "the duplicate instrument."; + } + + if (existing_instrument.unit_ != new_instrument.unit_) + { + resolution_info += "\nDifferent instrument units found."; + } + + if (existing_instrument.description_ != new_instrument.description_) + { + resolution_info += + "\nDifferent instrument descriptions found. Consider configuring a View to change the " + "description of the duplicate instrument."; + } + + OTEL_INTERNAL_LOG_WARN( + "[Meter::WarnOnDuplicateInstrument] Creating a duplicate instrument of the same " + "case-insensitive name. This may cause " + "semantic errors in the data exported from this meter." + << resolution_info << "\nScope: " << InstrumentationScopeLogStreamable{*scope} + << "\nExisting instrument: " << InstrumentDescriptorLogStreamable{existing_instrument} + << "\nDuplicate instrument: " << InstrumentDescriptorLogStreamable{new_instrument}); + return; + } + } +} + +// Implementation of the log message recommended by the SDK specification for name case conflicts. +// See +// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L910 +void Meter::WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope, + const InstrumentDescriptor &existing_instrument, + const InstrumentDescriptor &new_instrument) +{ + if (InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(existing_instrument.name_, + new_instrument.name_) && + existing_instrument.name_ != new_instrument.name_) + { + OTEL_INTERNAL_LOG_WARN( + "[Meter::WarnOnNameCaseConflict] Instrument name case conflict detected on creation. " + "Returning the existing instrument with the first-seen instrument name. To resolve this " + "warning consider configuring a View to rename the duplicate instrument." + << "\nScope: " << InstrumentationScopeLogStreamable{*scope} + << "\nExisting instrument: " << InstrumentDescriptorLogStreamable{existing_instrument} + << "\nDuplicate instrument: " << InstrumentDescriptorLogStreamable{new_instrument}); + } +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 7269330708..3eb65fbe30 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -34,7 +34,8 @@ foreach( observable_registry_test periodic_exporting_metric_reader_test instrument_metadata_validator_test - metric_test_stress) + metric_test_stress + instrument_descriptor_test) add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} diff --git a/sdk/test/metrics/instrument_descriptor_test.cc b/sdk/test/metrics/instrument_descriptor_test.cc new file mode 100644 index 0000000000..ee398a2c0c --- /dev/null +++ b/sdk/test/metrics/instrument_descriptor_test.cc @@ -0,0 +1,163 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include "opentelemetry/sdk/metrics/instruments.h" + +using namespace opentelemetry::sdk::metrics; + +InstrumentDescriptor CreateInstrumentDescriptor( + const std::string &name = "counter", + const std::string &description = "description", + const std::string &unit = "unit", + InstrumentType type = InstrumentType::kCounter, + InstrumentValueType value_type = InstrumentValueType::kLong) +{ + return {name, description, unit, type, value_type}; +} + +TEST(InstrumentDescriptorUtilTest, CaseInsensitiveAsciiEquals) +{ + // same name + EXPECT_TRUE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("counter", "counter")); + + // same case-insensitive name + EXPECT_TRUE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("counter", "COUNTer")); + EXPECT_TRUE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("CountER", "counter")); + + // different case-insensitive name same string length + EXPECT_FALSE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("Counter_1", "counter_2")); + EXPECT_FALSE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("counter_1", "counter_2")); + + // different case-sensitive name different string length + EXPECT_FALSE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("counter", "Counter1")); + EXPECT_FALSE(InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals("Counter1", "counter")); +} + +// The following tests cover the spec requirements on detecting identical and duplicate instruments +// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L869 +TEST(InstrumentDescriptorUtilTest, IsDuplicate) +{ + auto instrument_existing = CreateInstrumentDescriptor("counter"); + + // not a duplicate - different name + auto instrument_different_name = instrument_existing; + instrument_different_name.name_ = "another_name"; + EXPECT_FALSE( + InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_name)); + + // not a duplicate - identical instrument + auto instrument_identical = instrument_existing; + EXPECT_FALSE(InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_identical)); + + // not a duplicate - instrument with same case-insensitive name + auto instrument_same_name_case_insensitive = instrument_existing; + instrument_same_name_case_insensitive.name_ = "COUNTER"; + EXPECT_FALSE(InstrumentDescriptorUtil::IsDuplicate(instrument_existing, + instrument_same_name_case_insensitive)); + + // is duplicate by description + auto instrument_different_desc = instrument_existing; + instrument_different_desc.description_ = "another_description"; + EXPECT_TRUE( + InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_desc)); + + // is duplicate by unit + auto instrument_different_unit = instrument_existing; + instrument_different_unit.unit_ = "another_unit"; + EXPECT_TRUE( + InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_unit)); + + // is duplicate by kind - instrument type + auto instrument_different_type = instrument_existing; + instrument_different_type.type_ = InstrumentType::kHistogram; + EXPECT_TRUE( + InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_type)); + + // is duplicate by kind - instrument value_type + auto instrument_different_valuetype = instrument_existing; + instrument_different_valuetype.value_type_ = InstrumentValueType::kDouble; + EXPECT_TRUE( + InstrumentDescriptorUtil::IsDuplicate(instrument_existing, instrument_different_valuetype)); +} + +TEST(InstrumentDescriptorTest, EqualNameCaseInsensitiveOperator) +{ + // equal by name, description, unit, type and value type + InstrumentEqualNameCaseInsensitive equal_operator{}; + auto instrument_existing = CreateInstrumentDescriptor("counter"); + auto instrument_identical = instrument_existing; + EXPECT_TRUE(equal_operator(instrument_existing, instrument_identical)); + + // equal by name with different case + auto instrument_name_case_conflict = instrument_existing; + instrument_name_case_conflict.name_ = "COUNTER"; + EXPECT_TRUE(equal_operator(instrument_existing, instrument_name_case_conflict)); + + // not equal by name + auto instrument_different_name = instrument_existing; + instrument_different_name.name_ = "another_counter"; + EXPECT_FALSE(equal_operator(instrument_existing, instrument_different_name)); + + // not equal by instrument value type + auto instrument_different_valuetype = instrument_existing; + instrument_different_valuetype.value_type_ = InstrumentValueType::kDouble; + EXPECT_FALSE(equal_operator(instrument_existing, instrument_different_valuetype)); + + // not equal by instrument type + auto instrument_different_type = instrument_existing; + instrument_different_type.type_ = InstrumentType::kObservableCounter; + EXPECT_FALSE(equal_operator(instrument_existing, instrument_different_type)); + + // not equal by description + auto instrument_different_desc = instrument_existing; + instrument_different_desc.description_ = "another description"; + EXPECT_FALSE(equal_operator(instrument_existing, instrument_different_desc)); + + // not equal by unit + auto instrument_different_unit = instrument_existing; + instrument_different_unit.unit_ = "another unit"; + EXPECT_FALSE(equal_operator(instrument_existing, instrument_different_unit)); +} + +TEST(InstrumentDescriptorTest, HashOperator) +{ + InstrumentDescriptorHash hash_operator{}; + + // identical instrument - hash must match + auto instrument_existing = CreateInstrumentDescriptor("counter"); + auto instrument_identical = instrument_existing; + EXPECT_EQ(hash_operator(instrument_existing), hash_operator(instrument_identical)); + + // name case conflict - hash must match + auto instrument_name_case_conflict = instrument_existing; + instrument_name_case_conflict.name_ = "COUNTER"; + EXPECT_EQ(hash_operator(instrument_existing), hash_operator(instrument_name_case_conflict)); + + // different name + auto instrument_different_name = instrument_existing; + instrument_different_name.name_ = "another_counter"; + EXPECT_NE(hash_operator(instrument_existing), hash_operator(instrument_different_name)); + + // different kind - instrument value type + auto instrument_different_valuetype = instrument_existing; + instrument_different_valuetype.value_type_ = InstrumentValueType::kFloat; + EXPECT_NE(hash_operator(instrument_existing), hash_operator(instrument_different_valuetype)); + + // different kind - instrument type + auto instrument_different_type = instrument_existing; + instrument_different_type.type_ = InstrumentType::kObservableUpDownCounter; + EXPECT_NE(hash_operator(instrument_existing), hash_operator(instrument_different_type)); + + // different description + auto instrument_different_desc = instrument_existing; + instrument_different_desc.description_ = "another description"; + EXPECT_NE(hash_operator(instrument_existing), hash_operator(instrument_different_desc)); + + // different unit + auto instrument_different_unit = instrument_existing; + instrument_different_unit.unit_ = "another unit"; + EXPECT_NE(hash_operator(instrument_existing), hash_operator(instrument_different_unit)); +} diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index e3335b9a0b..199ccc682c 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -32,14 +32,21 @@ #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/common/attribute_utils.h" +#include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" +#include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/metric_reader.h" +#include "opentelemetry/sdk/metrics/view/instrument_selector.h" +#include "opentelemetry/sdk/metrics/view/meter_selector.h" +#include "opentelemetry/sdk/metrics/view/view.h" using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationscope; using namespace opentelemetry::sdk::metrics; +using namespace opentelemetry::sdk::common::internal_log; namespace { @@ -86,6 +93,97 @@ std::shared_ptr GetMeterProviderWithScopeConfigurator( p->AddMetricReader(std::move(metric_reader)); return p; } + +class TestLogHandler : public LogHandler +{ +public: + void Handle(LogLevel level, + const char * /*file*/, + int /*line*/, + const char *msg, + const sdk::common::AttributeMap & /*attributes*/) noexcept override + + { + if (LogLevel::Warning == level) + { + std::cout << msg << std::endl; + warnings.push_back(msg); + } + } + + bool HasNameCaseConflictWarning() const + { + return std::any_of(warnings.begin(), warnings.end(), [](const std::string &warning) { + return warning.find("WarnOnNameCaseConflict") != std::string::npos; + }); + } + + bool HasDuplicateInstrumentWarning() const + { + return std::any_of(warnings.begin(), warnings.end(), [](const std::string &warning) { + return warning.find("WarnOnDuplicateInstrument") != std::string::npos; + }); + } + +private: + std::vector warnings; +}; + +class MeterCreateInstrumentTest : public ::testing::Test +{ +protected: + void SetUp() override + { + ASSERT_TRUE(log_handler_ != nullptr); + ASSERT_TRUE(metric_reader_ptr_ != nullptr); + ASSERT_TRUE(provider_ != nullptr); + GlobalLogHandler::SetLogHandler(std::static_pointer_cast(log_handler_)); + GlobalLogHandler::SetLogLevel(LogLevel::Warning); + + provider_->AddMetricReader(metric_reader_ptr_); + meter_ = provider_->GetMeter("test_meter"); + ASSERT_TRUE(meter_ != nullptr); + } + + void TearDown() override {} + + void AddNameCorrectionView(const std::string &name, + const std::string &unit, + InstrumentType type, + const std::string &new_name) + { + std::unique_ptr corrective_view{new View(new_name)}; + std::unique_ptr instrument_selector{ + new InstrumentSelector(type, name, unit)}; + + std::unique_ptr meter_selector{new MeterSelector("test_meter", "", "")}; + + provider_->AddView(std::move(instrument_selector), std::move(meter_selector), + std::move(corrective_view)); + } + + void AddDescriptionCorrectionView(const std::string &name, + const std::string &unit, + InstrumentType type, + const std::string &new_description) + { + std::unique_ptr corrective_view{new View(name, new_description)}; + std::unique_ptr instrument_selector{ + new InstrumentSelector(type, name, unit)}; + + std::unique_ptr meter_selector{new MeterSelector("test_meter", "", "")}; + + provider_->AddView(std::move(instrument_selector), std::move(meter_selector), + std::move(corrective_view)); + } + + std::shared_ptr provider_{new sdk::metrics::MeterProvider()}; + std::shared_ptr log_handler_{new TestLogHandler()}; + opentelemetry::nostd::shared_ptr meter_{nullptr}; + + std::shared_ptr metric_reader_ptr_{new MockMetricReader()}; +}; + } // namespace TEST(MeterTest, BasicAsyncTests) @@ -372,3 +470,384 @@ TEST(MeterTest, MeterWithCustomConfig) return true; }); } + +TEST_F(MeterCreateInstrumentTest, IdenticalSyncInstruments) +{ + auto counter1 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + auto counter2 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + + counter1->Add(1.0, {{"key", "value1"}}); + counter2->Add(2.5, {{"key", "value2"}}); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 2); + auto &point_data1 = + metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_[0].point_data; + auto &point_data2 = + metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_[1].point_data; + + auto sum_point_data1 = nostd::get(point_data1); + auto sum_point_data2 = nostd::get(point_data2); + + const double sum = + nostd::get(sum_point_data1.value_) + nostd::get(sum_point_data2.value_); + EXPECT_DOUBLE_EQ(sum, 3.5); + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, NameCaseConflictSyncInstruments) +{ + auto counter1 = meter_->CreateUInt64Counter("My_CountER", "desc", "unit"); + auto counter2 = meter_->CreateUInt64Counter("my_counter", "desc", "unit"); + + counter1->Add(1); + counter2->Add(2); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + auto &point_data = + metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_[0].point_data; + auto sum_point_data = nostd::get(point_data); + const auto sum = nostd::get(sum_point_data.value_); + EXPECT_EQ(sum, 3); + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_TRUE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, ViewCorrectedNameCaseConflictSyncInstruments) +{ + InstrumentDescriptor descriptor{"My_CountER", "desc", "unit", InstrumentType::kCounter, + InstrumentValueType::kLong}; + + AddNameCorrectionView(descriptor.name_, descriptor.unit_, descriptor.type_, "my_counter"); + + auto counter1 = + meter_->CreateUInt64Counter("My_CountER", descriptor.description_, descriptor.unit_); + auto counter2 = + meter_->CreateUInt64Counter("my_counter", descriptor.description_, descriptor.unit_); + + counter1->Add(1); + counter2->Add(2); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + auto &point_data = + metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_[0].point_data; + auto sum_point_data = nostd::get(point_data); + const auto sum = nostd::get(sum_point_data.value_); + EXPECT_EQ(sum, 3); + // no warnings expected after correction with the view + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateSyncInstrumentsByKind) +{ + auto counter1 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + auto counter2 = meter_->CreateUInt64Counter("my_counter", "desc", "unit"); + + counter1->Add(1, {{"key", "value1"}}); + counter2->Add(1, {{"key", "value2"}}); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[1].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateSyncInstrumentsByUnits) +{ + auto counter1 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + auto counter2 = meter_->CreateDoubleCounter("my_counter", "desc", "another_unit"); + + counter1->Add(1, {{"key", "value1"}}); + counter2->Add(1, {{"key", "value2"}}); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[1].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateSyncInstrumentsByDescription) +{ + auto counter1 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + auto counter2 = meter_->CreateDoubleCounter("my_counter", "another_desc", "unit"); + + counter1->Add(1, {{"key", "value1"}}); + counter2->Add(1, {{"key", "value2"}}); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[1].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, ViewCorrectedDuplicateSyncInstrumentsByDescription) +{ + InstrumentDescriptor descriptor{"my_counter", "desc", "unit", InstrumentType::kCounter, + InstrumentValueType::kDouble}; + AddDescriptionCorrectionView(descriptor.name_, descriptor.unit_, descriptor.type_, + descriptor.description_); + + auto counter1 = meter_->CreateDoubleCounter("my_counter", "desc", "unit"); + auto counter2 = meter_->CreateDoubleCounter("my_counter", "another_desc", "unit"); + + counter1->Add(1, {{"key", "value1"}}); + counter2->Add(1, {{"key", "value2"}}); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + // only one metric_data object expected after correction with the view + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 2); + // no warnings expected after correction with the view + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, IdenticalAsyncInstruments) +{ + auto observable_counter1 = + meter_->CreateInt64ObservableCounter("observable_counter", "desc", "unit"); + auto observable_counter2 = + meter_->CreateInt64ObservableCounter("observable_counter", "desc", "unit"); + + auto callback1 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_long = + nostd::get>>(observer); + observer_long->Observe(12, {{"key", "value1"}}); + }; + + auto callback2 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_long = + nostd::get>>(observer); + observer_long->Observe(2, {{"key", "value2"}}); + }; + + observable_counter1->AddCallback(callback1, nullptr); + observable_counter2->AddCallback(callback2, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + auto &point_data_attr = metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_; + EXPECT_EQ(point_data_attr.size(), 2); + + auto &point_data1 = point_data_attr[0].point_data; + auto &point_data2 = point_data_attr[1].point_data; + + auto sum_point_data1 = nostd::get(point_data1); + auto sum_point_data2 = nostd::get(point_data2); + + int64_t sum = + nostd::get(sum_point_data1.value_) + nostd::get(sum_point_data2.value_); + EXPECT_EQ(sum, 14); + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, NameCaseConflictAsyncInstruments) +{ + auto observable_counter1 = + meter_->CreateDoubleObservableCounter("OBServable_CounTER", "desc", "unit"); + auto observable_counter2 = + meter_->CreateDoubleObservableCounter("observable_counter", "desc", "unit"); + + auto callback1 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_double = + nostd::get>>(observer); + observer_double->Observe(22.22, {{"key", "value1"}}); + }; + + auto callback2 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_double = + nostd::get>>(observer); + observer_double->Observe(55.55, {{"key", "value2"}}); + }; + + observable_counter1->AddCallback(callback1, nullptr); + observable_counter2->AddCallback(callback2, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + auto &point_data_attr = metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_; + EXPECT_EQ(point_data_attr.size(), 2); + + auto &point_data1 = point_data_attr[0].point_data; + auto &point_data2 = point_data_attr[1].point_data; + auto sum_point_data1 = nostd::get(point_data1); + auto sum_point_data2 = nostd::get(point_data2); + + const double sum = + nostd::get(sum_point_data1.value_) + nostd::get(sum_point_data2.value_); + EXPECT_DOUBLE_EQ(sum, 77.77); + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_TRUE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, ViewCorrectedNameCaseConflictAsyncInstruments) +{ + AddNameCorrectionView("OBServable_CounTER", "unit", InstrumentType::kObservableCounter, + "observable_counter"); + + auto observable_counter1 = + meter_->CreateDoubleObservableCounter("OBServable_CounTER", "desc", "unit"); + auto observable_counter2 = + meter_->CreateDoubleObservableCounter("observable_counter", "desc", "unit"); + + auto callback1 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_double = + nostd::get>>(observer); + observer_double->Observe(22.22, {{"key", "value1"}}); + }; + + auto callback2 = [](opentelemetry::metrics::ObserverResult observer, void * /* state */) { + auto observer_double = + nostd::get>>(observer); + observer_double->Observe(55.55, {{"key", "value2"}}); + }; + + observable_counter1->AddCallback(callback1, nullptr); + observable_counter2->AddCallback(callback2, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + auto &point_data_attr = metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_; + EXPECT_EQ(point_data_attr.size(), 2); + + auto &point_data1 = point_data_attr[0].point_data; + auto &point_data2 = point_data_attr[1].point_data; + auto sum_point_data1 = nostd::get(point_data1); + auto sum_point_data2 = nostd::get(point_data2); + + const double sum = + nostd::get(sum_point_data1.value_) + nostd::get(sum_point_data2.value_); + EXPECT_DOUBLE_EQ(sum, 77.77); + // no warnings expected after correction with the view + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateAsyncInstrumentsByKind) +{ + auto observable_counter1 = meter_->CreateDoubleObservableCounter("observable_counter"); + auto observable_counter2 = meter_->CreateDoubleObservableGauge("observable_counter"); + + observable_counter1->AddCallback(asyc_generate_measurements_double, nullptr); + observable_counter2->AddCallback(asyc_generate_measurements_double, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateAsyncInstrumentsByUnits) +{ + auto observable_counter1 = + meter_->CreateDoubleObservableCounter("observable_counter", "desc", "unit"); + auto observable_counter2 = + meter_->CreateDoubleObservableCounter("observable_counter", "desc", "another_unit"); + + observable_counter1->AddCallback(asyc_generate_measurements_double, nullptr); + observable_counter2->AddCallback(asyc_generate_measurements_double, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, DuplicateAsyncInstrumentsByDescription) +{ + auto observable_counter1 = + meter_->CreateDoubleObservableCounter("observable_counter", "desc", "unit"); + auto observable_counter2 = + meter_->CreateDoubleObservableCounter("observable_counter", "another_desc", "unit"); + + observable_counter1->AddCallback(asyc_generate_measurements_double, nullptr); + observable_counter2->AddCallback(asyc_generate_measurements_double, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 2); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + EXPECT_TRUE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +} + +TEST_F(MeterCreateInstrumentTest, ViewCorrectedDuplicateAsyncInstrumentsByDescription) +{ + InstrumentDescriptor descriptor{"observable_counter", "desc", "unit", + InstrumentType::kObservableCounter, InstrumentValueType::kDouble}; + + AddDescriptionCorrectionView(descriptor.name_, descriptor.unit_, descriptor.type_, + descriptor.description_); + + auto observable_counter1 = meter_->CreateDoubleObservableCounter( + descriptor.name_, descriptor.description_, descriptor.unit_); + auto observable_counter2 = + meter_->CreateDoubleObservableCounter(descriptor.name_, "another_desc", descriptor.unit_); + + observable_counter1->AddCallback(asyc_generate_measurements_double, nullptr); + observable_counter2->AddCallback(asyc_generate_measurements_double, nullptr); + + metric_reader_ptr_->Collect([this](ResourceMetrics &metric_data) { + EXPECT_EQ(metric_data.scope_metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_.size(), 1); + EXPECT_EQ(metric_data.scope_metric_data_[0].metric_data_[0].point_data_attr_.size(), 1); + // no warnings expected after correction with the view + EXPECT_FALSE(log_handler_->HasDuplicateInstrumentWarning()); + EXPECT_FALSE(log_handler_->HasNameCaseConflictWarning()); + return true; + }); +}