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).
open-telemetry · Dec 5, 2023 · 3267cce · 3267cce
1 parent d1143ab
commit 3267cce
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 8 deletions.
diff --git 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 <regex>
 #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
@@ -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 <regex>
 #endif
 

diff --git a/sdk/src/metrics/instrument_metadata_validator.cc b/sdk/src/metrics/instrument_metadata_validator.cc
@@ -8,7 +8,7 @@
 #include <algorithm>
 #include <iostream>
 
-#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX)
+#if OPENTELEMETRY_HAVE_WORKING_REGEX
 #  include <regex>
 #endif
 

diff --git a/sdk/test/metrics/histogram_test.cc b/sdk/test/metrics/histogram_test.cc
@@ -13,7 +13,7 @@
 
 #include <gtest/gtest.h>
 
-#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX)
+#if OPENTELEMETRY_HAVE_WORKING_REGEX
 #  include <regex>
 #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
@@ -8,7 +8,7 @@
 
 #include <gtest/gtest.h>
 
-#if defined(OPENTELEMETRY_HAVE_WORKING_REGEX)
+#if OPENTELEMETRY_HAVE_WORKING_REGEX
 #  include <regex>
 #endif