Skip to content

Commit 59e1e9c

Browse files
lobsterkatiejjbayer
authored andcommitted
ref(grouping): Allow rate-limiting of grouping config upgrades (#102441)
This adds a sample rate to grouping config upgrades, in case we decide we want to roll out the new config bit by bit. It can also serve as a killswitch should we ever decide we need one.
1 parent b900dd6 commit 59e1e9c

File tree

3 files changed

+96
-0
lines changed

3 files changed

+96
-0
lines changed

src/sentry/grouping/ingest/config.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import logging
44
import time
55
from collections.abc import MutableMapping
6+
from random import random
67
from typing import Any
78

89
from django.conf import settings
@@ -59,6 +60,17 @@ def update_or_set_grouping_config_if_needed(project: Project, source: str) -> st
5960
if cache.get(cache_key) is not None:
6061
return "skipped - race condition"
6162

63+
# Check if we're rate-limiting upgrades. Note that we only allow skips here if the project will
64+
# be left with a valid config record even if it's skipped.
65+
upgrade_sample_rate = options.get("grouping.config_transition.config_upgrade_sample_rate")
66+
if (
67+
upgrade_sample_rate < 1
68+
and current_config_is_valid
69+
and project_option_exists
70+
and random() > upgrade_sample_rate
71+
):
72+
return "skipped - sample rate"
73+
6274
try:
6375
with locks.get(lock_key, duration=60, name="grouping-update-lock").acquire():
6476
if cache.get(cache_key) is not None:

src/sentry/options/defaults.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2821,6 +2821,15 @@
28212821
flags=FLAG_AUTOMATOR_MODIFIABLE,
28222822
)
28232823

2824+
# This is here in case we want to roll out a new config gradually. Can also be used as a killswitch
2825+
# for config upgrades if one is ever needed.
2826+
register(
2827+
"grouping.config_transition.config_upgrade_sample_rate",
2828+
type=Float,
2829+
default=1.0,
2830+
flags=FLAG_AUTOMATOR_MODIFIABLE,
2831+
)
2832+
28242833

28252834
# Sample rate for double writing to experimental dsn
28262835
register(

tests/sentry/event_manager/test_event_manager_grouping.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from sentry.models.project import Project
2020
from sentry.testutils.cases import TestCase
2121
from sentry.testutils.helpers.eventprocessing import save_new_event
22+
from sentry.testutils.helpers.options import override_options
2223
from sentry.testutils.pytest.fixtures import django_db_all
2324
from sentry.testutils.silo import assume_test_silo_mode_of
2425
from sentry.testutils.skips import requires_snuba
@@ -239,6 +240,80 @@ def test_no_ops_if_grouping_config_project_option_exists_and_is_current(
239240
assert self.project.get_option("sentry:secondary_grouping_config") is None
240241
assert self.project.get_option("sentry:secondary_grouping_expiry") == 0
241242

243+
@patch(
244+
"sentry.event_manager.update_or_set_grouping_config_if_needed",
245+
wraps=update_or_set_grouping_config_if_needed,
246+
)
247+
def test_no_ops_if_sample_rate_test_fails(self, update_config_spy: MagicMock):
248+
with (
249+
# Ensure our die roll will fall outside the sample rate
250+
patch("sentry.grouping.ingest.config.random", return_value=0.1121),
251+
override_options({"grouping.config_transition.config_upgrade_sample_rate": 0.0908}),
252+
):
253+
self.project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
254+
assert self.project.get_option("sentry:grouping_config") == NO_MSG_PARAM_CONFIG
255+
256+
save_new_event({"message": "Dogs are great!"}, self.project)
257+
258+
update_config_spy.assert_called_with(self.project, "ingest")
259+
260+
# After the function has been called, the config hasn't changed and no transition has
261+
# started
262+
assert self.project.get_option("sentry:grouping_config") == NO_MSG_PARAM_CONFIG
263+
assert self.project.get_option("sentry:secondary_grouping_config") is None
264+
assert self.project.get_option("sentry:secondary_grouping_expiry") == 0
265+
266+
@patch(
267+
"sentry.event_manager.update_or_set_grouping_config_if_needed",
268+
wraps=update_or_set_grouping_config_if_needed,
269+
)
270+
def test_ignores_sample_rate_if_current_config_is_invalid(self, update_config_spy: MagicMock):
271+
with (
272+
# Ensure our die roll will fall outside the sample rate
273+
patch("sentry.grouping.ingest.config.random", return_value=0.1121),
274+
override_options({"grouping.config_transition.config_upgrade_sample_rate": 0.0908}),
275+
):
276+
self.project.update_option("sentry:grouping_config", "not_a_real_config")
277+
assert self.project.get_option("sentry:grouping_config") == "not_a_real_config"
278+
279+
save_new_event({"message": "Dogs are great!"}, self.project)
280+
281+
update_config_spy.assert_called_with(self.project, "ingest")
282+
283+
# The config has been updated, but no transition has started because we can't calculate
284+
# a secondary hash using a config that doesn't exist
285+
assert self.project.get_option("sentry:grouping_config") == DEFAULT_GROUPING_CONFIG
286+
assert self.project.get_option("sentry:secondary_grouping_config") is None
287+
assert self.project.get_option("sentry:secondary_grouping_expiry") == 0
288+
289+
@patch(
290+
"sentry.event_manager.update_or_set_grouping_config_if_needed",
291+
wraps=update_or_set_grouping_config_if_needed,
292+
)
293+
def test_ignores_sample_rate_if_no_record_exists(self, update_config_spy: MagicMock):
294+
with (
295+
# Ensure our die roll will fall outside the sample rate
296+
patch("sentry.grouping.ingest.config.random", return_value=0.1121),
297+
override_options({"grouping.config_transition.config_upgrade_sample_rate": 0.0908}),
298+
):
299+
assert self.project.get_option("sentry:grouping_config") == DEFAULT_GROUPING_CONFIG
300+
assert not ProjectOption.objects.filter(
301+
project_id=self.project.id, key="sentry:grouping_config"
302+
).exists()
303+
304+
save_new_event({"message": "Dogs are great!"}, self.project)
305+
306+
update_config_spy.assert_called_with(self.project, "ingest")
307+
308+
# The config hasn't been updated, but now the project has its own record. No transition
309+
# has started because the config was already up to date.
310+
assert self.project.get_option("sentry:grouping_config") == DEFAULT_GROUPING_CONFIG
311+
assert ProjectOption.objects.filter(
312+
project_id=self.project.id, key="sentry:grouping_config"
313+
).exists()
314+
assert self.project.get_option("sentry:secondary_grouping_config") is None
315+
assert self.project.get_option("sentry:secondary_grouping_expiry") == 0
316+
242317

243318
class PlaceholderTitleTest(TestCase):
244319
"""

0 commit comments

Comments
 (0)