From 732676b604214d034f36bfc806668971b85d53ef Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 15 Oct 2020 17:34:37 -0700 Subject: [PATCH 1/5] add has_tracing_enabled function --- sentry_sdk/utils.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index d39b0c1e40..983465b26f 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -968,3 +968,13 @@ def run(self): integer_configured_timeout ) ) + + +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")) From 06102cc513f529983449b84df3e126cec2afed39 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 15 Oct 2020 21:57:07 -0700 Subject: [PATCH 2/5] add sample rate validation --- sentry_sdk/tracing.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 80b4b377d9..0cb532c1ed 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1,9 +1,11 @@ import re import uuid import contextlib +import math import time from datetime import datetime, timedelta +from numbers import Real import sentry_sdk @@ -535,6 +537,37 @@ def finish(self, hub=None): ) +def _is_valid_sample_rate(rate): + # type: (Any) -> bool + """ + Checks the given sample rate to make sure it is valid type and value (a + boolean or a number between 0 and 1, inclusive). + """ + + # both booleans and NaN are instances of Real, so a) checking for Real + # checks for the possibility of a boolean also, and b) we have to check + # separately for NaN + if not isinstance(rate, Real) or math.isnan(rate): + logger.warning( + "[Tracing] Given sample rate is invalid. Sample rate must be a boolean or a number between 0 and 1. Got {rate} of type {type}.".format( + rate=rate, type=type(rate) + ) + ) + return False + + # in case rate is a boolean, it will get cast to 1 if it's True and 0 if it's False + rate = float(rate) + if rate < 0 or rate > 1: + logger.warning( + "[Tracing] Given sample rate is invalid. Sample rate must be between 0 and 1. Got {rate}.".format( + rate=rate + ) + ) + return False + + return True + + def _format_sql(cursor, sql): # type: (Any, str) -> Optional[str] From 4b56ec74063aa7c565a2c63edfa6760aa4b114a8 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 16 Oct 2020 00:31:13 -0700 Subject: [PATCH 3/5] add rate validation tests --- tests/tracing/test_sampling.py | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index 476d5e78c9..d166efb0a4 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -1,4 +1,13 @@ +import pytest + from sentry_sdk import start_span, start_transaction +from sentry_sdk.tracing import _is_valid_sample_rate +from sentry_sdk.utils import logger + +try: + from unittest import mock # python 3.3 and above +except ImportError: + import mock # python < 3.3 def test_sampling_decided_only_for_transactions(sentry_init, capture_events): @@ -32,3 +41,35 @@ def test_no_double_sampling(sentry_init, capture_events): pass assert len(events) == 1 + + +@pytest.mark.parametrize( + "rate", + [0.0, 0.1231, 1.0, True, False], +) +def test_accepts_valid_sample_rate(rate): + with mock.patch.object(logger, "warning", mock.Mock()): + result = _is_valid_sample_rate(rate) + assert logger.warning.called is False + assert result is True + + +@pytest.mark.parametrize( + "rate", + [ + "dogs are great", # wrong type + (0, 1), # wrong type + {"Maisey": "Charllie"}, # wrong type + [True, True], # wrong type + {0.2012}, # wrong type + float("NaN"), # wrong type + None, # wrong type + -1.121, # wrong value + 1.231, # wrong value + ], +) +def test_warns_on_invalid_sample_rate(rate, StringContaining): # noqa: N803 + with mock.patch.object(logger, "warning", mock.Mock()): + result = _is_valid_sample_rate(rate) + logger.warning.assert_any_call(StringContaining("Given sample rate is invalid")) + assert result is False From 41791fd2a3121f7f4aa5e4544d3d5c8424857d84 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 15 Oct 2020 22:44:13 -0700 Subject: [PATCH 4/5] remove unused client param in Span.to_json --- sentry_sdk/tracing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 0cb532c1ed..ab3b8c4b14 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -409,8 +409,8 @@ def finish(self, hub=None): _maybe_create_breadcrumbs_from_span(hub, self) return None - def to_json(self, client): - # type: (Optional[sentry_sdk.Client]) -> Dict[str, Any] + def to_json(self): + # type: () -> Dict[str, Any] rv = { "trace_id": self.trace_id, "span_id": self.span_id, @@ -519,7 +519,7 @@ def finish(self, hub=None): return None finished_spans = [ - span.to_json(client) + span.to_json() for span in self._span_recorder.spans if span is not self and span.timestamp is not None ] From 554c878d3a8d3b0afe58f764cb33d6aa8e73abe5 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 15 Oct 2020 23:13:41 -0700 Subject: [PATCH 5/5] add to_json method to Transaction class --- sentry_sdk/tracing.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index ab3b8c4b14..c908120032 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -536,6 +536,16 @@ def finish(self, hub=None): } ) + def to_json(self): + # type: () -> Dict[str, Any] + rv = super(Transaction, self).to_json() + + rv["name"] = self.name + rv["sampled"] = self.sampled + rv["parent_sampled"] = self.parent_sampled + + return rv + def _is_valid_sample_rate(rate): # type: (Any) -> bool