From 8edd55e7aea62220e0d5b5fba6b0a57a2b2af389 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 9 Dec 2020 22:01:38 +0100 Subject: [PATCH 1/3] fix: Fix sample decision propagation via headers --- sentry_sdk/tracing.py | 13 +++++++------ setup.py | 4 ++-- tests/tracing/test_integration_tests.py | 9 ++++++--- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 5e8a21e027..260255844f 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -583,22 +583,23 @@ def _set_initial_sampling_decision(self, sampling_context): decision, `traces_sample_rate` will be used. """ + # if the user has forced a sampling decision by passing a `sampled` + # value when starting the transaction, go with that + if self.sampled is not None: + return + hub = self.hub or sentry_sdk.Hub.current client = hub.client - options = (client and client.options) or {} transaction_description = "{op}transaction <{name}>".format( op=("<" + self.op + "> " if self.op else ""), name=self.name ) # nothing to do if there's no client or if tracing is disabled - if not client or not has_tracing_enabled(options): + if not client or not has_tracing_enabled(client.options): self.sampled = False return - # if the user has forced a sampling decision by passing a `sampled` - # value when starting the transaction, go with that - if self.sampled is not None: - return + options = client.options # we would have bailed already if neither `traces_sampler` nor # `traces_sample_rate` were defined, so one of these should work; prefer diff --git a/setup.py b/setup.py index 59aef3600c..074a80eebb 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ def get_file_text(file_name): with open(os.path.join(here, file_name)) as in_file: return in_file.read() - + setup( name="sentry-sdk", version="0.19.4", @@ -31,7 +31,7 @@ def get_file_text(file_name): }, description="Python client for Sentry (https://sentry.io)", long_description=get_file_text("README.md"), - long_description_content_type='text/markdown', + long_description_content_type="text/markdown", packages=find_packages(exclude=("tests", "tests.*")), # PEP 561 package_data={"sentry_sdk": ["py.typed"]}, diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index 298f460d59..e5586a25c0 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -47,12 +47,15 @@ def test_basic(sentry_init, capture_events, sample_rate): @pytest.mark.parametrize("sampled", [True, False, None]) -def test_continue_from_headers(sentry_init, capture_events, sampled): - sentry_init(traces_sample_rate=1.0) +@pytest.mark.parametrize( + "sample_rate", [0.0, 1.0] +) # ensure sampling decision is actually passed along via headers +def test_continue_from_headers(sentry_init, capture_events, sampled, sample_rate): + sentry_init(traces_sample_rate=sample_rate) events = capture_events() # make a parent transaction (normally this would be in a different service) - with start_transaction(name="hi"): + with start_transaction(name="hi", sampled=True): with start_span() as old_span: old_span.sampled = sampled headers = dict(Hub.current.iter_trace_propagation_headers()) From 3e68049b251926ccf2912c3a92adc097ad9709e0 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 9 Dec 2020 22:10:32 +0100 Subject: [PATCH 2/3] fix tracing again --- sentry_sdk/tracing.py | 14 ++------------ tests/tracing/test_integration_tests.py | 2 +- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 260255844f..73531894ef 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -594,8 +594,8 @@ def _set_initial_sampling_decision(self, sampling_context): op=("<" + self.op + "> " if self.op else ""), name=self.name ) - # nothing to do if there's no client or if tracing is disabled - if not client or not has_tracing_enabled(client.options): + # nothing to do if there's no client + if not client: self.sampled = False return @@ -663,16 +663,6 @@ def _set_initial_sampling_decision(self, sampling_context): ) -def has_tracing_enabled(options): - # type: (Dict[str, Any]) -> bool - """ - Returns True if either traces_sample_rate or traces_sampler is - non-zero/defined, False otherwise. - """ - - return bool(options.get("traces_sample_rate") or options.get("traces_sampler")) - - def _is_valid_sample_rate(rate): # type: (Any) -> bool """ diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index e5586a25c0..75e5398ff1 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -55,7 +55,7 @@ def test_continue_from_headers(sentry_init, capture_events, sampled, sample_rate events = capture_events() # make a parent transaction (normally this would be in a different service) - with start_transaction(name="hi", sampled=True): + 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(Hub.current.iter_trace_propagation_headers()) From 856cf8e081652cfb61bfbac922618df23734a6a0 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 9 Dec 2020 22:18:43 +0100 Subject: [PATCH 3/3] fix test --- tests/tracing/test_integration_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index 75e5398ff1..c4c316be96 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -87,7 +87,7 @@ def test_continue_from_headers(sentry_init, capture_events, sampled, sample_rate scope.transaction = "ho" capture_message("hello") - if sampled is False: + if sampled is False or (sample_rate == 0 and sampled is None): trace1, message = events assert trace1["transaction"] == "hi"