-
Notifications
You must be signed in to change notification settings - Fork 549
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
Fix tracing tests #3799
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixes another twp bug |
||
self._otel_span, ReadableSpan | ||
) | ||
|
||
@property | ||
def sampled(self): | ||
|
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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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)], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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() |
There was a problem hiding this comment.
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