feat(integrations): Instrument PR issue resolution

src/sentry/integrations/source_code_management/metrics.py

@@ -50,6 +50,8 @@ class SCMIntegrationInteractionType(StrEnum):
 
     # Releases
     COMPARE_COMMITS = "compare_commits"
+    RESOLVE_ISSUE_VIA_COMMIT = "resolve_issue_via_commit"
+    RESOLVE_ISSUE_VIA_PR = "resolve_issue_via_pr"
 
     # Status Checks
     CREATE_STATUS_CHECK = "create_status_check"
@@ -62,6 +64,10 @@ class SCMIntegrationInteractionType(StrEnum):
     GET_REPO_TREE = "get_repo_tree"
 
 
+class SCMResolutionHaltReason(StrEnum):
+    ISSUE_ALREADY_RESOLVED = "issue_already_resolved"
+
+
 @dataclass
 class SCMIntegrationInteractionEvent(IntegrationEventLifecycleMetric):
     """

src/sentry/models/releases/set_commits.py

@@ -3,12 +3,18 @@
 import itertools
 import logging
 import re
+from contextlib import nullcontext
 from typing import Any, TypedDict
 
 from django.db import IntegrityError, router
 
 from sentry.constants import ObjectStatus
 from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
+from sentry.integrations.source_code_management.metrics import (
+    SCMIntegrationInteractionEvent,
+    SCMIntegrationInteractionType,
+    SCMResolutionHaltReason,
+)
 from sentry.issues.action_log import SYSTEM_ACTOR, ActionSource, action_context_scope
 from sentry.locks import locks
 from sentry.models.activity import Activity
@@ -200,11 +206,6 @@ def update_group_resolutions(release, commit_author_by_commit):
         ).values_list("group_id", "linked_id")
     )
 
-    commit_group_authors = [
-        (cr[0], commit_author_by_commit.get(cr[1]))
-        for cr in commit_resolutions  # group_id
-    ]
-
     # repository_id is fetched here so we don't need a second PullRequest query
     # below to resolve providers (pull_request_resolutions is always a subset).
     pr_repo_ids = dict(
@@ -232,20 +233,20 @@ def update_group_resolutions(release, commit_author_by_commit):
 
     pr_authors_dict = {pra.id: pra.author for pra in pr_authors}
 
-    pull_request_group_authors = [
-        (prr[0], pr_authors_dict.get(prr[1])) for prr in pull_request_resolutions
-    ]
-
     # Map each resolved group to the (normalized) provider of the commit/PR that
     # resolved it, so the issue_resolved analytics event records e.g. "github".
     # commit -> repository_id comes for free from the select_related above;
     # pr -> repository_id was already collected in pr_repo_ids above.
     commit_repo_ids = {rc["commit_id"]: rc["commit__repository_id"] for rc in release_commits}
-    repo_providers = dict(
-        Repository.objects.filter(
+    repo_details = {
+        repo_id: (provider, integration_id)
+        for repo_id, provider, integration_id in Repository.objects.filter(
             id__in=set(commit_repo_ids.values()) | set(pr_repo_ids.values())
-        ).values_list("id", "provider")
-    )
+        ).values_list("id", "provider", "integration_id")
+    }
+    repo_providers = {
+        repo_id: provider for repo_id, (provider, _integration_id) in repo_details.items()
+    }
     # Iterate PRs first, commits second, so a commit-derived provider wins when a
     # group is resolved by both. The repo_id guards prevent a repo-less link from
     # overwriting a valid provider with None.
@@ -276,59 +277,106 @@ def update_group_resolutions(release, commit_author_by_commit):
 
     user_by_author: dict[CommitAuthor | None, RpcUser | None] = {None: None}
 
-    commits_and_prs = list(itertools.chain(commit_group_authors, pull_request_group_authors))
-    group_project_lookup = dict(
-        Group.objects.filter(id__in=[group_id for group_id, _ in commits_and_prs]).values_list(
-            "id", "project_id"
+    resolution_attempts = list(
+        itertools.chain(
+            (
+                (
+                    group_id,
+                    commit_author_by_commit.get(commit_id),
+                    SCMIntegrationInteractionType.RESOLVE_ISSUE_VIA_COMMIT,
+                    commit_repo_ids.get(commit_id),
+                )
+                for group_id, commit_id in commit_resolutions
+            ),
+            (
+                (
+                    group_id,
+                    pr_authors_dict.get(pr_id),
+                    SCMIntegrationInteractionType.RESOLVE_ISSUE_VIA_PR,
+                    pr_repo_ids.get(pr_id),
+                )
+                for group_id, pr_id in pull_request_resolutions
+            ),
         )
     )
+    group_project_lookup = dict(
+        Group.objects.filter(
+            id__in=[
+                group_id for group_id, _author, _interaction_type, _repo_id in resolution_attempts
+            ]
+        ).values_list("id", "project_id")
+    )
 
-    for group_id, author in commits_and_prs:
+    for group_id, author, interaction_type, repo_id in resolution_attempts:
         if author is not None and author not in user_by_author:
             try:
                 user_by_author[author] = author.find_users()[0]
             except IndexError:
                 user_by_author[author] = None
         actor = user_by_author[author]
 
-        with atomic_transaction(
-            using=(
-                router.db_for_write(GroupResolution),
-                router.db_for_write(Group),
-                # inside the remove_group_from_inbox
-                router.db_for_write(GroupInbox),
-                router.db_for_write(Activity),
-            )
-        ):
-            resolution, _ = GroupResolution.objects.update_or_create(
-                group_id=group_id,
-                defaults={
-                    "release": release,
-                    "type": GroupResolution.Type.in_release,
-                    "status": GroupResolution.Status.resolved,
-                    "actor_id": actor.id if actor is not None else None,
-                },
+        provider, integration_id = (
+            repo_details.get(repo_id, (None, None)) if repo_id is not None else (None, None)
+        )
+        normalized_provider = normalize_provider_key(provider)
+        event = (
+            SCMIntegrationInteractionEvent(
+                interaction_type=interaction_type,
+                provider_key=normalized_provider,
+                organization_id=release.organization_id,
+                integration_id=integration_id,
             )
-            group = Group.objects.get(id=group_id)
-            should_create_resolution_activity = group.status != GroupStatus.RESOLVED
-            group.update(status=GroupStatus.RESOLVED, substatus=None)
-            remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED, user=actor)
-            if should_create_resolution_activity:
-                activity_data: dict[str, Any] = {"version": release.version}
-                resolution_commit_id = commit_id_by_group.get(group_id)
-                if resolution_commit_id is not None:
-                    activity_data["commit"] = resolution_commit_id
-                Activity.objects.create(
-                    project_id=group.project_id,
-                    group=group,
-                    type=ActivityType.SET_RESOLVED_IN_RELEASE.value,
-                    user_id=actor.id if actor is not None else None,
-                    ident=resolution.id,
-                    data=activity_data,
+            if interaction_type is not None and normalized_provider is not None
+            else None
+        )
+        lifecycle_context = (
+            event.capture(assume_success=False) if event is not None else nullcontext()
+        )
+        with lifecycle_context as lifecycle:
+            with atomic_transaction(
+                using=(
+                    router.db_for_write(GroupResolution),
+                    router.db_for_write(Group),
+                    # inside the remove_group_from_inbox
+                    router.db_for_write(GroupInbox),
+                    router.db_for_write(Activity),
                 )
-            record_group_history(group, GroupHistoryStatus.RESOLVED, actor=actor)
-
-            metrics.incr("group.resolved", instance="in_commit", skip_internal=True)
+            ):
+                resolution, _ = GroupResolution.objects.update_or_create(
+                    group_id=group_id,
+                    defaults={
+                        "release": release,
+                        "type": GroupResolution.Type.in_release,
+                        "status": GroupResolution.Status.resolved,
+                        "actor_id": actor.id if actor is not None else None,
+                    },
+                )
+                group = Group.objects.get(id=group_id)
+                should_create_resolution_activity = group.status != GroupStatus.RESOLVED
+                group.update(status=GroupStatus.RESOLVED, substatus=None)
+                remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED, user=actor)
+                if should_create_resolution_activity:
+                    activity_data: dict[str, Any] = {"version": release.version}
+                    resolution_commit_id = commit_id_by_group.get(group_id)
+                    if resolution_commit_id is not None:
+                        activity_data["commit"] = resolution_commit_id
+                    Activity.objects.create(
+                        project_id=group.project_id,
+                        group=group,
+                        type=ActivityType.SET_RESOLVED_IN_RELEASE.value,
+                        user_id=actor.id if actor is not None else None,
+                        ident=resolution.id,
+                        data=activity_data,
+                    )
+                record_group_history(group, GroupHistoryStatus.RESOLVED, actor=actor)
+
+                metrics.incr("group.resolved", instance="in_commit", skip_internal=True)
+
+            if lifecycle is not None:
+                if should_create_resolution_activity:
+                    lifecycle.record_success()
+                else:
+                    lifecycle.record_halt(SCMResolutionHaltReason.ISSUE_ALREADY_RESOLVED)
 
         issue_resolved.send_robust(
             organization_id=release.organization_id,

tests/sentry/models/test_release.py

@@ -1,5 +1,5 @@
 from datetime import timedelta
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 from django.core.exceptions import ValidationError
@@ -10,6 +10,7 @@
 from sentry.api.release_search import INVALID_SEMVER_MESSAGE
 from sentry.exceptions import InvalidSearchQuery
 from sentry.integrations.models.external_issue import ExternalIssue
+from sentry.integrations.source_code_management.metrics import SCMIntegrationInteractionType
 from sentry.issues.action_log import SYSTEM_ACTOR, ActionSource
 from sentry.issues.action_log.types import SetResolvedInReleaseAction
 from sentry.models.commit import Commit
@@ -695,15 +696,21 @@ def test_commit_resolves_only_when_released_to_issue_project(self) -> None:
         assert resolution.status == GroupResolution.Status.resolved
         assert Group.objects.get(id=frontend_group.id).status == GroupStatus.RESOLVED
 
+    @patch("sentry.models.releases.set_commits.SCMIntegrationInteractionEvent")
     @patch("sentry.analytics.record")
     @receivers_raise_on_send()
-    def test_resolution_records_commit_provider(self, mock_record: MagicMock) -> None:
+    def test_resolution_records_commit_provider(
+        self, mock_record: MagicMock, mock_lifecycle_event: MagicMock
+    ) -> None:
         org = self.create_organization(owner=Factories.create_user())
         project = self.create_project(organization=org, name="foo")
         group = self.create_group(project=project)
         add_group_to_inbox(group, GroupInboxReason.MANUAL)
         repo = Repository.objects.create(
-            organization_id=org.id, name="test/repo", provider="integrations:github"
+            organization_id=org.id,
+            name="test/repo",
+            provider="integrations:github",
+            integration_id=123,
         )
         commit = Commit.objects.create(
             organization_id=org.id,
@@ -731,6 +738,19 @@ def test_resolution_records_commit_provider(self, mock_record: MagicMock) -> Non
                 issue_category=group.issue_category.name.lower(),
             ),
         )
+        mock_lifecycle_event.assert_called_once_with(
+            interaction_type=SCMIntegrationInteractionType.RESOLVE_ISSUE_VIA_COMMIT,
+            provider_key="github",
+            organization_id=org.id,
+            integration_id=repo.integration_id,
+        )
+        mock_lifecycle_event.return_value.capture.assert_called_once_with(assume_success=False)
+        capture_context = mock_lifecycle_event.return_value.capture.return_value
+        lifecycle = capture_context.__enter__.return_value
+        lifecycle.record_success.assert_called_once_with()
+        assert capture_context.mock_calls.index(
+            call.__enter__().record_success()
+        ) < capture_context.mock_calls.index(call.__exit__(None, None, None))
 
     @patch("sentry.analytics.record")
     @receivers_raise_on_send()
@@ -785,17 +805,23 @@ def test_resolution_attributes_to_system(self) -> None:
             actor=SYSTEM_ACTOR,
         )
 
+    @patch("sentry.models.releases.set_commits.SCMIntegrationInteractionEvent")
     @patch("sentry.analytics.record")
     @receivers_raise_on_send()
-    def test_resolution_via_pull_request_records_merge_commit(self, mock_record: MagicMock) -> None:
+    def test_resolution_via_pull_request_records_merge_commit(
+        self, mock_record: MagicMock, mock_lifecycle_event: MagicMock
+    ) -> None:
         """A group resolved by a pull request records the PR's merge commit id on
         the IssueResolvedEvent, resolved through the release's matching commit."""
         org = self.create_organization(owner=Factories.create_user())
         project = self.create_project(organization=org, name="foo")
         group = self.create_group(project=project)
         add_group_to_inbox(group, GroupInboxReason.MANUAL)
         repo = Repository.objects.create(
-            organization_id=org.id, name="test/repo", provider="integrations:github"
+            organization_id=org.id,
+            name="test/repo",
+            provider="integrations:github",
+            integration_id=123,
         )
         commit = Commit.objects.create(organization_id=org.id, repository_id=repo.id, key="b" * 40)
         pull_request = self.create_pull_request(repository_id=repo.id, organization_id=org.id)
@@ -827,6 +853,15 @@ def test_resolution_via_pull_request_records_merge_commit(self, mock_record: Mag
                 issue_category=group.issue_category.name.lower(),
             ),
         )
+        mock_lifecycle_event.assert_called_once_with(
+            interaction_type=SCMIntegrationInteractionType.RESOLVE_ISSUE_VIA_PR,
+            provider_key="github",
+            organization_id=org.id,
+            integration_id=repo.integration_id,
+        )
+        mock_lifecycle_event.return_value.capture.assert_called_once_with(assume_success=False)
+        lifecycle = mock_lifecycle_event.return_value.capture.return_value.__enter__.return_value
+        lifecycle.record_success.assert_called_once_with()
 
     @patch("sentry.integrations.example.integration.ExampleIntegration.sync_status_outbound")
     @receivers_raise_on_send()