Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sentry_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,8 @@ def _capture_telemetry(
serialized = telemetry._to_json() # type: ignore[union-attr]

if before_send is not None:
serialized = before_send(serialized, {}) # type: ignore[arg-type]
with capture_internal_exceptions():
serialized = before_send(serialized, {}) # type: ignore[arg-type]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception keeps telemetry instead of dropping

Medium Severity

When before_send_log or before_send_metric raises, serialized keeps its prior value and the item is still queued. That diverges from before_send and before_send_transaction in the same module, which treat a raised callback like a drop. A failing scrubbing callback can therefore still emit the original, possibly sensitive, payload.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d2f4ee7. Configure here.


if ty in ("log", "metric"):
# Logs and metrics can be dropped in their respective
Expand Down
5 changes: 4 additions & 1 deletion sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,10 @@ def _set_initial_sampling_decision(
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)
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
Expand Down
21 changes: 16 additions & 5 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1544,7 +1544,10 @@ def add_sentry_baggage_to_headers(
stripped_existing_baggage + separator + sentry_baggage
)

def _get_fallback_sample_rate(client: "BaseClient", propagation_context: "PropagationContext") -> "Union[float, bool]":

def _get_fallback_sample_rate(
client: "BaseClient", propagation_context: "PropagationContext"
) -> "Union[float, bool]":
if propagation_context.parent_sampled is not None:
propagation_context_sample_rate = propagation_context._sample_rate()

Expand All @@ -1555,6 +1558,7 @@ def _get_fallback_sample_rate(client: "BaseClient", propagation_context: "Propag
else:
return client.options["traces_sample_rate"]


def _make_sampling_decision(
name: str,
attributes: "Optional[Attributes]",
Expand Down Expand Up @@ -1601,18 +1605,25 @@ def _make_sampling_decision(
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_fallback_sample_rate(client=client, propagation_context=propagation_context)
logger.warning(
"[Tracing] traces_sampler raised; falling back to parent sample rate or traces_sample_rate",
exc_info=True,
)
sample_rate = _get_fallback_sample_rate(
client=client, propagation_context=propagation_context
)
else:
sample_rate = _get_fallback_sample_rate(client=client, propagation_context=propagation_context)
sample_rate = _get_fallback_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"
Expand Down
26 changes: 26 additions & 0 deletions tests/test_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,32 @@ 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 original,
# unmodified log is sent.
assert len(logs) == 1
assert logs[0]["body"] == "This is an error log..."
assert logs[0]["attributes"]["sentry.severity_text"] == "error"


@minimum_python_37
def test_logs_attributes(sentry_init, capture_items):
"""
Expand Down
23 changes: 23 additions & 0 deletions tests/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,29 @@ 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 original,
# unmodified metric is sent.
metrics = [item.payload for item in items]
assert len(metrics) == 1
assert metrics[0]["name"] == "test.keep"


def test_transport_format(sentry_init, capture_envelopes):
sentry_init(server_name="test-server", release="1.0.0")

Expand Down
29 changes: 29 additions & 0 deletions tests/tracing/test_span_streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,35 @@ 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):
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"):
...

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"


def test_span_attributes(sentry_init, capture_items):
sentry_init(
traces_sample_rate=1.0,
Expand Down
Loading