From 3267cce7fc0b02cb04744e676aa0d73fb7e2aa45 Mon Sep 17 00:00:00 2001 From: Kyle Loveless Date: Tue, 5 Dec 2023 12:39:03 -0500 Subject: [PATCH] Improve the handling of OPENTELEMETRY_HAVE_WORKING_REGEX. It used to be some places were checking for defined(OPENTELEMETRY_HAVE_WORKING_REGEX), but it is always being set (to either 0 or 1) in api/include/opentelemetry/common/macros.h. Thus move to just checking its value everywhere. Additionally, even if we did have the feature off - compilation would fail, since not all the functions were ifdef'd. My initial motivation for this was to reduce compile times, since specificing the definition with an std::regex adds a significant penalty to each translation unit (~700ms on my 5-year old laptop). In a separate patch, I think this should be moved to a .cc file - happy to receive advice on how (since api/ seems not to have .cc's). --- api/include/opentelemetry/trace/trace_state.h | 6 ++++-- sdk/include/opentelemetry/sdk/metrics/view/predicate.h | 2 +- sdk/src/metrics/instrument_metadata_validator.cc | 2 +- sdk/test/metrics/histogram_test.cc | 6 +++--- sdk/test/metrics/view_registry_test.cc | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/include/opentelemetry/trace/trace_state.h b/api/include/opentelemetry/trace/trace_state.h index f424b01997..8a25f4da5c 100644 --- a/api/include/opentelemetry/trace/trace_state.h +++ b/api/include/opentelemetry/trace/trace_state.h @@ -14,7 +14,7 @@ #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/version.h" -#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif @@ -246,6 +246,7 @@ class OPENTELEMETRY_EXPORT TraceState return str.substr(left, right - left + 1); } +#if OPENTELEMETRY_HAVE_WORKING_REGEX static bool IsValidKeyRegEx(nostd::string_view key) { static std::regex reg_key("^[a-z0-9][a-z0-9*_\\-/]{0,255}$"); @@ -267,7 +268,7 @@ class OPENTELEMETRY_EXPORT TraceState // Need to benchmark without regex, as a string object is created here. return std::regex_match(std::string(value.data(), value.size()), reg_value); } - +#else static bool IsValidKeyNonRegEx(nostd::string_view key) { if (key.empty() || key.size() > kKeyMaxSize || !IsLowerCaseAlphaOrDigit(key[0])) @@ -307,6 +308,7 @@ class OPENTELEMETRY_EXPORT TraceState } return true; } +#endif static bool IsLowerCaseAlphaOrDigit(char c) { return isdigit(c) || islower(c); } diff --git a/sdk/include/opentelemetry/sdk/metrics/view/predicate.h b/sdk/include/opentelemetry/sdk/metrics/view/predicate.h index 5346a753ff..cfa1b16eb3 100644 --- a/sdk/include/opentelemetry/sdk/metrics/view/predicate.h +++ b/sdk/include/opentelemetry/sdk/metrics/view/predicate.h @@ -8,7 +8,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" -#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif diff --git a/sdk/src/metrics/instrument_metadata_validator.cc b/sdk/src/metrics/instrument_metadata_validator.cc index ff963c3dd1..0b00404447 100644 --- a/sdk/src/metrics/instrument_metadata_validator.cc +++ b/sdk/src/metrics/instrument_metadata_validator.cc @@ -8,7 +8,7 @@ #include #include -#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif diff --git a/sdk/test/metrics/histogram_test.cc b/sdk/test/metrics/histogram_test.cc index a11a9c1f59..4daceb685b 100644 --- a/sdk/test/metrics/histogram_test.cc +++ b/sdk/test/metrics/histogram_test.cc @@ -13,7 +13,7 @@ #include -#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif @@ -73,7 +73,7 @@ TEST(Histogram, Double) actual.counts_); } -#if (OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX // FIXME - View Preficate search is only supported through regex TEST(Histogram, DoubleCustomBuckets) { @@ -188,7 +188,7 @@ TEST(Histogram, UInt64) actual.counts_); } -#if (OPENTELEMETRY_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 49e475a9f1..fb188e7124 100644 --- a/sdk/test/metrics/view_registry_test.cc +++ b/sdk/test/metrics/view_registry_test.cc @@ -8,7 +8,7 @@ #include -#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX) +#if OPENTELEMETRY_HAVE_WORKING_REGEX # include #endif