fix(suspect-resolutions): Tweak commit correlation logic and track total events (#37891)

Author: komal-saini

src/sentry/utils/suspect_resolutions/__init__.py

@@ -2,4 +2,4 @@
 
 # make sure to increment this when making changes to anything within the 'suspect_resolutions' directory
 # keeps track of changes to how we process suspect commits, so we can filter out analytics events by the algo version
-ALGO_VERSION = "0.0.3"
+ALGO_VERSION = "0.0.4"

src/sentry/utils/suspect_resolutions/analytics.py

@@ -16,6 +16,8 @@ class SuspectResolutionEvaluation(analytics.Event):
         analytics.Attribute("is_commit_correlated"),
         analytics.Attribute("resolved_issue_release_ids"),
         analytics.Attribute("candidate_issue_release_ids"),
+        analytics.Attribute("resolved_issue_total_events"),
+        analytics.Attribute("candidate_issue_total_events"),
     )
 
 

src/sentry/utils/suspect_resolutions/commit_correlation.py

@@ -4,7 +4,7 @@
 from datetime import datetime, timedelta
 from typing import Sequence, Set
 
-from sentry.models import CommitFileChange, Group, GroupRelease, ReleaseCommit
+from sentry.models import CommitFileChange, Group, GroupRelease, Release, ReleaseCommit
 
 
 @dataclass
@@ -40,19 +40,20 @@ def is_issue_commit_correlated(
     return CommitCorrelatedResult(
         not resolved_filechanges.files_changed.isdisjoint(candidate_filechanges.files_changed),
         resolved_filechanges.release_ids,
-        resolved_filechanges.release_ids,
+        candidate_filechanges.release_ids,
     )
 
 
 def get_files_changed_in_releases(
     resolved_issue_time: datetime, issue_id: int, project_id: int
 ) -> ReleaseCommitFileChanges:
-    releases = GroupRelease.objects.filter(
-        group_id=issue_id,
-        project_id=project_id,
-        last_seen__gte=(resolved_issue_time - timedelta(hours=5)),
-    ).values_list("release_id", flat=True)
-
+    releases = Release.objects.filter(
+        id__in=GroupRelease.objects.filter(
+            group_id=issue_id,
+            project_id=project_id,
+        ).values_list("release_id", flat=True),
+        date_added__gte=(resolved_issue_time - timedelta(hours=5)),
+    )
     if not releases:
         return ReleaseCommitFileChanges([], set())
 

src/sentry/utils/suspect_resolutions/get_suspect_resolutions.py

@@ -1,6 +1,8 @@
 from datetime import timedelta
 from typing import Sequence
 
+from django.utils import timezone
+
 from sentry import features
 from sentry.models import Activity, Group, GroupStatus
 from sentry.signals import issue_resolved
@@ -15,7 +17,19 @@ def record_suspect_resolutions(
     organization_id, project, group, user, resolution_type, **kwargs
 ) -> None:
     if features.has("projects:suspect-resolutions", project):
-        get_suspect_resolutions.delay(group.id)
+        if (
+            resolution_type == "in_next_release"
+            or resolution_type == "in_release"
+            or resolution_type == "with_commit"
+            or resolution_type == "in_commit"
+        ):
+            get_suspect_resolutions.delay(
+                group.id,
+                eta=timezone.now() + timedelta(hours=1),
+                expires=timezone.now() + timedelta(hours=1, minutes=30),
+            )
+        else:
+            get_suspect_resolutions.delay(group.id)
 
 
 @instrumented_task(name="sentry.tasks.get_suspect_resolutions", queue="get_suspect_resolutions")
@@ -66,6 +80,8 @@ def get_suspect_resolutions(resolved_issue_id: int) -> Sequence[int]:
             is_commit_correlated=commit_correlation.is_correlated,
             resolved_issue_release_ids=commit_correlation.resolved_issue_release_ids,
             candidate_issue_release_ids=commit_correlation.candidate_issue_release_ids,
+            resolved_issue_total_events=metric_correlation_result.resolved_issue_total_events,
+            candidate_issue_total_events=metric_correlation_result.candidate_issue_total_events,
         )
 
     return correlated_issue_ids

src/sentry/utils/suspect_resolutions/metric_correlation.py

@@ -11,6 +11,8 @@ class CandidateMetricCorrResult:
     candidate_suspect_resolution_id: int
     is_correlated: bool
     coefficient: float
+    candidate_issue_total_events: int
+    resolved_issue_total_events: int
 
 
 @dataclass
@@ -47,13 +49,18 @@ def is_issue_error_rate_correlated(
     x = [events for _, events in data[resolved_issue.id]]
     y = {csr.id: [events for _, events in data[csr.id]] for csr in candidate_suspect_resolutions}
 
+    resolved_issue_total_events = sum(x)
+    candidate_issue_total_events = {csr: sum(events) for csr, events in y.items()}
+
     coefficients = {csr_id: calculate_pearson_correlation_coefficient(x, y[csr_id]) for csr_id in y}
 
     results = [
         CandidateMetricCorrResult(
             candidate_suspect_resolution_id=csr_id,
             is_correlated=coefficient > 0.4,
             coefficient=coefficient,
+            candidate_issue_total_events=candidate_issue_total_events[csr_id],
+            resolved_issue_total_events=resolved_issue_total_events,
         )
         for (csr_id, coefficient) in coefficients.items()
     ]

tests/sentry/utils/suspect_resolutions/test_commit_correlation.py

@@ -22,7 +22,9 @@ class CommitCorrelationTest(TestCase):
     def setup(self, status=GroupStatus.RESOLVED):
         project = self.create_project()
         issue = self.create_group(project=project, status=status, resolved_at=timezone.now())
-        release = self.create_release(project=project, version="1")
+        release = self.create_release(
+            project=project, version="1", date_added=timezone.now() - timedelta(hours=3)
+        )
         repo = self.create_repo(project=project, name=project.name)
         commit = Commit.objects.create(
             organization_id=project.organization_id, repository_id=repo.id, key="1"
@@ -40,7 +42,6 @@ def setup(self, status=GroupStatus.RESOLVED):
             project_id=project.id,
             group_id=issue.id,
             release_id=release.id,
-            last_seen=(timezone.now() - timedelta(hours=3)),
         )
 
         return project, issue, release, repo
@@ -53,7 +54,7 @@ def test_get_files_changed_resolved_in_release(self):
         files_changed = get_files_changed_in_releases(timezone.now(), issue.id, project.id)
 
         assert files_changed.files_changed == {".random", ".random2"}
-        assert files_changed.release_ids.first() == release.id
+        assert files_changed.release_ids.first().id == release.id
 
     def test_get_files_changed_resolved_in_commit(self):
         (project, issue, release, repo) = self.setup()
@@ -64,7 +65,7 @@ def test_get_files_changed_resolved_in_commit(self):
         release_ids, files_changed = (res.release_ids, res.files_changed)
 
         assert files_changed == {".random", ".random2"}
-        assert release_ids.first() == release.id
+        assert release_ids.first().id == release.id
 
     def test_get_files_changed_resolved_in_pull_request(self):
         (project, issue, release, repo) = self.setup()
@@ -75,23 +76,23 @@ def test_get_files_changed_resolved_in_pull_request(self):
         release_ids, files_changed = (res.release_ids, res.files_changed)
 
         assert files_changed == {".random", ".random2"}
-        assert release_ids.first() == release.id
+        assert release_ids.first().id == release.id
 
     def test_get_files_changed_unresolved_issue(self):
         (project, issue, release, repo) = self.setup(status=GroupStatus.UNRESOLVED)
         res = get_files_changed_in_releases(timezone.now(), issue.id, project.id)
         release_ids, files_changed = (res.release_ids, res.files_changed)
 
         assert files_changed == {".random", ".random2"}
-        assert release_ids.first() == release.id
+        assert release_ids.first().id == release.id
 
     def test_get_files_changed_manually_resolved(self):
         (project, issue, release, repo) = self.setup()
         res = get_files_changed_in_releases(timezone.now(), issue.id, project.id)
         release_ids, files_changed = (res.release_ids, res.files_changed)
 
         assert files_changed == {".random", ".random2"}
-        assert release_ids.first() == release.id
+        assert release_ids.first().id == release.id
 
     def test_no_files_changed(self):
         project = self.create_project()
@@ -127,8 +128,8 @@ def test_no_files_changed(self):
 
         assert res1.files_changed == set()
         assert res2.files_changed == set()
-        assert res1.release_ids.first() == release.id
-        assert res2.release_ids.first() == release2.id
+        assert res1.release_ids.first().id == release.id
+        assert res2.release_ids.first().id == release2.id
         assert not is_issue_commit_correlated(group1.id, group2.id, project.id).is_correlated
 
     def test_files_changed_unreleased_commits(self):
@@ -156,7 +157,7 @@ def test_files_changed_unreleased_commits(self):
         release_ids, files_changed = (res.release_ids, res.files_changed)
 
         assert files_changed == set()
-        assert release_ids.first() == release.id
+        assert release_ids.first().id == release.id
 
     def test_get_files_changed_shared_files(self):
         (project, issue, release, repo) = self.setup()
@@ -213,14 +214,16 @@ def test_get_files_changed_no_shared_files(self):
 
         assert res1.files_changed == {".random", ".random2"}
         assert res2.files_changed == {".gitignore"}
-        assert res1.release_ids.first() == release.id
-        assert res2.release_ids.first() == release2.id
+        assert res1.release_ids.first().id == release.id
+        assert res2.release_ids.first().id == release2.id
         assert not is_issue_commit_correlated(issue.id, issue2.id, project.id).is_correlated
 
     def get_files_changed_outside_of_time_window(self):
         project = self.create_project()
         group = self.create_group(project=project, resolved_at=timezone.now())
-        release = self.create_release(project=project, version="1")
+        release = self.create_release(
+            project=project, version="1", date_added=timezone.now() - timedelta(hours=8)
+        )
         repo = self.create_repo(project=project, name=project.name)
         commit = Commit.objects.create(
             organization_id=project.organization_id, repository_id=repo.id, key="1"
@@ -235,7 +238,6 @@ def get_files_changed_outside_of_time_window(self):
             project_id=project.id,
             group_id=group.id,
             release_id=release.id,
-            last_seen=(timezone.now() - timedelta(hours=8)),
         )
 
         res = get_files_changed_in_releases(group.resolved_at, group.id, project.id)

tests/sentry/utils/suspect_resolutions/test_get_suspect_resolutions.py

@@ -27,7 +27,7 @@ class GetSuspectResolutionsTest(TestCase):
         "sentry.utils.suspect_resolutions.get_suspect_resolutions.is_issue_error_rate_correlated",
         mock.Mock(
             return_value=IssueReleaseMetricCorrResult(
-                [(CandidateMetricCorrResult(0, True, 0.5))],
+                [(CandidateMetricCorrResult(0, True, 0.5, 100, 100))],
                 timezone.now(),
                 timezone.now() - timedelta(days=1),
                 timezone.now() - timedelta(hours=2),
@@ -56,7 +56,7 @@ def test_get_suspect_resolutions(self):
         "sentry.utils.suspect_resolutions.get_suspect_resolutions.is_issue_error_rate_correlated",
         mock.Mock(
             return_value=IssueReleaseMetricCorrResult(
-                [(CandidateMetricCorrResult(0, True, 0.5))],
+                [(CandidateMetricCorrResult(0, True, 0.5, 100, 100))],
                 timezone.now(),
                 timezone.now() - timedelta(days=1),
                 timezone.now() - timedelta(hours=2),
@@ -80,7 +80,7 @@ def test_get_suspect_resolutions_uncorrelated_commit_data(self):
         "sentry.utils.suspect_resolutions.get_suspect_resolutions.is_issue_error_rate_correlated",
         mock.Mock(
             return_value=IssueReleaseMetricCorrResult(
-                [(CandidateMetricCorrResult(0, False, 0.2))],
+                [(CandidateMetricCorrResult(0, False, 0.2, 100, 100))],
                 timezone.now(),
                 timezone.now() - timedelta(days=1),
                 timezone.now() - timedelta(hours=2),
@@ -103,7 +103,7 @@ def test_get_suspect_resolutions_uncorrelated_metric_data(self):
         "sentry.utils.suspect_resolutions.get_suspect_resolutions.is_issue_error_rate_correlated",
         mock.Mock(
             return_value=IssueReleaseMetricCorrResult(
-                [(CandidateMetricCorrResult(0, False, 0.2))],
+                [(CandidateMetricCorrResult(0, False, 0.2, 100, 100))],
                 timezone.now(),
                 timezone.now() - timedelta(days=1),
                 timezone.now() - timedelta(hours=2),
@@ -158,7 +158,7 @@ def test_get_suspect_resolutions_no_other_issues_in_project(self):
         "sentry.utils.suspect_resolutions.get_suspect_resolutions.is_issue_error_rate_correlated",
         mock.Mock(
             return_value=IssueReleaseMetricCorrResult(
-                [(CandidateMetricCorrResult(0, True, 0.5))],
+                [(CandidateMetricCorrResult(0, True, 0.5, 100, 100))],
                 datetime(2022, 1, 3),
                 datetime(2022, 1, 2),
                 datetime(2022, 1, 1),
@@ -193,6 +193,8 @@ def test_suspect_resolutions_evaluation_analytics_event(self, record):
                 is_commit_correlated=True,
                 resolved_issue_release_ids=[1, 2],
                 candidate_issue_release_ids=[3, 4],
+                resolved_issue_total_events=100,
+                candidate_issue_total_events=100,
             )
         ]
 
@@ -202,17 +204,15 @@ def test_record_suspect_resolutions(self, mock_record_suspect_resolutions):
         project = self.create_project(organization=organization)
         user = self.create_user()
         resolved_issue = self.create_group(project=project)
-        resolution_type = Activity.objects.create(
-            project=project, group=resolved_issue, type=ActivityType.SET_RESOLVED_IN_RELEASE.value
-        )
+        resolution_type = "in_release"
 
         with self.feature("projects:suspect-resolutions"):
             issue_resolved.send(
                 organization_id=organization.id,
                 project=project,
                 group=resolved_issue,
                 user=user,
-                resolution_type=resolution_type.type,
+                resolution_type=resolution_type,
                 sender=type(self.project),
             )
 

tests/sentry/utils/suspect_resolutions/test_metric_correlation.py

@@ -56,6 +56,9 @@ def test_correlated_issues(self, mock_get_range):
         group1_pearson_values = [events for _, events in group1_events]
         group2_pearson_values = [events for _, events in group2_events]
 
+        group1_total_events = sum(group1_pearson_values)
+        group2_total_events = sum(group2_pearson_values)
+
         coefficient = calculate_pearson_correlation_coefficient(
             group1_pearson_values, group2_pearson_values
         )
@@ -67,6 +70,8 @@ def test_correlated_issues(self, mock_get_range):
                 candidate_suspect_resolution_id=group2.id,
                 is_correlated=True,
                 coefficient=coefficient,
+                candidate_issue_total_events=group2_total_events,
+                resolved_issue_total_events=group1_total_events,
             )
         ]
         assert result.issue_resolved_time == group1.resolved_at
@@ -85,6 +90,9 @@ def test_uncorrelated_issues(self, mock_get_range):
         group1_pearson_values = [events for _, events in group1_events]
         group2_pearson_values = [events for _, events in group2_events]
 
+        group1_total_events = sum(group1_pearson_values)
+        group2_total_events = sum(group2_pearson_values)
+
         coefficient = calculate_pearson_correlation_coefficient(
             group1_pearson_values, group2_pearson_values
         )
@@ -96,6 +104,8 @@ def test_uncorrelated_issues(self, mock_get_range):
                 candidate_suspect_resolution_id=group2.id,
                 is_correlated=False,
                 coefficient=coefficient,
+                candidate_issue_total_events=group2_total_events,
+                resolved_issue_total_events=group1_total_events,
             )
         ]
         assert result.issue_resolved_time == group1.resolved_at
@@ -113,6 +123,9 @@ def test_perfect_correlation(self, mock_get_range):
         group1_pearson_values = [events for _, events in group1_events]
         group2_pearson_values = [events for _, events in group1_events]
 
+        group1_total_events = sum(group1_pearson_values)
+        group2_total_events = sum(group2_pearson_values)
+
         coefficient = calculate_pearson_correlation_coefficient(
             group1_pearson_values, group2_pearson_values
         )
@@ -124,6 +137,8 @@ def test_perfect_correlation(self, mock_get_range):
                 candidate_suspect_resolution_id=group2.id,
                 is_correlated=True,
                 coefficient=coefficient,
+                candidate_issue_total_events=group2_total_events,
+                resolved_issue_total_events=group1_total_events,
             )
         ]
         assert result.issue_resolved_time == group1.resolved_at
@@ -158,6 +173,11 @@ def test_multiple_groups(self, mock_get_range):
         group3_pearson_values = [events for _, events in group3_events]
         group4_pearson_values = [events for _, events in group4_events]
 
+        group1_total_events = sum(group1_pearson_values)
+        group2_total_events = sum(group2_pearson_values)
+        group3_total_events = sum(group3_pearson_values)
+        group4_total_events = sum(group4_pearson_values)
+
         coefficient_group2 = calculate_pearson_correlation_coefficient(
             group1_pearson_values, group2_pearson_values
         )
@@ -175,16 +195,22 @@ def test_multiple_groups(self, mock_get_range):
                 candidate_suspect_resolution_id=group2.id,
                 is_correlated=True,
                 coefficient=coefficient_group2,
+                candidate_issue_total_events=group2_total_events,
+                resolved_issue_total_events=group1_total_events,
             ),
             CandidateMetricCorrResult(
                 candidate_suspect_resolution_id=group3.id,
                 is_correlated=True,
                 coefficient=coefficient_group3,
+                candidate_issue_total_events=group3_total_events,
+                resolved_issue_total_events=group1_total_events,
             ),
             CandidateMetricCorrResult(
                 candidate_suspect_resolution_id=group4.id,
                 is_correlated=True,
                 coefficient=coefficient_group4,
+                candidate_issue_total_events=group4_total_events,
+                resolved_issue_total_events=group1_total_events,
             ),
         ]
         assert result.issue_resolved_time == group1.resolved_at