diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index b3dbde6f65..5e9ae8a0e0 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -67,28 +67,26 @@ def __iter__(self): class _SpanRecorder(object): - __slots__ = ("maxlen", "finished_spans", "open_span_count") + """Limits the number of spans recorded in a transaction.""" + + __slots__ = ("maxlen", "spans") def __init__(self, maxlen): # type: (int) -> None - self.maxlen = maxlen - self.open_span_count = 0 # type: int - self.finished_spans = [] # type: List[Span] - - def start_span(self, span): + # FIXME: this is `maxlen - 1` only to preserve historical behavior + # enforced by tests. + # Either this should be changed to `maxlen` or the JS SDK implementation + # should be changed to match a consistent interpretation of what maxlen + # limits: either transaction+spans or only child spans. + self.maxlen = maxlen - 1 + self.spans = [] # type: List[Span] + + def add(self, span): # type: (Span) -> None - - # This is just so that we don't run out of memory while recording a lot - # of spans. At some point we just stop and flush out the start of the - # trace tree (i.e. the first n spans with the smallest - # start_timestamp). - self.open_span_count += 1 - if self.open_span_count > self.maxlen: + if len(self.spans) > self.maxlen: span._span_recorder = None - - def finish_span(self, span): - # type: (Span) -> None - self.finished_spans.append(span) + else: + self.spans.append(span) class Span(object): @@ -157,7 +155,7 @@ def init_finished_spans(self, maxlen): # type: (int) -> None if self._span_recorder is None: self._span_recorder = _SpanRecorder(maxlen) - self._span_recorder.start_span(self) + self._span_recorder.add(self) def __repr__(self): # type: () -> str @@ -330,8 +328,6 @@ def finish(self, hub=None): if self._span_recorder is None: return None - self._span_recorder.finish_span(self) - if self.transaction is None: # If this has no transaction set we assume there's a parent # transaction for this span that would be flushed out eventually. @@ -354,6 +350,12 @@ def finish(self, hub=None): return None + finished_spans = [ + span.to_json(client) + for span in self._span_recorder.spans + if span is not self and span.timestamp is not None + ] + return hub.capture_event( { "type": "transaction", @@ -362,11 +364,7 @@ def finish(self, hub=None): "tags": self._tags, "timestamp": self.timestamp, "start_timestamp": self.start_timestamp, - "spans": [ - s.to_json(client) - for s in self._span_recorder.finished_spans - if s is not self - ], + "spans": finished_spans, } ) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index b3a08f5c50..3c26b426f5 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -518,10 +518,10 @@ def test_middleware_spans(sentry_init, client, capture_events): if DJANGO_VERSION >= (1, 10): reference_value = [ - "tests.integrations.django.myapp.settings.TestFunctionMiddleware.__call__", - "tests.integrations.django.myapp.settings.TestMiddleware.__call__", - "django.contrib.auth.middleware.AuthenticationMiddleware.__call__", "django.contrib.sessions.middleware.SessionMiddleware.__call__", + "django.contrib.auth.middleware.AuthenticationMiddleware.__call__", + "tests.integrations.django.myapp.settings.TestMiddleware.__call__", + "tests.integrations.django.myapp.settings.TestFunctionMiddleware.__call__", ] else: reference_value = [ diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index ee6e7c8c60..e2ae005d2a 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -140,13 +140,15 @@ def test_subprocess_basic( ( subprocess_init_span, - subprocess_wait_span, subprocess_communicate_span, + subprocess_wait_span, ) = transaction_event["spans"] - assert subprocess_init_span["op"] == "subprocess" - assert subprocess_communicate_span["op"] == "subprocess.communicate" - assert subprocess_wait_span["op"] == "subprocess.wait" + assert ( + subprocess_init_span["op"], + subprocess_communicate_span["op"], + subprocess_wait_span["op"], + ) == ("subprocess", "subprocess.communicate", "subprocess.wait") # span hierarchy assert (