[API] fix the trace state regex to comply with the w3c trace context level 2 (#4194)

Author: dbarker

CHANGELOG.md

@@ -15,6 +15,15 @@ Increment the:
 
 ## [Unreleased]
 
+* [API] Fix `TraceState::IsValidKey()` to comply with the W3C Trace Context
+  Level 2, where keys containing `@` and keys with more than 241 characters
+  before `@` or more than 14 characters after `@` are now accepted.
+  Only the total 256-character key length limit is enforced.
+  **Note**: this is a correctness fix to an inline API header; the observable
+  behavior of `IsValidKey`, `IsValidKeyRegEx`, and `IsValidKeyNonRegEx`
+  changes (see `docs/abi-policy.md`).
+  [#4194](https://github.com/open-telemetry/opentelemetry-cpp/pull/4194)
+
 * [SDK] Add `TracerProvider::UpdateTracerConfigurator()` and example
   [#4065](https://github.com/open-telemetry/opentelemetry-cpp/issues/4065)
 

api/include/opentelemetry/trace/trace_state.h

@@ -205,9 +205,9 @@ class OPENTELEMETRY_EXPORT TraceState
   /** Returns whether key is a valid key. See https://www.w3.org/TR/trace-context/#key
    * Identifiers MUST begin with a lowercase letter or a digit, and can only contain
    * lowercase letters (a-z), digits (0-9), underscores (_), dashes (-), asterisks (*),
-   * and forward slashes (/).
-   * For multi-tenant vendor scenarios, an at sign (@) can be used to prefix the vendor name.
-   *
+   * forward slashes (/), and at signs (@).
+   * An at sign (@) is treated as a regular character (keychar) with no structural meaning.
+   * Total key length must not exceed 256 characters.
    */
   static bool IsValidKey(nostd::string_view key) noexcept
   {
@@ -255,15 +255,9 @@ class OPENTELEMETRY_EXPORT TraceState
     try
     {
 #  endif
-      static std::regex reg_key("^[a-z0-9][a-z0-9*_\\-/]{0,255}$");
-      static std::regex reg_key_multitenant(
-          "^[a-z0-9][a-z0-9*_\\-/]{0,240}(@)[a-z0-9][a-z0-9*_\\-/]{0,13}$");
+      static std::regex reg_key("^[a-z0-9][a-z0-9*_\\-/@]{0,255}$");
       std::string key_s(key.data(), key.size());
-      if (std::regex_match(key_s, reg_key) || std::regex_match(key_s, reg_key_multitenant))
-      {
-        return true;
-      }
-      return false;
+      return std::regex_match(key_s, reg_key);
 #  if OPENTELEMETRY_HAVE_EXCEPTIONS
     }
     catch (const std::regex_error &)
@@ -300,18 +294,12 @@ class OPENTELEMETRY_EXPORT TraceState
       return false;
     }
 
-    int ats = 0;
-
     for (const char c : key)
     {
       if (!IsLowerCaseAlphaOrDigit(c) && c != '_' && c != '-' && c != '@' && c != '*' && c != '/')
       {
         return false;
       }
-      if ((c == '@') && (++ats > 1))
-      {
-        return false;
-      }
     }
     return true;
   }

api/test/trace/trace_state_test.cc

@@ -62,6 +62,9 @@ TEST(TraceStateTest, ValidateHeaderParsing)
                    {"k1=v1,k2=v2,invalidmember", ""},
                    {"1a-2f@foo=bar1,a*/foo-_/bar=bar4", "1a-2f@foo=bar1,a*/foo-_/bar=bar4"},
                    {"1a-2f@foo=bar1,*/foo-_/bar=bar4", ""},
+                   {"foo@@bar=1,baz=2", "foo@@bar=1,baz=2"},
+                   {"foo@bar@baz=1", "foo@bar@baz=1"},
+                   {"@foo=1", ""},
                    {",k1=v1", "k1=v1"},
                    {",", ""},
                    {",=,", ""},
@@ -71,6 +74,12 @@ TEST(TraceStateTest, ValidateHeaderParsing)
   {
     EXPECT_EQ(create_ts_return_header(testcase.input), testcase.expected);
   }
+  // Key length boundaries: 256-char key valid, 257-char invalid
+  EXPECT_NE(create_ts_return_header(std::string(256, 'z') + "=1"), "");
+  EXPECT_EQ(create_ts_return_header(std::string(257, 'z') + "=1"), "");
+  // Old vendor-section limits (241 before @, 14 after) are gone; only total length matters
+  EXPECT_NE(create_ts_return_header(std::string(242, 't') + "@v=1"), "");  // 244-char key with @
+  EXPECT_NE(create_ts_return_header("t@" + std::string(15, 'v') + "=1"), "");  // 15 chars after @
 }
 
 TEST(TraceStateTest, ExceedsMaxKeyValuePairs)
@@ -165,10 +174,23 @@ TEST(TraceStateTest, GetAllEntries)
 TEST(TraceStateTest, IsValidKey)
 {
   EXPECT_TRUE(TraceState::IsValidKey("valid-key23/*"));
+  // @ is a regular keychar per W3C Trace Context Level 2
+  EXPECT_TRUE(TraceState::IsValidKey("foo@"));
+  EXPECT_TRUE(TraceState::IsValidKey("foo@@bar"));
+  EXPECT_TRUE(TraceState::IsValidKey("foo@bar@baz"));
+  EXPECT_FALSE(TraceState::IsValidKey("@foo"));  // must start with lcalpha or DIGIT
   EXPECT_FALSE(TraceState::IsValidKey("Invalid_key"));
   EXPECT_FALSE(TraceState::IsValidKey("invalid$Key&"));
   EXPECT_FALSE(TraceState::IsValidKey(""));
   EXPECT_FALSE(TraceState::IsValidKey(kLongString));
+  // Key length boundary: exactly 256 chars valid, 257 invalid
+  EXPECT_TRUE(TraceState::IsValidKey(std::string(256, 'z')));
+  EXPECT_FALSE(TraceState::IsValidKey(std::string(257, 'z')));
+  // Old vendor-section limits (241 before @, 14 after) are gone; only total length matters
+  EXPECT_TRUE(
+      TraceState::IsValidKey(std::string(241, 't') + "@" + std::string(14, 'v')));  // 256 chars
+  EXPECT_TRUE(TraceState::IsValidKey(std::string(242, 't') + "@v"));                // 244 chars
+  EXPECT_TRUE(TraceState::IsValidKey("t@" + std::string(15, 'v')));                 // 17 chars
 }
 
 TEST(TraceStateTest, IsValidValue)