Repository: scoutapp/scout_apm_python
Pull request #315 (2019-10-28 23:09:44)

Improve objtrace tests
Ensure key pathways in the C level extension get tested.


Patch:
diff --git a/.travis.yml b/.travis.yml
index 53c0232..6a203c8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -48,7 +48,7 @@ jobs:
         - twine check dist/* wheelhouse/*
         - ls -al dist wheelhouse
         - if [ "${TRAVIS_TAG:-}" != "" ]; then
-            python -m twine upload --skip-existing dist/* wheelhouse/*;
+            twine upload --skip-existing dist/* wheelhouse/*;
           fi
 
 notifications:
diff --git a/src/scout_apm/api/__init__.py b/src/scout_apm/api/__init__.py
index 6736fc2..66c8825 100644
--- a/src/scout_apm/api/__init__.py
+++ b/src/scout_apm/api/__init__.py
@@ -80,7 +80,9 @@ class Transaction(ContextDecorator):
 
         tracked_request = TrackedRequest.instance()
         tracked_request.mark_real_request()
-        span = tracked_request.start_span(operation=operation)
+        span = tracked_request.start_span(
+            operation=operation, should_capture_backtrace=False
+        )
         for key, value in tags.items():
             tracked_request.tag(key, value)
         return span
diff --git a/src/scout_apm/bottle.py b/src/scout_apm/bottle.py
index beb8ddb..6e47cb0 100644
--- a/src/scout_apm/bottle.py
+++ b/src/scout_apm/bottle.py
@@ -58,7 +58,10 @@ class ScoutPlugin(object):
                 controller_name = "/home"
             if not controller_name.startswith("/"):
                 controller_name = "/" + controller_name
-            tracked_request.start_span(operation="Controller{}".format(controller_name))
+            tracked_request.start_span(
+                operation="Controller{}".format(controller_name),
+                should_capture_backtrace=False,
+            )
 
             # Determine a remote IP to associate with the request. The
             # value is spoofable by the requester so this is not suitable
diff --git a/src/scout_apm/core/tracked_request.py b/src/scout_apm/core/tracked_request.py
index f5e8256..4fba267 100644
--- a/src/scout_apm/core/tracked_request.py
+++ b/src/scout_apm/core/tracked_request.py
@@ -149,6 +149,7 @@ class Span(object):
         "tags",
         "start_objtrace_counts",
         "end_objtrace_counts",
+        "should_capture_backtrace",
     )
 
     def __init__(
@@ -158,6 +159,7 @@ class Span(object):
         ignore=False,
         ignore_children=False,
         parent=None,
+        should_capture_backtrace=True,
     ):
         self.span_id = "span-" + str(uuid4())
         self.start_time = dt.datetime.utcnow()
@@ -170,6 +172,7 @@ class Span(object):
         self.tags = {}
         self.start_objtrace_counts = objtrace.get_counts()
         self.end_objtrace_counts = (0, 0, 0, 0)
+        self.should_capture_backtrace = should_capture_backtrace
 
     def __repr__(self):
         # Incomplete to avoid TMI
@@ -200,11 +203,7 @@ class Span(object):
     # process of stopping this span.
     def annotate(self):
         self.add_allocation_tags()
-        # Don't capture backtraces for Controller or Middleware
-        if self.operation is not None and (
-            self.operation.startswith(("Controller", "Middleware"))
-            or self.operation == "QueueTime/Request"
-        ):
+        if not self.should_capture_backtrace:
             return
         slow_threshold = 0.5
         if self.duration() > slow_threshold:
diff --git a/src/scout_apm/django/middleware.py b/src/scout_apm/django/middleware.py
index 2a44d3a..fc078dc 100644
--- a/src/scout_apm/django/middleware.py
+++ b/src/scout_apm/django/middleware.py
@@ -86,7 +86,9 @@ class MiddlewareTimingMiddleware(object):
 
         tracked_request = TrackedRequest.instance()
 
-        tracked_request.start_span(operation="Middleware")
+        tracked_request.start_span(
+            operation="Middleware", should_capture_backtrace=False
+        )
         queue_time = request.META.get("HTTP_X_QUEUE_START") or request.META.get(
             "HTTP_X_REQUEST_START", ""
         )
@@ -129,7 +131,7 @@ class ViewTimingMiddleware(object):
 
         # This operation name won't be recorded unless changed later in
         # process_view
-        tracked_request.start_span(operation="Unknown")
+        tracked_request.start_span(operation="Unknown", should_capture_backtrace=False)
         try:
             return self.get_response(request)
         finally:
@@ -182,7 +184,9 @@ class OldStyleMiddlewareTimingMiddleware(object):
                 request.META.get("HTTP_X_AMZN_TRACE_ID", ""), tracked_request
             )
 
-        tracked_request.start_span(operation="Middleware")
+        tracked_request.start_span(
+            operation="Middleware", should_capture_backtrace=False
+        )
 
     def process_response(self, request, response):
         # Only stop span if there's a request, but presume we are balanced,
@@ -206,7 +210,9 @@ class OldStyleViewMiddleware(object):
 
         track_request_view_data(request, tracked_request)
 
-        span = tracked_request.start_span(operation=get_operation_name(request))
+        span = tracked_request.start_span(
+            operation=get_operation_name(request), should_capture_backtrace=False
+        )
         # Save the span into the request, so we can check
         # if we're matched up when stopping
         request._scout_view_span = span
diff --git a/src/scout_apm/falcon.py b/src/scout_apm/falcon.py
index 1fe7745..687ac6e 100644
--- a/src/scout_apm/falcon.py
+++ b/src/scout_apm/falcon.py
@@ -96,7 +96,9 @@ class ScoutMiddleware(object):
                 resource.__module__, resource.__class__.__name__, responder.__name__
             )
 
-        span = tracked_request.start_span(operation=operation)
+        span = tracked_request.start_span(
+            operation=operation, should_capture_backtrace=False
+        )
         req.context.scout_resource_span = span
 
     def process_response(self, req, resp, resource, req_succeeded):
diff --git a/src/scout_apm/flask/__init__.py b/src/scout_apm/flask/__init__.py
index 5e7ad7b..cd4ffb5 100644
--- a/src/scout_apm/flask/__init__.py
+++ b/src/scout_apm/flask/__init__.py
@@ -64,7 +64,9 @@ class ScoutApm(object):
 
         tracked_request = TrackedRequest.instance()
         tracked_request.mark_real_request()
-        span = tracked_request.start_span(operation=operation)
+        span = tracked_request.start_span(
+            operation=operation, should_capture_backtrace=False
+        )
         span.tag("name", name)
 
         werkzeug_track_request_data(request, tracked_request)
diff --git a/src/scout_apm/nameko.py b/src/scout_apm/nameko.py
index cf8a64f..484eeaf 100644
--- a/src/scout_apm/nameko.py
+++ b/src/scout_apm/nameko.py
@@ -36,7 +36,7 @@ class ScoutReporter(DependencyProvider):
             + "."
             + worker_ctx.entrypoint.method_name
         )
-        tracked_request.start_span(operation=operation)
+        tracked_request.start_span(operation=operation, should_capture_backtrace=False)
 
     def _track_request_data(self, request, tracked_request):
         if not isinstance(request, Request):
diff --git a/src/scout_apm/pyramid.py b/src/scout_apm/pyramid.py
index 6d814bf..c7e2ddb 100644
--- a/src/scout_apm/pyramid.py
+++ b/src/scout_apm/pyramid.py
@@ -29,7 +29,9 @@ def includeme(config):
 def instruments(handler, registry):
     def scout_tween(request):
         tracked_request = TrackedRequest.instance()
-        span = tracked_request.start_span(operation="Controller/Pyramid")
+        span = tracked_request.start_span(
+            operation="Controller/Pyramid", should_capture_backtrace=False
+        )
 
         try:
             path = request.path


Test patch:
diff --git a/tests/unit/core/test_objtrace.py b/tests/unit/core/test_objtrace.py
index 611caa8..0fd7c83 100644
--- a/tests/unit/core/test_objtrace.py
+++ b/tests/unit/core/test_objtrace.py
@@ -1,6 +1,8 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import sys
+
 import pytest
 
 from scout_apm.core import objtrace
@@ -22,21 +24,59 @@ def test_enables_and_disabled():
     objtrace.disable()
 
 
+def test_enable_twice():
+    objtrace.enable()
+    objtrace.enable()
+
+
+def test_disable_twice():
+    objtrace.disable()
+    objtrace.disable()
+
+
+def test_get_counts():
+    objtrace.enable()
+    counts = objtrace.get_counts()
+    assert isinstance(counts, tuple)
+    assert len(counts) == 4
+    assert all(isinstance(x, int) for x in counts)
+
+
 @skip_if_objtrace_not_extension
-def test_allocation_counts():
+def test_get_counts_allocations():
     lists = []
     objtrace.enable()
-    for _ in range(100):
-        lists.append([1])
+    lists.extend([1] for _ in range(100))
     objtrace.disable()
-    c = objtrace.get_counts()
-    assert c[0] > 0
+    counts = objtrace.get_counts()
+    assert counts[0] > 0
+
+
+@skip_if_objtrace_not_extension
+@pytest.mark.skipif(
+    sys.version_info < (3, 5), reason="Mulitple allocations on Python 3.5+ only"
+)
+def test_get_counts_multiple_allocations():
+    objtrace.enable()
+    bytes("123")
+    counts = objtrace.get_counts()
+    assert counts[1] > 0
+
+
+@skip_if_objtrace_not_extension
+def test_get_counts_reallocations():
+    items = []
+    objtrace.enable()
+    for i in range(1000):
+        items.append(i)
+    counts = objtrace.get_counts()
+    assert counts[2] > 0
 
 
 @skip_if_objtrace_not_extension
-def test_frees_counts():
+def test_get_counts_frees():
     objtrace.enable()
     for x in (1, 2, 3):
         y = x  # noqa: F841
-    c = objtrace.get_counts()
-    assert c[3] > 0
+    counts = objtrace.get_counts()
+    assert counts[3] > 0
diff --git a/tests/unit/core/test_tracked_request.py b/tests/unit/core/test_tracked_request.py
index fc4cbec..74bc3c6 100644
--- a/tests/unit/core/test_tracked_request.py
+++ b/tests/unit/core/test_tracked_request.py
@@ -129,13 +129,25 @@ def test_span_captures_backtrace(tracked_request):
     assert "stack" in span.tags
 
 
-def test_span_does_not_capture_backtrace(tracked_request):
-    controller = tracked_request.start_span(operation="Controller/Work")
-    middleware = tracked_request.start_span(operation="Middleware/Work")
+def test_should_capture_backtrace_default_true(tracked_request):
+    span = tracked_request.start_span("Something")
+    # Trigger 'slow' condition
+    span.start_time -= dt.timedelta(seconds=2)
+
     tracked_request.stop_span()
+
+    stack = span.tags["stack"]
+    assert all(set(i.keys()) == {"file", "line", "function"} for i in stack)
+
+
+def test_should_capture_backtrace_false(tracked_request):
+    span = tracked_request.start_span("Something", should_capture_backtrace=False)
+    # Trigger 'slow' condition
+    span.start_time -= dt.timedelta(seconds=2)
+
     tracked_request.stop_span()
-    assert "stack" not in controller.tags
-    assert "stack" not in middleware.tags
+
+    assert "stack" not in span.tags
 
 
 def test_extra_stop_span_is_ignored(tracked_request):