From 67875c2071ecf0eea8ff9dfc589e484e95d03c89 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Wed, 11 Jan 2023 14:25:44 -0800 Subject: [PATCH 1/2] Make macros.h available for all source files via version.h --- api/include/opentelemetry/common/macros.h | 2 -- api/include/opentelemetry/version.h | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/include/opentelemetry/common/macros.h b/api/include/opentelemetry/common/macros.h index 4741677a74..e8264a4b07 100644 --- a/api/include/opentelemetry/common/macros.h +++ b/api/include/opentelemetry/common/macros.h @@ -5,8 +5,6 @@ #include -#include "opentelemetry/version.h" - #if !defined(OPENTELEMETRY_LIKELY_IF) && defined(__cplusplus) // GCC 9 has likely attribute but do not support declare it at the beginning of statement # if defined(__has_cpp_attribute) && (defined(__clang__) || !defined(__GNUC__) || __GNUC__ > 9) diff --git a/api/include/opentelemetry/version.h b/api/include/opentelemetry/version.h index 35a0a2baff..01ea4d8e67 100644 --- a/api/include/opentelemetry/version.h +++ b/api/include/opentelemetry/version.h @@ -3,6 +3,7 @@ #pragma once +#include "opentelemetry/common/macros.h" #include "opentelemetry/detail/preprocessor.h" #define OPENTELEMETRY_ABI_VERSION_NO 1 From e03aef5a372ca8ee9125b4b27e7ba0ead4c5204b Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Tue, 17 Jan 2023 12:00:57 -0800 Subject: [PATCH 2/2] Address feedback --- api/include/opentelemetry/common/macros.h | 7 ++----- api/include/opentelemetry/trace/trace_state.h | 16 ++++++---------- .../sdk/metrics/instrument_metadata_validator.h | 4 ++-- .../opentelemetry/sdk/metrics/view/predicate.h | 8 ++++++-- sdk/src/metrics/instrument_metadata_validator.cc | 10 +++++++--- sdk/test/metrics/histogram_test.cc | 8 ++++++-- sdk/test/metrics/view_registry_test.cc | 12 ++++++++---- 7 files changed, 37 insertions(+), 28 deletions(-) diff --git a/api/include/opentelemetry/common/macros.h b/api/include/opentelemetry/common/macros.h index e8264a4b07..0f8e57ccc0 100644 --- a/api/include/opentelemetry/common/macros.h +++ b/api/include/opentelemetry/common/macros.h @@ -3,8 +3,6 @@ #pragma once -#include - #if !defined(OPENTELEMETRY_LIKELY_IF) && defined(__cplusplus) // GCC 9 has likely attribute but do not support declare it at the beginning of statement # if defined(__has_cpp_attribute) && (defined(__clang__) || !defined(__GNUC__) || __GNUC__ > 9) @@ -90,10 +88,9 @@ // Regex support #if (__GNUC__ == 4 && (__GNUC_MINOR__ == 8 || __GNUC_MINOR__ == 9)) -# define HAVE_WORKING_REGEX 0 +# define OPENTELEMETRY_HAVE_WORKING_REGEX 0 #else -# include -# define HAVE_WORKING_REGEX 1 +# define OPENTELEMETRY_HAVE_WORKING_REGEX 1 #endif /* clang-format off */ diff --git a/api/include/opentelemetry/trace/trace_state.h b/api/include/opentelemetry/trace/trace_state.h index 496cde30c4..886ad259d7 100644 --- a/api/include/opentelemetry/trace/trace_state.h +++ b/api/include/opentelemetry/trace/trace_state.h @@ -7,20 +7,16 @@ #include #include -#include -#if (__GNUC__ == 4 && (__GNUC_MINOR__ == 8 || __GNUC_MINOR__ == 9)) -# define HAVE_WORKING_REGEX 0 -#else -# define HAVE_WORKING_REGEX 1 -#endif - #include "opentelemetry/common/kv_properties.h" -#include "opentelemetry/common/macros.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" +#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +# include +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace trace { @@ -212,7 +208,7 @@ class TraceState */ static bool IsValidKey(nostd::string_view key) { -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX return IsValidKeyRegEx(key); #else return IsValidKeyNonRegEx(key); @@ -225,7 +221,7 @@ class TraceState */ static bool IsValidValue(nostd::string_view value) { -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX return IsValidValueRegEx(value); #else return IsValidValueNonRegEx(value); diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h b/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h index 0a8362e9e9..95848a8eec 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h @@ -8,7 +8,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif @@ -26,7 +26,7 @@ class InstrumentMetaDataValidator bool ValidateDescription(nostd::string_view description) const; private: -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX const std::regex name_reg_key_; const std::regex unit_reg_key_; #endif diff --git a/sdk/include/opentelemetry/sdk/metrics/view/predicate.h b/sdk/include/opentelemetry/sdk/metrics/view/predicate.h index b5dafcf51a..5346a753ff 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/predicate.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/predicate.h @@ -8,6 +8,10 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" +#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +# include +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -26,7 +30,7 @@ class PatternPredicate : public Predicate PatternPredicate(opentelemetry::nostd::string_view pattern) : reg_key_{pattern.data()} {} bool Match(opentelemetry::nostd::string_view str) const noexcept override { -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX return std::regex_match(str.data(), reg_key_); #else // TBD - Support regex match for GCC4.8 @@ -37,7 +41,7 @@ class PatternPredicate : public Predicate } private: -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX std::regex reg_key_; #else std::string reg_key_; diff --git a/sdk/src/metrics/instrument_metadata_validator.cc b/sdk/src/metrics/instrument_metadata_validator.cc index 50ee98e68a..4fde7d3f26 100644 --- a/sdk/src/metrics/instrument_metadata_validator.cc +++ b/sdk/src/metrics/instrument_metadata_validator.cc @@ -8,6 +8,10 @@ #include #include +#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +# include +#endif + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -20,7 +24,7 @@ const std::string kInstrumentUnitPattern = "[\x01-\x7F]{0,63}"; // instrument-unit = It can have a maximum length of 63 ASCII chars InstrumentMetaDataValidator::InstrumentMetaDataValidator() -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX // clang-format off : name_reg_key_{kInstrumentNamePattern}, unit_reg_key_{kInstrumentUnitPattern} @@ -31,7 +35,7 @@ InstrumentMetaDataValidator::InstrumentMetaDataValidator() bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const { -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX return std::regex_match(name.data(), name_reg_key_); #else const size_t kMaxSize = 63; @@ -53,7 +57,7 @@ bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const bool InstrumentMetaDataValidator::ValidateUnit(nostd::string_view unit) const { -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX return std::regex_match(unit.data(), unit_reg_key_); #else const size_t kMaxSize = 63; diff --git a/sdk/test/metrics/histogram_test.cc b/sdk/test/metrics/histogram_test.cc index ec77d99733..ee3aa939b6 100644 --- a/sdk/test/metrics/histogram_test.cc +++ b/sdk/test/metrics/histogram_test.cc @@ -11,6 +11,10 @@ #include +#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +# include +#endif + using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationscope; using namespace opentelemetry::sdk::metrics; @@ -111,7 +115,7 @@ TEST(Histogram, Double) actual.counts_); } -#if (HAVE_WORKING_REGEX) +#if (OPENTELEMETRY_HAVE_WORKING_REGEX) // FIXME - View Preficate search is only supported through regex TEST(Histogram, DoubleCustomBuckets) { @@ -223,7 +227,7 @@ TEST(Histogram, UInt64) actual.counts_); } -#if (HAVE_WORKING_REGEX) +#if (OPENTELEMETRY_HAVE_WORKING_REGEX) // FIXME - View Preficate search is only supported through regex TEST(Histogram, UInt64CustomBuckets) { diff --git a/sdk/test/metrics/view_registry_test.cc b/sdk/test/metrics/view_registry_test.cc index 6c07b639ce..59899dedb0 100644 --- a/sdk/test/metrics/view_registry_test.cc +++ b/sdk/test/metrics/view_registry_test.cc @@ -8,6 +8,10 @@ #include +#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +# include +#endif + using namespace opentelemetry::sdk::metrics; using namespace opentelemetry::sdk::instrumentationscope; @@ -28,14 +32,14 @@ TEST(ViewRegistry, FindViewsEmptyRegistry) registry.FindViews(default_instrument_descriptor, *default_instrumentation_scope.get(), [&count](const View &view) { count++; -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX EXPECT_EQ(view.GetName(), ""); EXPECT_EQ(view.GetDescription(), ""); #endif EXPECT_EQ(view.GetAggregationType(), AggregationType::kDefault); return true; }); -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX EXPECT_EQ(count, 1); EXPECT_EQ(status, true); #endif @@ -73,13 +77,13 @@ TEST(ViewRegistry, FindNonExistingView) registry.FindViews(default_instrument_descriptor, *default_instrumentation_scope.get(), [&count, &view_name, &view_description](const View &view) { count++; -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX EXPECT_EQ(view.GetName(), view_name); EXPECT_EQ(view.GetDescription(), view_description); #endif return true; }); -#if HAVE_WORKING_REGEX +#if OPENTELEMETRY_HAVE_WORKING_REGEX EXPECT_EQ(count, 1); EXPECT_EQ(status, true); #endif