Skip to content

Commit f421fee

Browse files
authored
feat(suspect-commits): Remove suspect commit recalculation period (when all-frames is enabled) (#58415)
Closes #57438 We are no longer going to run the `process_commit_context` task when new events come in. To increase accuracy, we want to calculate the suspect commit only at the time the issue was created. Otherwise we might select commits that were made _after_ the issue started.
1 parent 24f74ad commit f421fee

File tree

2 files changed

+76
-2
lines changed

2 files changed

+76
-2
lines changed

src/sentry/tasks/post_process.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,14 @@ def process_commits(job: PostProcessJob) -> None:
10261026
features.has("organizations:commit-context", event.project.organization)
10271027
and has_integrations
10281028
):
1029+
if (
1030+
features.has(
1031+
"organizations:suspect-commits-all-frames", event.project.organization
1032+
)
1033+
and not job["group_state"]["is_new"]
1034+
):
1035+
return
1036+
10291037
cache_key = DEBOUNCE_CACHE_KEY(event.group_id)
10301038
if cache.get(cache_key):
10311039
metrics.incr("sentry.tasks.process_commit_context.debounce")

tests/sentry/tasks/test_post_process.py

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from sentry.eventstore.models import Event
2020
from sentry.eventstore.processing import event_processing_store
2121
from sentry.ingest.transaction_clusterer import ClustererNamespace
22+
from sentry.integrations.mixins.commit_context import CommitInfo, FileBlameInfo
2223
from sentry.issues.escalating import manage_issue_states
2324
from sentry.issues.grouptype import PerformanceNPlusOneGroupType, ProfileFileIOGroupType
2425
from sentry.issues.ingest import save_issue_occurrence
@@ -1379,8 +1380,7 @@ def setUp(self):
13791380
)
13801381
self.cache_key = write_event_to_cache(self.created_event)
13811382
self.repo = self.create_repo(
1382-
name="example",
1383-
integration_id=self.integration.id,
1383+
name="example", integration_id=self.integration.id, provider="github"
13841384
)
13851385
self.code_mapping = self.create_code_mapping(
13861386
repo=self.repo, project=self.project, stack_root="src/"
@@ -1394,6 +1394,23 @@ def setUp(self):
13941394
message="placeholder commit message",
13951395
)
13961396

1397+
self.github_blame_all_files_return_value = [
1398+
FileBlameInfo(
1399+
code_mapping=self.code_mapping,
1400+
lineno=39,
1401+
path="sentry/models/release.py",
1402+
ref="master",
1403+
repo=self.repo,
1404+
commit=CommitInfo(
1405+
commitId="asdfwreqr",
1406+
committedDate=(datetime.now(timezone.utc) - timedelta(days=2)),
1407+
commitMessage="placeholder commit message",
1408+
commitAuthorName="",
1409+
commitAuthorEmail="admin@localhost",
1410+
),
1411+
)
1412+
]
1413+
13971414
@with_feature("organizations:commit-context")
13981415
@patch(
13991416
"sentry.integrations.github.GitHubIntegration.get_commit_context",
@@ -1434,6 +1451,55 @@ def test_logic_fallback_no_scm(self, mock_get_commit_context):
14341451
)
14351452
assert not cache.has_key(f"process-commit-context-{self.created_event.group_id}")
14361453

1454+
@with_feature("organizations:commit-context")
1455+
@with_feature("organizations:suspect-commits-all-frames")
1456+
@patch("sentry.integrations.github.GitHubIntegration.get_commit_context_all_frames")
1457+
def test_skip_when_not_is_new(self, mock_get_commit_context):
1458+
"""
1459+
Tests that when the organizations:suspect-commits-all-frames feature is enabled,
1460+
and the group is not new, that we do not process commit context.
1461+
"""
1462+
with self.tasks():
1463+
self.call_post_process_group(
1464+
is_new=False,
1465+
is_regression=False,
1466+
is_new_group_environment=True,
1467+
event=self.created_event,
1468+
)
1469+
assert not mock_get_commit_context.called
1470+
assert not GroupOwner.objects.filter(
1471+
group=self.created_event.group,
1472+
project=self.created_event.project,
1473+
organization=self.created_event.project.organization,
1474+
type=GroupOwnerType.SUSPECT_COMMIT.value,
1475+
).exists()
1476+
1477+
@with_feature("organizations:commit-context")
1478+
@with_feature("organizations:suspect-commits-all-frames")
1479+
@patch(
1480+
"sentry.integrations.github.GitHubIntegration.get_commit_context_all_frames",
1481+
)
1482+
def test_does_not_skip_when_is_new(self, mock_get_commit_context):
1483+
"""
1484+
Tests that when the organizations:suspect-commits-all-frames feature is enabled,
1485+
and the group is new, the commit context should be processed.
1486+
"""
1487+
mock_get_commit_context.return_value = self.github_blame_all_files_return_value
1488+
with self.tasks():
1489+
self.call_post_process_group(
1490+
is_new=True,
1491+
is_regression=False,
1492+
is_new_group_environment=True,
1493+
event=self.created_event,
1494+
)
1495+
assert mock_get_commit_context.called
1496+
assert GroupOwner.objects.get(
1497+
group=self.created_event.group,
1498+
project=self.created_event.project,
1499+
organization=self.created_event.project.organization,
1500+
type=GroupOwnerType.SUSPECT_COMMIT.value,
1501+
)
1502+
14371503

14381504
class SnoozeTestMixin(BasePostProgressGroupMixin):
14391505
@with_feature("organizations:escalating-issues")

0 commit comments

Comments
 (0)