Skip to content

ref(seer grouping): Stop using projects:similarity-embeddings-metadata flag #74914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 25, 2024
Merged

ref(seer grouping): Stop using projects:similarity-embeddings-metadata flag #74914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a28e74c
stop checking flag in `_project_has_similarity_grouping_enabled`
lobsterkatie Jul 24, 2024
aa63f72
remove feature flag check in `get_seer_similar_issues`
lobsterkatie Jul 24, 2024
5c99698
fix tests
lobsterkatie Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/sentry/grouping/ingest/seer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,15 @@ def should_call_seer_for_grouping(event: Event, primary_hashes: CalculatedHashes


def _project_has_similarity_grouping_enabled(project: Project) -> bool:
has_either_seer_grouping_feature = features.has(
"projects:similarity-embeddings-metadata", project
) or features.has("projects:similarity-embeddings-grouping", project)
has_seer_grouping_flag_on = features.has("projects:similarity-embeddings-grouping", project)

# TODO: This is a hack to get ingest to turn on for projects as soon as they're backfilled. When
# the backfill script completes, we turn on this option, enabling ingest immediately rather than
# forcing the project to wait until it's been manually added to a feature handler. Once all
# projects have been backfilled, the option (and this check) can go away.
has_been_backfilled = project.get_option("sentry:similarity_backfill_completed")

return has_either_seer_grouping_feature or has_been_backfilled
return has_seer_grouping_flag_on or has_been_backfilled


# TODO: Here we're including events with hybrid fingerprints (ones which are `{{ default }}`
Expand Down Expand Up @@ -199,11 +197,7 @@ def get_seer_similar_issues(
)
parent_group = (
Group.objects.filter(id=seer_results[0].parent_group_id).first()
if (
seer_results
and seer_results[0].should_group
and features.has("projects:similarity-embeddings-grouping", event.project)
)
if seer_results and seer_results[0].should_group
else None
)

Expand Down
72 changes: 2 additions & 70 deletions tests/sentry/event_manager/grouping/test_seer_grouping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,7 @@ def test_obeys_seer_similarity_flags(self):
),
):

with Feature(
{
"projects:similarity-embeddings-metadata": False,
"projects:similarity-embeddings-grouping": False,
}
):
with Feature({"projects:similarity-embeddings-grouping": False}):
new_event = save_new_event({"message": "Adopt don't shop"}, self.project)

# We checked whether to make the call, but didn't go through with it
Expand All @@ -74,76 +69,14 @@ def test_obeys_seer_similarity_flags(self):
should_call_seer_spy.reset_mock()
get_seer_similar_issues_spy.reset_mock()

with Feature(
{
"projects:similarity-embeddings-metadata": True,
"projects:similarity-embeddings-grouping": False,
}
):
with Feature({"projects:similarity-embeddings-grouping": True}):
new_event = save_new_event({"message": "Maisey is silly"}, self.project)
expected_metadata = {**metadata_base, "request_hash": new_event.get_primary_hash()}

# We checked whether to make the call, and then made it
assert should_call_seer_spy.call_count == 1
assert get_seer_similar_issues_spy.call_count == 1

# Metadata returned and stored
assert get_seer_similar_issues_return_values[0][0] == expected_metadata
assert (
NonNone(new_event.group).data["metadata"]["seer_similarity"]
== expected_metadata
)
assert new_event.data["seer_similarity"] == expected_metadata

# No parent group returned or used (even though `should_group` is True)
assert get_seer_similar_issues_return_values[0][1] is None
assert new_event.group_id != existing_event.group_id

should_call_seer_spy.reset_mock()
get_seer_similar_issues_spy.reset_mock()
get_seer_similar_issues_return_values.pop()

with Feature(
{
"projects:similarity-embeddings-metadata": False,
"projects:similarity-embeddings-grouping": True,
}
):
new_event = save_new_event({"message": "Charlie is goofy"}, self.project)
expected_metadata = {**metadata_base, "request_hash": new_event.get_primary_hash()}

# We checked whether to make the call, and then made it
assert should_call_seer_spy.call_count == 1
assert get_seer_similar_issues_spy.call_count == 1

# Metadata returned and stored (metadata flag being off doesn't matter because
# grouping flag takes precedence)
assert get_seer_similar_issues_return_values[0][0] == expected_metadata
assert new_event.data["seer_similarity"] == expected_metadata

# Parent group returned and used
assert get_seer_similar_issues_return_values[0][1] == existing_event.group
assert new_event.group_id == existing_event.group_id

should_call_seer_spy.reset_mock()
get_seer_similar_issues_spy.reset_mock()
get_seer_similar_issues_return_values.pop()

with Feature(
{
"projects:similarity-embeddings-metadata": True,
"projects:similarity-embeddings-grouping": True,
}
):
new_event = save_new_event(
{"message": "Cori and Bodhi are ridiculous"}, self.project
)
expected_metadata = {**metadata_base, "request_hash": new_event.get_primary_hash()}

# We checked whether to make the call, and then made it
assert should_call_seer_spy.call_count == 1
assert get_seer_similar_issues_spy.call_count == 1

# Metadata returned and stored
assert get_seer_similar_issues_return_values[0][0] == expected_metadata
assert new_event.data["seer_similarity"] == expected_metadata
Expand Down Expand Up @@ -208,7 +141,6 @@ def test_stores_seer_results_in_metadata(self, _):
"results": [asdict(seer_result_data)],
}

assert NonNone(new_event.group).data["metadata"]["seer_similarity"] == expected_metadata
assert new_event.data["seer_similarity"] == expected_metadata

@with_feature("projects:similarity-embeddings-grouping")
Expand Down
59 changes: 8 additions & 51 deletions tests/sentry/grouping/ingest/test_seer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,36 +46,20 @@ def setUp(self):
self.primary_hashes = self.event.get_hashes()

def test_obeys_feature_enablement_check(self):
for metadata_flag, grouping_flag, backfill_completed_option, expected_result in [
# TODO: This manual cartesian product business is gross, but thankfully it's temporary -
# the metadata flag is about to go away and the backfill completed option will go away
# once all projects are backfilled.
(False, False, None, False),
(True, False, None, True),
(False, True, None, True),
(True, True, None, True),
(False, False, 11211231, True),
(True, False, 11211231, True),
(False, True, 11211231, True),
(True, True, 11211231, True),
for grouping_flag, backfill_completed_option, expected_result in [
(False, None, False),
(True, None, True),
(False, 11211231, True),
(True, 11211231, True),
]:
with (
Feature(
{
"projects:similarity-embeddings-metadata": metadata_flag,
"projects:similarity-embeddings-grouping": grouping_flag,
}
),
# Having too many cases above makes us hit the project rate limit if we don't patch this
patch("sentry.grouping.ingest.seer._ratelimiting_enabled", return_value=False),
):
with Feature({"projects:similarity-embeddings-grouping": grouping_flag}):
self.project.update_option(
"sentry:similarity_backfill_completed", backfill_completed_option
)
assert (
should_call_seer_for_grouping(self.event, self.primary_hashes)
is expected_result
), f"Case (metadata {metadata_flag}, grouping {grouping_flag}, backfill completed {backfill_completed_option}) failed."
), f"Case (grouping {grouping_flag}, backfill completed {backfill_completed_option}) failed."

@with_feature("projects:similarity-embeddings-grouping")
def test_obeys_content_filter(self):
Expand Down Expand Up @@ -186,7 +170,7 @@ def setUp(self):
)
self.new_event_hashes = CalculatedHashes(["20130809201315042012311220122111"])

@patch("sentry.grouping.ingest.seer.get_similarity_data_from_seer")
@patch("sentry.grouping.ingest.seer.get_similarity_data_from_seer", return_value=[])
def test_sends_expected_data_to_seer(self, mock_get_similarity_data: MagicMock):
new_event = Event(
project_id=self.project.id,
Expand Down Expand Up @@ -234,31 +218,6 @@ def test_sends_expected_data_to_seer(self, mock_get_similarity_data: MagicMock):
}
)

@with_feature({"projects:similarity-embeddings-grouping": False})
def test_returns_metadata_but_no_group_if_seer_grouping_flag_off(self):
seer_result_data = SeerSimilarIssueData(
parent_hash=NonNone(self.existing_event.get_primary_hash()),
parent_group_id=NonNone(self.existing_event.group_id),
stacktrace_distance=0.01,
message_distance=0.05,
should_group=True,
)
expected_metadata = {
"similarity_model_version": SEER_SIMILARITY_MODEL_VERSION,
"request_hash": self.new_event_hashes.hashes[0],
"results": [asdict(seer_result_data)],
}

with patch(
"sentry.grouping.ingest.seer.get_similarity_data_from_seer",
return_value=[seer_result_data],
):
assert get_seer_similar_issues(self.new_event, self.new_event_hashes) == (
expected_metadata,
None, # No group returned, even though `should_group` is True
)

@with_feature("projects:similarity-embeddings-grouping")
def test_returns_metadata_and_group_if_sufficiently_close_group_found(self):
seer_result_data = SeerSimilarIssueData(
parent_hash=NonNone(self.existing_event.get_primary_hash()),
Expand All @@ -282,7 +241,6 @@ def test_returns_metadata_and_group_if_sufficiently_close_group_found(self):
self.existing_event.group,
)

@with_feature("projects:similarity-embeddings-grouping")
def test_returns_metadata_but_no_group_if_similar_group_insufficiently_close(self):
seer_result_data = SeerSimilarIssueData(
parent_hash=NonNone(self.existing_event.get_primary_hash()),
Expand All @@ -306,7 +264,6 @@ def test_returns_metadata_but_no_group_if_similar_group_insufficiently_close(sel
None,
)

@with_feature("projects:similarity-embeddings-grouping")
def test_returns_no_group_and_empty_metadata_if_no_similar_group_found(self):
expected_metadata = {
"similarity_model_version": SEER_SIMILARITY_MODEL_VERSION,
Expand Down