Improve W3C Trace Context compliance (#343)

* improve W3C TraceContext compliance

* improve W3C TraceContext compliance

* improve code coverage

* fix lint

Author: reyang

opencensus/trace/propagation/trace_context_http_header_format.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import logging
 import re
 
 from opencensus.trace.span_context import SpanContext
@@ -22,64 +21,15 @@
 
 _TRACEPARENT_HEADER_NAME = 'traceparent'
 _TRACESTATE_HEADER_NAME = 'tracestate'
-_TRACE_CONTEXT_HEADER_FORMAT = \
-    '([0-9a-f]{2})(-([0-9a-f]{32}))(-([0-9a-f]{16}))?(-([0-9a-f]{2}))?'
-_TRACE_CONTEXT_HEADER_RE = re.compile(_TRACE_CONTEXT_HEADER_FORMAT)
+_TRACEPARENT_HEADER_FORMAT = \
+    '^[ \t]*([0-9a-f]{2})-([0-9a-f]{32})-([0-9a-f]{16})-([0-9a-f]{2})' + \
+    '(-.*)?[ \t]*$'
+_TRACEPARENT_HEADER_FORMAT_RE = re.compile(_TRACEPARENT_HEADER_FORMAT)
 
 
 class TraceContextPropagator(object):
     """Propagator for processing the trace context HTTP header format."""
 
-    def from_header(self, header):
-        """Generate a SpanContext object using the trace context header.
-
-        :type header: str
-        :param header: Trace context header which was extracted from the HTTP
-                       request headers.
-
-        :rtype: :class:`~opencensus.trace.span_context.SpanContext`
-        :returns: SpanContext generated from the trace context header.
-        """
-        if header is None:
-            return SpanContext()
-
-        try:
-            match = re.search(_TRACE_CONTEXT_HEADER_RE, header)
-        except TypeError:
-            logging.warning(
-                'Header should be str, got {}. Cannot parse the header.'
-                .format(header.__class__.__name__))
-            raise
-
-        if match:
-            version = match.group(1)
-
-            if version == '00':
-                trace_id = match.group(3)
-                span_id = match.group(5)
-                trace_options = match.group(7)
-
-                if trace_options is None:
-                    trace_options = 1
-
-                # Need to convert span_id from hex string to int
-                span_context = SpanContext(
-                    trace_id=trace_id,
-                    span_id=span_id,
-                    trace_options=TraceOptions(trace_options),
-                    from_header=True)
-                return span_context
-            else:
-                logging.warning(
-                    'Header format version {} is not supported, generate a new'
-                    'context instead.'.format(version))
-        else:
-            logging.warning(
-                'Cannot parse the header {}, generate a new context instead.'
-                .format(header))
-
-        return SpanContext()
-
     def from_headers(self, headers):
         """Generate a SpanContext object using the W3C Distributed Tracing headers.
 
@@ -91,42 +41,46 @@ def from_headers(self, headers):
         """
         if headers is None:
             return SpanContext()
+
         header = headers.get(_TRACEPARENT_HEADER_NAME)
         if header is None:
             return SpanContext()
-        header = str(header.encode('utf-8'))
-
-        span_context = self.from_header(header)
 
-        header = headers.get(_TRACESTATE_HEADER_NAME)
-        if header is not None:
-            span_context.tracestate = \
-                TracestateStringFormatter().from_string(header)
-
-        return span_context
+        match = re.search(_TRACEPARENT_HEADER_FORMAT_RE, header)
+        if not match:
+            return SpanContext()
 
-    def to_header(self, span_context):
-        """Convert a SpanContext object to header string, using version 0.
+        version = match.group(1)
+        trace_id = match.group(2)
+        span_id = match.group(3)
+        trace_options = match.group(4)
 
-        :type span_context:
-            :class:`~opencensus.trace.span_context.SpanContext`
-        :param span_context: SpanContext object.
+        if trace_id == '0' * 32 or span_id == '0' * 16:
+            return SpanContext()
 
-        :rtype: str
-        :returns: A trace context header string in trace context HTTP format.
-        """
-        trace_id = span_context.trace_id
-        span_id = span_context.span_id
-        trace_options = span_context.trace_options.enabled
+        if version == '00':
+            if match.group(5):
+                return SpanContext()
+        if version == 'ff':
+            return SpanContext()
 
-        # Convert the trace options
-        trace_options = '01' if trace_options else '00'
+        span_context = SpanContext(
+            trace_id=trace_id,
+            span_id=span_id,
+            trace_options=TraceOptions(trace_options),
+            from_header=True)
 
-        header = '00-{}-{}-{}'.format(
-            trace_id,
-            span_id,
-            trace_options)
-        return header
+        header = headers.get(_TRACESTATE_HEADER_NAME)
+        if header is None:
+            return span_context
+        try:
+            tracestate = TracestateStringFormatter().from_string(header)
+            if tracestate.is_valid():
+                span_context.tracestate = \
+                    TracestateStringFormatter().from_string(header)
+        except ValueError:
+            pass
+        return span_context
 
     def to_headers(self, span_context):
         """Convert a SpanContext object to W3C Distributed Tracing headers,
@@ -139,8 +93,19 @@ def to_headers(self, span_context):
         :rtype: dict
         :returns: W3C Distributed Tracing headers.
         """
+        trace_id = span_context.trace_id
+        span_id = span_context.span_id
+        trace_options = span_context.trace_options.enabled
+
+        # Convert the trace options
+        trace_options = '01' if trace_options else '00'
+
         headers = {
-            _TRACEPARENT_HEADER_NAME: self.to_header(span_context),
+            _TRACEPARENT_HEADER_NAME: '00-{}-{}-{}'.format(
+                trace_id,
+                span_id,
+                trace_options
+            ),
         }
         tracestate = span_context.tracestate
         if tracestate:

opencensus/trace/propagation/tracestate_string_format.py

@@ -32,6 +32,8 @@ def from_string(self, string):
             if not match:
                 raise ValueError('illegal key-value format %r' % (member))
             key, eq, value = match.groups()
+            if key in tracestate:
+                raise ValueError('conflict key {!r}'.format(key))
             tracestate[key] = value
         return tracestate
 

opencensus/trace/tracestate.py

@@ -15,8 +15,12 @@
 from collections import OrderedDict
 import re
 
-_KEY_FORMAT = r'[a-z][_0-9a-z\-\*\/]{0,255}'
-_VALUE_FORMAT = r'[\x20-\x2b\x2d-\x3c\x3e-\x7e]{1,256}'
+_KEY_WITHOUT_VENDOR_FORMAT = r'[a-z][_0-9a-z\-\*\/]{0,255}'
+_KEY_WITH_VENDOR_FORMAT = \
+    r'[a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}'
+_KEY_FORMAT = _KEY_WITHOUT_VENDOR_FORMAT + '|' + _KEY_WITH_VENDOR_FORMAT
+_VALUE_FORMAT = \
+    r'[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]'
 
 _KEY_VALIDATION_RE = re.compile('^' + _KEY_FORMAT + '$')
 _VALUE_VALIDATION_RE = re.compile('^' + _VALUE_FORMAT + '$')