Repository: getsentry/sentry-python

Issue:
Issues Sampling (like `traces_sampler`)
### Problem Statement

I ran into the same problem as was voiced in: https://github.com/getsentry/sentry/issues/28792#issuecomment-926373402, https://github.com/getsentry/sentry/issues/46670, https://github.com/getsentry/sentry/issues/27884.
In short: there is a service that periodically restarts due to heavy load on it, we can't do anything about it. At the moment of restarting the service - there is a large (hundreds of thousands) number of sending logged messages to Sentry. It is impossible to disable messages at all, because we need to control the situation when the service has stopped responding at all.

There is a way to set rate limit for tracing in the [documentation](https://docs.sentry.io/platforms/python/configuration/filtering/#using-platformidentifier-nametraces-sampler-) (and it is mentioned in https://github.com/getsentry/sentry/issues/46670#issuecomment-1496560154). But there is no way to set rate limit by message text from the logger.

We are using Django, and if we try to do according to the example in the documentation, there is no useful information about the message itself in `sampling_context`. There's just the transaction, no error, no logger message text, no traceback.


### Solution Brainstorm

Please make `issues_sampler` so that it would be possible to filter a errors before sending it to Sentry.

Discussion:
cleptric: An issue in Sentry represents N events, so calling it `issue_sampler` isn't the best way forward. IMO, this option should sample everything that ends up in `capture_event`.

szokeasaurusrex: > @szokeasaurusrex Is it only errors passing through `issues_sampler` or also other event types?
> 
> If it's only errors, calling the option `error_sampler` might be better for clarity.

The `_should_sample_error` function that I have modified here is called in the `capture_event` functions, so I _believe_ all events will be sampled. In light of @cleptric's comment, though, we might consider renaming `issues_sampler` to `events_sampler`. What do you think?


> More importantly, are the events from the [original issue](https://github.com/getsentry/sentry-python/issues/2440) errors (as opposed to e.g. messages) and would they pass through the `issues_sampler`?

I am unsure given the information I see in the original issue. @saippuakauppias, could you please provide the code where these errors/messages originate from, or provide us more detail with how these are being generated, so we can answer this question?
sentrivana: > The `_should_sample_error` function that I have modified here is called in the `capture_event` functions, so I _believe_ all events will be sampled. In light of @cleptric's comment, though, we might consider renaming `issues_sampler` to something like `events_sampler`. What do you think?

Yes, looks like it's everything but transactions and checkins: https://github.com/getsentry/sentry-python/blob/4d10edfe7233d5adc2ceeeb984d8f93dfa3a29eb/sentry_sdk/client.py#L556-L560

In that case `events_sampler`/`event_sampler` sounds good! And it also means that it can be used to set the sample rate for any event, so that answers my second question.

> IMO, this option should sample everything that ends up in capture_event.

@cleptric Checkins and transactions also end up in `capture_event` but wouldn't be run through the sampler, which is ok I think. For transactions there is `traces_sampler`/`traces_sample_rate` and I assume there's currently no demand for being able to set checkin sample rates. And if there is, `before_send` is still a thing.
sentrivana: Once we've settled on the API please don't forget to update the docs @szokeasaurusrex 📖 
szokeasaurusrex: I discussed with @cleptric, and we now think we should pick a different name than `event_sampler`, since transactions are also technically events.

The best idea we came up with was to just call the new sampler `sampler`. This name would also fit the pattern where our `sampler` corresponds to the `sample_rate` the same way the `traces_sampler` corresponds to the `traces_sample_rate`. Do you have any other ideas @sentrivana, or are you okay with us proceeding naming the sampler `sampler`.
saippuakauppias: > I am unsure given the information I see in the original issue. @saippuakauppias, could you please provide the code where these errors/messages originate from, or provide us more detail with how these are being generated, so we can answer this question?

I'm using django, so it's going to be a bit of a specific example here, but I think it's reproducible.



```python
# settings/production.py

LOGGING = {
  'version': 1,
  'formatters': {
    'simple': {
        'format': '[%(asctime)s] %(levelname)s %(module)s %(message)s',
        'datefmt': '%Y-%m-%d %H:%M:%S'
    },
  },

  'handlers': {
    'sentry': {
      'level': 'WARNING',
      'class': 'base.sentry.SentryHandler',
    },
    'streamlogger': {
      'level': 'DEBUG',
      'class': 'logging.StreamHandler',
      'formatter': 'simple'
    },
  },

  'loggers': {
    'warn.logger': {
      'handlers': ['sentry', 'streamlogger'],
      'level': 'WARNING',
      'propagate': False
    },
  },
  
  'root': {
    'level': 'INFO',
    'handlers': ['streamlogger'],
  },
}
```

```python
# base/sentry.py
from sentry_sdk.integrations.atexit import AtexitIntegration
from sentry_sdk.integrations.dedupe import DedupeIntegration
from sentry_sdk.integrations.django import DjangoIntegration
from sentry_sdk.integrations.excepthook import ExcepthookIntegration
from sentry_sdk.integrations.logging import EventHandler, LoggingIntegration
from sentry_sdk.integrations.modules import ModulesIntegration
from sentry_sdk.integrations.stdlib import StdlibIntegration


class SentryHandler(EventHandler):
    ...


def initialize(dsn, service, release, branch, debug: bool, breadcrumb_filters: list[dict] = None):
    from django.conf import settings

    sample_events_by_path = {
        f'{settings.PROJECT_ROOT}/some/module.py': 0.0001,
    }

    def prepare_event(event, hint):
        request_data = get_request_data()
        tags = event.setdefault('tags', {})

        # my hack for filtration here
        is_sampled = False
        if isinstance(hint, dict) and 'log_record' in hint:
            pathname = getattr(hint['log_record'], 'pathname', '')
            is_sampled = pathname in sample_events_by_path
            sample_rate = sample_events_by_path.get(pathname, 1)

            if sample_rate < 1.0 and random.random() < sample_rate:
                return None

        tags['is_sampled'] = is_sampled
        
        return event

    sentry_sdk.init(
        dsn=dsn,
        release=release or None,
        before_send=prepare_event,
        default_integrations=False,
        integrations=[
            StdlibIntegration(),
            ExcepthookIntegration(),
            DedupeIntegration(),
            AtexitIntegration(),
            ModulesIntegration(),
            DjangoIntegration(),
            LoggingIntegration(level=None, event_level=None),
        ],
    )
```

```python
# some/module.py
import logging
import requests


logger = logging.getLogger('warn.logger')


def some_fun():
    try:
        resp = requsts.get('http://some.service.name')
        resp.raise_for_status()
    except Exception as e:
        logger.warning('Error get value from service', exc_info=True)
```
sentrivana: > I discussed with @cleptric, and we now think we should pick a different name than `event_sampler`, since transactions are also technically events.
> 
> The best idea we came up with was to just call the new sampler `sampler`. This name would also fit the pattern where our `sampler` corresponds to the `sample_rate` the same way the `traces_sampler` corresponds to the `traces_sample_rate`. Do you have any other ideas @sentrivana, or are you okay with us proceeding naming the sampler `sampler`?

I'm not opposed to calling it `sampler` per se, but I don't follow the reasoning. If the issue with `event_sampler` is that it's too broad, then `sampler` sounds even broader? 😄 

FWIW I don't think there's an ideal solution here. But if I saw `sampler` as a user without reading the docs, I would assume it samples everything and is a generalization of/supersedes `traces_sampler`. On the other hand, `event_sampler` suggests some sort of dichotomy to `traces_sampler` -- i.e., those two probably sample different things.
szokeasaurusrex: I discussed with @antonpirker this morning about naming the sampler as `sampler` versus `events_sampler`, he also was leaning towards `events_sampler`. Since the majority consensus here seems to be to go with `events_sampler`, let's go with `events_sampler` for now.

Patch:
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
index b65c3f0c..749ab23c 100644
--- a/sentry_sdk/client.py
+++ b/sentry_sdk/client.py
@@ -454,12 +454,34 @@ class _Client(object):
     def _should_sample_error(
         self,
         event,  # type: Event
+        hint,  # type: Hint
     ):
         # type: (...) -> bool
-        not_in_sample_rate = (
-            self.options["sample_rate"] < 1.0
-            and random.random() >= self.options["sample_rate"]
-        )
+        sampler = self.options.get("error_sampler", None)
+
+        if callable(sampler):
+            with capture_internal_exceptions():
+                sample_rate = sampler(event, hint)
+        else:
+            sample_rate = self.options["sample_rate"]
+
+        try:
+            not_in_sample_rate = sample_rate < 1.0 and random.random() >= sample_rate
+        except TypeError:
+            parameter, verb = (
+                ("error_sampler", "returned")
+                if callable(sampler)
+                else ("sample_rate", "contains")
+            )
+            logger.warning(
+                "The provided %s %s an invalid value of %s. The value should be a float or a bool. Defaulting to sampling the event."
+                % (parameter, verb, repr(sample_rate))
+            )
+
+            # If the sample_rate has an invalid value, we should sample the event, since the default behavior
+            # (when no sample_rate or error_sampler is provided) is to sample all events.
+            not_in_sample_rate = False
+
         if not_in_sample_rate:
             # because we will not sample this event, record a "lost event".
             if self.transport:
@@ -556,7 +578,7 @@ class _Client(object):
         if (
             not is_transaction
             and not is_checkin
-            and not self._should_sample_error(event)
+            and not self._should_sample_error(event, hint)
         ):
             return None
 
diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py
index 5bc3e2aa..60cb65bc 100644
--- a/sentry_sdk/consts.py
+++ b/sentry_sdk/consts.py
@@ -22,6 +22,7 @@ if TYPE_CHECKING:
         BreadcrumbProcessor,
         Event,
         EventProcessor,
+        Hint,
         ProfilerMode,
         TracesSampler,
         TransactionProcessor,
@@ -261,6 +262,7 @@ class ClientConstructor(object):
         event_scrubber=None,  # type: Optional[sentry_sdk.scrubber.EventScrubber]
         max_value_length=DEFAULT_MAX_VALUE_LENGTH,  # type: int
         enable_backpressure_handling=True,  # type: bool
+        error_sampler=None,  # type: Optional[Callable[[Event, Hint], Union[float, bool]]]
     ):
         # type: (...) -> None
         pass


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index bf3e4e79..5a7a5cff 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -25,6 +25,12 @@ from sentry_sdk.utils import HAS_CHAINED_EXCEPTIONS
 from sentry_sdk.utils import logger
 from sentry_sdk.serializer import MAX_DATABAG_BREADTH
 from sentry_sdk.consts import DEFAULT_MAX_BREADCRUMBS, DEFAULT_MAX_VALUE_LENGTH
+from sentry_sdk._types import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from collections.abc import Callable
+    from typing import Any, Optional, Union
+    from sentry_sdk._types import Event
 
 try:
     from unittest import mock  # python 3.3 and above
@@ -1196,3 +1202,114 @@ def test_debug_option(
         assert "something is wrong" in caplog.text
     else:
         assert "something is wrong" not in caplog.text
+
+
+class IssuesSamplerTestConfig:
+    def __init__(
+        self,
+        expected_events,
+        sampler_function=None,
+        sample_rate=None,
+        exception_to_raise=Exception,
+    ):
+        # type: (int, Optional[Callable[[Event], Union[float, bool]]], Optional[float], type[Exception]) -> None
+        self.sampler_function_mock = (
+            None
+            if sampler_function is None
+            else mock.MagicMock(side_effect=sampler_function)
+        )
+        self.expected_events = expected_events
+        self.sample_rate = sample_rate
+        self.exception_to_raise = exception_to_raise
+
+    def init_sdk(self, sentry_init):
+        # type: (Callable[[*Any], None]) -> None
+        sentry_init(
+            error_sampler=self.sampler_function_mock, sample_rate=self.sample_rate
+        )
+
+    def raise_exception(self):
+        # type: () -> None
+        raise self.exception_to_raise()
+
+
+@mock.patch("sentry_sdk.client.random.random", return_value=0.618)
+@pytest.mark.parametrize(
+    "test_config",
+    (
+        # Baseline test with error_sampler only, both floats and bools
+        IssuesSamplerTestConfig(sampler_function=lambda *_: 1.0, expected_events=1),
+        IssuesSamplerTestConfig(sampler_function=lambda *_: 0.7, expected_events=1),
+        IssuesSamplerTestConfig(sampler_function=lambda *_: 0.6, expected_events=0),
+        IssuesSamplerTestConfig(sampler_function=lambda *_: 0.0, expected_events=0),
+        IssuesSamplerTestConfig(sampler_function=lambda *_: True, expected_events=1),
+        IssuesSamplerTestConfig(sampler_function=lambda *_: False, expected_events=0),
+        # Baseline test with sample_rate only
+        IssuesSamplerTestConfig(sample_rate=1.0, expected_events=1),
+        IssuesSamplerTestConfig(sample_rate=0.7, expected_events=1),
+        IssuesSamplerTestConfig(sample_rate=0.6, expected_events=0),
+        IssuesSamplerTestConfig(sample_rate=0.0, expected_events=0),
+        # error_sampler takes precedence over sample_rate
+        IssuesSamplerTestConfig(
+            sampler_function=lambda *_: 1.0, sample_rate=0.0, expected_events=1
+        ),
+        IssuesSamplerTestConfig(
+            sampler_function=lambda *_: 0.0, sample_rate=1.0, expected_events=0
+        ),
+        # Different sample rates based on exception, retrieved both from event and hint
+        IssuesSamplerTestConfig(
+            sampler_function=lambda event, _: {
+                "ZeroDivisionError": 1.0,
+                "AttributeError": 0.0,
+            }[event["exception"]["values"][0]["type"]],
+            exception_to_raise=ZeroDivisionError,
+            expected_events=1,
+        ),
+        IssuesSamplerTestConfig(
+            sampler_function=lambda event, _: {
+                "ZeroDivisionError": 1.0,
+                "AttributeError": 0.0,
+            }[event["exception"]["values"][0]["type"]],
+            exception_to_raise=AttributeError,
+            expected_events=0,
+        ),
+        IssuesSamplerTestConfig(
+            sampler_function=lambda _, hint: {
+                ZeroDivisionError: 1.0,
+                AttributeError: 0.0,
+            }[hint["exc_info"][0]],
+            exception_to_raise=ZeroDivisionError,
+            expected_events=1,
+        ),
+        IssuesSamplerTestConfig(
+            sampler_function=lambda _, hint: {
+                ZeroDivisionError: 1.0,
+                AttributeError: 0.0,
+            }[hint["exc_info"][0]],
+            exception_to_raise=AttributeError,
+            expected_events=0,
+        ),
+        # If sampler returns invalid value, we should still send the event
+        IssuesSamplerTestConfig(
+            sampler_function=lambda *_: "This is an invalid return value for the sampler",
+            expected_events=1,
+        ),
+    ),
+)
+def test_error_sampler(_, sentry_init, capture_events, test_config):
+    test_config.init_sdk(sentry_init)
+
+    events = capture_events()
+
+    try:
+        test_config.raise_exception()
+    except Exception:
+        capture_exception()
+
+    assert len(events) == test_config.expected_events
+
+    if test_config.sampler_function_mock is not None:
+        assert test_config.sampler_function_mock.call_count == 1
+
+        # Ensure two arguments (the event and hint) were passed to the sampler function
+        assert len(test_config.sampler_function_mock.call_args[0]) == 2