Skip to content

Sample on root span level #3767

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 13 commits into from
Nov 14, 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
Diff view
Diff view
3 changes: 1 addition & 2 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh

- The SDK now supports Python 3.7 and higher.
- `sentry_sdk.start_span` now only takes keyword arguments.
- `sentry_sdk.start_span` no longer takes an explicit `span` argument.
- `sentry_sdk.start_span` no longer takes explicit `trace_id`, `span_id` or `parent_span_id` arguments.
- `sentry_sdk.start_transaction`/`sentry_sdk.start_span` no longer takes the following arguments: `span`, `parent_sampled`, `trace_id`, `span_id` or `parent_span_id`.
- The `Span()` constructor does not accept a `hub` parameter anymore.
- `Span.finish()` does not accept a `hub` parameter anymore.
- The `Profile()` constructor does not accept a `hub` parameter anymore.
Expand Down
22 changes: 15 additions & 7 deletions sentry_sdk/integrations/opentelemetry/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,29 @@ def should_sample(
if not has_tracing_enabled(client.options):
return dropped_result(parent_span_context, attributes)

# parent_span_context.is_valid means this span has a parent, remote or local
is_root_span = not parent_span_context.is_valid or parent_span_context.is_remote

# Explicit sampled value provided at start_span
if attributes.get(SentrySpanAttribute.CUSTOM_SAMPLED) is not None:
sample_rate = float(attributes[SentrySpanAttribute.CUSTOM_SAMPLED])
if sample_rate > 0:
return sampled_result(parent_span_context, attributes, sample_rate)
if is_root_span:
sample_rate = float(attributes[SentrySpanAttribute.CUSTOM_SAMPLED])
if sample_rate > 0:
return sampled_result(parent_span_context, attributes, sample_rate)
else:
return dropped_result(parent_span_context, attributes)
else:
return dropped_result(parent_span_context, attributes)
logger.debug(
f"[Tracing] Ignoring sampled param for non-root span {name}"
)

sample_rate = None

# Check if there is a traces_sampler
# Traces_sampler is responsible to check parent sampled to have full transactions.
has_traces_sampler = callable(client.options.get("traces_sampler"))
if has_traces_sampler:

if is_root_span and has_traces_sampler:
sampling_context = {
"transaction_context": {
"name": name,
Expand Down Expand Up @@ -161,8 +170,7 @@ def should_sample(
return dropped_result(parent_span_context, attributes)

# Down-sample in case of back pressure monitor says so
# TODO: this should only be done for transactions (aka root spans)
if client.monitor:
if is_root_span and client.monitor:
sample_rate /= 2**client.monitor.downsample_factor

# Roll the dice on sample rate
Expand Down
81 changes: 43 additions & 38 deletions tests/tracing/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,33 @@

import sentry_sdk
from sentry_sdk import start_span, start_transaction, capture_exception
from sentry_sdk.tracing import Transaction
from sentry_sdk.utils import logger


def test_sampling_decided_only_for_transactions(sentry_init, capture_events):
def test_sampling_decided_only_for_root_spans(sentry_init):
sentry_init(traces_sample_rate=0.5)

with start_transaction(name="hi") as transaction:
assert transaction.sampled is not None
with start_span(name="outer1") as root_span1:
assert root_span1.sampled is not None

with start_span() as span:
assert span.sampled == transaction.sampled
with start_span(name="inner") as span:
assert span.sampled == root_span1.sampled

with start_span() as span:
assert span.sampled is None
with start_span(name="outer2") as root_span2:
assert root_span2.sampled is not None


@pytest.mark.parametrize("sampled", [True, False])
def test_nested_transaction_sampling_override(sentry_init, sampled):
def test_nested_span_sampling_override(sentry_init, sampled):
sentry_init(traces_sample_rate=1.0)

with start_transaction(name="outer", sampled=sampled) as outer_transaction:
assert outer_transaction.sampled is sampled
with start_transaction(
name="inner", sampled=(not sampled)
) as inner_transaction:
assert inner_transaction.sampled is not sampled
assert outer_transaction.sampled is sampled
with start_span(name="outer", sampled=sampled) as outer_span:
assert outer_span.sampled is sampled
with start_span(name="inner", sampled=(not sampled)) as inner_span:
# won't work because the child span inherits the sampling decision
# from the parent
assert inner_span.sampled is sampled
assert outer_span.sampled is sampled


def test_no_double_sampling(sentry_init, capture_events):
Expand Down Expand Up @@ -147,10 +146,17 @@ def test_ignores_inherited_sample_decision_when_traces_sampler_defined(
traces_sampler = mock.Mock(return_value=not parent_sampling_decision)
sentry_init(traces_sampler=traces_sampler)

transaction = start_transaction(
name="dogpark", parent_sampled=parent_sampling_decision
sentry_trace_header = (
"12312012123120121231201212312012-1121201211212012-{sampled}".format(
sampled=int(parent_sampling_decision)
)
)
assert transaction.sampled is not parent_sampling_decision

with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
with sentry_sdk.start_span(name="dogpark") as span:
pass

assert span.sampled is not parent_sampling_decision


@pytest.mark.parametrize("explicit_decision", [True, False])
Expand All @@ -176,39 +182,38 @@ def test_inherits_parent_sampling_decision_when_traces_sampler_undefined(
sentry_init(traces_sample_rate=0.5)
mock_random_value = 0.25 if parent_sampling_decision is False else 0.75

with mock.patch.object(random, "random", return_value=mock_random_value):
transaction = start_transaction(
name="dogpark", parent_sampled=parent_sampling_decision
sentry_trace_header = (
"12312012123120121231201212312012-1121201211212012-{sampled}".format(
sampled=int(parent_sampling_decision)
)
assert transaction.sampled is parent_sampling_decision
)
with mock.patch.object(random, "random", return_value=mock_random_value):
with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
with start_span(name="dogpark") as span:
pass

assert span.sampled is parent_sampling_decision


@pytest.mark.parametrize("parent_sampling_decision", [True, False])
def test_passes_parent_sampling_decision_in_sampling_context(
sentry_init, parent_sampling_decision
):
sentry_init(traces_sample_rate=1.0)
def dummy_traces_sampler(sampling_context):
assert sampling_context["parent_sampled"] is parent_sampling_decision
return 1.0

sentry_init(traces_sample_rate=1.0, traces_sampler=dummy_traces_sampler)

sentry_trace_header = (
"12312012123120121231201212312012-1121201211212012-{sampled}".format(
sampled=int(parent_sampling_decision)
)
)

transaction = Transaction.continue_from_headers(
headers={"sentry-trace": sentry_trace_header}, name="dogpark"
)
spy = mock.Mock(wraps=transaction)
start_transaction(transaction=spy)

# there's only one call (so index at 0) and kwargs are always last in a call
# tuple (so index at -1)
sampling_context = spy._set_initial_sampling_decision.mock_calls[0][-1][
"sampling_context"
]
assert "parent_sampled" in sampling_context
# because we passed in a spy, attribute access requires unwrapping
assert sampling_context["parent_sampled"]._mock_wraps is parent_sampling_decision
with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
with sentry_sdk.start_span(name="dogpark"):
pass


def test_passes_attributes_from_start_span_to_traces_sampler(
Expand Down
Loading