8000 Fix tracing tests by sl0thentr0py · Pull Request #3799 · getsentry/sentry-python · GitHub
[go: up one dir, main page]

Skip to content

Fix tracing tests #3799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading 8000
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/opentelemetry/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes bug in the unsampled case


dsc_sampled = parent_context.trace_state.get(TRACESTATE_SAMPLED_KEY)
if dsc_sampled == "true":
Expand Down
8 changes: 0 additions & 8 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this opaque behavior, we will not rely on it anymore

# 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):
Expand Down
7 changes: 5 additions & 2 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ class TransactionKwargs(SpanKwargs, total=False):
}

DEFAULT_SPAN_ORIGIN = "manual"
DEFAULT_SPAN_NAME = "<unlabeled span>"

tracer = otel_trace.get_tracer(__name__)

Expand Down Expand Up @@ -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 {}
Expand Down Expand Up @@ -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(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes another twp bug

self._otel_span, ReadableSpan
)

@property
def sampled(self):
Expand Down
179 changes: 61 additions & 118 deletions tests/tracing/test_integration_tests.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 2364 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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplified this test since it was too confusing and we can write a separate test for explicit sampling later if really wanted

): # noqa:N803
"""
Ensure data is actually passed along via headers, and that they are read
correctly.
Expand All @@ -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()
Expand All @@ -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,
}
Expand All @@ -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)],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer relevant

)
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
Expand All @@ -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

Expand All @@ -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:]
Expand All @@ -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()
Loading
Loading
0