diff --git a/src/sentry/rules/conditions/event_frequency.py b/src/sentry/rules/conditions/event_frequency.py index 0f322bda690fe9..7c2c6bcdf8f3a5 100644 --- a/src/sentry/rules/conditions/event_frequency.py +++ b/src/sentry/rules/conditions/event_frequency.py @@ -140,13 +140,17 @@ def passes(self, event: GroupEvent, state: EventState) -> bool: if state.is_new and value > 1: return False - # TODO(mgaeta): Bug: Rule is optional. + comparison_type = self.get_option("comparisonType", ComparisonType.COUNT) + comparison_interval_option = self.get_option("comparisonInterval", "5m") + comparison_interval = COMPARISON_INTERVALS[comparison_interval_option][1] + _, duration = self.intervals[interval] try: - current_value = self.get_rate(event, interval, self.rule.environment_id) # type: ignore[arg-type, union-attr] + current_value = self.get_rate(duration=duration, comparison_interval=comparison_interval, event=event, environment_id=self.rule.environment_id, comparison_type=comparison_type) # type: ignore[arg-type, union-attr] # XXX(CEO): once inc-666 work is concluded, rm try/except except RateLimitExceeded: metrics.incr("rule.event_frequency.snuba_query_limit") return False + logging.info("event_frequency_rule current: %s, threshold: %s", current_value, value) return current_value > value @@ -214,26 +218,44 @@ def batch_query_hook( """ raise NotImplementedError - def get_rate(self, event: GroupEvent, interval: str, environment_id: int) -> int: - _, duration = self.intervals[interval] - end = timezone.now() + def get_option_override(self, duration: timedelta) -> contextlib.AbstractContextManager[object]: # For conditions with interval >= 1 hour we don't need to worry about read your writes # consistency. Disable it so that we can scale to more nodes. option_override_cm: contextlib.AbstractContextManager[object] = contextlib.nullcontext() if duration >= timedelta(hours=1): option_override_cm = options_override({"consistent": False}) - with option_override_cm: - result: int = self.query(event, end - duration, end, environment_id=environment_id) - comparison_type = self.get_option("comparisonType", ComparisonType.COUNT) + return option_override_cm + + def get_comparison_start_end( + self, interval: timedelta, duration: timedelta + ) -> tuple[datetime, datetime]: + """ + Calculate the start and end times for the query. `interval` is only used for EventFrequencyPercentCondition + as the '5 minutes' in The issue affects more than 100 percent of sessions in 5 minutes, otherwise it's the current time. + `duration` is the time frame in which the condition is measuring counts, e.g. the '10 minutes' in + "The issue is seen more than 100 times in 10 minutes" + """ + end = timezone.now() - interval + start = end - duration + return (start, end) + + def get_rate( + self, + duration: timedelta, + comparison_interval: timedelta, + event: GroupEvent, + environment_id: int, + comparison_type: str, + ) -> int: + start, end = self.get_comparison_start_end(timedelta(), duration) + with self.get_option_override(duration): + result = self.query(event, start, end, environment_id=environment_id) if comparison_type == ComparisonType.PERCENT: - comparison_interval = COMPARISON_INTERVALS[self.get_option("comparisonInterval")][1] - comparison_end = end - comparison_interval # TODO: Figure out if there's a way we can do this less frequently. All queries are # automatically cached for 10s. We could consider trying to cache this and the main # query for 20s to reduce the load. - comparison_result = self.query( - event, comparison_end - duration, comparison_end, environment_id=environment_id - ) + start, end = self.get_comparison_start_end(comparison_interval, duration) + comparison_result = self.query(event, start, end, environment_id=environment_id) result = percent_increase(result, comparison_result) return result diff --git a/tests/snuba/rules/conditions/test_event_frequency.py b/tests/snuba/rules/conditions/test_event_frequency.py index 209e1e153d3301..66176e558c45ff 100644 --- a/tests/snuba/rules/conditions/test_event_frequency.py +++ b/tests/snuba/rules/conditions/test_event_frequency.py @@ -290,43 +290,63 @@ def _run_test(self, minutes, data, passes, add_events=False): self.assertDoesNotPass(environment_rule, event, is_new=False) def test_one_minute_with_events(self): - data = {"interval": "1m", "value": 6} + data = {"interval": "1m", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=1, passes=True, add_events=True) - data = {"interval": "1m", "value": 16} + data = { + "interval": "1m", + "value": 16, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=1, passes=False) def test_one_hour_with_events(self): - data = {"interval": "1h", "value": 6} + data = {"interval": "1h", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=60, passes=True, add_events=True) - data = {"interval": "1h", "value": 16} + data = { + "interval": "1h", + "value": 16, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=60, passes=False) def test_one_day_with_events(self): - data = {"interval": "1d", "value": 6} + data = {"interval": "1d", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=1440, passes=True, add_events=True) - data = {"interval": "1d", "value": 16} + data = { + "interval": "1d", + "value": 16, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=1440, passes=False) def test_one_week_with_events(self): - data = {"interval": "1w", "value": 6} + data = {"interval": "1w", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=10080, passes=True, add_events=True) - data = {"interval": "1w", "value": 16} + data = { + "interval": "1w", + "value": 16, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=10080, passes=False) def test_one_minute_no_events(self): - data = {"interval": "1m", "value": 6} + data = {"interval": "1m", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=1, passes=False) def test_one_hour_no_events(self): - data = {"interval": "1h", "value": 6} + data = {"interval": "1h", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=60, passes=False) def test_one_day_no_events(self): - data = {"interval": "1d", "value": 6} + data = {"interval": "1d", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=1440, passes=False) def test_one_week_no_events(self): - data = {"interval": "1w", "value": 6} + data = {"interval": "1w", "value": 6, "comparisonType": "count", "comparisonInterval": "5m"} self._run_test(data=data, minutes=10080, passes=False) def test_comparison(self): @@ -516,57 +536,117 @@ def increment(self, event, count, environment=None, timestamp=None): @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_five_minutes_with_events(self): self._make_sessions(60) - data = {"interval": "5m", "value": 39} + data = { + "interval": "5m", + "value": 39, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=5, passes=True, add_events=True) - data = {"interval": "5m", "value": 41} + data = { + "interval": "5m", + "value": 41, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=5, passes=False) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_ten_minutes_with_events(self): self._make_sessions(60) - data = {"interval": "10m", "value": 49} + data = { + "interval": "10m", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=10, passes=True, add_events=True) - data = {"interval": "10m", "value": 51} + data = { + "interval": "10m", + "value": 51, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=10, passes=False) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_thirty_minutes_with_events(self): self._make_sessions(60) - data = {"interval": "30m", "value": 49} + data = { + "interval": "30m", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=30, passes=True, add_events=True) - data = {"interval": "30m", "value": 51} + data = { + "interval": "30m", + "value": 51, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=30, passes=False) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_one_hour_with_events(self): self._make_sessions(60) - data = {"interval": "1h", "value": 49} + data = { + "interval": "1h", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=60, add_events=True, passes=True) - data = {"interval": "1h", "value": 51} + data = { + "interval": "1h", + "value": 51, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=60, passes=False) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_five_minutes_no_events(self): self._make_sessions(60) - data = {"interval": "5m", "value": 39} + data = { + "interval": "5m", + "value": 39, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=5, passes=True, add_events=True) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_ten_minutes_no_events(self): self._make_sessions(60) - data = {"interval": "10m", "value": 49} + data = { + "interval": "10m", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=10, passes=True, add_events=True) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_thirty_minutes_no_events(self): self._make_sessions(60) - data = {"interval": "30m", "value": 49} + data = { + "interval": "30m", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=30, passes=True, add_events=True) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1) def test_one_hour_no_events(self): self._make_sessions(60) - data = {"interval": "1h", "value": 49} + data = { + "interval": "1h", + "value": 49, + "comparisonType": "count", + "comparisonInterval": "5m", + } self._run_test(data=data, minutes=60, passes=False) @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)