feat(tracing): Make spans point to their transactions (getsentry#870)

lobsterkatie · web-flow · commit 5bb6ffc729cc · 2020-10-21T11:32:22.000-07:00
diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py
@@ -109,6 +109,9 @@ class Span(object):
         "_span_recorder",
         "hub",
         "_context_manager_state",
+        # TODO: rename this "transaction" once we fully and truly deprecate the
+        # old "transaction" attribute (which was actually the transaction name)?
+        "_containing_transaction",
     )
 
     def __new__(cls, **kwargs):
@@ -164,6 +167,7 @@ def __init__(
         self.timestamp = None  # type: Optional[datetime]
 
         self._span_recorder = None  # type: Optional[_SpanRecorder]
+        self._containing_transaction = None  # type: Optional[Transaction]
 
     def init_span_recorder(self, maxlen):
         # type: (int) -> None
@@ -210,15 +214,20 @@ def start_child(self, **kwargs):
         Start a sub-span from the current span or transaction.
 
         Takes the same arguments as the initializer of :py:class:`Span`. The
-        trace id, sampling decision, and span recorder are inherited from the
-        current span/transaction.
+        trace id, sampling decision, transaction pointer, and span recorder are
+        inherited from the current span/transaction.
         """
         kwargs.setdefault("sampled", self.sampled)
 
         rv = Span(
             trace_id=self.trace_id, span_id=None, parent_span_id=self.span_id, **kwargs
         )
 
+        if isinstance(self, Transaction):
+            rv._containing_transaction = self
+        else:
+            rv._containing_transaction = self._containing_transaction
+
         rv._span_recorder = recorder = self._span_recorder
         if recorder:
             recorder.add(rv)
diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py
@@ -1,7 +1,7 @@
 import pytest
 
-from sentry_sdk import start_span, start_transaction
-from sentry_sdk.tracing import Transaction
+from sentry_sdk import Hub, start_span, start_transaction
+from sentry_sdk.tracing import Span, Transaction
 
 
 def test_span_trimming(sentry_init, capture_events):
@@ -49,3 +49,82 @@ def test_transaction_method_signature(sentry_init, capture_events):
     with start_transaction(Transaction(name="c")):
         pass
     assert len(events) == 4
+
+
+def test_finds_transaction_on_scope(sentry_init):
+    sentry_init(traces_sample_rate=1.0)
+
+    transaction = start_transaction(name="dogpark")
+
+    scope = Hub.current.scope
+
+    # See note in Scope class re: getters and setters of the `transaction`
+    # property. For the moment, assigning to scope.transaction merely sets the
+    # transaction name, rather than putting the transaction on the scope, so we
+    # have to assign to _span directly.
+    scope._span = transaction
+
+    # Reading scope.property, however, does what you'd expect, and returns the
+    # transaction on the scope.
+    assert scope.transaction is not None
+    assert isinstance(scope.transaction, Transaction)
+    assert scope.transaction.name == "dogpark"
+
+    # If the transaction is also set as the span on the scope, it can be found
+    # by accessing _span, too.
+    assert scope._span is not None
+    assert isinstance(scope._span, Transaction)
+    assert scope._span.name == "dogpark"
+
+
+def test_finds_transaction_when_decedent_span_is_on_scope(
+    sentry_init,
+):
+    sentry_init(traces_sample_rate=1.0)
+
+    transaction = start_transaction(name="dogpark")
+    child_span = transaction.start_child(op="sniffing")
+
+    scope = Hub.current.scope
+    scope._span = child_span
+
+    # this is the same whether it's the transaction itself or one of its
+    # decedents directly attached to the scope
+    assert scope.transaction is not None
+    assert isinstance(scope.transaction, Transaction)
+    assert scope.transaction.name == "dogpark"
+
+    # here we see that it is in fact the span on the scope, rather than the
+    # transaction itself
+    assert scope._span is not None
+    assert isinstance(scope._span, Span)
+    assert scope._span.op == "sniffing"
+
+
+def test_finds_orphan_span_on_scope(sentry_init):
+    # this is deprecated behavior which may be removed at some point (along with
+    # the start_span function)
+    sentry_init(traces_sample_rate=1.0)
+
+    span = start_span(op="sniffing")
+
+    scope = Hub.current.scope
+    scope._span = span
+
+    assert scope._span is not None
+    assert isinstance(scope._span, Span)
+    assert scope._span.op == "sniffing"
+
+
+def test_finds_non_orphan_span_on_scope(sentry_init):
+    sentry_init(traces_sample_rate=1.0)
+
+    transaction = start_transaction(name="dogpark")
+    child_span = transaction.start_child(op="sniffing")
+
+    scope = Hub.current.scope
+    scope._span = child_span
+
+    assert scope._span is not None
+    assert isinstance(scope._span, Span)
+    assert scope._span.op == "sniffing"
