Skip to content

Commit

Permalink
chore(grouping): Use optimized grouping logic for 20% of non-transiti…
Browse files Browse the repository at this point in the history
…oning projects (#76614)

This enables using the new grouping logic (`_save_aggregate_new`) for 20% of projects not currently in a grouping config transition. (It's already rolled out to all transitioning projects.) Unlike with transitioning projects, `_save_aggregate_new` doesn't actually do anything different than `_save_aggregate` does with events from non-transitioning projects, so this change is essentially just switching the projects in question to a refactored version of what's already happening.
  • Loading branch information
lobsterkatie authored Aug 28, 2024
1 parent 97bea3e commit c26c6ef
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
12 changes: 8 additions & 4 deletions src/sentry/grouping/ingest/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ def project_uses_optimized_grouping(project: Project) -> bool:
if has_mobile_config or options.get("grouping.config_transition.killswitch_enabled"):
return False

return features.has(
"organizations:grouping-suppress-unnecessary-secondary-hash",
project.organization,
) or (is_in_transition(project))
return (
features.has(
"organizations:grouping-suppress-unnecessary-secondary-hash",
project.organization,
)
or (is_in_transition(project))
or project.id % 5 < 1 # 20% of all non-transition projects
)
12 changes: 8 additions & 4 deletions tests/sentry/event_manager/grouping/test_assign_to_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,13 +496,15 @@ def test_existing_group_new_hash_exists(
@pytest.mark.parametrize(
"mobile_config", (True, False), ids=(" mobile_config: True ", " mobile_config: False ")
)
@pytest.mark.parametrize("id_even", (True, False), ids=(" id_even: True ", " id_even: False "))
@pytest.mark.parametrize(
"id_qualifies", (True, False), ids=(" id_qualifies: True ", " id_qualifies: False ")
)
@patch("sentry.event_manager._save_aggregate_new", wraps=_save_aggregate_new)
@patch("sentry.event_manager._save_aggregate", wraps=_save_aggregate)
def test_uses_regular_or_optimized_grouping_as_appropriate(
mock_save_aggregate: MagicMock,
mock_save_aggregate_new: MagicMock,
id_even: bool,
id_qualifies: bool,
mobile_config: bool,
in_transition: bool,
flag_on: bool,
Expand All @@ -513,10 +515,10 @@ def test_uses_regular_or_optimized_grouping_as_appropriate(
with patch(
"sentry.utils.snowflake.generate_snowflake_id", side_effect=lambda _: randint(1, 1000)
):
# Keep making projects until we get an id of the correct parity
# Keep making projects until we get an id which matches `id_qualifies`
org = Factories.create_organization()
project = Factories.create_project(organization=org)
while project.id % 2 == (1 if id_even else 0):
while (project.id % 5 >= 1) if id_qualifies else (project.id % 5 < 1):
project = Factories.create_project(organization=org)

with (
Expand All @@ -533,5 +535,7 @@ def test_uses_regular_or_optimized_grouping_as_appropriate(
assert mock_save_aggregate_new.call_count == 1
elif in_transition:
assert mock_save_aggregate_new.call_count == 1
elif id_qualifies:
assert mock_save_aggregate_new.call_count == 1
else:
assert mock_save_aggregate.call_count == 1

0 comments on commit c26c6ef

Please sign in to comment.