Browse Source

feat(commit-context): Add missing commit to sentry_commit (#44677)

This should be safe as far as other Commit-based logic. We rely on the
commit date for [this
query](https://github.com/getsentry/sentry/blob/8167e5a909faa997e14304c5949e267a5d7e2546/src/sentry/api/endpoints/organization_member_unreleased_commits.py#L11-L33),
so we can skip inserting commits that don't have this field.

WOR-2691
Snigdha Sharma 2 years ago
parent
commit
b13e69be0b
2 changed files with 37 additions and 20 deletions
  1. 28 14
      src/sentry/tasks/commit_context.py
  2. 9 6
      tests/sentry/tasks/test_commit_context.py

+ 28 - 14
src/sentry/tasks/commit_context.py

@@ -167,6 +167,7 @@ def process_commit_context(
                 return
 
             commit = None
+            new_commit = None
             for commit_context, selected_code_mapping in found_contexts:
                 try:
                     # Find commit and break
@@ -176,6 +177,13 @@ def process_commit_context(
                     )
                     break
                 except Commit.DoesNotExist:
+                    # If the commit has no date, we will not add it to avoid breaking other commit ordered-based logic.
+                    if not new_commit and commit_context.get("committedDate"):
+                        new_commit = {
+                            "context": commit_context,
+                            "repository_id": selected_code_mapping.repository_id,
+                            "code_mapping_id": selected_code_mapping.id,
+                        }
 
                     logger.info(
                         "process_commit_context.no_commit_in_sentry",
@@ -188,27 +196,33 @@ def process_commit_context(
                         },
                     )
 
-            if not commit:
-                # None of the commits found from the integrations' contexts exist in sentry_commit.
-
-                # We couldn't find the commit in Sentry, so we will debounce the task for 1 day.
-                # TODO(nisanthan): We will not get the commit history for new customers, only the commits going forward from when they installed the source-code integration. We need a long-term fix.
-                cache.set(cache_key, True, timedelta(days=1).total_seconds())
-
-                metrics.incr(
-                    "sentry.tasks.process_commit_context.aborted",
-                    tags={
-                        "detail": "commit_sha_does_not_exist_in_sentry",
-                    },
+            if not commit and new_commit:
+                context = new_commit["context"]
+                # If none of the commits exist in sentry_commit, we add the first commit we found
+                commit_author, _ = CommitAuthor.objects.get_or_create(
+                    organization_id=project.organization_id,
+                    email=context.get("commitAuthorEmail"),
+                    defaults={"name": context.get("commitAuthorName")},
                 )
+                commit = Commit.objects.create(
+                    organization_id=project.organization_id,
+                    repository_id=new_commit["repository_id"],
+                    key=context.get("commitId"),
+                    date_added=context.get("committedDate"),
+                    author=commit_author,
+                    message=context.get("message"),
+                )
+
                 logger.info(
-                    "process_commit_context.aborted.no_commit_in_sentry",
+                    "process_commit_context.added_commit_to_sentry_commit",
                     extra={
                         **basic_logging_details,
+                        "sha": new_commit.get("commitId"),
+                        "repository_id": new_commit["repository_id"],
+                        "code_mapping_id": new_commit["code_mapping_id"],
                         "reason": "commit_sha_does_not_exist_in_sentry_for_all_code_mappings",
                     },
                 )
-                return
 
             authors = list(CommitAuthor.objects.get_many_from_cache([commit.author_id]))
             author_to_user = get_users_for_authors(commit.organization_id, authors)

+ 9 - 6
tests/sentry/tasks/test_commit_context.py

@@ -6,6 +6,7 @@ from celery.exceptions import MaxRetriesExceededError
 from django.utils import timezone
 
 from sentry.models import Repository
+from sentry.models.commit import Commit
 from sentry.models.groupowner import GroupOwner, GroupOwnerType
 from sentry.shared_integrations.exceptions.base import ApiError
 from sentry.tasks.commit_context import process_commit_context
@@ -76,7 +77,7 @@ class TestCommitContext(TestCase):
         "sentry.integrations.github.GitHubIntegration.get_commit_context",
         return_value={
             "commitId": "asdfwreqr",
-            "committedDate": "",
+            "committedDate": "2023-02-14T11:11Z",
             "commitMessage": "placeholder commit message",
             "commitAuthorName": "",
             "commitAuthorEmail": "admin@localhost",
@@ -138,7 +139,7 @@ class TestCommitContext(TestCase):
         "sentry.integrations.github.GitHubIntegration.get_commit_context",
         return_value={
             "commitId": "asdfasdf",
-            "committedDate": "",
+            "committedDate": "2023-02-14T11:11Z",
             "commitMessage": "placeholder commit message",
             "commitAuthorName": "",
             "commitAuthorEmail": "admin@localhost",
@@ -147,6 +148,7 @@ class TestCommitContext(TestCase):
     def test_no_matching_commit_in_db(self, mock_get_commit_context):
         with self.tasks():
             assert not GroupOwner.objects.filter(group=self.event.group).exists()
+            assert not Commit.objects.filter(key="asdfasdf").exists()
             event_frames = get_frame_paths(self.event)
             process_commit_context(
                 event_id=self.event.event_id,
@@ -155,13 +157,14 @@ class TestCommitContext(TestCase):
                 group_id=self.event.group_id,
                 project_id=self.event.project_id,
             )
-        assert not GroupOwner.objects.filter(group=self.event.group).exists()
+        assert Commit.objects.filter(key="asdfasdf").exists()
+        assert GroupOwner.objects.filter(group=self.event.group).exists()
 
     @patch(
         "sentry.integrations.github.GitHubIntegration.get_commit_context",
         return_value={
             "commitId": "asdfwreqr",
-            "committedDate": "",
+            "committedDate": "2023-02-14T11:11Z",
             "commitMessage": "placeholder commit message",
             "commitAuthorName": "",
             "commitAuthorEmail": "admin@localhost",
@@ -244,7 +247,7 @@ class TestCommitContext(TestCase):
         "sentry.integrations.github.GitHubIntegration.get_commit_context",
         return_value={
             "commitId": "somekey",
-            "committedDate": "",
+            "committedDate": "2023-02-14T11:11Z",
             "commitMessage": "placeholder commit message",
             "commitAuthorName": "",
             "commitAuthorEmail": "randomuser@sentry.io",
@@ -284,7 +287,7 @@ class TestCommitContext(TestCase):
         "sentry.integrations.github.GitHubIntegration.get_commit_context",
         return_value={
             "commitId": "somekey",
-            "committedDate": "",
+            "committedDate": "2023-02-14T11:11Z",
             "commitMessage": "placeholder commit message",
             "commitAuthorName": "",
             "commitAuthorEmail": "randomuser@sentry.io",