Skip to content

Commit 59a48d9

Browse files
mtopo27claude
andcommitted
feat(preprod): Use sequential (n-1) comparisons for size analysis diff monitors
Change size analysis diff monitors to use sequential comparisons instead of git-based (merge-base) comparisons. When a new build arrives, each diff-threshold monitor now resolves its base by finding the most recent prior artifact matching the detector's query filters and structural identity (app_id, artifact_type, build_configuration). - Add get_sequential_base_artifact() to artifact_search.py for n-1 base lookup with batch optimization per unique query string - Refactor maybe_emit_issues into maybe_emit_issues_from_diff_size_results which owns all diff monitor evaluation via sequential comparison - Remove maybe_emit_issues call from git-based _run_size_analysis_comparison so the git path only persists comparison records for status checks - Restructure compare_preprod_artifact_size_analysis so the commit_comparison check guards only the git block; sequential monitor eval runs unconditionally Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f20d53c commit 59a48d9

File tree

4 files changed

+434
-233
lines changed

4 files changed

+434
-233
lines changed

src/sentry/preprod/artifact_search.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,3 +315,49 @@ def apply_filters(
315315
q = ~q
316316
queryset = queryset.filter(q)
317317
return queryset
318+
319+
320+
def get_sequential_base_artifact(
321+
artifact: PreprodArtifact,
322+
query: str,
323+
organization: Organization,
324+
) -> PreprodArtifact | None:
325+
"""
326+
Find the most recent prior artifact matching the given query and structural
327+
identity fields, with completed size metrics.
328+
329+
Used by sequential (n-1) monitors to resolve the base artifact for diff
330+
comparisons. Unlike the git-based lookup (get_base_artifact_for_commit),
331+
this orders by date_added rather than commit ancestry.
332+
333+
Args:
334+
artifact: The current (head) artifact to find a base for.
335+
query: The detector's query string (e.g. "app_name:MyApp"). May be empty.
336+
organization: The organization for scoping query filters.
337+
338+
Returns:
339+
The most recent prior PreprodArtifact matching the criteria, or None.
340+
"""
341+
queryset = PreprodArtifact.objects.get_queryset()
342+
343+
if query and query.strip():
344+
search_filters = parse_search_query(
345+
query, config=search_config, get_field_type=get_field_type
346+
)
347+
queryset = apply_filters(queryset, search_filters, organization)
348+
349+
queryset = queryset.filter(
350+
project_id=artifact.project_id,
351+
app_id=artifact.app_id,
352+
artifact_type=artifact.artifact_type,
353+
build_configuration=artifact.build_configuration,
354+
)
355+
356+
queryset = queryset.filter(
357+
preprodartifactsizemetrics__state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
358+
preprodartifactsizemetrics__metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
359+
)
360+
361+
queryset = queryset.exclude(pk=artifact.pk)
362+
363+
return queryset.order_by("-date_added").first()

src/sentry/preprod/size_analysis/tasks.py

Lines changed: 105 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
from django.utils import timezone
99

1010
from sentry import features
11+
from sentry.exceptions import InvalidSearchQuery
1112
from sentry.models.files.file import File
13+
from sentry.preprod.artifact_search import get_sequential_base_artifact
1214
from sentry.preprod.models import (
1315
PreprodArtifact,
1416
PreprodArtifactSizeComparison,
@@ -21,7 +23,7 @@
2123
SizeAnalysisMetadata,
2224
SizeAnalysisValue,
2325
)
24-
from sentry.preprod.size_analysis.models import ComparisonResults, SizeAnalysisResults
26+
from sentry.preprod.size_analysis.models import SizeAnalysisResults
2527
from sentry.preprod.size_analysis.utils import build_size_metrics_map, can_compare_size_metrics
2628
from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task
2729
from sentry.silo.base import SiloMode
@@ -91,6 +93,8 @@ def compare_preprod_artifact_size_analysis(
9193
"caller": "compare_build_config_mismatch",
9294
}
9395
)
96+
# Still evaluate sequential monitors even on build config mismatch
97+
maybe_emit_issues_from_diff_size_results(artifact, org_id)
9498
return
9599

96100
base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
@@ -269,6 +273,9 @@ def compare_preprod_artifact_size_analysis(
269273
extra={"artifact_id": artifact_id},
270274
)
271275

276+
# Evaluate sequential monitors unconditionally — independent of git metadata
277+
maybe_emit_issues_from_diff_size_results(artifact, org_id)
278+
272279

273280
@instrumented_task(
274281
name="sentry.preprod.tasks.manual_size_analysis_comparison",
@@ -533,50 +540,39 @@ def _run_size_analysis_comparison(
533540
comparison.state = PreprodArtifactSizeComparison.State.SUCCESS
534541
comparison.save()
535542

536-
maybe_emit_issues(
537-
comparison_results=comparison_results,
538-
head_metric=head_size_metric,
539-
base_metric=base_size_metric,
540-
)
541-
542543
logger.info(
543544
"preprod.size_analysis.compare.success",
544545
extra={"comparison_id": comparison.id},
545546
)
546547

547548

548-
def maybe_emit_issues(
549-
comparison_results: ComparisonResults,
550-
head_metric: PreprodArtifactSizeMetrics,
551-
base_metric: PreprodArtifactSizeMetrics,
549+
def maybe_emit_issues_from_diff_size_results(
550+
artifact: PreprodArtifact,
551+
org_id: int,
552552
) -> None:
553553
try:
554-
_maybe_emit_issues(
555-
comparison_results=comparison_results, head_metric=head_metric, base_metric=base_metric
556-
)
554+
_maybe_emit_issues_from_diff_size_results(artifact=artifact, org_id=org_id)
557555
except Exception:
558-
logger.exception("Error emitting issues")
556+
logger.exception("Error emitting issues from diff size results")
559557

560558

561559
def _get_platform(artifact: PreprodArtifact) -> str:
562560
return artifact.platform or "unknown"
563561

564562

565-
def _maybe_emit_issues(
566-
comparison_results: ComparisonResults,
567-
head_metric: PreprodArtifactSizeMetrics,
568-
base_metric: PreprodArtifactSizeMetrics,
563+
def _maybe_emit_issues_from_diff_size_results(
564+
artifact: PreprodArtifact,
565+
org_id: int,
569566
) -> None:
570-
project = head_metric.preprod_artifact.project
567+
project = artifact.project
571568
project_id = project.id
572-
organization_id = project.organization.id
573569

574570
if not features.has("organizations:preprod-issues", project.organization):
575571
logger.info(
576-
"preprod.size_analysis.compare.issues.disabled",
572+
"preprod.size_analysis.diff_results.issues.disabled",
577573
extra={
578574
"project_id": project_id,
579-
"organization_id": organization_id,
575+
"organization_id": org_id,
580576
},
581577
)
582578
return
@@ -590,62 +586,107 @@ def _maybe_emit_issues(
590586
)
591587
if not detectors:
592588
logger.info(
593-
"preprod.size_analysis.no_detectors",
589+
"preprod.size_analysis.diff_results.no_detectors",
594590
extra={"project_id": project_id},
595591
)
596592
return
597593

598-
# Only process diff-based detectors from the comparison path
599594
detectors = [d for d in detectors if d.config.get("threshold_type") in DIFF_THRESHOLD_TYPES]
600595
if not detectors:
601596
logger.info(
602-
"preprod.size_analysis.no_diff_detectors",
597+
"preprod.size_analysis.diff_results.no_diff_detectors",
603598
extra={"project_id": project_id},
604599
)
605600
return
606601

607-
diff = comparison_results.size_metric_diff_item
608-
head_artifact = head_metric.preprod_artifact
609-
base_artifact = base_metric.preprod_artifact
602+
head_metrics = list(
603+
PreprodArtifactSizeMetrics.objects.filter(
604+
preprod_artifact=artifact,
605+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
606+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
607+
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
608+
)
609+
if not head_metrics:
610+
logger.info(
611+
"preprod.size_analysis.diff_results.no_head_metrics",
612+
extra={"artifact_id": artifact.id},
613+
)
614+
return
610615

611-
metadata: SizeAnalysisMetadata = {
612-
"platform": _get_platform(head_artifact),
613-
"head_metric_id": head_metric.id,
614-
"base_metric_id": base_metric.id,
615-
"head_artifact_id": head_artifact.id,
616-
"base_artifact_id": base_artifact.id,
617-
"head_artifact": head_artifact,
618-
"base_artifact": base_artifact,
619-
}
616+
head_metric = head_metrics[0]
617+
organization = project.organization
620618

621-
size_data: SizeAnalysisValue = {
622-
"head_install_size_bytes": diff.head_install_size,
623-
"head_download_size_bytes": diff.head_download_size,
624-
"base_install_size_bytes": diff.base_install_size,
625-
"base_download_size_bytes": diff.base_download_size,
626-
"metadata": metadata,
627-
}
619+
# Batch-resolve sequential bases by unique query string
620+
query_to_base: dict[str, PreprodArtifact | None] = {}
621+
for detector in detectors:
622+
query = detector.config.get("query", "")
623+
normalized_query = query.strip() if query else ""
624+
if normalized_query not in query_to_base:
625+
try:
626+
base = get_sequential_base_artifact(artifact, normalized_query, organization)
627+
except InvalidSearchQuery:
628+
logger.exception(
629+
"preprod.size_analysis.diff_results.invalid_query",
630+
extra={"detector_id": detector.id, "query": normalized_query},
631+
)
632+
base = None
633+
query_to_base[normalized_query] = base
634+
635+
for detector in detectors:
636+
query = detector.config.get("query", "")
637+
normalized_query = query.strip() if query else ""
638+
base_artifact = query_to_base.get(normalized_query)
639+
if base_artifact is None:
640+
logger.info(
641+
"preprod.size_analysis.diff_results.no_base",
642+
extra={"detector_id": detector.id, "artifact_id": artifact.id},
643+
)
644+
continue
628645

629-
data_packet: SizeAnalysisDataPacket = DataPacket(
630-
source_id=f"preprod-size-analysis:{project_id}",
631-
packet=size_data,
632-
)
646+
base_metrics = list(
647+
PreprodArtifactSizeMetrics.objects.filter(
648+
preprod_artifact=base_artifact,
649+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
650+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
651+
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
652+
)
653+
if not base_metrics:
654+
continue
633655

634-
logger.info(
635-
"preprod.size_analysis.process_detectors.starting",
636-
extra={
637-
"project_id": project_id,
638-
"detector_count": len(detectors),
639-
},
640-
)
641-
results = process_detectors(data_packet, detectors)
642-
logger.info(
643-
"preprod.size_analysis.process_detectors.completed",
644-
extra={
645-
"project_id": project_id,
646-
"detector_count": len(results),
647-
},
648-
)
656+
base_metric = base_metrics[0]
657+
658+
metadata: SizeAnalysisMetadata = {
659+
"platform": _get_platform(artifact),
660+
"head_metric_id": head_metric.id,
661+
"base_metric_id": base_metric.id,
662+
"head_artifact_id": artifact.id,
663+
"base_artifact_id": base_artifact.id,
664+
"head_artifact": artifact,
665+
"base_artifact": base_artifact,
666+
}
667+
668+
size_data: SizeAnalysisValue = {
669+
"head_install_size_bytes": head_metric.max_install_size or 0,
670+
"head_download_size_bytes": head_metric.max_download_size or 0,
671+
"base_install_size_bytes": base_metric.max_install_size or 0,
672+
"base_download_size_bytes": base_metric.max_download_size or 0,
673+
"metadata": metadata,
674+
}
675+
676+
data_packet: SizeAnalysisDataPacket = DataPacket(
677+
source_id=f"preprod-size-analysis:{project_id}",
678+
packet=size_data,
679+
)
680+
681+
logger.info(
682+
"preprod.size_analysis.diff_results.evaluating",
683+
extra={
684+
"detector_id": detector.id,
685+
"artifact_id": artifact.id,
686+
"base_artifact_id": base_artifact.id,
687+
},
688+
)
689+
process_detectors(data_packet, [detector])
649690

650691

651692
def maybe_emit_issues_from_absolute_size_results(

0 commit comments

Comments
 (0)