fix: "tracestate" parsing must allow whitespace around list-members (#1728)

* fix: "tracestate" parsing must allow whitespace around list-members

Per https://w3c.github.io/trace-context/#tracestate-header-field-values
the tracestate header can have whitespace around list-members. E.g.
    'vendor1=foo, vendor2=bar ,\t vendor3=baz '

Refs: elastic/apm-agent-rum-js#1334

* I think this is what the pre-commit test failure in CI is complaining about.

* Remove superfluous question mark

* Use the same regex for tracestate reconstruction

* Strip optional whitespace around tracestate

* Revert my removal of the `*?` in Trent's original regex

* CHANGELOG

---------

Co-authored-by: Colton Myers <[email protected]>

Author: trentm

CHANGELOG.asciidoc

@@ -43,6 +43,7 @@ endif::[]
 ===== Bug fixes
 
 * Fix an async issue with long elasticsearch queries {pull}1725[#1725]
+* Fix a minor inconsistency with the W3C tracestate spec {pull}1728[#1728]
 
 
 [[release-notes-6.x]]

elasticapm/utils/disttracing.py

@@ -199,7 +199,11 @@ def _parse_tracestate(self, tracestate) -> Dict[str, str]:
         made up of key:value pairs, separated by semicolons. It is meant to
         be parsed into a dict.
 
-            tracestate: es=key:value;key:value...,othervendor=<opaque>
+            tracestate: es=key:value;key:value... , othervendor=<opaque>
+
+        Per https://w3c.github.io/trace-context/#tracestate-header-field-values
+        there can be optional whitespace (OWS) between the comma-separated
+        list-members.
         """
         if not tracestate:
             return {}
@@ -208,7 +212,7 @@ def _parse_tracestate(self, tracestate) -> Dict[str, str]:
 
         ret = {}
         try:
-            state = re.search(r"(?:,|^)es=([^,]*)", tracestate).group(1).split(";")
+            state = re.search(r"(?:,|^)\s*es=([^,]*?)\s*(?:,|$)", tracestate).group(1).split(";")
         except IndexError:
             return {}
         for keyval in state:
@@ -230,8 +234,9 @@ def _set_tracestate(self):
             return elastic_state
         else:
             # Remove es=<stuff> from the tracestate, and add the new es state to the end
-            otherstate = re.sub(r"(?:,|^)es=([^,]*)", "", self.tracestate)
-            otherstate = otherstate.lstrip(",")
+            otherstate = re.sub(r"(?:,|^)\s*es=([^,]*?)\s*(?:,|$)", "", self.tracestate)
+            otherstate = otherstate.lstrip(",")  # in case `es=` was the first entry
+            otherstate = re.sub(r",,", ",", otherstate)  # remove potential double commas
             # No validation of `otherstate` required, since we're downstream. We only need to check `es=`
             # since we introduced it, and that validation has already been done at this point.
             if otherstate:

tests/utils/test_disttracing_header.py

@@ -125,7 +125,7 @@ def test_trace_parent_binary_invalid_length(caplog):
     "state_header",
     [
         "es=foo:bar;baz:qux,othervendor=<opaque>",
-        "snes=x:y,es=foo:bar;baz:qux,othervendor=<opaque>",
+        "snes=x:y, es=foo:bar;baz:qux ,\tothervendor=<opaque>",  # whitespace (OWS)
         "othervendor=<opaque>,es=foo:bar;baz:qux",
     ],
 )