Skip to content

Commit 2c48c43

Browse files
authored
feat(preprod): record if the status check failed (#104039)
Records in the `extras` field whether the status check succeeded or not, which we can later display in the UI.
1 parent f5b3cda commit 2c48c43

File tree

2 files changed

+240
-14
lines changed

2 files changed

+240
-14
lines changed

src/sentry/preprod/vcs/status_checks/size/tasks.py

Lines changed: 74 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
import logging
44
from abc import ABC, abstractmethod
55
from datetime import datetime
6+
from enum import StrEnum
67
from typing import Any
78

9+
from django.db import router, transaction
10+
811
from sentry.constants import ObjectStatus
912
from sentry.integrations.base import IntegrationInstallation
1013
from sentry.integrations.github.status_check import GitHubCheckConclusion, GitHubCheckStatus
@@ -129,19 +132,24 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None:
129132
if GITHUB_STATUS_CHECK_STATUS_MAPPING[status] == GitHubCheckStatus.COMPLETED:
130133
completed_at = preprod_artifact.date_updated
131134

132-
check_id = provider.create_status_check(
133-
repo=commit_comparison.head_repo_name,
134-
sha=commit_comparison.head_sha,
135-
status=status,
136-
title=title,
137-
subtitle=subtitle,
138-
text=None, # TODO(telkins): add text field support
139-
summary=summary,
140-
external_id=str(preprod_artifact.id),
141-
target_url=target_url,
142-
started_at=preprod_artifact.date_added,
143-
completed_at=completed_at,
144-
)
135+
try:
136+
check_id = provider.create_status_check(
137+
repo=commit_comparison.head_repo_name,
138+
sha=commit_comparison.head_sha,
139+
status=status,
140+
title=title,
141+
subtitle=subtitle,
142+
text=None, # TODO(telkins): add text field support
143+
summary=summary,
144+
external_id=str(preprod_artifact.id),
145+
target_url=target_url,
146+
started_at=preprod_artifact.date_added,
147+
completed_at=completed_at,
148+
)
149+
except Exception as e:
150+
_update_posted_status_check(preprod_artifact, check_type="size", success=False, error=e)
151+
raise
152+
145153
if check_id is None:
146154
logger.error(
147155
"preprod.status_checks.create.failed",
@@ -151,8 +159,13 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None:
151159
"organization_slug": preprod_artifact.project.organization.slug,
152160
},
153161
)
162+
_update_posted_status_check(preprod_artifact, check_type="size", success=False)
154163
return
155164

165+
_update_posted_status_check(
166+
preprod_artifact, check_type="size", success=True, check_id=check_id
167+
)
168+
156169
logger.info(
157170
"preprod.status_checks.create.success",
158171
extra={
@@ -165,6 +178,54 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None:
165178
)
166179

167180

181+
def _update_posted_status_check(
182+
preprod_artifact: PreprodArtifact,
183+
check_type: str,
184+
success: bool,
185+
check_id: str | None = None,
186+
error: Exception | None = None,
187+
) -> None:
188+
"""Update the posted_status_checks field in the artifact's extras."""
189+
with transaction.atomic(router.db_for_write(PreprodArtifact)):
190+
artifact = PreprodArtifact.objects.select_for_update().get(id=preprod_artifact.id)
191+
extras = artifact.extras or {}
192+
193+
posted_status_checks = extras.get("posted_status_checks", {})
194+
195+
check_result: dict[str, Any] = {"success": success}
196+
if success and check_id:
197+
check_result["check_id"] = check_id
198+
if not success:
199+
check_result["error_type"] = _get_error_type(error).value
200+
201+
posted_status_checks[check_type] = check_result
202+
extras["posted_status_checks"] = posted_status_checks
203+
artifact.extras = extras
204+
artifact.save(update_fields=["extras"])
205+
206+
207+
def _get_error_type(error: Exception | None) -> StatusCheckErrorType:
208+
"""Determine the error type from an exception."""
209+
if error is None:
210+
return StatusCheckErrorType.UNKNOWN
211+
if isinstance(error, IntegrationConfigurationError):
212+
return StatusCheckErrorType.INTEGRATION_ERROR
213+
if isinstance(error, ApiError):
214+
return StatusCheckErrorType.API_ERROR
215+
return StatusCheckErrorType.UNKNOWN
216+
217+
218+
class StatusCheckErrorType(StrEnum):
219+
"""Error types for status check creation failures."""
220+
221+
UNKNOWN = "unknown"
222+
"""An unknown error occurred (e.g., API returned null check_id)."""
223+
API_ERROR = "api_error"
224+
"""A retryable API error (5xx, rate limit, transient issues)."""
225+
INTEGRATION_ERROR = "integration_error"
226+
"""An integration configuration error (permissions, invalid request, etc.)."""
227+
228+
168229
def _compute_overall_status(
169230
artifacts: list[PreprodArtifact], size_metrics_map: dict[int, list[PreprodArtifactSizeMetrics]]
170231
) -> StatusCheckStatus:

tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py

Lines changed: 166 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111
from sentry.models.commitcomparison import CommitComparison
1212
from sentry.models.repository import Repository
1313
from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics
14-
from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task
14+
from sentry.preprod.vcs.status_checks.size.tasks import (
15+
StatusCheckErrorType,
16+
create_preprod_status_check_task,
17+
)
1518
from sentry.shared_integrations.exceptions import IntegrationConfigurationError
1619
from sentry.testutils.cases import TestCase
1720
from sentry.testutils.silo import region_silo_test
@@ -988,3 +991,165 @@ def test_sibling_deduplication_with_same_app_id_different_platforms(self):
988991
assert (
989992
ios_artifact.id not in sibling_ids_from_ios_new
990993
), "Old iOS artifact should be deduplicated (not the triggering artifact)"
994+
995+
def test_posted_status_check_success(self):
996+
"""Test that successful status check posts are recorded in artifact extras."""
997+
preprod_artifact = self._create_preprod_artifact(
998+
state=PreprodArtifact.ArtifactState.PROCESSED
999+
)
1000+
1001+
PreprodArtifactSizeMetrics.objects.create(
1002+
preprod_artifact=preprod_artifact,
1003+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
1004+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
1005+
min_download_size=1024 * 1024,
1006+
max_download_size=1024 * 1024,
1007+
min_install_size=2 * 1024 * 1024,
1008+
max_install_size=2 * 1024 * 1024,
1009+
)
1010+
1011+
_, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup(
1012+
preprod_artifact
1013+
)
1014+
mock_provider.create_status_check.return_value = "check_12345"
1015+
1016+
with client_patch, provider_patch:
1017+
with self.tasks():
1018+
create_preprod_status_check_task(preprod_artifact.id)
1019+
1020+
preprod_artifact.refresh_from_db()
1021+
assert preprod_artifact.extras is not None
1022+
assert "posted_status_checks" in preprod_artifact.extras
1023+
assert "size" in preprod_artifact.extras["posted_status_checks"]
1024+
assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is True
1025+
assert preprod_artifact.extras["posted_status_checks"]["size"]["check_id"] == "check_12345"
1026+
1027+
def test_posted_status_check_failure_null_check_id(self):
1028+
"""Test that failed status check posts (null check_id) are recorded in artifact extras."""
1029+
preprod_artifact = self._create_preprod_artifact(
1030+
state=PreprodArtifact.ArtifactState.PROCESSED
1031+
)
1032+
1033+
PreprodArtifactSizeMetrics.objects.create(
1034+
preprod_artifact=preprod_artifact,
1035+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
1036+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
1037+
min_download_size=1024 * 1024,
1038+
max_download_size=1024 * 1024,
1039+
min_install_size=2 * 1024 * 1024,
1040+
max_install_size=2 * 1024 * 1024,
1041+
)
1042+
1043+
_, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup(
1044+
preprod_artifact
1045+
)
1046+
mock_provider.create_status_check.return_value = None # Simulate API returning no check_id
1047+
1048+
with client_patch, provider_patch:
1049+
with self.tasks():
1050+
create_preprod_status_check_task(preprod_artifact.id)
1051+
1052+
preprod_artifact.refresh_from_db()
1053+
assert preprod_artifact.extras is not None
1054+
assert "posted_status_checks" in preprod_artifact.extras
1055+
assert "size" in preprod_artifact.extras["posted_status_checks"]
1056+
assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is False
1057+
assert (
1058+
preprod_artifact.extras["posted_status_checks"]["size"]["error_type"]
1059+
== StatusCheckErrorType.UNKNOWN.value
1060+
)
1061+
1062+
@responses.activate
1063+
def test_posted_status_check_failure_integration_error(self):
1064+
"""Test that integration errors during status check creation are recorded in artifact extras."""
1065+
preprod_artifact = self._create_preprod_artifact(
1066+
state=PreprodArtifact.ArtifactState.PROCESSED
1067+
)
1068+
1069+
PreprodArtifactSizeMetrics.objects.create(
1070+
preprod_artifact=preprod_artifact,
1071+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
1072+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
1073+
min_download_size=1024 * 1024,
1074+
max_download_size=1024 * 1024,
1075+
min_install_size=2 * 1024 * 1024,
1076+
max_install_size=2 * 1024 * 1024,
1077+
)
1078+
1079+
integration = self.create_integration(
1080+
organization=self.organization,
1081+
external_id="test-integration-error",
1082+
provider="github",
1083+
metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"},
1084+
)
1085+
1086+
Repository.objects.create(
1087+
organization_id=self.organization.id,
1088+
name="owner/repo",
1089+
provider="integrations:github",
1090+
integration_id=integration.id,
1091+
)
1092+
1093+
responses.add(
1094+
responses.POST,
1095+
"https://api.github.com/repos/owner/repo/check-runs",
1096+
status=403,
1097+
json={
1098+
"message": "Resource not accessible by integration",
1099+
"documentation_url": "https://docs.github.com/rest/checks/runs#create-a-check-run",
1100+
},
1101+
)
1102+
1103+
with self.tasks():
1104+
try:
1105+
create_preprod_status_check_task(preprod_artifact.id)
1106+
except IntegrationConfigurationError:
1107+
pass # Expected
1108+
1109+
preprod_artifact.refresh_from_db()
1110+
assert preprod_artifact.extras is not None
1111+
assert "posted_status_checks" in preprod_artifact.extras
1112+
assert "size" in preprod_artifact.extras["posted_status_checks"]
1113+
assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is False
1114+
assert (
1115+
preprod_artifact.extras["posted_status_checks"]["size"]["error_type"]
1116+
== StatusCheckErrorType.INTEGRATION_ERROR.value
1117+
)
1118+
1119+
def test_posted_status_check_preserves_existing_extras(self):
1120+
"""Test that recording status check result preserves other fields in extras."""
1121+
preprod_artifact = self._create_preprod_artifact(
1122+
state=PreprodArtifact.ArtifactState.PROCESSED
1123+
)
1124+
preprod_artifact.extras = {"existing_field": "existing_value", "another_field": 123}
1125+
preprod_artifact.save()
1126+
1127+
PreprodArtifactSizeMetrics.objects.create(
1128+
preprod_artifact=preprod_artifact,
1129+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
1130+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
1131+
min_download_size=1024 * 1024,
1132+
max_download_size=1024 * 1024,
1133+
min_install_size=2 * 1024 * 1024,
1134+
max_install_size=2 * 1024 * 1024,
1135+
)
1136+
1137+
_, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup(
1138+
preprod_artifact
1139+
)
1140+
mock_provider.create_status_check.return_value = "check_67890"
1141+
1142+
with client_patch, provider_patch:
1143+
with self.tasks():
1144+
create_preprod_status_check_task(preprod_artifact.id)
1145+
1146+
preprod_artifact.refresh_from_db()
1147+
assert preprod_artifact.extras is not None
1148+
# Verify existing fields are preserved
1149+
assert preprod_artifact.extras["existing_field"] == "existing_value"
1150+
assert preprod_artifact.extras["another_field"] == 123
1151+
# Verify new field is added
1152+
assert "posted_status_checks" in preprod_artifact.extras
1153+
assert "size" in preprod_artifact.extras["posted_status_checks"]
1154+
assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is True
1155+
assert preprod_artifact.extras["posted_status_checks"]["size"]["check_id"] == "check_67890"

0 commit comments

Comments
 (0)