From c017b015f33ed28ed5d9842eea28f16f757ed6da Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 10:09:21 +0100 Subject: [PATCH 01/12] Sample on root span level --- MIGRATION_GUIDE.md | 1 + .../integrations/opentelemetry/sampler.py | 21 ++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 0095cafab7..052f4df0ba 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -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. +- The `sampled` argument of `sentry_sdk.start_span` is 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. diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 79e2ec7d8f..1b097950df 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -120,20 +120,28 @@ def should_sample( if not has_tracing_enabled(client.options): return dropped_result(parent_span_context, attributes) + has_parent = parent_span_context.is_valid + is_root_span = not has_parent 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, @@ -161,8 +169,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 From b47bc70b814f078b870ccc8c3fc4f7809e6473e7 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 10:11:05 +0100 Subject: [PATCH 02/12] note --- sentry_sdk/integrations/opentelemetry/sampler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 1b097950df..74a09d5056 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -120,8 +120,8 @@ def should_sample( if not has_tracing_enabled(client.options): return dropped_result(parent_span_context, attributes) - has_parent = parent_span_context.is_valid - is_root_span = not has_parent or parent_span_context.is_remote + # 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: From 1d77484ea093b080bd13a72d5f533b0d6da11ec6 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 10:15:13 +0100 Subject: [PATCH 03/12] better wording --- MIGRATION_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 052f4df0ba..3d6a7e896a 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -11,7 +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. -- The `sampled` argument of `sentry_sdk.start_span` is only taken into account for root spans (previously known as transactions). +- `sentry_sdk.start_span` now accepts the `sampled` argument. It is however 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. From cefb27ca9af9e7cefc47e5d17c8101f0936389a2 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 10:24:25 +0100 Subject: [PATCH 04/12] reword and support parent_sampled --- MIGRATION_GUIDE.md | 2 +- .../integrations/opentelemetry/consts.py | 3 +++ .../integrations/opentelemetry/sampler.py | 6 ++++- sentry_sdk/tracing.py | 5 +++++ tests/tracing/test_sampling.py | 22 +++++++++---------- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 3d6a7e896a..11843083c0 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -11,7 +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 taken into account for root spans (previously known as transactions). +- `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. diff --git a/sentry_sdk/integrations/opentelemetry/consts.py b/sentry_sdk/integrations/opentelemetry/consts.py index 1585e8d893..8594b5c11c 100644 --- a/sentry_sdk/integrations/opentelemetry/consts.py +++ b/sentry_sdk/integrations/opentelemetry/consts.py @@ -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) + ) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 74a09d5056..b5dc872ff3 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -154,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: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index a69a6f98be..e4b4691fab 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -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 @@ -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 diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index 70baacc951..192cba133f 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -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): @@ -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]) From 3fcbf65005ab2344928099f2644746025eba8474 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 11:26:39 +0100 Subject: [PATCH 05/12] tests --- .../integrations/opentelemetry/sampler.py | 17 ++++++--- tests/tracing/test_sampling.py | 38 ++++++++----------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index b5dc872ff3..aa18ba1300 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -141,24 +141,29 @@ def should_sample( # 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")) + custom_parent_sampled = attributes.get( + SentrySpanAttribute.CUSTOM_PARENT_SAMPLED + ) + if custom_parent_sampled is not None: + parent_sampled = custom_parent_sampled + else: + parent_sampled = get_parent_sampled(parent_span_context, trace_id) + print("custom_parent_sampled", custom_parent_sampled) + print("get parent sampled", get_parent_sampled(parent_span_context, trace_id)) + if is_root_span and has_traces_sampler: sampling_context = { "transaction_context": { "name": name, "op": attributes.get(SentrySpanAttribute.OP), }, - "parent_sampled": get_parent_sampled(parent_span_context, trace_id), + "parent_sampled": parent_sampled, } sampling_context.update(attributes) sample_rate = client.options["traces_sampler"](sampling_context) else: # Check if there is a parent with a sampling decision - 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: diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index 192cba133f..ce540050f7 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -6,21 +6,20 @@ 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]) @@ -185,7 +184,11 @@ def test_inherits_parent_sampling_decision_when_traces_sampler_undefined( 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( @@ -193,20 +196,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( From 9600484725c330697609c5eb722f8058fe3f3cf5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 11:27:10 +0100 Subject: [PATCH 06/12] . --- sentry_sdk/integrations/opentelemetry/sampler.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index aa18ba1300..9f1a5de8c9 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -148,8 +148,6 @@ def should_sample( parent_sampled = custom_parent_sampled else: parent_sampled = get_parent_sampled(parent_span_context, trace_id) - print("custom_parent_sampled", custom_parent_sampled) - print("get parent sampled", get_parent_sampled(parent_span_context, trace_id)) if is_root_span and has_traces_sampler: sampling_context = { From 69dfe92ce655636ac0ca5ef173f06a3d32a19c45 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 11:30:53 +0100 Subject: [PATCH 07/12] remove, this is not a change afait --- MIGRATION_GUIDE.md | 1 - 1 file changed, 1 deletion(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 11843083c0..0095cafab7 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -11,7 +11,6 @@ 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. From 1b8521b66a6de4f26c0e3e57765864f6aed34fad Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 11:34:33 +0100 Subject: [PATCH 08/12] simplify --- sentry_sdk/integrations/opentelemetry/sampler.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 9f1a5de8c9..77878bb5ed 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -141,12 +141,8 @@ def should_sample( # 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")) - custom_parent_sampled = attributes.get( - SentrySpanAttribute.CUSTOM_PARENT_SAMPLED - ) - if custom_parent_sampled is not None: - parent_sampled = custom_parent_sampled - else: + parent_sampled = attributes.get(SentrySpanAttribute.CUSTOM_PARENT_SAMPLED) + if parent_sampled is None: parent_sampled = get_parent_sampled(parent_span_context, trace_id) if is_root_span and has_traces_sampler: From d9b7f7178344a45f635353e42748902591e46a26 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 15:19:06 +0100 Subject: [PATCH 09/12] remove parent_sampled --- MIGRATION_GUIDE.md | 2 +- .../integrations/opentelemetry/consts.py | 3 -- .../integrations/opentelemetry/sampler.py | 6 ++-- sentry_sdk/tracing.py | 4 --- tests/tracing/test_sampling.py | 30 ------------------- 5 files changed, 3 insertions(+), 42 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 0095cafab7..7b45b480bb 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`. - 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/consts.py b/sentry_sdk/integrations/opentelemetry/consts.py index 8594b5c11c..1585e8d893 100644 --- a/sentry_sdk/integrations/opentelemetry/consts.py +++ b/sentry_sdk/integrations/opentelemetry/consts.py @@ -31,6 +31,3 @@ 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) - ) diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 77878bb5ed..cb722694ac 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -141,9 +141,6 @@ def should_sample( # 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")) - parent_sampled = attributes.get(SentrySpanAttribute.CUSTOM_PARENT_SAMPLED) - if parent_sampled is None: - parent_sampled = get_parent_sampled(parent_span_context, trace_id) if is_root_span and has_traces_sampler: sampling_context = { @@ -151,13 +148,14 @@ def should_sample( "name": name, "op": attributes.get(SentrySpanAttribute.OP), }, - "parent_sampled": parent_sampled, + "parent_sampled": get_parent_sampled(parent_span_context, trace_id), } sampling_context.update(attributes) sample_rate = client.options["traces_sampler"](sampling_context) else: # Check if there is a parent with a sampling decision + parent_sampled = get_parent_sampled(parent_span_context, trace_id) if parent_sampled is not None: sample_rate = parent_sampled else: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index e4b4691fab..535b936261 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1253,10 +1253,6 @@ 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 diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index ce540050f7..bee1a6a6d4 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -137,21 +137,6 @@ def test_prefers_traces_sampler_to_traces_sample_rate( assert transaction.sampled is traces_sampler_return_value -@pytest.mark.parametrize("parent_sampling_decision", [True, False]) -def test_ignores_inherited_sample_decision_when_traces_sampler_defined( - sentry_init, parent_sampling_decision -): - # make traces_sampler pick the opposite of the inherited decision, to prove - # that traces_sampler takes precedence - 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 - ) - assert transaction.sampled is not parent_sampling_decision - - @pytest.mark.parametrize("explicit_decision", [True, False]) def test_traces_sampler_doesnt_overwrite_explicitly_passed_sampling_decision( sentry_init, explicit_decision @@ -165,21 +150,6 @@ def test_traces_sampler_doesnt_overwrite_explicitly_passed_sampling_decision( assert transaction.sampled is explicit_decision -@pytest.mark.parametrize("parent_sampling_decision", [True, False]) -def test_inherits_parent_sampling_decision_when_traces_sampler_undefined( - sentry_init, parent_sampling_decision -): - # make sure the parent sampling decision is the opposite of what - # traces_sample_rate would produce, to prove the inheritance takes - # precedence - 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): - 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]) def test_passes_parent_sampling_decision_in_sampling_context( sentry_init, parent_sampling_decision From 8ae99c087874024b755c58ac39386f09602c5b9c Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 15:31:23 +0100 Subject: [PATCH 10/12] fix tests --- tests/tracing/test_sampling.py | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index bee1a6a6d4..2c6f2ca8fc 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -137,6 +137,28 @@ def test_prefers_traces_sampler_to_traces_sample_rate( assert transaction.sampled is traces_sampler_return_value +@pytest.mark.parametrize("parent_sampling_decision", [True, False]) +def test_ignores_inherited_sample_decision_when_traces_sampler_defined( + sentry_init, parent_sampling_decision +): + # make traces_sampler pick the opposite of the inherited decision, to prove + # that traces_sampler takes precedence + traces_sampler = mock.Mock(return_value=not parent_sampling_decision) + sentry_init(traces_sampler=traces_sampler) + + sentry_trace_header = ( + "12312012123120121231201212312012-1121201211212012-{sampled}".format( + sampled=int(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]) def test_traces_sampler_doesnt_overwrite_explicitly_passed_sampling_decision( sentry_init, explicit_decision @@ -150,6 +172,24 @@ def test_traces_sampler_doesnt_overwrite_explicitly_passed_sampling_decision( assert transaction.sampled is explicit_decision +@pytest.mark.parametrize("parent_sampling_decision", [True, False]) +def test_inherits_parent_sampling_decision_when_traces_sampler_undefined( + sentry_init, parent_sampling_decision +): + # make sure the parent sampling decision is the opposite of what + # traces_sample_rate would produce, to prove the inheritance takes + # precedence + 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): + with start_span(name="catpark", sampled=parent_sampling_decision): + 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 From a11ff463cdce702abfbdb8fc7b6969fcd9cbbf19 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 13 Nov 2024 15:32:39 +0100 Subject: [PATCH 11/12] remove all parent_sampled stuff --- sentry_sdk/tracing.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 535b936261..a69a6f98be 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1212,7 +1212,6 @@ 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 From 032a6c4fe4ede48d4d9b495001ca32df7bcc91a3 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Thu, 14 Nov 2024 11:55:27 +0100 Subject: [PATCH 12/12] fix test --- tests/tracing/test_sampling.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index 2c6f2ca8fc..8ef362a1e8 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -182,8 +182,13 @@ 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 + sentry_trace_header = ( + "12312012123120121231201212312012-1121201211212012-{sampled}".format( + sampled=int(parent_sampling_decision) + ) + ) with mock.patch.object(random, "random", return_value=mock_random_value): - with start_span(name="catpark", sampled=parent_sampling_decision): + with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}): with start_span(name="dogpark") as span: pass