Skip to content

Commit

Permalink
fix(client-reports): Record lost sample_rate events only if tracing…
Browse files Browse the repository at this point in the history
… is enabled (#1268)
  • Loading branch information
sl0thentr0py authored Dec 10, 2021
1 parent 3a7943b commit d09221d
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ A major release `N` implies the previous release `N-1` will no longer receive up
## Unreleased

- Fix django legacy url resolver regex substitution due to upstream CVE-2021-44420 fix #1272
- Record lost `sample_rate` events only if tracing is enabled

## 1.5.0

Expand Down
10 changes: 5 additions & 5 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,24 +543,24 @@ def finish(self, hub=None):
hub = hub or self.hub or sentry_sdk.Hub.current
client = hub.client

if client is None:
# We have no client and therefore nowhere to send this transaction.
return None

# This is a de facto proxy for checking if sampled = False
if self._span_recorder is None:
logger.debug("Discarding transaction because sampled = False")

# This is not entirely accurate because discards here are not
# exclusively based on sample rate but also traces sampler, but
# we handle this the same here.
if client and client.transport:
if client.transport and has_tracing_enabled(client.options):
client.transport.record_lost_event(
"sample_rate", data_category="transaction"
)

return None

if client is None:
# We have no client and therefore nowhere to send this transaction.
return None

if not self.name:
logger.warning(
"Transaction has no name, falling back to `<unlabeled transaction>`."
Expand Down
2 changes: 1 addition & 1 deletion sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ 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.
defined, False otherwise.
"""

return bool(
Expand Down
58 changes: 58 additions & 0 deletions tests/tracing/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,61 @@ def test_warns_and_sets_sampled_to_false_on_invalid_traces_sampler_return_value(
transaction = start_transaction(name="dogpark")
logger.warning.assert_any_call(StringContaining("Given sample rate is invalid"))
assert transaction.sampled is False


@pytest.mark.parametrize(
"traces_sample_rate,sampled_output,reports_output",
[
(None, False, []),
(0.0, False, [("sample_rate", "transaction")]),
(1.0, True, []),
],
)
def test_records_lost_event_only_if_traces_sample_rate_enabled(
sentry_init, traces_sample_rate, sampled_output, reports_output, monkeypatch
):
reports = []

def record_lost_event(reason, data_category=None, item=None):
reports.append((reason, data_category))

sentry_init(traces_sample_rate=traces_sample_rate)

monkeypatch.setattr(
Hub.current.client.transport, "record_lost_event", record_lost_event
)

transaction = start_transaction(name="dogpark")
assert transaction.sampled is sampled_output
transaction.finish()

assert reports == reports_output


@pytest.mark.parametrize(
"traces_sampler,sampled_output,reports_output",
[
(None, False, []),
(lambda _x: 0.0, False, [("sample_rate", "transaction")]),
(lambda _x: 1.0, True, []),
],
)
def test_records_lost_event_only_if_traces_sampler_enabled(
sentry_init, traces_sampler, sampled_output, reports_output, monkeypatch
):
reports = []

def record_lost_event(reason, data_category=None, item=None):
reports.append((reason, data_category))

sentry_init(traces_sampler=traces_sampler)

monkeypatch.setattr(
Hub.current.client.transport, "record_lost_event", record_lost_event
)

transaction = start_transaction(name="dogpark")
assert transaction.sampled is sampled_output
transaction.finish()

assert reports == reports_output

0 comments on commit d09221d

Please sign in to comment.