Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions src/sentry/api/endpoints/event_file_committers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.bases.project import ProjectEndpoint
from sentry.models.commit import Commit
from sentry.models.group import Group
from sentry.models.release import Release
from sentry.services import eventstore
from sentry.utils.committers import get_serialized_event_file_committers

Expand Down Expand Up @@ -39,18 +36,12 @@ def get(self, request: Request, project, event_id) -> Response:
elif event.group_id is None:
raise NotFound(detail="Issue not found")

try:
committers = get_serialized_event_file_committers(
project, event, frame_limit=int(request.GET.get("frameLimit", 25))
)
committers = get_serialized_event_file_committers(
project, event, frame_limit=int(request.GET.get("frameLimit", 25))
)

# TODO(nisanthan): Remove the Group.DoesNotExist and Release.DoesNotExist once Commit Context goes GA
except Group.DoesNotExist:
raise NotFound(detail="Issue not found")
except Release.DoesNotExist:
raise NotFound(detail="Release not found")
except Commit.DoesNotExist:
raise NotFound(detail="No Commits found for Release")
if not committers:
raise NotFound(detail="No committers found")

data = {
"committers": committers,
Expand Down
61 changes: 12 additions & 49 deletions src/sentry/utils/committers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@
from sentry.models.commit import Commit
from sentry.models.commitfilechange import CommitFileChange
from sentry.models.group import Group
from sentry.models.groupowner import GroupOwner, GroupOwnerType
from sentry.models.groupowner import GroupOwner, GroupOwnerType, SuspectCommitStrategy
from sentry.models.project import Project
from sentry.models.release import Release
from sentry.models.releasecommit import ReleaseCommit
from sentry.services.eventstore.models import Event, GroupEvent
from sentry.users.services.user.service import user_service
from sentry.utils import metrics
from sentry.utils.event_frames import find_stack_frames, get_sdk_name, munged_filename_and_frames
from sentry.utils.event_frames import find_stack_frames, munged_filename_and_frames
from sentry.utils.hashlib import hash_values

PATH_SEPARATORS = frozenset(["/", "\\"])
Expand Down Expand Up @@ -188,6 +187,14 @@ def _get_serialized_committers_from_group_owners(
if serialized_owners:
author = serialized_owners[0]

# Map the suspect commit strategy to the appropriate type
strategy = owner.context.get("suspectCommitStrategy")
if strategy == SuspectCommitStrategy.RELEASE_BASED:
suspect_commit_type = SuspectCommitType.RELEASE_COMMIT.value
else:
# Default to SCM integration for SCM_BASED or any other/unknown strategy
suspect_commit_type = SuspectCommitType.INTEGRATION_COMMIT.value

return [
{
"author": author,
Expand All @@ -196,7 +203,7 @@ def _get_serialized_committers_from_group_owners(
commit,
serializer=CommitSerializer(
exclude=["author"],
type=SuspectCommitType.INTEGRATION_COMMIT.value,
type=suspect_commit_type,
),
)
],
Expand Down Expand Up @@ -367,52 +374,8 @@ def get_serialized_event_file_committers(
result = _get_serialized_committers_from_group_owners(project, event.group_id)
if result is not None:
return result

# TODO(nisanthan): We create GroupOwner records for
# legacy Suspect Commits in process_suspect_commits task.
# We should refactor to query GroupOwner rather than recalculate.
# But we need to store the commitId and a way to differentiate
# if the Suspect Commit came from ReleaseCommits or CommitContext.
else:
event_frames = get_frame_paths(event)
sdk_name = get_sdk_name(event.data)
committers = get_event_file_committers(
project,
event.group_id,
event_frames,
event.platform,
frame_limit=frame_limit,
sdk_name=sdk_name,
)
commits = [commit for committer in committers for commit in committer["commits"]]
serialized_commits: Sequence[MutableMapping[str, Any]] = serialize(
[c for (c, score) in commits],
serializer=CommitSerializer(
exclude=["author"],
type=SuspectCommitType.RELEASE_COMMIT.value,
),
)

serialized_commits_by_id = {}

for (commit, score), serialized_commit in zip(commits, serialized_commits):
serialized_commit["score"] = score
serialized_commits_by_id[commit.id] = serialized_commit

serialized_committers: list[AuthorCommitsSerialized] = []
for committer in committers:
commit_ids = [commit.id for (commit, _) in committer["commits"]]
commits_result = [serialized_commits_by_id[commit_id] for commit_id in commit_ids]
serialized_committers.append(
{"author": committer["author"], "commits": dedupe_commits(commits_result)}
)

metrics.incr(
"feature.owners.has-committers",
instance="hit" if committers else "miss",
skip_internal=False,
)
return serialized_committers
return []


def dedupe_commits(
Expand Down
77 changes: 24 additions & 53 deletions tests/sentry/api/endpoints/test_event_committers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.urls import reverse

from sentry.models.groupowner import GroupOwner, GroupOwnerType
from sentry.models.groupowner import GroupOwner, GroupOwnerType, SuspectCommitStrategy
from sentry.models.pullrequest import PullRequest
from sentry.models.repository import Repository
from sentry.testutils.cases import APITestCase
Expand All @@ -19,18 +19,31 @@ def test_simple(self) -> None:

project = self.create_project()

release = self.create_release(project, self.user)
min_ago = before_now(minutes=1).isoformat()
event = self.store_event(
data={
"fingerprint": ["group1"],
"timestamp": min_ago,
"release": release.version,
},
project_id=project.id,
default_event_type=EventType.DEFAULT,
)

# Create a commit and GroupOwner to simulate SCM-based suspect commit detection
repo = self.create_repo(project=project, name="example/repo")
commit = self.create_commit(project=project, repo=repo)
GroupOwner.objects.create(
group_id=event.group.id,
project=project,
organization_id=project.organization_id,
type=GroupOwnerType.SUSPECT_COMMIT.value,
user_id=self.user.id,
context={
"commitId": commit.id,
"suspectCommitStrategy": SuspectCommitStrategy.RELEASE_BASED,
},
)

url = reverse(
"sentry-api-0-event-file-committers",
kwargs={
Expand All @@ -44,10 +57,10 @@ def test_simple(self) -> None:
assert response.status_code == 200, response.content
assert len(response.data["committers"]) == 1
assert response.data["committers"][0]["author"]["username"] == "admin@localhost"
assert len(response.data["committers"][0]["commits"]) == 1
assert (
response.data["committers"][0]["commits"][0]["message"] == "placeholder commit message"
)
commits = response.data["committers"][0]["commits"]
assert len(commits) == 1
assert commits[0]["message"] == commit.message
assert commits[0]["suspectCommitType"] == "via commit in release"

def test_no_group(self) -> None:
self.login_as(user=self.user)
Expand All @@ -74,7 +87,8 @@ def test_no_group(self) -> None:
assert response.status_code == 404, response.content
assert response.data["detail"] == "Issue not found"

def test_no_release(self) -> None:
def test_no_committers(self) -> None:
"""Test that events without GroupOwners return 404"""
self.login_as(user=self.user)

project = self.create_project()
Expand All @@ -95,51 +109,9 @@ def test_no_release(self) -> None:

response = self.client.get(url, format="json")
assert response.status_code == 404, response.content
assert response.data["detail"] == "Release not found"

def test_null_stacktrace(self) -> None:
self.login_as(user=self.user)

project = self.create_project()

release = self.create_release(project, self.user)

min_ago = before_now(minutes=1).isoformat()
event = self.store_event(
data={
"fingerprint": ["group1"],
"environment": "production",
"type": "default",
"exception": {
"values": [
{
"type": "ValueError",
"value": "My exception value",
"module": "__builtins__",
"stacktrace": None,
}
]
},
"tags": [["environment", "production"], ["sentry:release", release.version]],
"release": release.version,
"timestamp": min_ago,
},
project_id=project.id,
)

url = reverse(
"sentry-api-0-event-file-committers",
kwargs={
"event_id": event.event_id,
"project_id_or_slug": event.project.slug,
"organization_id_or_slug": event.project.organization.slug,
},
)
assert response.data["detail"] == "No committers found"

response = self.client.get(url, format="json")
assert response.status_code == 200, response.content

def test_with_commit_context(self) -> None:
def test_with_committers(self) -> None:
self.login_as(user=self.user)
self.repo = Repository.objects.create(
organization_id=self.organization.id,
Expand Down Expand Up @@ -242,7 +214,6 @@ def test_with_commit_context_pull_request(self) -> None:

response = self.client.get(url, format="json")
assert response.status_code == 200, response.content

commits = response.data["committers"][0]["commits"]
assert len(commits) == 1
assert "pullRequest" in commits[0]
Expand Down
Loading
Loading