Repository: scoutapp/scout_apm_python
Pull request #261 (2019-10-02 23:03:58)

Minimize TrackedRequest and Span classes
Replicate the changes from #255 into these key classes. Also remove unused constructor arguments and rename `req_id` to `request_id`.


Patch:
diff --git a/src/scout_apm/core/commands.py b/src/scout_apm/core/commands.py
index d8f074c..497c540 100644
--- a/src/scout_apm/core/commands.py
+++ b/src/scout_apm/core/commands.py
@@ -182,13 +182,13 @@ class BatchCommand(object):
         # The TrackedRequest must be finished
         commands = []
         commands.append(
-            StartRequest(timestamp=request.start_time, request_id=request.req_id)
+            StartRequest(timestamp=request.start_time, request_id=request.request_id)
         )
         for key in request.tags:
             commands.append(
                 TagRequest(
                     timestamp=request.start_time,
-                    request_id=request.req_id,
+                    request_id=request.request_id,
                     tag=key,
                     value=request.tags[key],
                 )
@@ -209,7 +209,7 @@ class BatchCommand(object):
                 commands.append(
                     TagSpan(
                         timestamp=span.start_time,
-                        request_id=request.req_id,
+                        request_id=request.request_id,
                         span_id=span.span_id,
                         tag=key,
                         value=span.tags[key],
@@ -225,7 +225,7 @@ class BatchCommand(object):
             )
 
         commands.append(
-            FinishRequest(timestamp=request.end_time, request_id=request.req_id)
+            FinishRequest(timestamp=request.end_time, request_id=request.request_id)
         )
 
         return cls(commands)
diff --git a/src/scout_apm/core/thread_local.py b/src/scout_apm/core/thread_local.py
index 1f9ed61..51a8b98 100644
--- a/src/scout_apm/core/thread_local.py
+++ b/src/scout_apm/core/thread_local.py
@@ -40,6 +40,6 @@ class ThreadLocalSingleton(object):
                 self.__class__._thread_lookup.instance = None
 
     @classmethod
-    def __new_instance(cls, *args, **kwargs):
+    def __new_instance(cls, args, kwargs):
         cls._thread_lookup = threading.local()
-        cls._thread_lookup.instance = cls(args, kwargs)
+        cls._thread_lookup.instance = cls(*args, **kwargs)
diff --git a/src/scout_apm/core/tracked_request.py b/src/scout_apm/core/tracked_request.py
index af675e4..cc6defe 100644
--- a/src/scout_apm/core/tracked_request.py
+++ b/src/scout_apm/core/tracked_request.py
@@ -27,34 +27,48 @@ class TrackedRequest(ThreadLocalSingleton):
     their keyname
     """
 
-    def __init__(self, *args, **kwargs):
-        self.req_id = "req-" + str(uuid4())
-        self.start_time = kwargs.get("start_time", dt.datetime.utcnow())
-        self.end_time = kwargs.get("end_time", None)
-        self.active_spans = kwargs.get("active_spans", [])
-        self.complete_spans = kwargs.get("complete_spans", [])
-        self.tags = kwargs.get("tags", {})
-        self.real_request = kwargs.get("real_request", False)
-        self.memory_start = kwargs.get("memory_start", Memory.rss_in_mb())
+    __slots__ = (
+        "request_id",
+        "start_time",
+        "end_time",
+        "active_spans",
+        "complete_spans",
+        "tags",
+        "_is_real_request",
+        "memory_start",
+        "callset",
+    )
+
+    def __init__(self):
+        self.request_id = "req-" + str(uuid4())
+        self.start_time = dt.datetime.utcnow()
+        self.end_time = None
+        self.active_spans = []
+        self.complete_spans = []
+        self.tags = {}
+        self._is_real_request = False
+        self.memory_start = Memory.rss_in_mb()
         self.callset = NPlusOneCallSet()
-        logger.debug("Starting request: %s", self.req_id)
+        logger.debug("Starting request: %s", self.request_id)
 
     def __repr__(self):
         # Incomplete to avoid TMI
-        return "<TrackedRequest(req_id={}, tags={})>".format(
-            repr(self.req_id), repr(self.tags)
+        return "<TrackedRequest(request_id={}, tags={})>".format(
+            repr(self.request_id), repr(self.tags)
         )
 
     def mark_real_request(self):
-        self.real_request = True
+        self._is_real_request = True
 
     def is_real_request(self):
-        return self.real_request
+        return self._is_real_request
 
     def tag(self, key, value):
         if key in self.tags:
             logger.debug(
-                "Overwriting previously set tag for request %s: %s", self.req_id, key
+                "Overwriting previously set tag for request %s: %s",
+                self.request_id,
+                key,
             )
         self.tags[key] = value
 
@@ -70,7 +84,7 @@ class TrackedRequest(ThreadLocalSingleton):
             parent_id = None
 
         kwargs["parent"] = parent_id
-        kwargs["request_id"] = self.req_id
+        kwargs["request_id"] = self.request_id
 
         new_span = Span(**kwargs)
         self.active_spans.append(new_span)
@@ -98,7 +112,7 @@ class TrackedRequest(ThreadLocalSingleton):
 
     # Request is done, release any info we have about it.
     def finish(self):
-        logger.debug("Stopping request: %s", self.req_id)
+        logger.debug("Stopping request: %s", self.request_id)
         if self.end_time is None:
             self.end_time = dt.datetime.utcnow()
         if self.is_real_request():
@@ -121,25 +135,42 @@ class TrackedRequest(ThreadLocalSingleton):
 
 
 class Span(object):
-    def __init__(self, *args, **kwargs):
-        self.span_id = kwargs.get("span_id", "span-" + str(uuid4()))
-        self.start_time = kwargs.get("start_time", dt.datetime.utcnow())
-        self.end_time = kwargs.get("end_time", None)
-        self.request_id = kwargs.get("request_id", None)
-        self.operation = kwargs.get("operation", None)
-        self.ignore = kwargs.get("ignore", False)
-        self.ignore_children = kwargs.get("ignore_children", False)
-        self.parent = kwargs.get("parent", None)
-        self.tags = kwargs.get("tags", {})
+    __slots__ = (
+        "span_id",
+        "start_time",
+        "end_time",
+        "request_id",
+        "operation",
+        "ignore",
+        "ignore_children",
+        "parent",
+        "tags",
+        "start_objtrace_counts",
+        "end_objtrace_counts",
+    )
+
+    def __init__(
+        self,
+        request_id=None,
+        operation=None,
+        ignore=False,
+        ignore_children=False,
+        parent=None,
+    ):
+        self.span_id = "span-" + str(uuid4())
+        self.start_time = dt.datetime.utcnow()
+        self.end_time = None
+        self.request_id = request_id
+        self.operation = operation
+        self.ignore = ignore
+        self.ignore_children = ignore_children
+        self.parent = parent
+        self.tags = {}
         if objtrace is not None:
-            self.start_objtrace_counts = kwargs.get(
-                "start_objtrace_counts", objtrace.get_counts()
-            )
+            self.start_objtrace_counts = objtrace.get_counts()
         else:
-            self.start_objtrace_counts = kwargs.get(
-                "start_objtrace_counts", (0, 0, 0, 0)
-            )
-        self.end_objtrace_counts = kwargs.get("end_objtrace_counts", (0, 0, 0, 0))
+            self.start_objtrace_counts = (0, 0, 0, 0)
+        self.end_objtrace_counts = (0, 0, 0, 0)
 
     def __repr__(self):
         # Incomplete to avoid TMI


Test patch:
diff --git a/tests/unit/core/test_commands.py b/tests/unit/core/test_commands.py
index 2eec5b5..1085e32 100644
--- a/tests/unit/core/test_commands.py
+++ b/tests/unit/core/test_commands.py
@@ -167,8 +167,8 @@ def make_tracked_request_instance_deterministic(tr):
     Override values in a TrackedRequest instance to make tests determistic.
 
     """
-    assert type(tr.req_id) == type(REQUEST_ID)
-    tr.req_id = REQUEST_ID
+    assert type(tr.request_id) == type(REQUEST_ID)
+    tr.request_id = REQUEST_ID
 
     assert type(tr.start_time) == type(START_TIME)
     tr.start_time = START_TIME
diff --git a/tests/unit/core/test_tracked_request.py b/tests/unit/core/test_tracked_request.py
index 4903e1f..6cfa5cf 100644
--- a/tests/unit/core/test_tracked_request.py
+++ b/tests/unit/core/test_tracked_request.py
@@ -129,22 +129,16 @@ def test_start_span_ignores_children(tr):
 
 
 def test_span_captures_backtrace(tr):
-    span = tr.start_span(
-        operation="Sql/Work", start_time=dt.datetime.utcnow() - dt.timedelta(seconds=1)
-    )
+    span = tr.start_span(operation="Sql/Work")
+    # Pretend it was started 1 second ago
+    span.start_time = dt.datetime.utcnow() - dt.timedelta(seconds=1)
     tr.stop_span()
-    assert span.tags["stack"]
+    assert "stack" in span.tags
 
 
 def test_span_does_not_capture_backtrace(tr):
-    controller = tr.start_span(
-        operation="Controller/Work",
-        start_time=dt.datetime.utcnow() - dt.timedelta(seconds=10),
-    )
-    middleware = tr.start_span(
-        operation="Middleware/Work",
-        start_time=dt.datetime.utcnow() - dt.timedelta(seconds=10),
-    )
+    controller = tr.start_span(operation="Controller/Work")
+    middleware = tr.start_span(operation="Middleware/Work")
     tr.stop_span()
     tr.stop_span()
     assert "stack" not in controller.tags