fix(w3c trace context): do not pass down unknown flags.

The W3C trace context specification mandates that a participant must
only send known flags downstream. See
https://www.w3.org/TR/trace-context/#other-flags: "The behavior of other
flags, such as (00000100) is not defined and is reserved for future use.
Vendors MUST set those to zero."

In particular, this commit changes the way traceparent.py handles the
flags. Instead of persisting the complete flags field from traceparent,
we specifically parse the flag(s) that we understand and keep them as
individual boolean attributes. This will also make it easier to
handle the random trace ID flag correctly when updating support to W3C
trace context level 2.

In addition we now correctly pass down the version field as 00, since
that is the traceparent version we support (instead of passing down the
incoming version value).

Signed-off-by: Bastian Krol <[email protected]>

Author: Bastian Krol

instana/w3c_trace_context/traceparent.py

@@ -4,6 +4,8 @@
 from ..log import logger
 import re
 
+# See https://www.w3.org/TR/trace-context-2/#trace-flags for details on the bitmasks.
+SAMPLED_BITMASK = 0b1;
 
 class Traceparent:
     SPECIFICATION_VERSION = "00"
@@ -27,15 +29,16 @@ def get_traceparent_fields(traceparent):
         """
         Parses the validated traceparent header into its fields and returns the fields
         :param traceparent: the original validated traceparent header
-        :return: version, trace_id, parent_id, trace_flags
+        :return: version, trace_id, parent_id, sampled_flag
         """
         try:
             traceparent_properties = traceparent.split("-")
             version = traceparent_properties[0]
             trace_id = traceparent_properties[1]
             parent_id = traceparent_properties[2]
-            trace_flags = traceparent_properties[3]
-            return version, trace_id, parent_id, trace_flags
+            flags = int(traceparent_properties[3])
+            sampled_flag = (flags & SAMPLED_BITMASK) == SAMPLED_BITMASK
+            return version, trace_id, parent_id, sampled_flag
         except Exception:  # This method is intended to be called with a version 00 validated traceparent
             # This exception handling is added just for making sure we do not throw any unhandled exception
             # if somebody calls the method in the future without a validated traceparent
@@ -51,21 +54,24 @@ def update_traceparent(self, traceparent, in_trace_id, in_span_id, level):
         :param level: instana level, used to determine the value of sampled flag of the traceparent header
         :return: the updated traceparent header
         """
-        mask = 1 << 0
-        trace_flags = 0
         if traceparent is None:  # modify the trace_id part only when it was not present at all
             trace_id = in_trace_id.zfill(32)
-            version = self.SPECIFICATION_VERSION
         else:
-            version, trace_id, _, trace_flags = self.get_traceparent_fields(traceparent)
-            trace_flags = int(trace_flags, 16)
+            # - We do not need the incoming upstream parent span ID for the header we sent downstream.
+            # - We also do not care about the incoming version: The version field we sent downstream needs to match the
+            #   format of the traceparent header we produce here, so we always send the version _we_ support downstream,
+            #   even if the header coming from upstream supported a different version.
+            # - Finally, we also do not care about the incoming sampled flag , we only need to communicate our own
+            #   sampling decision downstream. The sampling decisions from our upstream is irrelevant for what we send
+            #   downstream.
+            _, trace_id, _, _ = self.get_traceparent_fields(traceparent)
 
         parent_id = in_span_id.zfill(16)
-        trace_flags = (trace_flags & ~mask) | ((level << 0) & mask)
-        trace_flags = format(trace_flags, '0>2x')
+        flags = level & SAMPLED_BITMASK
+        flags = format(flags, '0>2x')
 
-        traceparent = "{version}-{traceid}-{parentid}-{trace_flags}".format(version=version,
+        traceparent = "{version}-{traceid}-{parentid}-{flags}".format(version=self.SPECIFICATION_VERSION,
                                                                         traceid=trace_id,
                                                                         parentid=parent_id,
-                                                                        trace_flags=trace_flags)
+                                                                        flags=flags)
         return traceparent

tests/frameworks/test_django.py

@@ -338,7 +338,9 @@ def test_with_incoming_context(self):
         self.assertEqual('1', response.headers['X-INSTANA-L'])
 
         assert ('traceparent' in response.headers)
-        self.assertEqual('01-4bf92f3577b34da6a3ce929d0e0e4736-{}-01'.format(django_span.s),
+        # The incoming traceparent header had version 01 (which does not exist at the time of writing), but since we
+        # support version 00, we also need to pass down 00 for the version field.
+        self.assertEqual('00-4bf92f3577b34da6a3ce929d0e0e4736-{}-01'.format(django_span.s),
                          response.headers['traceparent'])
 
         assert ('tracestate' in response.headers)

tests/propagators/test_http_propagator.py

@@ -28,7 +28,7 @@ def test_extract_carrier_dict(self, mock_validate, mock_get_traceparent_fields):
             'X-INSTANA-L': '1, correlationType=web; correlationId=1234567890abcdef'
         }
         mock_validate.return_value = '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01'
-        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", "01"]
+        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", True]
         ctx = self.hptc.extract(carrier)
         self.assertEqual(ctx.correlation_id, '1234567890abcdef')
         self.assertEqual(ctx.correlation_type, "web")
@@ -54,7 +54,7 @@ def test_extract_carrier_list(self, mock_validate, mock_get_traceparent_fields):
                    ('X-INSTANA-L', '1')]
 
         mock_validate.return_value = '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01'
-        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", "01"]
+        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", True]
         ctx = self.hptc.extract(carrier)
         self.assertIsNone(ctx.correlation_id)
         self.assertIsNone(ctx.correlation_type)
@@ -124,7 +124,7 @@ def test_extract_carrier_dict_corrupted_level_header(self, mock_validate, mock_g
             'X-INSTANA-L': '1, correlationTypeweb; correlationId1234567890abcdef'
         }
         mock_validate.return_value = '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01'
-        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", "01"]
+        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", True]
         ctx = self.hptc.extract(carrier)
         self.assertIsNone(ctx.correlation_id)
         self.assertIsNone(ctx.correlation_type)
@@ -156,7 +156,7 @@ def test_extract_carrier_dict_level_header_not_splitable(self, mock_validate, mo
             'X-INSTANA-L': ['1']
         }
         mock_validate.return_value = '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01'
-        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", "01"]
+        mock_get_traceparent_fields.return_value = ["00", "4bf92f3577b34da6a3ce929d0e0e4736", "00f067aa0ba902b7", True]
         ctx = self.hptc.extract(carrier)
         self.assertIsNone(ctx.correlation_id)
         self.assertIsNone(ctx.correlation_type)

tests/w3c_trace_context/test_traceparent.py

@@ -23,21 +23,31 @@ def test_validate_traceparent_None(self):
 
     def test_get_traceparent_fields(self):
         traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"
-        version, trace_id, parent_id, trace_flags = self.tp.get_traceparent_fields(traceparent)
+        version, trace_id, parent_id, sampled_flag = self.tp.get_traceparent_fields(traceparent)
         self.assertEqual(trace_id, "4bf92f3577b34da6a3ce929d0e0e4736")
         self.assertEqual(parent_id, "00f067aa0ba902b7")
+        self.assertTrue(sampled_flag)
+
+    def test_get_traceparent_fields_unsampled(self):
+        traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00"
+        version, trace_id, parent_id, sampled_flag = self.tp.get_traceparent_fields(traceparent)
+        self.assertEqual(trace_id, "4bf92f3577b34da6a3ce929d0e0e4736")
+        self.assertEqual(parent_id, "00f067aa0ba902b7")
+        self.assertFalse(sampled_flag)
 
     def test_get_traceparent_fields_None_input(self):
         traceparent = None
-        version, trace_id, parent_id, trace_flags = self.tp.get_traceparent_fields(traceparent)
+        version, trace_id, parent_id, sampled_flag = self.tp.get_traceparent_fields(traceparent)
         self.assertIsNone(trace_id)
         self.assertIsNone(parent_id)
+        self.assertFalse(sampled_flag)
 
     def test_get_traceparent_fields_string_input_no_dash(self):
         traceparent = "invalid"
-        version, trace_id, parent_id, trace_flags = self.tp.get_traceparent_fields(traceparent)
+        version, trace_id, parent_id, sampled_flag = self.tp.get_traceparent_fields(traceparent)
         self.assertIsNone(trace_id)
         self.assertIsNone(parent_id)
+        self.assertFalse(sampled_flag)
 
     def test_update_traceparent(self):
         traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"