Handle X-INSTANA-L according to specifications (#352)

* feat(propagators): Properly propagate tracing level (X-INSTANA-L)

* fix(level): Inject level and only return spans with level!=0

* fix(injection): Only inject trace_id and span_id when required

* fix(recording): Don't record spans with suppression

* fix(recording): Return early in case of suppression

* fix(propagation): Avoid adding future trace context values accidentally

* fix(propagation): Ensure that the context level is not overridden

* test(flask): Add TC for suppression with w3c 'tracestate'

* refactor(http_propagation): Remove duplication from carrier field population

* fix(ci): Pin "responses" version

* As stated in the changelog here:
  https://github.com/getsentry/responses/releases/tag/0.18.0
  0.18.0 Removed internal `_matches` attribute of RequestsMock object.
  Which has been used by our UT TCs, so before we refactor that
  we need to pin the last known working version.

* feat(span_context): Use a dedicated property for signaling suppression

* feat(propagator): Use suppression property in base_propagator

For skipping tracestate update in suppression mode

Co-authored-by: Andrey Slotin <[email protected]>

Co-authored-by: Andrey Slotin <[email protected]>

Author: Ferenc-

instana/instrumentation/flask/vanilla.py

@@ -20,8 +20,6 @@
 def before_request_with_instana(*argv, **kwargs):
     try:
         env = flask.request.environ
-        ctx = None
-
         ctx = tracer.extract(opentracing.Format.HTTP_HEADERS, env)
 
         flask.g.scope = tracer.start_active_span('wsgi', child_of=ctx)

instana/propagators/base_propagator.py

@@ -130,15 +130,22 @@ def _get_participating_trace_context(self, span_context):
         traceparent = span_context.traceparent
         tracestate = span_context.tracestate
         traceparent = self._tp.update_traceparent(traceparent, tp_trace_id, span_context.span_id, span_context.level)
+
+        # In suppression mode do not update the tracestate and
+        # do not add the 'in=' key-value pair to the incoming tracestate
+        # Just propagate the incoming tracestate (if any) unchanged.
+        if span_context.suppression:
+            return traceparent, tracestate
+
         tracestate = self._ts.update_tracestate(tracestate, span_context.trace_id, span_context.span_id)
         return traceparent, tracestate
 
     def __determine_span_context(self, trace_id, span_id, level, synthetic, traceparent, tracestate,
                                  disable_w3c_trace_context):
         """
         This method determines the span context depending on a set of conditions being met
-        Detailed description of the conditions can be found here:
-        https://github.com/instana/technical-documentation/tree/master/tracing/specification#http-processing-for-instana-tracers
+        Detailed description of the conditions can be found in the instana internal technical-documentation,
+        under section http-processing-for-instana-tracers
         :param trace_id: instana trace id
         :param span_id: instana span id
         :param level: instana level
@@ -157,11 +164,15 @@ def __determine_span_context(self, trace_id, span_id, level, synthetic, tracepar
             correlation = True
 
         ctx_level = self._get_ctx_level(level)
+        if ctx_level == 0 or level == '0':
+            trace_id = ctx.trace_id = None
+            span_id = ctx.span_id = None
+            ctx.correlation_type = None
+            ctx.correlation_id = None
 
         if trace_id and span_id:
             ctx.trace_id = trace_id[-16:]  # only the last 16 chars
             ctx.span_id = span_id[-16:]  # only the last 16 chars
-            ctx.level = ctx_level
             ctx.synthetic = synthetic is not None
 
             if len(trace_id) > 16:
@@ -176,7 +187,6 @@ def __determine_span_context(self, trace_id, span_id, level, synthetic, tracepar
             if disable_traceparent == "":
                 ctx.trace_id = tp_trace_id[-16:]
                 ctx.span_id = tp_parent_id
-                ctx.level = ctx_level
                 ctx.synthetic = synthetic is not None
                 ctx.trace_parent = True
                 ctx.instana_ancestor = instana_ancestor
@@ -185,7 +195,6 @@ def __determine_span_context(self, trace_id, span_id, level, synthetic, tracepar
                 if instana_ancestor:
                     ctx.trace_id = instana_ancestor.t
                     ctx.span_id = instana_ancestor.p
-                    ctx.level = ctx_level
                     ctx.synthetic = synthetic is not None
 
         elif synthetic:
@@ -198,6 +207,8 @@ def __determine_span_context(self, trace_id, span_id, level, synthetic, tracepar
             ctx.traceparent = traceparent
             ctx.tracestate = tracestate
 
+        ctx.level = ctx_level
+
         return ctx
 
     def __extract_instana_headers(self, dc):
@@ -263,7 +274,7 @@ def __extract_w3c_trace_context_headers(self, dc):
 
     def extract(self, carrier, disable_w3c_trace_context=False):
         """
-        This method overrides the one of the Baseclass as with the introduction of W3C trace context for the HTTP
+        This method overrides one of the Baseclasses as with the introduction of W3C trace context for the HTTP
         requests more extracting steps and logic was required
         :param disable_w3c_trace_context:
         :param carrier:
@@ -288,4 +299,4 @@ def extract(self, carrier, disable_w3c_trace_context=False):
 
             return ctx
         except Exception:
-            logger.debug("extract error:", exc_info=True)
+            logger.debug("extract error:", exc_info=True)

instana/propagators/http_propagator.py

@@ -19,49 +19,36 @@ def __init__(self):
         super(HTTPPropagator, self).__init__()
 
     def inject(self, span_context, carrier, disable_w3c_trace_context=False):
-        try:
-            trace_id = span_context.trace_id
-            span_id = span_context.span_id
-            level = span_context.level
-
-            if disable_w3c_trace_context:
-                traceparent, tracestate = [None] * 2
-            else:
-                traceparent, tracestate = self._get_participating_trace_context(span_context)
-
-            if isinstance(carrier, dict) or hasattr(carrier, "__dict__"):
-                if traceparent and tracestate:
-                    carrier[self.HEADER_KEY_TRACEPARENT] = traceparent
-                    carrier[self.HEADER_KEY_TRACESTATE] = tracestate
-                carrier[self.HEADER_KEY_T] = trace_id
-                carrier[self.HEADER_KEY_S] = span_id
-                carrier[self.HEADER_KEY_L] = "1"
-            elif isinstance(carrier, list):
-                if traceparent and tracestate:
-                    carrier.append((self.HEADER_KEY_TRACEPARENT, traceparent))
-                    carrier.append((self.HEADER_KEY_TRACESTATE, tracestate))
-                carrier.append((self.HEADER_KEY_T, trace_id))
-                carrier.append((self.HEADER_KEY_S, span_id))
-                carrier.append((self.HEADER_KEY_L, "1"))
-            elif hasattr(carrier, '__setitem__'):
-                if traceparent and tracestate:
-                    carrier.__setitem__(self.HEADER_KEY_TRACEPARENT, traceparent)
-                    carrier.__setitem__(self.HEADER_KEY_TRACESTATE, tracestate)
-                carrier.__setitem__(self.HEADER_KEY_T, trace_id)
-                carrier.__setitem__(self.HEADER_KEY_S, span_id)
-                carrier.__setitem__(self.HEADER_KEY_L, "1")
+        trace_id = span_context.trace_id
+        span_id = span_context.span_id
+        serializable_level = str(span_context.level)
+
+        if disable_w3c_trace_context:
+            traceparent, tracestate = [None] * 2
+        else:
+            traceparent, tracestate = self._get_participating_trace_context(span_context)
+
+        def inject_key_value(carrier, key, value):
+            if isinstance(carrier, list):
+                carrier.append((key, value))
+            elif isinstance(carrier, dict) or '__setitem__' in dir(carrier):
+                carrier[key] = value
             else:
                 raise Exception("Unsupported carrier type", type(carrier))
 
-        except Exception:
-            logger.debug("inject error:", exc_info=True)
-
-
-
-
-
-
+        try:
+            inject_key_value(carrier, self.HEADER_KEY_L, serializable_level)
 
+            if traceparent:
+                inject_key_value(carrier, self.HEADER_KEY_TRACEPARENT, traceparent)
+            if tracestate:
+                inject_key_value(carrier, self.HEADER_KEY_TRACESTATE, tracestate)
 
+            if span_context.suppression:
+                return
 
+            inject_key_value(carrier, self.HEADER_KEY_T, trace_id)
+            inject_key_value(carrier, self.HEADER_KEY_S, span_id)
 
+        except Exception:
+            logger.debug("inject error:", exc_info=True)

instana/recorder.py

@@ -75,6 +75,9 @@ def record_span(self, span):
         """
         Convert the passed BasicSpan into and add it to the span queue
         """
+        if span.context.suppression:
+            return
+
         if self.agent.can_send():
             service_name = None
             source = self.agent.get_from_structure()

instana/span.py

@@ -106,6 +106,7 @@ def __init__(self, span, source, service_name, **kwargs):
         self.t = span.context.trace_id
         self.p = span.parent_id
         self.s = span.context.span_id
+        self.l = span.context.level
         self.ts = int(round(span.start_time * 1000))
         self.d = int(round(span.duration * 1000))
         self.f = source

instana/span_context.py

@@ -88,11 +88,16 @@ def correlation_id(self, value):
     def baggage(self):
         return self._baggage
 
+    @property
+    def suppression(self):
+        return self.level == 0
+
     def with_baggage_item(self, key, value):
         new_baggage = self._baggage.copy()
         new_baggage[key] = value
         return SpanContext(
             trace_id=self.trace_id,
             span_id=self.span_id,
             sampled=self.sampled,
-            baggage=new_baggage)
+            level=self.level,
+            baggage=new_baggage)

instana/tracer.py

@@ -92,6 +92,7 @@ def start_span(self,
             ctx.long_trace_id = parent_ctx.long_trace_id
             ctx.trace_parent = parent_ctx.trace_parent
             ctx.instana_ancestor = parent_ctx.instana_ancestor
+            ctx.level = parent_ctx.level
             ctx.correlation_type = parent_ctx.correlation_type
             ctx.correlation_id = parent_ctx.correlation_id
             ctx.traceparent = parent_ctx.traceparent
@@ -100,6 +101,7 @@ def start_span(self,
             ctx.trace_id = gid
             ctx.sampled = self.sampler.sampled(ctx.trace_id)
             if parent_ctx is not None:
+                ctx.level = parent_ctx.level
                 ctx.correlation_type = parent_ctx.correlation_type
                 ctx.correlation_id = parent_ctx.correlation_id
                 ctx.traceparent = parent_ctx.traceparent

tests/frameworks/test_flask.py

@@ -102,6 +102,52 @@ def test_get_request(self):
         # We should NOT have a path template for this route
         self.assertIsNone(wsgi_span.data["http"]["path_tpl"])
 
+    def test_get_request_with_suppression(self):
+        headers = {'X-INSTANA-L':'0'}
+        response = self.http.urlopen('GET', testenv["wsgi_server"] + '/', headers=headers)
+
+        spans = self.recorder.queued_spans()
+
+        self.assertEqual(response.headers.get('X-INSTANA-L', None), '0')
+        # The traceparent has to be present
+        self.assertIsNotNone(response.headers.get('traceparent', None))
+        # The last digit of the traceparent has to be 0
+        self.assertEqual(response.headers['traceparent'][-1], '0')
+
+        # This should not be present
+        self.assertIsNone(response.headers.get('tracestate', None))
+
+        # Assert that there isn't any span, where level is not 0!
+        self.assertFalse(any(map(lambda x: x.l != 0, spans)))
+
+        # Assert that there are no spans in the recorded list
+        self.assertEquals(spans, [])
+
+    def test_get_request_with_suppression_and_w3c(self):
+        headers = {
+                'X-INSTANA-L':'0',
+                'traceparent': '00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01',
+                'tracestate': 'congo=ucfJifl5GOE,rojo=00f067aa0ba902b7'}
+
+        response = self.http.urlopen('GET', testenv["wsgi_server"] + '/', headers=headers)
+
+        spans = self.recorder.queued_spans()
+
+        self.assertEqual(response.headers.get('X-INSTANA-L', None), '0')
+        self.assertIsNotNone(response.headers.get('traceparent', None))
+        self.assertEqual(response.headers['traceparent'][-1], '0')
+        # The tracestate has to be present
+        self.assertIsNotNone(response.headers.get('tracestate', None))
+
+        # The 'in=' section can not be in the tracestate
+        self.assertTrue('in=' not in response.headers['tracestate'])
+
+        # Assert that there isn't any span, where level is not 0!
+        self.assertFalse(any(map(lambda x: x.l != 0, spans)))
+
+        # Assert that there are no spans in the recorded list
+        self.assertEquals(spans, [])
+
     def test_synthetic_request(self):
         headers = {
             'X-INSTANA-SYNTHETIC': '1'

tests/requirements-310.txt

@@ -35,6 +35,7 @@ pytest>=6.2.4
 pytest-celery
 redis>=3.5.3
 requests-mock
+responses<=0.17.0
 sanic>=19.0.0,<21.9.0
 sqlalchemy>=1.4.15
 spyne>=2.13.16

tests/requirements.txt

@@ -23,6 +23,7 @@ pytest>=6.2.4
 pytest-celery
 redis>=3.5.3
 requests-mock
+responses<=0.17.0
 sanic>=19.0.0,<21.9.0
 sqlalchemy>=1.4.15
 spyne>=2.13.16