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 4 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
1 change: 1 addition & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,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` now accepts the `sampled` argument. It is however only taken into account for root spans (previously known as transactions).
- 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
3 changes: 3 additions & 0 deletions sentry_sdk/integrations/opentelemetry/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ class SentrySpanAttribute:
SOURCE = "sentry.source"
CONTEXT = "sentry.context"
CUSTOM_SAMPLED = "sentry.custom_sampled" # used for saving start_span(sampled=X)
CUSTOM_PARENT_SAMPLED = (
"sentry.custom_parent_sampled" # used for saving start_span(parent_sampled=X)
)
27 changes: 19 additions & 8 deletions sentry_sdk/integrations/opentelemetry/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,28 @@ 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 All @@ -146,7 +154,11 @@ def should_sample(

else:
# Check if there is a parent with a sampling decision
parent_sampled = get_parent_sampled(parent_span_context, trace_id)
parent_sampled = attributes.get(SentrySpanAttribute.CUSTOM_PARENT_SAMPLED)

if parent_sampled is None:
parent_sampled = get_parent_sampled(parent_span_context, trace_id)

if parent_sampled is not None:
sample_rate = parent_sampled
else:
Expand All @@ -161,8 +173,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
5 changes: 5 additions & 0 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ def __init__(
attributes=None, # type: OTelSpanAttributes
only_if_parent=False, # type: bool
otel_span=None, # type: Optional[OtelSpan]
parent_sampled=None, # type: Optional[bool]
**_, # type: dict[str, object]
):
# type: (...) -> None
Expand Down Expand Up @@ -1252,6 +1253,10 @@ def __init__(
attributes[SentrySpanAttribute.OP] = op
if sampled is not None:
attributes[SentrySpanAttribute.CUSTOM_SAMPLED] = sampled
if parent_sampled is not None:
attributes[SentrySpanAttribute.CUSTOM_PARENT_SAMPLED] = (
parent_sampled
)

self._otel_span = tracer.start_span(
span_name, start_time=start_timestamp, attributes=attributes
Expand Down
22 changes: 10 additions & 12 deletions tests/tracing/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ def test_sampling_decided_only_for_transactions(sentry_init, capture_events):


@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 @@ -177,10 +177,8 @@ def test_inherits_parent_sampling_decision_when_traces_sampler_undefined(
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
)
assert transaction.sampled is parent_sampling_decision
span = start_span(name="dogpark", parent_sampled=parent_sampling_decision)
assert span.sampled is parent_sampling_decision


@pytest.mark.parametrize("parent_sampling_decision", [True, False])
Expand Down
Loading