Repository: scoutapp/scout_apm_python
Pull request #309 (2019-10-28 16:39:44)

Simplify samplers
Following looking at them inn #307:

* Make `metric_type`, `metric_name`, and `human_name` simple class properties since they never change
* Remove static methods from `Memory`, simplifying down to a function `get_rss_in_mb()` and the delta method on `TrackedRequest`


Patch:
diff --git a/src/scout_apm/core/samplers/cpu.py b/src/scout_apm/core/samplers/cpu.py
index 12ac9c8..fb20b37 100644
--- a/src/scout_apm/core/samplers/cpu.py
+++ b/src/scout_apm/core/samplers/cpu.py
@@ -10,23 +10,18 @@ logger = logging.getLogger(__name__)
 
 
 class Cpu(object):
+    metric_type = "CPU"
+    metric_name = "Utilization"
+    human_name = "Process CPU"
+
     def __init__(self):
         self.last_run = dt.datetime.utcnow()
         self.last_cpu_times = psutil.Process().cpu_times()
         self.num_processors = psutil.cpu_count()
         if self.num_processors is None:
-            logger.debug("Could not determine CPU count - assume there is one.")
+            logger.debug("Could not determine CPU count - assuming there is one.")
             self.num_processors = 1
 
-    def metric_type(self):
-        return "CPU"
-
-    def metric_name(self):
-        return "Utilization"
-
-    def human_name(self):
-        return "Process CPU"
-
     def run(self):
         now = dt.datetime.utcnow()
         process = psutil.Process()  # get a handle on the current process
@@ -37,7 +32,7 @@ class Cpu(object):
             self.save_times(now, cpu_times)
             logger.debug(
                 "%s: Negative time elapsed. now: %s, last_run: %s.",
-                self.human_name(),
+                self.human_name,
                 now,
                 self.last_run,
             )
@@ -55,7 +50,7 @@ class Cpu(object):
                 "%s: Negative process time elapsed. "
                 "utime: %s, stime: %s, total time: %s. "
                 "This is normal to see when starting a forking web server.",
-                self.human_name(),
+                self.human_name,
                 utime_elapsed,
                 stime_elapsed,
                 process_elapsed,
@@ -73,7 +68,7 @@ class Cpu(object):
 
         self.save_times(now, cpu_times)
 
-        logger.debug("%s: %s [%s CPU(s)]", self.human_name(), res, self.num_processors)
+        logger.debug("%s: %s [%s CPU(s)]", self.human_name, res, self.num_processors)
 
         return res
 
diff --git a/src/scout_apm/core/samplers/memory.py b/src/scout_apm/core/samplers/memory.py
index ace3c8d..7728d82 100644
--- a/src/scout_apm/core/samplers/memory.py
+++ b/src/scout_apm/core/samplers/memory.py
@@ -8,44 +8,17 @@ import psutil
 logger = logging.getLogger(__name__)
 
 
+def get_rss_in_mb():
+    rss_in_bytes = psutil.Process().memory_info().rss
+    return rss_in_bytes / (1024 * 1024)
+
+
 class Memory(object):
-    @staticmethod
-    def rss_to_mb(rss_in_bytes):
-        """
-        Convert a number of bytes to a number of megabytes.
-        """
-        bytes_per_kb = 1024
-        kb_per_megabyte = 1024
-        return float(rss_in_bytes) / bytes_per_kb / kb_per_megabyte
-
-    @staticmethod
-    def rss():
-        """
-        Returns the memory usage (RSS) of this process, in bytes
-        """
-        return psutil.Process().memory_info().rss
-
-    @staticmethod
-    def rss_in_mb():
-        return Memory.rss_to_mb(Memory.rss())
-
-    @staticmethod
-    def get_delta(prior_rss_in_mb):
-        mem = Memory.rss_in_mb()
-        if mem > prior_rss_in_mb:
-            return mem - prior_rss_in_mb
-        return 0.0
-
-    def metric_type(self):
-        return "Memory"
-
-    def metric_name(self):
-        return "Physical"
-
-    def human_name(self):
-        return "Process Memory"
+    metric_type = "Memory"
+    metric_name = "Physical"
+    human_name = "Process Memory"
 
     def run(self):
-        res = self.__class__.rss_in_mb()
-        logger.debug("%s: #%s", self.human_name(), res)
-        return res
+        value = get_rss_in_mb()
+        logger.debug("%s: #%s", self.human_name, value)
+        return value
diff --git a/src/scout_apm/core/samplers/thread.py b/src/scout_apm/core/samplers/thread.py
index 735f0f2..5f90feb 100644
--- a/src/scout_apm/core/samplers/thread.py
+++ b/src/scout_apm/core/samplers/thread.py
@@ -56,7 +56,7 @@ class SamplersThread(threading.Thread):
             for instance in instances:
                 event_value = instance.run()
                 if event_value is not None:
-                    event_type = instance.metric_type() + "/" + instance.metric_name()
+                    event_type = instance.metric_type + "/" + instance.metric_name
                     event = ApplicationEvent(
                         event_value=event_value,
                         event_type=event_type,
diff --git a/src/scout_apm/core/tracked_request.py b/src/scout_apm/core/tracked_request.py
index fe29ea6..f5e8256 100644
--- a/src/scout_apm/core/tracked_request.py
+++ b/src/scout_apm/core/tracked_request.py
@@ -9,7 +9,7 @@ from scout_apm.core import backtrace, objtrace
 from scout_apm.core.commands import BatchCommand
 from scout_apm.core.context import AgentContext
 from scout_apm.core.n_plus_one_call_set import NPlusOneCallSet
-from scout_apm.core.samplers.memory import Memory
+from scout_apm.core.samplers.memory import get_rss_in_mb
 from scout_apm.core.samplers.thread import SamplersThread
 from scout_apm.core.thread_local import ThreadLocalSingleton
 
@@ -31,7 +31,7 @@ class TrackedRequest(ThreadLocalSingleton):
         "complete_spans",
         "tags",
         "_is_real_request",
-        "memory_start",
+        "_memory_start",
         "callset",
     )
 
@@ -43,7 +43,7 @@ class TrackedRequest(ThreadLocalSingleton):
         self.complete_spans = []
         self.tags = {}
         self._is_real_request = False
-        self.memory_start = Memory.rss_in_mb()
+        self._memory_start = get_rss_in_mb()
         self.callset = NPlusOneCallSet()
         logger.debug("Starting request: %s", self.request_id)
 
@@ -112,7 +112,7 @@ class TrackedRequest(ThreadLocalSingleton):
         if self.end_time is None:
             self.end_time = dt.datetime.utcnow()
         if self.is_real_request():
-            self.tag("mem_delta", Memory.get_delta(self.memory_start))
+            self.tag("mem_delta", self._get_mem_delta())
             if not self.is_ignored():
                 batch_command = BatchCommand.from_tracked_request(self)
                 AgentContext.socket().send(batch_command)
@@ -125,6 +125,12 @@ class TrackedRequest(ThreadLocalSingleton):
         except Exception:
             pass
 
+    def _get_mem_delta(self):
+        current_mem = get_rss_in_mb()
+        if current_mem > self._memory_start:
+            return current_mem - self._memory_start
+        return 0.0
+
     # A request is ignored if the tag "ignore_transaction" is set to True
     def is_ignored(self):
         return self.tags.get("ignore_transaction", False)


Test patch:
diff --git a/tests/unit/core/samplers/test_cpu.py b/tests/unit/core/samplers/test_cpu.py
index f529d8b..7a4de7d 100644
--- a/tests/unit/core/samplers/test_cpu.py
+++ b/tests/unit/core/samplers/test_cpu.py
@@ -10,18 +10,6 @@ from scout_apm.core.samplers.cpu import Cpu
 from tests.compat import mock
 
 
-def test_metric_type():
-    assert Cpu().metric_type() == "CPU"
-
-
-def test_metric_name():
-    assert Cpu().metric_name() == "Utilization"
-
-
-def test_human_name():
-    assert Cpu().human_name() == "Process CPU"
-
-
 def test_run_negative_time_elapsed(caplog):
     cpu = Cpu()
     cpu.last_run = dt.datetime.utcnow() + dt.timedelta(days=100)
@@ -103,7 +91,7 @@ def test_run_undetermined_cpus(caplog):
     assert record_tuples[0] == (
         "scout_apm.core.samplers.cpu",
         logging.DEBUG,
-        "Could not determine CPU count - assume there is one.",
+        "Could not determine CPU count - assuming there is one.",
     )
     _, level, message = record_tuples[1]
     assert level == logging.DEBUG
diff --git a/tests/unit/core/samplers/test_memory.py b/tests/unit/core/samplers/test_memory.py
index 0702fbc..45d0414 100644
--- a/tests/unit/core/samplers/test_memory.py
+++ b/tests/unit/core/samplers/test_memory.py
@@ -3,57 +3,9 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 import logging
 
-import pytest
-import six
-
 from scout_apm.core.samplers.memory import Memory
 
 
-@pytest.mark.parametrize(
-    "given, expected",
-    [
-        (0, 0.0),
-        (1024 * 1024, 1.0),
-        (2 * 1024 * 1024, 2.0),
-        (2 * 1024 * 1024 + 1, 2.0000009536743164),
-    ],
-)
-def test_rss_to_mb(given, expected):
-    assert Memory.rss_to_mb(given) == expected
-
-
-def test_rss():
-    result = Memory.rss()
-    assert isinstance(result, six.integer_types) and result > 0
-
-
-def test_rss_in_mb():
-    result = Memory.rss_in_mb()
-    assert isinstance(result, float) and result > 0.0
-
-
-def test_get_delta():
-    result = Memory.get_delta(1.0)
-    assert isinstance(result, float) and result > 0.0
-
-
-def test_get_delta_big():
-    result = Memory.get_delta(1e12)
-    assert result == 0.0
-
-
-def test_metric_type():
-    assert Memory().metric_type() == "Memory"
-
-
-def test_metric_name():
-    assert Memory().metric_name() == "Physical"
-
-
-def test_human_name():
-    assert Memory().human_name() == "Process Memory"
-
-
 def test_run(caplog):
     result = Memory().run()
     assert isinstance(result, float) and result > 0.0