fix: Better memory usage for tracing

untitaker · untitaker · commit 614ec68dd1b4 · 2019-07-13T18:01:12.000+02:00
Fix getsentry#430
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 and client:
+            span.init_finished_spans(client.options["max_breadcrumbs"])
+
         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
@@ -2,23 +2,25 @@
 import uuid
 import contextlib
 
+from collections import deque
 from datetime import datetime
 
 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
     from typing import Any
     from typing import Dict
-    from typing import List
+    from typing import Deque
 
 _traceparent_header_format_re = re.compile(
     "^[ \t]*"  # whitespace
@@ -76,15 +78,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 +98,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[Deque[Span]]
         self.start_timestamp = datetime.now()
 
         #: End timestamp of span
         self.timestamp = None
 
+    def init_finished_spans(self, maxlen):
+        if self._finished_spans is None:
+            self._finished_spans = deque(maxlen=maxlen)
+
     def __repr__(self):
         return (
             "<%s(transaction=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>"
@@ -184,7 +191,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,38 @@ 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",
+    [
+        # Tracing is enabled, but the max amount of spans is 0
+        ({"traces_sample_rate": 1.0, "max_breadcrumbs": 0}, 0),
+        # Tracing is enabled, but the max amount of spans is 10
+        ({"traces_sample_rate": 1.0, "max_breadcrumbs": 10}, 10),
+        # Tracing is disabled, so max amount of spans should not matter
+        ({"traces_sample_rate": 0.0, "max_breadcrumbs": 100}, 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
+        gc.collect()
+        assert len(references) == expected_refcount
