diff --git a/src/sentry/integrations/github/integration.py b/src/sentry/integrations/github/integration.py index 41d8cabbe74d5e..ba3a67706bf63f 100644 --- a/src/sentry/integrations/github/integration.py +++ b/src/sentry/integrations/github/integration.py @@ -2,12 +2,13 @@ import logging import re -from datetime import datetime +from datetime import timezone from typing import Any, Collection, Dict, Mapping, Sequence from django.http import HttpResponse from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ +from isodate import parse_datetime from rest_framework.request import Request from sentry import features, options @@ -249,9 +250,7 @@ def get_commit_context( for blame in blame_range if blame.get("startingLine", 0) <= lineno <= blame.get("endingLine", 0) ), - key=lambda blame: datetime.strptime( - blame.get("commit", {}).get("committedDate"), "%Y-%m-%dT%H:%M:%SZ" - ), + key=lambda blame: parse_datetime(blame.get("commit", {}).get("committedDate")), default={}, ) if not commit: @@ -263,9 +262,13 @@ def get_commit_context( if not commitInfo: return None else: + committed_date = parse_datetime(commitInfo.get("committed_date")).astimezone( + timezone.utc + ) + return { "commitId": commitInfo.get("oid"), - "committedDate": commitInfo.get("committedDate"), + "committedDate": committed_date, "commitMessage": commitInfo.get("message"), "commitAuthorName": commitInfo.get("author", {}).get("name"), "commitAuthorEmail": commitInfo.get("author", {}).get("email"), diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index e6d647250dd132..88a3330a1de637 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -1,12 +1,13 @@ from __future__ import annotations -from datetime import datetime, timezone +from datetime import timezone from typing import Any, Mapping, Sequence from urllib.parse import urlparse from django import forms from django.http import HttpResponse from django.utils.translation import gettext_lazy as _ +from isodate import parse_datetime from rest_framework.request import Request from sentry.identity.gitlab import get_oauth_data, get_user_info @@ -166,13 +167,10 @@ def get_commit_context( except ApiError as e: raise e - date_format_expected = "%Y-%m-%dT%H:%M:%S.%f%z" try: commit = max( blame_range, - key=lambda blame: datetime.strptime( - blame.get("commit", {}).get("committed_date"), date_format_expected - ), + key=lambda blame: parse_datetime(blame.get("commit", {}).get("committed_date")), ) except (ValueError, IndexError): return None @@ -181,11 +179,11 @@ def get_commit_context( if not commitInfo: return None else: - committed_date = "{}Z".format( - datetime.strptime(commitInfo.get("committed_date"), date_format_expected) - .astimezone(timezone.utc) - .strftime("%Y-%m-%dT%H:%M:%S.%f")[:-3] + # TODO(nisanthan): Use dateutil.parser.isoparse once on python 3.11 + committed_date = parse_datetime(commitInfo.get("committed_date")).astimezone( + timezone.utc ) + return { "commitId": commitInfo.get("id"), "committedDate": committed_date, diff --git a/src/sentry/integrations/utils/commit_context.py b/src/sentry/integrations/utils/commit_context.py index 0c9da764c7b033..e556447d825d85 100644 --- a/src/sentry/integrations/utils/commit_context.py +++ b/src/sentry/integrations/utils/commit_context.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from datetime import datetime, timedelta, timezone from typing import Any, List, Mapping, Sequence, Tuple import sentry_sdk @@ -108,7 +109,12 @@ def find_commit_context_for_event( }, ) - if commit_context: + # Only return suspect commits that are less than a year old + if commit_context and is_date_less_than_year(commit_context["committedDate"]): result.append((commit_context, code_mapping)) return result, installation + + +def is_date_less_than_year(date: datetime): + return date > datetime.now(tz=timezone.utc) - timedelta(days=365) diff --git a/tests/sentry/integrations/gitlab/test_integration.py b/tests/sentry/integrations/gitlab/test_integration.py index 39ed558f8b8d39..14a0d7fc8136f5 100644 --- a/tests/sentry/integrations/gitlab/test_integration.py +++ b/tests/sentry/integrations/gitlab/test_integration.py @@ -5,6 +5,7 @@ import responses from django.core.cache import cache from django.test import override_settings +from isodate import parse_datetime from fixtures.gitlab import GET_COMMIT_RESPONSE, GitLabTestCase from sentry.integrations.gitlab import GitlabIntegrationProvider @@ -425,7 +426,7 @@ def test_get_commit_context(self): commit_context_expected = { "commitId": "d42409d56517157c48bf3bd97d3f75974dde19fb", - "committedDate": "2015-12-18T08:12:22.000Z", + "committedDate": parse_datetime("2015-12-18T08:12:22.000Z"), "commitMessage": "Add installation instructions", "commitAuthorName": "Nisanthan Nanthakumar", "commitAuthorEmail": "nisanthan.nanthakumar@sentry.io", diff --git a/tests/sentry/tasks/test_commit_context.py b/tests/sentry/tasks/test_commit_context.py index 7cb3e6882988b8..565f63f3a4f09e 100644 --- a/tests/sentry/tasks/test_commit_context.py +++ b/tests/sentry/tasks/test_commit_context.py @@ -1,4 +1,5 @@ -from datetime import timedelta +from datetime import datetime, timedelta +from datetime import timezone as datetime_timezone from unittest.mock import Mock, patch import pytest @@ -83,7 +84,7 @@ class TestCommitContext(TestCommitContextMixin): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfwreqr", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -141,11 +142,47 @@ def test_failed_to_fetch_commit_context_record(self, mock_get_commit_context, mo error_message="integration_failed", ) + @patch("sentry.tasks.commit_context.logger") @patch( "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfasdf", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=370)), + "commitMessage": "placeholder commit message", + "commitAuthorName": "", + "commitAuthorEmail": "admin@localhost", + }, + ) + def test_found_commit_is_too_old(self, mock_get_commit_context, mock_logger): + with self.tasks(): + assert not GroupOwner.objects.filter(group=self.event.group).exists() + event_frames = get_frame_paths(self.event) + process_commit_context( + event_id=self.event.event_id, + event_platform=self.event.platform, + event_frames=event_frames, + group_id=self.event.group_id, + project_id=self.event.project_id, + ) + + assert mock_logger.info.call_count == 1 + mock_logger.info.assert_called_with( + "process_commit_context.find_commit_context", + extra={ + "event": self.event.event_id, + "group": self.event.group_id, + "organization": self.event.group.project.organization_id, + "reason": "could_not_fetch_commit_context", + "code_mappings_count": 1, + "fallback": True, + }, + ) + + @patch( + "sentry.integrations.github.GitHubIntegration.get_commit_context", + return_value={ + "commitId": "asdfasdf", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -170,7 +207,7 @@ def test_no_matching_commit_in_db(self, mock_get_commit_context): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfwreqr", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -255,7 +292,7 @@ def test_no_inapp_frame_in_stacktrace(self, mock_process_suspect_commits): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -296,7 +333,7 @@ def test_commit_author_not_in_sentry(self, mock_get_commit_context): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -337,7 +374,7 @@ def test_commit_author_no_user(self, mock_get_commit_context, mock_get_users_for "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -423,7 +460,7 @@ def after_return(self, status, retval, task_id, args, kwargs, einfo): Mock( return_value={ "commitId": "asdfwreqr", - "committedDate": "2023-02-14T11:11Z", + "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index 24a7811258799f..fc7ce446b46f39 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -1303,7 +1303,7 @@ def test_issue_owners_should_ratelimit(self, logger): class ProcessCommitsTestMixin(BasePostProgressGroupMixin): github_blame_return_value = { "commitId": "asdfwreqr", - "committedDate": "", + "committedDate": (datetime.now(timezone.utc) - timedelta(days=2)), "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost",