Skip to content

Commit aa02773

Browse files
authored
ref(grouping): Add separate test-only grouping config (#95741)
When we're testing anything having to do with grouping config upgrade or transition, it's helpful to have a config which we know will produce a different hash than the default config produces. We've been using the legacy config for that, because it doesn't parameterize messages, but it's about to disappear. This therefore replaces its use in tests with a new config, explicitly for the purpose of generating a different hash, which is just the default config with message parameterization turned off. So that this new config doesn't count as valid in prod, it's only registered when `tests/grouping/__init__.py` is loaded (i.e., when grouping tests are run).
1 parent 8767711 commit aa02773

File tree

8 files changed

+106
-87
lines changed

8 files changed

+106
-87
lines changed

tests/sentry/event_manager/grouping/test_assign_to_group.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import pytest
99

10-
from sentry.conf.server import DEFAULT_GROUPING_CONFIG, LEGACY_GROUPING_CONFIG
10+
from sentry.conf.server import DEFAULT_GROUPING_CONFIG
1111
from sentry.event_manager import _create_group
1212
from sentry.eventstore.models import Event
1313
from sentry.grouping.ingest.hashing import (
@@ -22,6 +22,7 @@
2222
from sentry.testutils.pytest.fixtures import django_db_all
2323
from sentry.testutils.pytest.mocking import capture_results
2424
from sentry.testutils.skips import requires_snuba
25+
from tests.sentry.grouping import NO_MSG_PARAM_CONFIG
2526

2627
pytestmark = [requires_snuba]
2728

@@ -265,7 +266,7 @@ def test_new_group(
265266
event_data=event_data,
266267
project=project,
267268
primary_config=DEFAULT_GROUPING_CONFIG,
268-
secondary_config=LEGACY_GROUPING_CONFIG,
269+
secondary_config=NO_MSG_PARAM_CONFIG,
269270
in_transition=in_transition,
270271
)
271272

@@ -316,14 +317,14 @@ def test_existing_group_no_new_hash(
316317
event_data = {"message": "testing, testing, 123"}
317318

318319
# Set the stage by creating a group with the soon-to-be-secondary hash
319-
existing_event = save_event_with_grouping_config(event_data, project, LEGACY_GROUPING_CONFIG)
320+
existing_event = save_event_with_grouping_config(event_data, project, NO_MSG_PARAM_CONFIG)
320321

321322
# Now save a new, identical, event with an updated grouping config
322323
results = get_results_from_saving_event(
323324
event_data=event_data,
324325
project=project,
325326
primary_config=DEFAULT_GROUPING_CONFIG,
326-
secondary_config=LEGACY_GROUPING_CONFIG,
327+
secondary_config=NO_MSG_PARAM_CONFIG,
327328
in_transition=in_transition,
328329
existing_group_id=existing_event.group_id,
329330
)
@@ -378,13 +379,14 @@ def test_existing_group_new_hash_exists(
378379
project = default_project
379380
event_data = {"message": "testing, testing, 123"}
380381

381-
# Set the stage by creating a group tied to the new hash (and possibly the legacy hash as well)
382+
# Set the stage by creating a group tied to the new hash (and, depending on the value of
383+
# `secondary_hash_exists`, a secondary hash tied to the same group as well)
382384
if secondary_hash_exists:
383385
existing_event_with_secondary_hash = save_event_with_grouping_config(
384-
event_data, project, LEGACY_GROUPING_CONFIG
386+
event_data, project, NO_MSG_PARAM_CONFIG
385387
)
386388
existing_event_with_primary_hash = save_event_with_grouping_config(
387-
event_data, project, DEFAULT_GROUPING_CONFIG, LEGACY_GROUPING_CONFIG, True
389+
event_data, project, DEFAULT_GROUPING_CONFIG, NO_MSG_PARAM_CONFIG, True
388390
)
389391
group_id = existing_event_with_primary_hash.group_id
390392

@@ -407,7 +409,7 @@ def test_existing_group_new_hash_exists(
407409
event_data=event_data,
408410
project=project,
409411
primary_config=DEFAULT_GROUPING_CONFIG,
410-
secondary_config=LEGACY_GROUPING_CONFIG,
412+
secondary_config=NO_MSG_PARAM_CONFIG,
411413
in_transition=in_transition,
412414
existing_group_id=group_id,
413415
)

tests/sentry/event_manager/grouping/test_grouphash_metadata.py

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from typing import Any
55
from unittest.mock import ANY, MagicMock, patch
66

7-
from sentry.conf.server import DEFAULT_GROUPING_CONFIG, LEGACY_GROUPING_CONFIG
7+
from sentry.conf.server import DEFAULT_GROUPING_CONFIG
88
from sentry.eventstore.models import Event
99
from sentry.grouping.ingest.grouphash_metadata import create_or_update_grouphash_metadata_if_needed
1010
from sentry.models.grouphash import GroupHash
@@ -13,6 +13,7 @@
1313
from sentry.testutils.helpers.eventprocessing import save_new_event
1414
from sentry.testutils.helpers.options import override_options
1515
from sentry.testutils.skips import requires_snuba
16+
from tests.sentry.grouping import NO_MSG_PARAM_CONFIG
1617

1718
pytestmark = [requires_snuba]
1819

@@ -100,22 +101,23 @@ def test_stores_expected_properties(self):
100101
def test_stores_expected_properties_for_secondary_hashes(self):
101102
project = self.project
102103

103-
project.update_option("sentry:grouping_config", LEGACY_GROUPING_CONFIG)
104+
project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
104105

105-
# Ensure the legacy grouphash doesn't have metadata added when it's created as the primary
106-
# grouphash, so we can test how the metadata code handles it when it's a seconary grouphash
106+
# Ensure the older grouphash doesn't have metadata added when it's created as the primary
107+
# grouphash, so we can test how the metadata code handles it when that same grouphash comes
108+
# up as a secondary grouphash
107109
with override_options({"grouping.grouphash_metadata.ingestion_writes_enabled": False}):
108110
event1 = save_new_event(
109111
{"message": "Dogs are great! 1231", "platform": "python"}, self.project
110112
)
111-
legacy_config_grouphash = GroupHash.objects.filter(
113+
older_config_grouphash = GroupHash.objects.filter(
112114
project=self.project, hash=event1.get_primary_hash()
113115
).first()
114-
assert legacy_config_grouphash and not legacy_config_grouphash.metadata
116+
assert older_config_grouphash and not older_config_grouphash.metadata
115117

116118
# Update the project's grouping config, and set it in transition mode
117119
project.update_option("sentry:grouping_config", DEFAULT_GROUPING_CONFIG)
118-
project.update_option("sentry:secondary_grouping_config", LEGACY_GROUPING_CONFIG)
120+
project.update_option("sentry:secondary_grouping_config", NO_MSG_PARAM_CONFIG)
119121
project.update_option("sentry:secondary_grouping_expiry", time() + 3600)
120122

121123
event2 = save_new_event(
@@ -124,40 +126,41 @@ def test_stores_expected_properties_for_secondary_hashes(self):
124126
default_config_grouphash = GroupHash.objects.filter(
125127
project=self.project, hash=event2.get_primary_hash()
126128
).first()
127-
# The events should end up in the same group, but their hashes should be different, because
128-
# the legacy config won't parameterize the number in the message, while the new one will
129+
# The events should end up in the same group, but their hashes should be different (because
130+
# NO_MSG_PARAM_CONFIG won't parameterize the number in the message, while the default config
131+
# will)
129132
assert event1.group_id == event2.group_id
130133
assert (
131134
default_config_grouphash
132-
and default_config_grouphash.hash != legacy_config_grouphash.hash
135+
and default_config_grouphash.hash != older_config_grouphash.hash
133136
)
134137

135138
# This time metadata was added
136-
legacy_config_grouphash.refresh_from_db()
137-
assert legacy_config_grouphash.metadata
139+
older_config_grouphash.refresh_from_db()
140+
assert older_config_grouphash.metadata
138141
self.assert_metadata_values(
139-
legacy_config_grouphash,
142+
older_config_grouphash,
140143
{
141144
"schema_version": GROUPHASH_METADATA_SCHEMA_VERSION,
142-
"latest_grouping_config": LEGACY_GROUPING_CONFIG,
145+
"latest_grouping_config": NO_MSG_PARAM_CONFIG,
143146
"platform": "python",
144147
},
145148
)
146149
# No hash basis or hashing metadata because secondary grouphashes don't come with variants
147-
assert legacy_config_grouphash.metadata.hash_basis is None
148-
assert legacy_config_grouphash.metadata.hashing_metadata is None
150+
assert older_config_grouphash.metadata.hash_basis is None
151+
assert older_config_grouphash.metadata.hashing_metadata is None
149152

150153
@override_options({"grouping.grouphash_metadata.backfill_sample_rate": 1.0})
151154
@patch("sentry.grouping.ingest.grouphash_metadata.metrics.incr")
152155
def test_does_grouping_config_update(self, mock_metrics_incr: MagicMock):
153-
self.project.update_option("sentry:grouping_config", LEGACY_GROUPING_CONFIG)
156+
self.project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
154157

155158
event1 = save_new_event({"message": "Dogs are great!"}, self.project)
156159
grouphash1 = GroupHash.objects.filter(
157160
project=self.project, hash=event1.get_primary_hash()
158161
).first()
159162

160-
self.assert_metadata_values(grouphash1, {"latest_grouping_config": LEGACY_GROUPING_CONFIG})
163+
self.assert_metadata_values(grouphash1, {"latest_grouping_config": NO_MSG_PARAM_CONFIG})
161164

162165
# Update the grouping config. Since there's nothing to parameterize in the message, the
163166
# hash should be the same under both configs, meaning we'll hit the same grouphash.
@@ -177,21 +180,21 @@ def test_does_grouping_config_update(self, mock_metrics_incr: MagicMock):
177180
"grouping.grouphash_metadata.db_hit",
178181
tags={
179182
"reason": "old_grouping_config",
180-
"current_config": LEGACY_GROUPING_CONFIG,
183+
"current_config": NO_MSG_PARAM_CONFIG,
181184
"new_config": DEFAULT_GROUPING_CONFIG,
182185
},
183186
)
184187

185188
@override_options({"grouping.grouphash_metadata.backfill_sample_rate": 0.415})
186189
def test_updates_obey_sample_rate(self):
187-
self.project.update_option("sentry:grouping_config", LEGACY_GROUPING_CONFIG)
190+
self.project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
188191

189192
event1 = save_new_event({"message": "Dogs are great!"}, self.project)
190193
grouphash1 = GroupHash.objects.filter(
191194
project=self.project, hash=event1.get_primary_hash()
192195
).first()
193196

194-
self.assert_metadata_values(grouphash1, {"latest_grouping_config": LEGACY_GROUPING_CONFIG})
197+
self.assert_metadata_values(grouphash1, {"latest_grouping_config": NO_MSG_PARAM_CONFIG})
195198

196199
# Update the grouping config. Since there's nothing to parameterize in the message, the
197200
# hash should be the same under both configs, meaning we'll hit the same grouphash.
@@ -208,9 +211,7 @@ def test_updates_obey_sample_rate(self):
208211
assert grouphash1 == grouphash2
209212

210213
# Grouping config wasn't updated
211-
self.assert_metadata_values(
212-
grouphash2, {"latest_grouping_config": LEGACY_GROUPING_CONFIG}
213-
)
214+
self.assert_metadata_values(grouphash2, {"latest_grouping_config": NO_MSG_PARAM_CONFIG})
214215

215216
# Under the sample rate cutoff, so record should be updated
216217
with patch("sentry.grouping.ingest.grouphash_metadata.random.random", return_value=0.1231):
@@ -288,7 +289,7 @@ def test_does_schema_update(self, mock_metrics_incr: MagicMock):
288289
@override_options({"grouping.grouphash_metadata.backfill_sample_rate": 1.0})
289290
@patch("sentry.grouping.ingest.grouphash_metadata.metrics.incr")
290291
def test_does_both_updates(self, mock_metrics_incr: MagicMock):
291-
self.project.update_option("sentry:grouping_config", LEGACY_GROUPING_CONFIG)
292+
self.project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
292293

293294
with patch(
294295
"sentry.grouping.ingest.grouphash_metadata.GROUPHASH_METADATA_SCHEMA_VERSION", "11"
@@ -301,7 +302,7 @@ def test_does_both_updates(self, mock_metrics_incr: MagicMock):
301302
self.assert_metadata_values(
302303
grouphash1,
303304
{
304-
"latest_grouping_config": LEGACY_GROUPING_CONFIG,
305+
"latest_grouping_config": NO_MSG_PARAM_CONFIG,
305306
"schema_version": "11",
306307
"hashing_metadata": {
307308
"message_source": "message",
@@ -346,7 +347,7 @@ def test_does_both_updates(self, mock_metrics_incr: MagicMock):
346347
"grouping.grouphash_metadata.db_hit",
347348
tags={
348349
"reason": "config_and_schema",
349-
"current_config": LEGACY_GROUPING_CONFIG,
350+
"current_config": NO_MSG_PARAM_CONFIG,
350351
"new_config": DEFAULT_GROUPING_CONFIG,
351352
"current_version": "11",
352353
"new_version": "12",

tests/sentry/event_manager/test_event_manager_grouping.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@
88
import pytest
99

1010
from sentry import audit_log
11-
from sentry.conf.server import (
12-
DEFAULT_GROUPING_CONFIG,
13-
LEGACY_GROUPING_CONFIG,
14-
SENTRY_GROUPING_CONFIG_TRANSITION_DURATION,
15-
)
11+
from sentry.conf.server import DEFAULT_GROUPING_CONFIG, SENTRY_GROUPING_CONFIG_TRANSITION_DURATION
1612
from sentry.event_manager import _get_updated_group_title
1713
from sentry.eventtypes.base import DefaultEvent
1814
from sentry.grouping.api import get_grouping_config_dict_for_project
@@ -26,6 +22,7 @@
2622
from sentry.testutils.pytest.fixtures import django_db_all
2723
from sentry.testutils.silo import assume_test_silo_mode_of
2824
from sentry.testutils.skips import requires_snuba
25+
from tests.sentry.grouping import NO_MSG_PARAM_CONFIG
2926

3027
pytestmark = [requires_snuba]
3128

@@ -151,7 +148,7 @@ def test_auto_updates_grouping_config_even_if_config_is_gone(self):
151148
assert self.project.get_option("sentry:secondary_grouping_config") is None
152149

153150
def test_auto_updates_grouping_config(self):
154-
self.project.update_option("sentry:grouping_config", LEGACY_GROUPING_CONFIG)
151+
self.project.update_option("sentry:grouping_config", NO_MSG_PARAM_CONFIG)
155152

156153
save_new_event({"message": "Adopt don't shop"}, self.project)
157154
assert self.project.get_option("sentry:grouping_config") == DEFAULT_GROUPING_CONFIG
@@ -164,7 +161,7 @@ def test_auto_updates_grouping_config(self):
164161

165162
assert audit_log_entry.data == {
166163
"sentry:grouping_config": DEFAULT_GROUPING_CONFIG,
167-
"sentry:secondary_grouping_config": LEGACY_GROUPING_CONFIG,
164+
"sentry:secondary_grouping_config": NO_MSG_PARAM_CONFIG,
168165
"sentry:secondary_grouping_expiry": ANY, # tested separately below
169166
"id": self.project.id,
170167
"slug": self.project.slug,
@@ -440,8 +437,8 @@ def test_records_avg_calculations_per_event_metrics(self, mock_metrics_incr: Mag
440437
project = self.project
441438

442439
cases: list[Any] = [
443-
["Dogs are great!", LEGACY_GROUPING_CONFIG, None, None, 1],
444-
["Adopt don't shop", DEFAULT_GROUPING_CONFIG, LEGACY_GROUPING_CONFIG, time() + 3600, 2],
440+
["Dogs are great!", NO_MSG_PARAM_CONFIG, None, None, 1],
441+
["Adopt don't shop", DEFAULT_GROUPING_CONFIG, NO_MSG_PARAM_CONFIG, time() + 3600, 2],
445442
]
446443

447444
for (
@@ -485,10 +482,10 @@ def test_adds_correct_tags_to_avg_calculations_per_event_metrics(
485482
project = self.project
486483

487484
in_transition_cases: list[Any] = [
488-
[LEGACY_GROUPING_CONFIG, None, None, "False"], # Not in transition
485+
[NO_MSG_PARAM_CONFIG, None, None, "False"], # Not in transition
489486
[
490487
DEFAULT_GROUPING_CONFIG,
491-
LEGACY_GROUPING_CONFIG,
488+
NO_MSG_PARAM_CONFIG,
492489
time() + 3600,
493490
"True",
494491
], # In transition
@@ -541,7 +538,7 @@ def test_records_hash_comparison_metric(
541538
):
542539
project = default_project
543540
project.update_option("sentry:grouping_config", DEFAULT_GROUPING_CONFIG)
544-
project.update_option("sentry:secondary_grouping_config", LEGACY_GROUPING_CONFIG)
541+
project.update_option("sentry:secondary_grouping_config", NO_MSG_PARAM_CONFIG)
545542
project.update_option("sentry:secondary_grouping_expiry", time() + 3600)
546543

547544
with mock.patch(

tests/sentry/grouping/__init__.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from sentry.grouping.component import BaseGroupingComponent
2323
from sentry.grouping.enhancer import Enhancements
2424
from sentry.grouping.fingerprinting import FingerprintingRules
25-
from sentry.grouping.strategies.configurations import CONFIGURATIONS
25+
from sentry.grouping.strategies.configurations import CONFIGURATIONS, register_strategy_config
2626
from sentry.grouping.variants import BaseVariant
2727
from sentry.models.project import Project
2828
from sentry.stacktraces.processing import normalize_stacktraces_for_grouping
@@ -32,6 +32,16 @@
3232
GROUPING_INPUTS_DIR = path.join(path.dirname(__file__), "grouping_inputs")
3333
FINGERPRINT_INPUTS_DIR = path.join(path.dirname(__file__), "fingerprint_inputs")
3434

35+
# Create a grouping config to be used only in tests, in which message parameterization is turned
36+
# off. This lets us easily force an event to have different hashes for different configs. (We use a
37+
# purposefully old date so that it can be used as a secondary config.)
38+
NO_MSG_PARAM_CONFIG = "no-msg-param-tests-only:2012-12-31"
39+
register_strategy_config(
40+
id=NO_MSG_PARAM_CONFIG,
41+
base=DEFAULT_GROUPING_CONFIG,
42+
initial_context={"normalize_message": False},
43+
)
44+
3545

3646
class GroupingInput:
3747
def __init__(self, inputs_dir: str, filename: str):

tests/sentry/grouping/test_benchmark.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import pytest
22

33
from sentry.grouping.strategies.configurations import CONFIGURATIONS
4-
from tests.sentry.grouping import GROUPING_INPUTS_DIR, GroupingInput, get_grouping_inputs
4+
from tests.sentry.grouping import (
5+
GROUPING_INPUTS_DIR,
6+
NO_MSG_PARAM_CONFIG,
7+
GroupingInput,
8+
get_grouping_inputs,
9+
)
510

611
GROUPING_INPUTS = get_grouping_inputs(GROUPING_INPUTS_DIR)
712

@@ -18,7 +23,8 @@ def benchmark_available() -> bool:
1823
@pytest.mark.skipif(not benchmark_available(), reason="requires pytest-benchmark")
1924
@pytest.mark.parametrize(
2025
"config_name",
21-
sorted(CONFIGURATIONS.keys()),
26+
# NO_MSG_PARAM_CONFIG is only used in tests, so no need to benchmark it
27+
sorted(set(CONFIGURATIONS.keys()) - {NO_MSG_PARAM_CONFIG}),
2228
ids=lambda config_name: config_name.replace("-", "_"),
2329
)
2430
def test_benchmark_grouping(config_name, benchmark):

tests/sentry/grouping/test_grouphash_metadata.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from sentry.utils import json
2525
from tests.sentry.grouping import (
2626
GROUPING_INPUTS_DIR,
27+
NO_MSG_PARAM_CONFIG,
2728
GroupingInput,
2829
dump_variant,
2930
get_snapshot_path,
@@ -38,7 +39,8 @@
3839
@with_grouping_inputs("grouping_input", GROUPING_INPUTS_DIR)
3940
@pytest.mark.parametrize(
4041
"config_name",
41-
set(CONFIGURATIONS.keys()) - {DEFAULT_GROUPING_CONFIG},
42+
# The default config is tested below, and NO_MSG_PARAM_CONFIG is only meant for use in unit tests
43+
set(CONFIGURATIONS.keys()) - {DEFAULT_GROUPING_CONFIG, NO_MSG_PARAM_CONFIG},
4244
ids=lambda config_name: config_name.replace("-", "_"),
4345
)
4446
def test_hash_basis_with_legacy_configs(
@@ -97,7 +99,8 @@ def test_hash_basis_with_current_default_config(
9799
@django_db_all
98100
@pytest.mark.parametrize(
99101
"config_name",
100-
CONFIGURATIONS.keys(),
102+
# NO_MSG_PARAM_CONFIG is only meant for use in unit tests
103+
set(CONFIGURATIONS.keys()) - {NO_MSG_PARAM_CONFIG},
101104
ids=lambda config_name: config_name.replace("-", "_"),
102105
)
103106
def test_unknown_hash_basis(

0 commit comments

Comments
 (0)