Fix tracing tests

Author: sl0thentr0py

sentry_sdk/integrations/opentelemetry/potel_span_processor.py

@@ -179,7 +179,7 @@ def _root_span_to_transaction_event(self, span):
 
         transaction_name, transaction_source = extract_transaction_name_source(span)
         span_data = extract_span_data(span)
-        (_, description, _, http_status, _) = span_data
+        (_, description, status, http_status, _) = span_data
 
         trace_context = get_trace_context(span, span_data=span_data)
         contexts = {"trace": trace_context}
@@ -241,6 +241,9 @@ def _span_to_json(self, span):
             }
         )
 
+        if status:
+            span_json.setdefault("tags", {})["status"] = status
+
         if parent_span_id:
             span_json["parent_span_id"] = parent_span_id
 

sentry_sdk/integrations/opentelemetry/sampler.py

@@ -34,8 +34,8 @@ def get_parent_sampled(parent_context, trace_id):
     # Only inherit sample rate if `traceId` is the same
     if is_span_context_valid and parent_context.trace_id == trace_id:
         # this is getSamplingDecision in JS
-        if parent_context.trace_flags.sampled:
-            return True
+        if parent_context.trace_flags.sampled is not None:
+            return parent_context.trace_flags.sampled
 
         dsc_sampled = parent_context.trace_state.get(TRACESTATE_SAMPLED_KEY)
         if dsc_sampled == "true":

sentry_sdk/integrations/opentelemetry/utils.py

@@ -336,7 +336,7 @@ def trace_state_from_baggage(baggage):
 
 def baggage_from_trace_state(trace_state):
     # type: (TraceState) -> Baggage
-    return Baggage(dsc_from_trace_state(trace_state))
+    return Baggage(dsc_from_trace_state(trace_state), mutable=False)
 
 
 def serialize_trace_state(trace_state):

sentry_sdk/scope.py

@@ -786,14 +786,6 @@ def span(self, span):
         # type: (Optional[Span]) -> None
         """Set current tracing span."""
         self._span = span
-        # XXX: this differs from the implementation in JS, there Scope.setSpan
-        # does not set Scope._transactionName.
-        if isinstance(span, Transaction):
-            transaction = span
-            if transaction.name:
-                self._transaction = transaction.name
-                if transaction.source:
-                    self._transaction_info["source"] = transaction.source
 
     @property
     def profile(self):

sentry_sdk/tracing.py

@@ -163,6 +163,7 @@ class TransactionKwargs(SpanKwargs, total=False):
 }
 
 DEFAULT_SPAN_ORIGIN = "manual"
+DEFAULT_SPAN_NAME = "<unlabeled span>"
 
 tracer = otel_trace.get_tracer(__name__)
 
@@ -1249,7 +1250,7 @@ def __init__(
                     # OTel timestamps have nanosecond precision
                     start_timestamp = convert_to_otel_timestamp(start_timestamp)
 
-                span_name = name or description or op or ""
+                span_name = name or description or op or DEFAULT_SPAN_NAME
 
                 # Prepopulate some attrs so that they're accessible in traces_sampler
                 attributes = attributes or {}
@@ -1398,7 +1399,9 @@ def span_id(self):
     @property
     def is_valid(self):
         # type: () -> bool
-        return self._otel_span.get_span_context().is_valid
+        return self._otel_span.get_span_context().is_valid and isinstance(
+            self._otel_span, ReadableSpan
+        )
 
     @property
     def sampled(self):

tests/tracing/test_integration_tests.py

@@ -1,27 +1,24 @@
-import weakref
-import gc
 import re
 import pytest
 import random
 
 import sentry_sdk
 from sentry_sdk import (
     capture_message,
+    continue_trace,
     start_span,
-    start_transaction,
 )
 from sentry_sdk.consts import SPANSTATUS
 from sentry_sdk.transport import Transport
-from sentry_sdk.tracing import Transaction
 
 
 @pytest.mark.parametrize("sample_rate", [0.0, 1.0])
 def test_basic(sentry_init, capture_events, sample_rate):
     sentry_init(traces_sample_rate=sample_rate)
     events = capture_events()
 
-    with start_transaction(name="hi") as transaction:
-        transaction.set_status(SPANSTATUS.OK)
+    with start_span(name="hi") as root_span:
+        root_span.set_status(SPANSTATUS.OK)
         with pytest.raises(ZeroDivisionError):
             with start_span(op="foo", name="foodesc"):
                 1 / 0
@@ -39,21 +36,23 @@ def test_basic(sentry_init, capture_events, sample_rate):
         span1, span2 = event["spans"]
         parent_span = event
         assert span1["tags"]["status"] == "internal_error"
+        assert span1["status"] == "internal_error"
         assert span1["op"] == "foo"
         assert span1["description"] == "foodesc"
         assert "status" not in span2.get("tags", {})
         assert span2["op"] == "bar"
         assert span2["description"] == "bardesc"
         assert parent_span["transaction"] == "hi"
-        assert "status" not in event["tags"]
+        assert "status" not in event.get("tags", {})
         assert event["contexts"]["trace"]["status"] == "ok"
     else:
         assert not events
 
 
-@pytest.mark.parametrize("sampled", [True, False, None])
 @pytest.mark.parametrize("sample_rate", [0.0, 1.0])
-def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_rate):
+def test_continue_trace(
+    sentry_init, capture_envelopes, sample_rate, SortedBaggage
+):  # noqa:N803
     """
     Ensure data is actually passed along via headers, and that they are read
     correctly.
@@ -62,55 +61,41 @@ def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_r
     envelopes = capture_envelopes()
 
     # make a parent transaction (normally this would be in a different service)
-    with start_transaction(name="hi", sampled=True if sample_rate == 0 else None):
-        with start_span() as old_span:
-            old_span.sampled = sampled
-            headers = dict(
-                sentry_sdk.get_current_scope().iter_trace_propagation_headers(old_span)
-            )
-            headers["baggage"] = (
-                "other-vendor-value-1=foo;bar;baz, "
-                "sentry-trace_id=771a43a4192642f0b136d5159a501700, "
-                "sentry-public_key=49d0f7386ad645858ae85020e393bef3, "
-                "sentry-sample_rate=0.01337, sentry-user_id=Amelie, "
-                "other-vendor-value-2=foo;bar;"
-            )
+    with start_span(name="hi"):
+        with start_span(name="inner") as old_span:
+            headers = dict(old_span.iter_headers())
+            assert headers["sentry-trace"]
+            assert headers["baggage"]
 
     # child transaction, to prove that we can read 'sentry-trace' header data correctly
-    child_transaction = Transaction.continue_from_headers(headers, name="WRONG")
-    assert child_transaction is not None
-    assert child_transaction.parent_sampled == sampled
-    assert child_transaction.trace_id == old_span.trace_id
-    assert child_transaction.same_process_as_parent is False
-    assert child_transaction.parent_span_id == old_span.span_id
-    assert child_transaction.span_id != old_span.span_id
-
-    baggage = child_transaction._baggage
-    assert baggage
-    assert not baggage.mutable
-    assert baggage.sentry_items == {
-        "public_key": "49d0f7386ad645858ae85020e393bef3",
-        "trace_id": "771a43a4192642f0b136d5159a501700",
-        "user_id": "Amelie",
-        "sample_rate": "0.01337",
-    }
-
-    # add child transaction to the scope, to show that the captured message will
-    # be tagged with the trace id (since it happens while the transaction is
-    # open)
-    with start_transaction(child_transaction):
-        # change the transaction name from "WRONG" to make sure the change
-        # is reflected in the final data
-        sentry_sdk.get_current_scope().transaction = "ho"
-        capture_message("hello")
+    with continue_trace(headers):
+        with start_span(name="WRONG") as child_root_span:
+            assert child_root_span is not None
+            assert child_root_span.sampled == (sample_rate == 1.0)
+            if child_root_span.sampled:
+                assert child_root_span.parent_span_id == old_span.span_id
+            assert child_root_span.trace_id == old_span.trace_id
+            assert child_root_span.span_id != old_span.span_id
+
+            baggage = child_root_span.get_baggage()
+            assert baggage.serialize() == SortedBaggage(headers["baggage"])
+
+            # change the transaction name from "WRONG" to make sure the change
+            # is reflected in the final data
+            sentry_sdk.get_current_scope().set_transaction_name("ho")
+            # to show that the captured message will be tagged with the trace id
+            # (since it happens while the transaction is open)
+            capture_message("hello")
 
     # in this case the child transaction won't be captured
-    if sampled is False or (sample_rate == 0 and sampled is None):
-        trace1, message = envelopes
+    # but message follows twp spec
+    if sample_rate == 0.0:
+        (message,) = envelopes
         message_payload = message.get_event()
-        trace1_payload = trace1.get_transaction_event()
-
-        assert trace1_payload["transaction"] == "hi"
+        assert message_payload["transaction"] == "ho"
+        assert (
+            child_root_span.trace_id == message_payload["contexts"]["trace"]["trace_id"]
+        )
     else:
         trace1, message, trace2 = envelopes
         trace1_payload = trace1.get_transaction_event()
@@ -123,56 +108,43 @@ def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_r
         assert (
             trace1_payload["contexts"]["trace"]["trace_id"]
             == trace2_payload["contexts"]["trace"]["trace_id"]
-            == child_transaction.trace_id
+            == child_root_span.trace_id
             == message_payload["contexts"]["trace"]["trace_id"]
         )
 
         assert trace2.headers["trace"] == baggage.dynamic_sampling_context()
-        assert trace2.headers["trace"] == {
-            "public_key": "49d0f7386ad645858ae85020e393bef3",
-            "trace_id": "771a43a4192642f0b136d5159a501700",
-            "user_id": "Amelie",
-            "sample_rate": "0.01337",
-        }
 
     assert message_payload["message"] == "hello"
 
 
 @pytest.mark.parametrize("sample_rate", [0.5, 1.0])
 def test_dynamic_sampling_head_sdk_creates_dsc(
-    sentry_init, capture_envelopes, sample_rate, monkeypatch
+    sentry_init,
+    capture_envelopes,
+    sample_rate,
+    monkeypatch,
+    SortedBaggage,  # noqa: N803
 ):
     sentry_init(traces_sample_rate=sample_rate, release="foo")
     envelopes = capture_envelopes()
 
     # make sure transaction is sampled for both cases
     monkeypatch.setattr(random, "random", lambda: 0.1)
 
-    transaction = Transaction.continue_from_headers({}, name="Head SDK tx")
-
-    # will create empty mutable baggage
-    baggage = transaction._baggage
-    assert baggage
-    assert baggage.mutable
-    assert baggage.sentry_items == {}
-    assert baggage.third_party_items == ""
-
-    with start_transaction(transaction):
-        with start_span(op="foo", name="foodesc"):
-            pass
+    with continue_trace({}):
+        with start_span(name="Head SDK tx"):
+            with start_span(op="foo", name="foodesc") as span:
+                baggage = span.get_baggage()
 
-    # finish will create a new baggage entry
-    baggage = transaction._baggage
-    trace_id = transaction.trace_id
+    trace_id = span.trace_id
 
     assert baggage
-    assert not baggage.mutable
     assert baggage.third_party_items == ""
     assert baggage.sentry_items == {
         "environment": "production",
         "release": "foo",
         "sample_rate": str(sample_rate),
-        "sampled": "true" if transaction.sampled else "false",
+        "sampled": "true" if span.sampled else "false",
         "transaction": "Head SDK tx",
         "trace_id": trace_id,
     }
@@ -184,59 +156,30 @@ def test_dynamic_sampling_head_sdk_creates_dsc(
         "sentry-transaction=Head%%20SDK%%20tx,"
         "sentry-sample_rate=%s,"
         "sentry-sampled=%s"
-        % (trace_id, sample_rate, "true" if transaction.sampled else "false")
+        % (trace_id, sample_rate, "true" if span.sampled else "false")
     )
-    assert baggage.serialize() == expected_baggage
+    assert baggage.serialize() == SortedBaggage(expected_baggage)
 
     (envelope,) = envelopes
     assert envelope.headers["trace"] == baggage.dynamic_sampling_context()
     assert envelope.headers["trace"] == {
         "environment": "production",
         "release": "foo",
         "sample_rate": str(sample_rate),
-        "sampled": "true" if transaction.sampled else "false",
+        "sampled": "true" if span.sampled else "false",
         "transaction": "Head SDK tx",
         "trace_id": trace_id,
     }
 
 
-@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 start_transaction(name="hi"):
-        for i in range(100):
-            with start_span(op="helloworld", name="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
-
-
 def test_transactions_do_not_go_through_before_send(sentry_init, capture_events):
     def before_send(event, hint):
         raise RuntimeError("should not be called")
 
     sentry_init(traces_sample_rate=1.0, before_send=before_send)
     events = capture_events()
 
-    with start_transaction(name="/"):
+    with start_span(name="/"):
         pass
 
     assert len(events) == 1
@@ -254,7 +197,7 @@ def capture_event(self, event):
     sentry_init(traces_sample_rate=1, transport=CustomTransport())
     events = capture_events()
 
-    with start_transaction(name="hi"):
+    with start_span(name="hi"):
         with start_span(op="bar", name="bardesc"):
             pass
 
@@ -264,14 +207,14 @@ def capture_event(self, event):
 def test_trace_propagation_meta_head_sdk(sentry_init):
     sentry_init(traces_sample_rate=1.0, release="foo")
 
-    transaction = Transaction.continue_from_headers({}, name="Head SDK tx")
     meta = None
     span = None
 
-    with start_transaction(transaction):
-        with start_span(op="foo", name="foodesc") as current_span:
-            span = current_span
-            meta = sentry_sdk.get_current_scope().trace_propagation_meta()
+    with continue_trace({}):
+        with start_span(name="Head SDK tx") as root_span:
+            with start_span(op="foo", name="foodesc") as current_span:
+                span = current_span
+                meta = sentry_sdk.get_current_scope().trace_propagation_meta()
 
     ind = meta.find(">") + 1
     sentry_trace, baggage = meta[:ind], meta[ind:]
@@ -282,4 +225,4 @@ def test_trace_propagation_meta_head_sdk(sentry_init):
 
     assert 'meta name="baggage"' in baggage
     baggage_content = re.findall('content="([^"]*)"', baggage)[0]
-    assert baggage_content == transaction.get_baggage().serialize()
+    assert baggage_content == root_span.get_baggage().serialize()