fix: Better memory usage for tracing (getsentry#431)

untitaker · web-flow · commit 520158dab56a · 2019-07-15T17:12:43.000+02:00
* fix: Better memory usage for tracing Fix getsentry#430 * fix: Remove useless gc.collect * Revert "fix: Remove useless gc.collect" This reverts commit 897b683. * doc: Add comment to gc.collect * fix: Do not trim at all
diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py
@@ -479,6 +479,9 @@ def start_span(
             sample_rate = client and client.options["traces_sample_rate"] or 0
             span.sampled = random.random() < sample_rate
 
+        if span.sampled:
+            span.init_finished_spans()
+
         return span
 
     def finish_span(
@@ -517,7 +520,9 @@ def finish_span(
                 "contexts": {"trace": span.get_trace_context()},
                 "timestamp": span.timestamp,
                 "start_timestamp": span.start_timestamp,
-                "spans": [s.to_json() for s in span._finished_spans if s is not span],
+                "spans": [
+                    s.to_json() for s in (span._finished_spans or ()) if s is not span
+                ],
             }
         )
 
diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py
@@ -6,13 +6,14 @@
 
 from sentry_sdk.utils import capture_internal_exceptions, concat_strings
 from sentry_sdk._compat import PY2
+from sentry_sdk._types import MYPY
 
 if PY2:
     from collections import Mapping
 else:
     from collections.abc import Mapping
 
-if False:
+if MYPY:
     import typing
 
     from typing import Optional
@@ -76,15 +77,16 @@ class Span(object):
 
     def __init__(
         self,
-        trace_id=None,
-        span_id=None,
-        parent_span_id=None,
-        same_process_as_parent=True,
-        sampled=None,
-        transaction=None,
-        op=None,
-        description=None,
+        trace_id=None,  # type: Optional[str]
+        span_id=None,  # type: Optional[str]
+        parent_span_id=None,  # type: Optional[str]
+        same_process_as_parent=True,  # type: bool
+        sampled=None,  # type: Optional[bool]
+        transaction=None,  # type: Optional[str]
+        op=None,  # type: Optional[str]
+        description=None,  # type: Optional[str]
     ):
+        # type: (...) -> None
         self.trace_id = trace_id or uuid.uuid4().hex
         self.span_id = span_id or uuid.uuid4().hex[16:]
         self.parent_span_id = parent_span_id
@@ -95,12 +97,16 @@ def __init__(
         self.description = description
         self._tags = {}  # type: Dict[str, str]
         self._data = {}  # type: Dict[str, Any]
-        self._finished_spans = []  # type: List[Span]
+        self._finished_spans = None  # type: Optional[List[Span]]
         self.start_timestamp = datetime.now()
 
         #: End timestamp of span
         self.timestamp = None
 
+    def init_finished_spans(self):
+        if self._finished_spans is None:
+            self._finished_spans = []
+
     def __repr__(self):
         return (
             "<%s(transaction=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>"
@@ -184,7 +190,8 @@ def set_data(self, key, value):
 
     def finish(self):
         self.timestamp = datetime.now()
-        self._finished_spans.append(self)
+        if self._finished_spans is not None:
+            self._finished_spans.append(self)
 
     def to_json(self):
         return {
diff --git a/tests/test_tracing.py b/tests/test_tracing.py
@@ -1,3 +1,6 @@
+import weakref
+import gc
+
 import pytest
 
 from sentry_sdk import Hub, capture_message
@@ -93,3 +96,34 @@ def test_sampling_decided_only_for_transactions(sentry_init, capture_events):
 
     with Hub.current.span() as span:
         assert span.sampled is None
+
+
+@pytest.mark.parametrize(
+    "args,expected_refcount",
+    [({"traces_sample_rate": 1.0}, 100), ({"traces_sample_rate": 0.0}, 0)],
+)
+def test_memory_usage(sentry_init, capture_events, args, expected_refcount):
+    sentry_init(**args)
+
+    references = weakref.WeakSet()
+
+    with Hub.current.span(transaction="hi"):
+        for i in range(100):
+            with Hub.current.span(
+                op="helloworld", description="hi {}".format(i)
+            ) as span:
+
+                def foo():
+                    pass
+
+                references.add(foo)
+                span.set_tag("foo", foo)
+                pass
+
+        del foo
+        del span
+
+        # required only for pypy (cpython frees immediately)
+        gc.collect()
+
+        assert len(references) == expected_refcount
