Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)
from sentry.models.organization import Organization
from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval
from sentry.preprod.vcs.pr_comments.snapshot_tasks import create_preprod_snapshot_pr_comment_task
from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
create_preprod_snapshot_status_check_task,
Expand Down Expand Up @@ -91,4 +92,12 @@ def post(self, request: Request, organization: Organization, artifact_id: str) -
caller="approval_endpoint",
)

if feature_type == PreprodComparisonApproval.FeatureType.SNAPSHOTS:
create_preprod_snapshot_pr_comment_task.apply_async(
kwargs={
"preprod_artifact_id": artifact.id,
"caller": "approval_endpoint",
},
)

return Response({"detail": "Approved"}, status=201)
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from sentry.preprod.analytics import PreprodArtifactApiRerunStatusChecksEvent
from sentry.preprod.api.bases.preprod_artifact_endpoint import PreprodArtifactEndpoint
from sentry.preprod.models import PreprodArtifact
from sentry.preprod.vcs.pr_comments.snapshot_tasks import create_preprod_snapshot_pr_comment_task
from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
create_preprod_snapshot_status_check_task,
Expand Down Expand Up @@ -100,6 +101,9 @@ def post(
create_preprod_snapshot_status_check_task.delay(
preprod_artifact_id=head_artifact.id, caller="rerun_endpoint"
)
create_preprod_snapshot_pr_comment_task.delay(
preprod_artifact_id=head_artifact.id, caller="rerun_endpoint"
)
case _:
continue
except Exception:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
find_head_snapshot_artifacts_awaiting_base,
)
from sentry.preprod.url_utils import get_preprod_artifact_url
from sentry.preprod.vcs.pr_comments.snapshot_tasks import create_preprod_snapshot_pr_comment_task
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
create_preprod_snapshot_status_check_task,
)
Expand Down Expand Up @@ -579,6 +580,12 @@ def post(self, request: Request, project: Project) -> Response:
"caller": "upload_completion",
},
)
create_preprod_snapshot_pr_comment_task.apply_async(
kwargs={
"preprod_artifact_id": artifact.id,
"caller": "upload_completion",
},
)

if base_sha and base_repo_name:
try:
Expand Down
13 changes: 13 additions & 0 deletions src/sentry/preprod/snapshots/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
SnapshotManifest,
)
from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
from sentry.preprod.vcs.pr_comments.snapshot_tasks import create_preprod_snapshot_pr_comment_task
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
create_preprod_snapshot_status_check_task,
)
Expand Down Expand Up @@ -717,6 +718,12 @@ def _fetch_hash(h: str) -> None:
"caller": "compare_completion",
},
)
create_preprod_snapshot_pr_comment_task.apply_async(
kwargs={
"preprod_artifact_id": head_artifact_id,
"caller": "compare_completion",
},
)

logger.info(
"Snapshot comparison complete",
Expand Down Expand Up @@ -759,4 +766,10 @@ def _fetch_hash(h: str) -> None:
"caller": "compare_failure",
},
)
create_preprod_snapshot_pr_comment_task.apply_async(
kwargs={
"preprod_artifact_id": head_artifact_id,
"caller": "compare_failure",
},
)
raise
256 changes: 256 additions & 0 deletions src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
from __future__ import annotations

import logging
from collections.abc import Sequence
from typing import Any

from django.db import router, transaction
from taskbroker_client.retry import Retry

from sentry import features
from sentry.models.commitcomparison import CommitComparison
from sentry.preprod.integration_utils import get_commit_context_client
from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval
from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
from sentry.preprod.vcs.pr_comments.snapshot_templates import format_snapshot_pr_comment
from sentry.preprod.vcs.pr_comments.tasks import _get_error_type
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
FAIL_ON_ADDED_OPTION_KEY,
FAIL_ON_REMOVED_OPTION_KEY,
_build_changes_map,
)
Comment thread
cursor[bot] marked this conversation as resolved.
from sentry.shared_integrations.exceptions import ApiError
from sentry.silo.base import SiloMode
from sentry.tasks.base import instrumented_task
from sentry.taskworker.namespaces import preprod_tasks

logger = logging.getLogger(__name__)

ENABLED_OPTION_KEY = "sentry:preprod_snapshot_pr_comments_enabled"
FEATURE_FLAG = "organizations:preprod-snapshot-pr-comments"


@instrumented_task(
name="sentry.preprod.tasks.create_preprod_snapshot_pr_comment",
namespace=preprod_tasks,
processing_deadline_duration=30,
silo_mode=SiloMode.CELL,
retry=Retry(times=5, delay=60 * 5),
)
def create_preprod_snapshot_pr_comment_task(
preprod_artifact_id: int, caller: str | None = None, **kwargs: Any
) -> None:
try:
artifact = PreprodArtifact.objects.select_related(
"mobile_app_info",
"commit_comparison",
"project",
"project__organization",
).get(id=preprod_artifact_id)
except PreprodArtifact.DoesNotExist:
logger.exception(
"preprod.snapshot_pr_comments.create.artifact_not_found",
extra={"artifact_id": preprod_artifact_id, "caller": caller},
)
return

if not artifact.commit_comparison:
logger.info(
"preprod.snapshot_pr_comments.create.no_commit_comparison",
extra={"artifact_id": artifact.id},
)
return

commit_comparison = artifact.commit_comparison
if (
not commit_comparison.pr_number
or not commit_comparison.head_repo_name
or not commit_comparison.provider
):
logger.info(
"preprod.snapshot_pr_comments.create.no_pr_info",
extra={
"artifact_id": artifact.id,
"pr_number": commit_comparison.pr_number,
"head_repo_name": commit_comparison.head_repo_name,
},
)
return

if not artifact.project.get_option(ENABLED_OPTION_KEY, default=True):
logger.info(
"preprod.snapshot_pr_comments.create.project_disabled",
extra={"artifact_id": artifact.id, "project_id": artifact.project.id},
)
return

organization = artifact.project.organization
if not features.has(FEATURE_FLAG, organization):
logger.info(
"preprod.snapshot_pr_comments.create.feature_disabled",
extra={"artifact_id": artifact.id, "organization_id": organization.id},
)
return

client = get_commit_context_client(
organization, commit_comparison.head_repo_name, commit_comparison.provider
)
if not client:
logger.info(
"preprod.snapshot_pr_comments.create.no_client",
extra={"artifact_id": artifact.id},
)
return

api_error: Exception | None = None

with transaction.atomic(router.db_for_write(CommitComparison)):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we lock here on the commit comparison. this is the same as what we do for build distirbution comments.

all_for_pr = list(
CommitComparison.objects.select_for_update()
.filter(
organization_id=commit_comparison.organization_id,
head_repo_name=commit_comparison.head_repo_name,
pr_number=commit_comparison.pr_number,
)
.order_by("id")
)

try:
cc = next(c for c in all_for_pr if c.id == commit_comparison.id)
except StopIteration:
raise CommitComparison.DoesNotExist(
f"CommitComparison {commit_comparison.id} was deleted before lock acquisition"
)

# Gather snapshot data
all_artifacts = list(artifact.get_sibling_artifacts_for_commit())

artifact_ids = [a.id for a in all_artifacts]
snapshot_metrics_qs = PreprodSnapshotMetrics.objects.filter(
preprod_artifact_id__in=artifact_ids,
)
snapshot_metrics_map: dict[int, PreprodSnapshotMetrics] = {
m.preprod_artifact_id: m for m in snapshot_metrics_qs
}

all_artifacts = [a for a in all_artifacts if a.id in snapshot_metrics_map]
if not all_artifacts:
return

metrics_ids = [m.id for m in snapshot_metrics_map.values()]
comparisons_qs = PreprodSnapshotComparison.objects.filter(
head_snapshot_metrics_id__in=metrics_ids,
)
comparisons_map: dict[int, PreprodSnapshotComparison] = {
c.head_snapshot_metrics_id: c for c in comparisons_qs
}

approvals_map: dict[int, PreprodComparisonApproval] = {}
approval_qs = PreprodComparisonApproval.objects.filter(
preprod_artifact_id__in=artifact_ids,
preprod_feature_type=PreprodComparisonApproval.FeatureType.SNAPSHOTS,
approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED,
)
for approval in approval_qs:
approvals_map[approval.preprod_artifact_id] = approval

base_artifact_map = PreprodArtifact.get_base_artifacts_for_commit(all_artifacts)

fail_on_added = artifact.project.get_option(FAIL_ON_ADDED_OPTION_KEY, default=False)
fail_on_removed = artifact.project.get_option(FAIL_ON_REMOVED_OPTION_KEY, default=True)
changes_map = _build_changes_map(
all_artifacts,
snapshot_metrics_map,
comparisons_map,
fail_on_added=fail_on_added,
fail_on_removed=fail_on_removed,
)

comment_body = format_snapshot_pr_comment(
all_artifacts,
snapshot_metrics_map,
comparisons_map,
base_artifact_map,
changes_map,
approvals_map=approvals_map,
)

existing_comment_id = _find_existing_comment_id(all_for_pr)

try:
if existing_comment_id:
client.update_comment(
repo=cc.head_repo_name,
issue_id=str(cc.pr_number),
comment_id=str(existing_comment_id),
data={"body": comment_body},
)
comment_id = existing_comment_id
logger.info(
"preprod.snapshot_pr_comments.create.updated",
extra={"artifact_id": artifact.id, "comment_id": comment_id},
)
else:
resp = client.create_comment(
repo=cc.head_repo_name,
issue_id=str(cc.pr_number),
data={"body": comment_body},
)
comment_id = str(resp["id"])
logger.info(
"preprod.snapshot_pr_comments.create.created",
extra={"artifact_id": artifact.id, "comment_id": comment_id},
)
except Exception as e:
extra: dict[str, Any] = {
"artifact_id": artifact.id,
"organization_id": organization.id,
"error_type": type(e).__name__,
}
if isinstance(e, ApiError):
extra["status_code"] = e.code
logger.exception("preprod.snapshot_pr_comments.create.failed", extra=extra)
_save_pr_comment_result(cc, success=False, error=e)
api_error = e
else:
_save_pr_comment_result(cc, success=True, comment_id=comment_id)

if api_error is not None:
raise api_error


def _find_existing_comment_id(
comparisons: Sequence[CommitComparison],
) -> str | None:
for cc in comparisons:
extras = cc.extras or {}
comment_id = extras.get("pr_comments", {}).get("snapshots", {}).get("comment_id")
if comment_id:
return str(comment_id)
return None


def _save_pr_comment_result(
commit_comparison: CommitComparison,
success: bool,
comment_id: str | None = None,
error: Exception | None = None,
) -> None:
extras = commit_comparison.extras or {}

# Preserve the existing comment_id on failure so retries use
# update_comment instead of creating a duplicate.
if not comment_id:
existing = extras.get("pr_comments", {}).get("snapshots", {})
comment_id = existing.get("comment_id")

result: dict[str, Any] = {"success": success}
if comment_id:
result["comment_id"] = comment_id
if not success:
result["error_type"] = _get_error_type(error)

pr_comments = extras.setdefault("pr_comments", {})
pr_comments["snapshots"] = result
commit_comparison.extras = extras
commit_comparison.save(update_fields=["extras"])
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
7 changes: 7 additions & 0 deletions src/sentry/preprod/vcs/webhooks/github_check_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from sentry.models.repository import Repository
from sentry.preprod.analytics import PreprodStatusCheckApprovalCreatedEvent
from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval
from sentry.preprod.vcs.pr_comments.snapshot_tasks import create_preprod_snapshot_pr_comment_task
from sentry.preprod.vcs.status_checks.size.tasks import (
APPROVE_SIZE_ACTION_IDENTIFIER,
create_preprod_status_check_task,
Expand Down Expand Up @@ -224,5 +225,11 @@ def handle_preprod_check_run_event(
preprod_artifact_id=artifact.id,
caller="github_approve_webhook",
)
create_preprod_snapshot_pr_comment_task.apply_async(
kwargs={
"preprod_artifact_id": artifact.id,
"caller": "github_approve_webhook",
},
)
else:
raise ValueError(f"Unknown identifier: {identifier}")
Loading
Loading