fix(tracing): handle exceptions raised within traces_sampler and other callbacks

sentry_sdk/client.py

@@ -1244,15 +1244,31 @@
             serialized = telemetry._to_json()  # type: ignore[union-attr]
 
         if before_send is not None:
-            serialized = before_send(serialized, {})  # type: ignore[arg-type]
+            exception_raised_in_before_send_func = False
+            with capture_internal_exceptions():
+                try:
+                    serialized = before_send(serialized, {})  # type: ignore[arg-type]
+                except Exception:
+                    exception_raised_in_before_send_func = True
+                    raise
 
             if ty in ("log", "metric"):
+                # We are ok with dropping metrics and logs when an exception is raised
+                # because we allow users to drop them in their respect before_send_*
+                # functions.
+                if exception_raised_in_before_send_func:
+                    return
                 # Logs and metrics can be dropped in their respective
                 # before_send, so if we get None, don't queue them for sending.
                 if serialized is None:
                     return
 
             elif ty == "span" and isinstance(telemetry, StreamedSpan):
+                # Reset the span to its original value before we attempted
+                # to call the `before_send_span` callback
+                if exception_raised_in_before_send_func:
+                    serialized = telemetry._to_json()
+
                 # Spans can't be dropped in before_send_span by design. They can
                 # be altered though (e.g. to sanitize). Only allow changes to
                 # name and attributes.

sentry_sdk/tracing.py

@@ -1200,16 +1200,25 @@ def _set_initial_sampling_decision(
         # we would have bailed already if neither `traces_sampler` nor
         # `traces_sample_rate` were defined, so one of these should work; prefer
         # the hook if so
-        sample_rate = (
-            client.options["traces_sampler"](sampling_context)
-            if callable(client.options.get("traces_sampler"))
-            # default inheritance behavior
-            else (
+        if callable(client.options.get("traces_sampler")):
+            try:
+                sample_rate = client.options["traces_sampler"](sampling_context)
+            except Exception:
+                logger.warning(
+                    "[Tracing] traces_sampler raised; falling back to parent sample rate or traces_sample_rate",
+                    exc_info=True,
+                )
+                sample_rate = (
+                    sampling_context["parent_sampled"]
+                    if sampling_context["parent_sampled"] is not None
+                    else client.options["traces_sample_rate"]
+                )
+        else:
+            sample_rate = (
                 sampling_context["parent_sampled"]
                 if sampling_context["parent_sampled"] is not None
                 else client.options["traces_sample_rate"]
             )
-        )
 
         # Since this is coming from the user (or from a function provided by the
         # user), who knows what we might get. (The only valid values are
@@ -1235,7 +1244,7 @@ def _set_initial_sampling_decision(
                 "[Tracing] Discarding {transaction_description} because {reason}".format(
                     transaction_description=transaction_description,
                     reason=(
-                        "traces_sampler returned 0 or False"
+                        "traces_sampler returned 0 or False, or is using a fallback sample rate that is 0 or False"
                         if callable(client.options.get("traces_sampler"))
                         else "traces_sample_rate is set to 0"
                     ),

sentry_sdk/tracing_utils.py

@@ -1575,6 +1575,20 @@ def add_sentry_baggage_to_headers(
     )
 
 
+def _get_effective_sample_rate(
+    client: "Any", propagation_context: "PropagationContext"
+) -> "Union[float, bool]":
+    if propagation_context.parent_sampled is not None:
+        propagation_context_sample_rate = propagation_context._sample_rate()
+
+        if propagation_context_sample_rate is not None:
+            return propagation_context_sample_rate
+        else:
+            return propagation_context.parent_sampled
+    else:
+        return client.options["traces_sample_rate"]
+
+
 def _make_sampling_decision(
     name: str,
     attributes: "Optional[Attributes]",
@@ -1630,26 +1644,31 @@ def _make_sampling_decision(
         if propagation_context.custom_sampling_context:
             sampling_context.update(propagation_context.custom_sampling_context)
 
-        sample_rate = client.options["traces_sampler"](sampling_context)
+        try:
+            sample_rate = client.options["traces_sampler"](sampling_context)
+        except Exception:
+            logger.warning(
+                "[Tracing] traces_sampler raised; falling back to parent sample rate or traces_sample_rate",
+                exc_info=True,
+            )
+            sample_rate = _get_effective_sample_rate(
+                client=client, propagation_context=propagation_context
+            )
     else:
-        if propagation_context.parent_sampled is not None:
-            if propagation_context._sample_rate() is not None:
-                sample_rate = propagation_context._sample_rate()
-            else:
-                sample_rate = propagation_context.parent_sampled
-        else:
-            sample_rate = client.options["traces_sample_rate"]
+        sample_rate = _get_effective_sample_rate(
+            client=client, propagation_context=propagation_context
+        )
 
     # Validate whether the sample_rate we got is actually valid. Since
     # traces_sampler is user-provided, it could return anything.
     if not is_valid_sample_rate(sample_rate, source="Tracing"):
         logger.warning(f"[Tracing] Discarding {name} because of invalid sample rate.")
         return False, None, None, "sample_rate"
 
-    sample_rate = float(sample_rate)  # type: ignore[arg-type]
+    sample_rate = float(sample_rate)
     if not sample_rate:
         if traces_sampler_defined:
-            reason = "traces_sampler returned 0 or False"
+            reason = "traces_sampler returned 0 or False, or is using a fallback sample rate that is 0 or False"
         else:
             reason = "traces_sample_rate is set to 0"
 

tests/test_logs.py

@@ -162,6 +162,29 @@ def _before_log(record, hint):
     assert before_log_called is True
 
 
+@minimum_python_37
+@pytest.mark.tests_internal_exceptions
+def test_logs_before_send_log_raises_does_not_crash_application(
+    sentry_init, capture_items
+):
+    def _before_log(record, hint):
+        raise ValueError("before_send_log error")
+
+    sentry_init(
+        enable_logs=True,
+        before_send_log=_before_log,
+    )
+    items = capture_items("log")
+
+    sentry_sdk.logger.error("This is an error log...")
+
+    get_client().flush()
+    logs = [item.payload for item in items]
+
+    # The exception in before_send_log is swallowed and the log is dropped.
+    assert not logs
+
+
 @minimum_python_37
 def test_logs_attributes(sentry_init, capture_items):
     """

tests/test_metrics.py

@@ -247,6 +247,28 @@ def _before_metric(record, hint):
     assert before_metric_called
 
 
+@pytest.mark.tests_internal_exceptions
+def test_metrics_before_send_raises_does_not_crash_application(
+    sentry_init, capture_items
+):
+    def _before_metric(record, hint):
+        raise ValueError("before_send_metric error")
+
+    sentry_init(
+        before_send_metric=_before_metric,
+    )
+    items = capture_items("trace_metric")
+
+    sentry_sdk.metrics.count("test.keep", 1)
+
+    get_client().flush()
+
+    # The exception in before_send_metric is swallowed and the metric is
+    # dropped.
+    metrics = [item.payload for item in items]
+    assert not metrics
+
+
 def test_transport_format(sentry_init, capture_envelopes):
     sentry_init(server_name="test-server", release="1.0.0")
 

tests/tracing/test_sampling.py

@@ -195,15 +195,124 @@ def test_tolerates_traces_sampler_returning_a_boolean_span_streaming(
 ):
     sentry_init(
         traces_sampler=mock.Mock(return_value=traces_sampler_return_value),
-        _experiments={
-            "trace_lifecycle": "stream",
-        },
+        trace_lifecycle="stream",
     )
 
     with sentry_sdk.traces.start_span(name="dogpark") as span:
         assert span.sampled is traces_sampler_return_value
 
 
+@pytest.mark.parametrize(
+    "traces_sample_rate,expected_decision",
+    [(0.0, False), (0.25, False), (0.75, True), (1.00, True)],
+)
+def test_traces_sampler_raising_falls_back_to_traces_sample_rate(
+    sentry_init,
+    traces_sample_rate,
+    expected_decision,
+):
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+        traces_sample_rate=traces_sample_rate,
+    )
+
+    baggage = Baggage(sentry_items={"sample_rand": "0.500000"})
+    transaction = start_transaction(name="dogpark", baggage=baggage)
+    assert transaction.sampled is expected_decision
+
+
+@pytest.mark.parametrize("parent_sampling_decision", [True, False])
+def test_traces_sampler_raising_falls_back_to_parent_sampling_decision(
+    sentry_init, parent_sampling_decision
+):
+    # set traces_sample_rate to produce the opposite of the parent decision,
+    # to prove the parent's decision takes precedence in the fallback
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+        traces_sample_rate=0.0 if parent_sampling_decision else 1.0,
+    )
+
+    baggage = Baggage(sentry_items={"sample_rand": "0.500000"})
+    transaction = start_transaction(
+        name="dogpark", baggage=baggage, parent_sampled=parent_sampling_decision
+    )
+    assert transaction.sampled is parent_sampling_decision
+
+
+@pytest.mark.parametrize(
+    "traces_sample_rate,expected_decision",
+    [(0.0, False), (0.25, False), (0.75, True), (1.00, True)],
+)
+def test_traces_sampler_raising_falls_back_to_traces_sample_rate_span_streaming(
+    sentry_init,
+    traces_sample_rate,
+    expected_decision,
+):
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+        traces_sample_rate=traces_sample_rate,
+        trace_lifecycle="stream",
+    )
+
+    sentry_sdk.traces.continue_trace(
+        {
+            "sentry-trace": "0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331",
+            "baggage": "sentry-sample_rand=0.500000",
+        }
+    )
+
+    with sentry_sdk.traces.start_span(name="dogpark") as span:
+        assert span.sampled is expected_decision
+
+
+@pytest.mark.parametrize(
+    "traces_sample_rate,expected_decision",
+    [(0.0, False), (0.25, False), (0.75, True), (1.00, True)],
+)
+def test_traces_sampler_raising_no_incoming_trace_falls_back_to_traces_sample_rate_span_streaming(
+    sentry_init,
+    traces_sample_rate,
+    expected_decision,
+    monkeypatch,
+):
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+        traces_sample_rate=traces_sample_rate,
+        trace_lifecycle="stream",
+    )
+
+    # no continue_trace, so no propagated sample_rand; make it deterministic
+    monkeypatch.setattr(
+        "sentry_sdk.tracing_utils._generate_sample_rand", lambda *a, **kw: 0.5
+    )
+
+    with sentry_sdk.traces.start_span(name="dogpark") as span:
+        assert span.sampled is expected_decision
+
+
+def test_traces_sampler_raising_no_incoming_trace_and_no_traces_sample_rate(
+    sentry_init,
+):
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+    )
+
+    transaction = start_transaction(name="dogpark")
+    assert transaction.sampled is False
+
+
+def test_traces_sampler_raising_no_incoming_trace_and_no_traces_sample_rate_span_streaming(
+    sentry_init,
+):
+    sentry_init(
+        traces_sampler=mock.Mock(side_effect=ValueError("boom")),
+        trace_lifecycle="stream",
+    )
+
+    with sentry_sdk.traces.start_span(name="dogpark") as span:
+        assert span.sampled is False
+
+
 @pytest.mark.parametrize("sampling_decision", [True, False])
 def test_only_captures_transaction_when_sampled_is_true(
     sentry_init, sampling_decision, capture_events

tests/tracing/test_span_streaming.py

@@ -359,6 +359,41 @@ def before_send_span(span, hint):
     assert not before_send_span_called
 
 
+@pytest.mark.tests_internal_exceptions
+def test_before_send_span_raises_does_not_crash_application(sentry_init, capture_items):
+    def before_send_span(span, hint):
+        # Mutate the span before raising to ensure the partial mutation
+        # is discarded when the exception is raised.
+        span["name"] = "Mutated span name"
+        span["attributes"]["mutated"] = True
+        raise ValueError("before_send_span error")
+
+    sentry_init(
+        traces_sample_rate=1.0,
+        _experiments={
+            "before_send_span": before_send_span,
+            "trace_lifecycle": "stream",
+        },
+    )
+
+    items = capture_items("span")
+
+    with sentry_sdk.traces.start_span(name="span", attributes={"original": "value"}):
+        ...
+
+    sentry_sdk.get_client().flush()
+    spans = [item.payload for item in items]
+
+    # The exception in before_send_span is swallowed and the original,
+    # unmodified span is sent.
+    assert len(spans) == 1
+    (span,) = spans
+
+    assert span["name"] == "span"
+    assert span["attributes"]["original"] == "value"
+    assert "mutated" not in span["attributes"]
+
+
 def test_span_attributes(sentry_init, capture_items):
     sentry_init(
         traces_sample_rate=1.0,