diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 8913550ae8..8d8ee0d682 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -10,7 +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_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. diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 3aaa563bd8..302b66aaaa 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -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, @@ -162,8 +171,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 diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index bafb639c34..432427b08e 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -535,8 +535,6 @@ async def handler(request): with start_transaction( name="/interactions/other-dogs/new-dog", op="greeting.sniff", - # make trace_id difference between transactions - trace_id="0123456789012345678901234567890", ) as transaction: client = await aiohttp_client(raw_server) resp = await client.get("/") @@ -572,14 +570,21 @@ async def handler(request): with start_transaction( name="/interactions/other-dogs/new-dog", op="greeting.sniff", - trace_id="0123456789012345678901234567890", - ): + ) as transaction: client = await aiohttp_client(raw_server) resp = await client.get("/", headers={"bagGage": "custom=value"}) assert ( - resp.request_info.headers["baggage"] - == "custom=value,sentry-trace_id=0123456789012345678901234567890,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true" + sorted(resp.request_info.headers["baggage"].split(",")) + == sorted([ + "custom=value", + f"sentry-trace_id={transaction.trace_id}", + "sentry-environment=production", + "sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42", + "sentry-transaction=/interactions/other-dogs/new-dog", + "sentry-sample_rate=1.0", + "sentry-sampled=true", + ]) ) diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index 70baacc951..8ef362a1e8 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -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): @@ -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]) @@ -176,18 +182,28 @@ 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( @@ -195,20 +211,9 @@ def test_passes_parent_sampling_decision_in_sampling_context( ) ) - 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(