Span: Add Stack Frame limits (#292)

* Apply Stack reporting limits in spans

* Don't collect backtrace on Entry spans

* Better stack processing and a hard limit

* Only slice if limit is exceeded

* Updates to follow changes

* Fix synthetic flag propagation

* Update to follow changes

Author: Peter Giacomo Lombardo

instana/span.py

@@ -104,12 +104,10 @@ def __init__(self, span, source, service_name, **kwargs):
         self.f = source
         self.ec = span.tags.pop('ec', None)
         self.data = DictionaryOfStan()
+        self.stack = span.stack
 
-        if span.synthetic:
-            self.sy = True
-
-        if span.stack:
-            self.stack = span.stack
+        if span.synthetic is True:
+            self.sy = span.synthetic
 
         self.__dict__.update(kwargs)
 

instana/tracer.py

@@ -104,10 +104,6 @@ def start_span(self,
         if operation_name in RegisteredSpan.EXIT_SPANS:
             self.__add_stack(span)
 
-        elif operation_name in RegisteredSpan.ENTRY_SPANS:
-            # For entry spans, add only a backtrace fingerprint
-            self.__add_stack(span, limit=2)
-
         return span
 
     def inject(self, span_context, format, carrier):
@@ -122,33 +118,42 @@ def extract(self, format, carrier):
 
         raise ot.UnsupportedFormatException()
 
-    def __add_stack(self, span, limit=None):
-        """ Adds a backtrace to this span """
-        span.stack = []
-        frame_count = 0
-
-        tb = traceback.extract_stack()
-        tb.reverse()
-        for frame in tb:
-            if limit is not None and frame_count >= limit:
-                break
-
-            # Exclude Instana frames unless we're in dev mode
-            if "INSTANA_DEBUG" not in os.environ:
-                if re_tracer_frame.search(frame[0]) is not None:
-                    continue
-
-                if re_with_stan_frame.search(frame[2]) is not None:
-                    continue
-
-            span.stack.append({
-                "c": frame[0],
-                "n": frame[1],
-                "m": frame[2]
-            })
-
-            if limit is not None:
-                frame_count += 1
+    def __add_stack(self, span, limit=30):
+        """
+        Adds a backtrace to <span>.  The default length limit for
+        stack traces is 30 frames.  A hard limit of 40 frames is enforced.
+        """
+        try:
+            sanitized_stack = []
+            if limit > 40:
+                limit = 40
+
+            trace_back = traceback.extract_stack()
+            trace_back.reverse()
+            for frame in trace_back:
+                # Exclude Instana frames unless we're in dev mode
+                if "INSTANA_DEBUG" not in os.environ:
+                    if re_tracer_frame.search(frame[0]) is not None:
+                        continue
+
+                    if re_with_stan_frame.search(frame[2]) is not None:
+                        continue
+
+                sanitized_stack.append({
+                    "c": frame[0],
+                    "n": frame[1],
+                    "m": frame[2]
+                })
+
+            if len(sanitized_stack) > limit:
+                # (limit * -1) gives us negative form of <limit> used for
+                # slicing from the end of the list. e.g. stack[-30:]
+                span.stack = sanitized_stack[(limit*-1):]
+            else:
+                span.stack = sanitized_stack
+        except Exception:
+            # No fail
+            pass
 
 
 # Used by __add_stack

tests/clients/test_urllib3.py

@@ -62,8 +62,7 @@ def test_get_request(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -110,8 +109,7 @@ def test_get_request_with_query(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -159,8 +157,7 @@ def test_get_request_with_alt_query(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -208,8 +205,7 @@ def test_put_request(self):
         self.assertEqual('PUT', wsgi_span.data["http"]["method"])
         self.assertEqual(404, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -265,17 +261,15 @@ def test_301_redirect(self):
         self.assertEqual('GET', wsgi_span1.data["http"]["method"])
         self.assertEqual(200, wsgi_span1.data["http"]["status"])
         self.assertIsNone(wsgi_span1.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span1.stack)
-        self.assertEqual(2, len(wsgi_span1.stack))
+        self.assertIsNone(wsgi_span1.stack)
 
         self.assertEqual("wsgi", wsgi_span2.n)
         self.assertEqual('127.0.0.1:' + str(testenv["wsgi_port"]), wsgi_span2.data["http"]["host"])
         self.assertEqual('/301', wsgi_span2.data["http"]["url"])
         self.assertEqual('GET', wsgi_span2.data["http"]["method"])
         self.assertEqual(301, wsgi_span2.data["http"]["status"])
         self.assertIsNone(wsgi_span2.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span2.stack)
-        self.assertEqual(2, len(wsgi_span2.stack))
+        self.assertIsNone(wsgi_span2.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -339,17 +333,15 @@ def test_302_redirect(self):
         self.assertEqual('GET', wsgi_span1.data["http"]["method"])
         self.assertEqual(200, wsgi_span1.data["http"]["status"])
         self.assertIsNone(wsgi_span1.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span1.stack)
-        self.assertEqual(2, len(wsgi_span1.stack))
+        self.assertIsNone(wsgi_span1.stack)
 
         self.assertEqual("wsgi", wsgi_span2.n)
         self.assertEqual('127.0.0.1:' + str(testenv["wsgi_port"]), wsgi_span2.data["http"]["host"])
         self.assertEqual('/302', wsgi_span2.data["http"]["url"])
         self.assertEqual('GET', wsgi_span2.data["http"]["method"])
         self.assertEqual(302, wsgi_span2.data["http"]["status"])
         self.assertIsNone(wsgi_span2.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span2.stack)
-        self.assertEqual(2, len(wsgi_span2.stack))
+        self.assertIsNone(wsgi_span2.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -405,8 +397,7 @@ def test_5xx_request(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(504, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -457,8 +448,7 @@ def test_exception_logging(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(500, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -545,8 +535,7 @@ def test_requestspkg_get(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -596,8 +585,7 @@ def test_requestspkg_get_with_custom_headers(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -643,8 +631,7 @@ def test_requestspkg_put(self):
         self.assertEqual('PUT', wsgi_span.data["http"]["method"])
         self.assertEqual(404, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])
@@ -694,8 +681,7 @@ def test_response_header_capture(self):
         self.assertEqual('GET', wsgi_span.data["http"]["method"])
         self.assertEqual(200, wsgi_span.data["http"]["status"])
         self.assertIsNone(wsgi_span.data["http"]["error"])
-        self.assertIsNotNone(wsgi_span.stack)
-        self.assertEqual(2, len(wsgi_span.stack))
+        self.assertIsNone(wsgi_span.stack)
 
         # urllib3
         self.assertEqual("test", test_span.data["sdk"]["name"])

tests/frameworks/test_aiohttp_server.py

@@ -72,9 +72,7 @@ async def test():
         self.assertEqual(testenv["aiohttp_server"] +
                          "/", aioserver_span.data["http"]["url"])
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(200, aioclient_span.data["http"]["status"])
@@ -136,9 +134,7 @@ async def test():
         self.assertEqual(testenv["aiohttp_server"] +
                          "/204", aioserver_span.data["http"]["url"])
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(204, aioclient_span.data["http"]["status"])
@@ -220,9 +216,7 @@ async def test():
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
         self.assertEqual("secret=<redacted>",
                          aioserver_span.data["http"]["params"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(200, aioclient_span.data["http"]["status"])
@@ -291,9 +285,7 @@ async def test():
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
         self.assertEqual("secret=<redacted>",
                          aioserver_span.data["http"]["params"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(200, aioclient_span.data["http"]["status"])
@@ -357,9 +349,7 @@ async def test():
         self.assertEqual(testenv["aiohttp_server"] +
                          "/401", aioserver_span.data["http"]["url"])
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(401, aioclient_span.data["http"]["status"])
@@ -416,9 +406,7 @@ async def test():
         self.assertEqual(testenv["aiohttp_server"] +
                          "/500", aioserver_span.data["http"]["url"])
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(500, aioclient_span.data["http"]["status"])
@@ -477,9 +465,7 @@ async def test():
         self.assertEqual(testenv["aiohttp_server"] +
                          "/exception", aioserver_span.data["http"]["url"])
         self.assertEqual("GET", aioserver_span.data["http"]["method"])
-        self.assertIsNotNone(aioserver_span.stack)
-        self.assertTrue(isinstance(aioserver_span.stack, list))
-        self.assertTrue(len(aioserver_span.stack) > 1)
+        self.assertIsNone(aioserver_span.stack)
 
         self.assertEqual("aiohttp-client", aioclient_span.n)
         self.assertEqual(500, aioclient_span.data["http"]["status"])

tests/frameworks/test_django.py

@@ -71,8 +71,7 @@ def test_basic_request(self):
         self.assertEqual('/', django_span.data["http"]["url"])
         self.assertEqual('GET', django_span.data["http"]["method"])
         self.assertEqual(200, django_span.data["http"]["status"])
-        assert django_span.stack
-        self.assertEqual(2, len(django_span.stack))
+        self.assertIsNone(django_span.stack)
 
     def test_synthetic_request(self):
         headers = {
@@ -154,8 +153,7 @@ def test_request_with_error(self):
         self.assertEqual('GET', django_span.data["http"]["method"])
         self.assertEqual(500, django_span.data["http"]["status"])
         self.assertEqual('This is a fake error: /cause-error', django_span.data["http"]["error"])
-        assert(django_span.stack)
-        self.assertEqual(2, len(django_span.stack))
+        self.assertIsNone(django_span.stack)
 
     def test_complex_request(self):
         with tracer.start_active_span('test'):
@@ -204,8 +202,7 @@ def test_complex_request(self):
         self.assertEqual(ot_span2.p, ot_span1.s)
 
         self.assertEqual(None, django_span.ec)
-        assert(django_span.stack)
-        self.assertEqual(2, len(django_span.stack))
+        self.assertIsNone(django_span.stack)
 
         self.assertEqual('/complex', django_span.data["http"]["url"])
         self.assertEqual('GET', django_span.data["http"]["method"])
@@ -244,8 +241,7 @@ def test_custom_header_capture(self):
         self.assertEqual(django_span.p, urllib3_span.s)
 
         self.assertEqual(None, django_span.ec)
-        assert(django_span.stack)
-        self.assertEqual(2, len(django_span.stack))
+        self.assertIsNone(django_span.stack)
 
         self.assertEqual('/', django_span.data["http"]["url"])
         self.assertEqual('GET', django_span.data["http"]["method"])