From 43ae18ce6adf1ecdb008b6a53f8846f6bca7efee Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Thu, 29 May 2025 17:43:57 -0700 Subject: [PATCH 1/6] Recreate backfill migration --- migrations_lockfile.txt | 2 +- .../migrations/0915_backfill_open_periods.py | 232 ++++++++++++++++++ .../test_0915_backfill_open_periods.py | 216 ++++++++++++++++ 3 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 src/sentry/migrations/0915_backfill_open_periods.py create mode 100644 tests/sentry/migrations/test_0915_backfill_open_periods.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 0e86af4fd86263..a2d94e4547af92 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -23,7 +23,7 @@ preprod: 0001_emerge_upload_models replays: 0001_squashed_0005_drop_replay_index -sentry: 0914_increase_orgmember_user_email_max_length +sentry: 0915_backfill_open_periods social_auth: 0001_squashed_0002_default_auto_field diff --git a/src/sentry/migrations/0915_backfill_open_periods.py b/src/sentry/migrations/0915_backfill_open_periods.py new file mode 100644 index 00000000000000..ba701add92ca08 --- /dev/null +++ b/src/sentry/migrations/0915_backfill_open_periods.py @@ -0,0 +1,232 @@ +# Generated by Django 5.2.1 on 2025-05-30 00:42 + +import logging +from collections import defaultdict +from datetime import datetime +from enum import Enum +from typing import TYPE_CHECKING, Any + +from django.conf import settings +from django.db import IntegrityError, migrations, router, transaction +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + +from sentry.new_migrations.migrations import CheckedMigration +from sentry.utils import redis +from sentry.utils.iterators import chunked +from sentry.utils.query import RangeQuerySetWrapperWithProgressBarApprox + +if TYPE_CHECKING: + from sentry.models.activity import Activity as ActivityModelType + + +logger = logging.getLogger(__name__) + +CHUNK_SIZE = 100 + + +# copied constants and enums +class ActivityType(Enum): + SET_UNRESOLVED = 2 + SET_REGRESSION = 6 + SET_RESOLVED = 1 + SET_RESOLVED_IN_RELEASE = 13 + SET_RESOLVED_BY_AGE = 15 + SET_RESOLVED_IN_COMMIT = 16 + SET_RESOLVED_IN_PULL_REQUEST = 21 + + +RESOLVED_ACTIVITY_TYPES = [ + ActivityType.SET_RESOLVED.value, + ActivityType.SET_RESOLVED_IN_RELEASE.value, + ActivityType.SET_RESOLVED_BY_AGE.value, + ActivityType.SET_RESOLVED_IN_COMMIT.value, + ActivityType.SET_RESOLVED_IN_PULL_REQUEST.value, +] + + +class GroupStatus: + UNRESOLVED = 0 + RESOLVED = 1 + + +# end copy + + +def get_open_periods_for_group( + apps: StateApps, + group_id: int, + status: int, + project_id: int, + first_seen: datetime, + activities: list[Any], + GroupOpenPeriod: Any, +) -> list[Any]: + # Filter to REGRESSION and SET_RESOLVED_XX activties to find the bounds of each open period. + # The only UNRESOLVED activity we would care about is the first UNRESOLVED activity for the group creation, + # but we don't create an entry for that. + open_periods = [] + start: datetime | None = None + end: datetime | None = None + end_activity: ActivityModelType | None = None + + # Handle currently open period + if status == GroupStatus.UNRESOLVED and len(activities) > 0: + open_periods.append( + GroupOpenPeriod( + group_id=group_id, + project_id=project_id, + date_started=activities[0].datetime, + date_ended=None, + resolution_activity=None, + user_id=None, + ) + ) + activities = activities[1:] + + for activity in activities: + if activity.type in RESOLVED_ACTIVITY_TYPES: + end = activity.datetime + end_activity = activity + elif activity.type == ActivityType.SET_REGRESSION.value: + start = activity.datetime + if end is None: + logger.error( + "No end activity found for group open period backfill", + extra={"group_id": group_id, "starting_activity": activity.id}, + ) + if start is not None and end is not None: + if start > end: + logger.error( + "Open period has invalid start and end dates", + extra={"group_id": group_id}, + ) + return [] + + open_periods.append( + GroupOpenPeriod( + group_id=group_id, + project_id=project_id, + date_started=start, + date_ended=end, + resolution_activity=end_activity, + user_id=end_activity.user_id if end_activity else None, + ) + ) + end = None + end_activity = None + # Add the very first open period, which has no UNRESOLVED activity for the group creation + open_periods.append( + GroupOpenPeriod( + group_id=group_id, + project_id=project_id, + date_started=first_seen, + date_ended=end if end else None, + resolution_activity=end_activity, + user_id=end_activity.user_id if end_activity else None, + ) + ) + return open_periods + + +def _backfill_group_open_periods( + apps: StateApps, group_data: list[tuple[int, datetime, int, int]] +) -> None: + GroupOpenPeriod = apps.get_model("sentry", "GroupOpenPeriod") + Activity = apps.get_model("sentry", "Activity") + + group_ids = [group_id for group_id, _, _, _ in group_data] + groups_with_open_periods = set( + GroupOpenPeriod.objects.filter(group_id__in=group_ids) + .values_list("group_id", flat=True) + .distinct() + ) + + group_ids = [group_id for group_id in group_ids if group_id not in groups_with_open_periods] + # Filter the relevant activities for groups that need to be backfilled. + activities = defaultdict(list) + for activity in Activity.objects.filter( + group_id__in=group_ids, + type__in=[ActivityType.SET_REGRESSION.value, *RESOLVED_ACTIVITY_TYPES], + ).order_by("-datetime"): + activities[activity.group_id].append(activity) + + open_periods = [] + for group_id, first_seen, status, project_id in group_data: + # Skip groups that already have open periods + if group_id in groups_with_open_periods: + continue + + open_periods.extend( + get_open_periods_for_group( + apps, + group_id, + status, + project_id, + first_seen, + activities[group_id], + GroupOpenPeriod, + ) + ) + + with transaction.atomic(router.db_for_write(GroupOpenPeriod)): + try: + GroupOpenPeriod.objects.bulk_create(open_periods) + except IntegrityError as e: + logger.exception( + "Error creating open period", + extra={"group_ids": group_ids, "error": e}, + ) + + +def backfill_group_open_periods(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + Group = apps.get_model("sentry", "Group") + + backfill_key = "backfill_group_open_periods_from_activity_1" + redis_client = redis.redis_clusters.get(settings.SENTRY_MONITORS_REDIS_CLUSTER) + + progress_id = int(redis_client.get(backfill_key) or 0) + for group_data in chunked( + RangeQuerySetWrapperWithProgressBarApprox( + Group.objects.filter(id__gt=progress_id).values_list( + "id", "first_seen", "status", "project_id" + ), + result_value_getter=lambda item: item[0], + ), + CHUNK_SIZE, + ): + logger.info( + "Processing batch for group open period backfill", + extra={"last_group_id": group_data[-1][0]}, + ) + _backfill_group_open_periods(apps, group_data) + # Save progress to redis in case we have to restart + redis_client.set(backfill_key, group_data[-1][0], ex=60 * 60 * 24 * 7) + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = True + + dependencies = [ + ("sentry", "0914_increase_orgmember_user_email_max_length"), + ] + + operations = [ + migrations.RunPython( + backfill_group_open_periods, + migrations.RunPython.noop, + hints={"tables": ["sentry_groupopenperiod"]}, + ), + ] diff --git a/tests/sentry/migrations/test_0915_backfill_open_periods.py b/tests/sentry/migrations/test_0915_backfill_open_periods.py new file mode 100644 index 00000000000000..ebb9c37696767b --- /dev/null +++ b/tests/sentry/migrations/test_0915_backfill_open_periods.py @@ -0,0 +1,216 @@ +from datetime import timedelta + +import pytest +from django.utils import timezone + +from sentry.models.activity import Activity +from sentry.models.group import Group, GroupStatus +from sentry.models.groupopenperiod import GroupOpenPeriod +from sentry.models.organization import Organization +from sentry.testutils.cases import TestMigrations +from sentry.types.activity import ActivityType +from sentry.types.group import GroupSubStatus + + +@pytest.mark.skip(reason="Skipping migration test to avoid blocking CI") +class BackfillGroupOpenPeriodsTest(TestMigrations): + migrate_from = "0877_integer_drift_release" + migrate_to = "0878_backfill_open_periods" + + def setup_before_migration(self, app): + now = timezone.now() + self.organization = Organization.objects.create(name="test", slug="test") + self.project = self.create_project(organization=self.organization) + + # Create a group that has been resolved and then regressed and resolved again + self.group_resolved = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=3), + ) + self.group_resolved_resolution_activity_1 = Activity.objects.create( + group=self.group_resolved, + project=self.project, + type=ActivityType.SET_RESOLVED.value, + datetime=now - timedelta(days=2), + ) + self.group_resolved.update( + status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED + ) + Activity.objects.create( + group=self.group_resolved, + project=self.project, + type=ActivityType.SET_REGRESSION.value, + datetime=now - timedelta(days=1), + ) + self.group_resolved.update(status=GroupStatus.RESOLVED, substatus=None) + self.group_resolved_resolution_activity_2 = Activity.objects.create( + group=self.group_resolved, + project=self.project, + type=ActivityType.SET_RESOLVED.value, + datetime=now, + ) + assert self.group_resolved.status == GroupStatus.RESOLVED + assert self.group_resolved.substatus is None + + # Create a group that has been resolved and then regressed + self.group_regressed = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=3), + ) + self.group_regressed.update(status=GroupStatus.RESOLVED, substatus=None) + self.group_regressed_resolution_activity = Activity.objects.create( + group=self.group_regressed, + project=self.project, + type=ActivityType.SET_RESOLVED.value, + datetime=now - timedelta(days=2), + ) + self.group_regressed.update( + status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED + ) + Activity.objects.create( + group=self.group_regressed, + project=self.project, + type=ActivityType.SET_REGRESSION.value, + datetime=now - timedelta(days=1), + ) + assert self.group_regressed.status == GroupStatus.UNRESOLVED + assert self.group_regressed.substatus == GroupSubStatus.REGRESSED + + # Create a new group that has never been resolved + self.group_new = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=3), + ) + assert self.group_new.status == GroupStatus.UNRESOLVED + assert self.group_new.substatus == GroupSubStatus.NEW + + # Create a group that has been ignored until escalating + self.group_ignored = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=3), + ) + self.group_ignored.update( + status=GroupStatus.IGNORED, substatus=GroupSubStatus.UNTIL_ESCALATING + ) + Activity.objects.create( + group=self.group_ignored, + project=self.project, + type=ActivityType.SET_IGNORED.value, + datetime=now - timedelta(days=2), + ) + assert self.group_ignored.status == GroupStatus.IGNORED + assert self.group_ignored.substatus == GroupSubStatus.UNTIL_ESCALATING + + # Create an unresolved group that already has an open period + self.unresolved_group_with_open_period = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=5), + ) + GroupOpenPeriod.objects.create( + group=self.unresolved_group_with_open_period, + project=self.project, + date_started=self.unresolved_group_with_open_period.first_seen, + ) + + assert ( + GroupOpenPeriod.objects.filter(group=self.unresolved_group_with_open_period).count() + == 1 + ) + + # Create a resolved group that already has an open period + self.resolved_group_with_open_period = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=now - timedelta(days=3), + ) + gop = GroupOpenPeriod.objects.create( + group=self.resolved_group_with_open_period, + project=self.project, + date_started=self.resolved_group_with_open_period.first_seen, + ) + self.resolved_group_with_open_period.update(status=GroupStatus.RESOLVED, substatus=None) + self.resolved_group_with_open_period_resolution_activity = Activity.objects.create( + group=self.resolved_group_with_open_period, + project=self.project, + type=ActivityType.SET_RESOLVED.value, + datetime=now, + ) + gop.update( + resolution_activity=self.resolved_group_with_open_period_resolution_activity, + date_ended=self.resolved_group_with_open_period_resolution_activity.datetime, + ) + assert ( + GroupOpenPeriod.objects.filter(group=self.resolved_group_with_open_period).count() == 1 + ) + assert self.resolved_group_with_open_period.status == GroupStatus.RESOLVED + assert self.resolved_group_with_open_period.substatus is None + + def test(self): + self.group_resolved.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.group_resolved).order_by( + "date_started" + ) + assert len(open_periods) == 2 + assert open_periods[0].date_started == self.group_resolved.first_seen + assert open_periods[0].date_ended == self.group_resolved_resolution_activity_1.datetime + assert open_periods[0].resolution_activity == self.group_resolved_resolution_activity_1 + assert open_periods[1].date_started > open_periods[0].date_started + assert open_periods[1].date_ended == self.group_resolved_resolution_activity_2.datetime + assert open_periods[1].resolution_activity == self.group_resolved_resolution_activity_2 + + self.group_regressed.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.group_regressed).order_by( + "date_started" + ) + assert len(open_periods) == 2 + assert open_periods[0].date_started == self.group_regressed.first_seen + assert open_periods[0].date_ended == self.group_regressed_resolution_activity.datetime + assert open_periods[1].date_started > open_periods[0].date_started + assert open_periods[1].date_ended is None + assert open_periods[1].resolution_activity is None + + self.group_new.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.group_new) + assert len(open_periods) == 1 + assert open_periods[0].date_started == self.group_new.first_seen + assert open_periods[0].date_ended is None + assert open_periods[0].resolution_activity is None + + self.group_ignored.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.group_ignored) + assert len(open_periods) == 1 + assert open_periods[0].date_started == self.group_ignored.first_seen + assert open_periods[0].date_ended is None + assert open_periods[0].resolution_activity is None + + # Ensure that the existing open periods were not touched + self.unresolved_group_with_open_period.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.unresolved_group_with_open_period) + assert len(open_periods) == 1 + assert open_periods[0].date_started == self.unresolved_group_with_open_period.first_seen + assert open_periods[0].date_ended is None + assert open_periods[0].resolution_activity is None + + self.resolved_group_with_open_period.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=self.resolved_group_with_open_period) + assert len(open_periods) == 1 + assert open_periods[0].date_started == self.resolved_group_with_open_period.first_seen + assert ( + open_periods[0].date_ended + == self.resolved_group_with_open_period_resolution_activity.datetime + ) + assert ( + open_periods[0].resolution_activity + == self.resolved_group_with_open_period_resolution_activity + ) From 091e8d27fb22f005fb890dc6485a5eb128dd18a3 Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Thu, 29 May 2025 22:37:49 -0700 Subject: [PATCH 2/6] Refactor test to reuse code --- .../test_0915_backfill_open_periods.py | 288 +++++++++--------- 1 file changed, 141 insertions(+), 147 deletions(-) diff --git a/tests/sentry/migrations/test_0915_backfill_open_periods.py b/tests/sentry/migrations/test_0915_backfill_open_periods.py index ebb9c37696767b..f3e762141a42a9 100644 --- a/tests/sentry/migrations/test_0915_backfill_open_periods.py +++ b/tests/sentry/migrations/test_0915_backfill_open_periods.py @@ -1,6 +1,5 @@ from datetime import timedelta -import pytest from django.utils import timezone from sentry.models.activity import Activity @@ -12,205 +11,200 @@ from sentry.types.group import GroupSubStatus -@pytest.mark.skip(reason="Skipping migration test to avoid blocking CI") class BackfillGroupOpenPeriodsTest(TestMigrations): - migrate_from = "0877_integer_drift_release" - migrate_to = "0878_backfill_open_periods" + migrate_from = "0914_increase_orgmember_user_email_max_length" + migrate_to = "0915_backfill_open_periods" - def setup_before_migration(self, app): - now = timezone.now() - self.organization = Organization.objects.create(name="test", slug="test") - self.project = self.create_project(organization=self.organization) - - # Create a group that has been resolved and then regressed and resolved again - self.group_resolved = Group.objects.create( + def _create_resolved_group(self): + group = Group.objects.create( project=self.project, status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=3), + first_seen=self.now - timedelta(days=3), ) - self.group_resolved_resolution_activity_1 = Activity.objects.create( - group=self.group_resolved, + resolution_activity_1 = Activity.objects.create( + group=group, project=self.project, type=ActivityType.SET_RESOLVED.value, - datetime=now - timedelta(days=2), - ) - self.group_resolved.update( - status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED + datetime=self.now - timedelta(days=2), ) - Activity.objects.create( - group=self.group_resolved, + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED) + regressed_activity = Activity.objects.create( + group=group, project=self.project, type=ActivityType.SET_REGRESSION.value, - datetime=now - timedelta(days=1), + datetime=self.now - timedelta(days=1), ) - self.group_resolved.update(status=GroupStatus.RESOLVED, substatus=None) - self.group_resolved_resolution_activity_2 = Activity.objects.create( - group=self.group_resolved, + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity_2 = Activity.objects.create( + group=group, project=self.project, type=ActivityType.SET_RESOLVED.value, - datetime=now, + datetime=self.now, + ) + return ( + group, + [group.first_seen, regressed_activity.datetime], + [ + resolution_activity_1.datetime, + resolution_activity_2.datetime, + ], + [resolution_activity_1, resolution_activity_2], ) - assert self.group_resolved.status == GroupStatus.RESOLVED - assert self.group_resolved.substatus is None - # Create a group that has been resolved and then regressed - self.group_regressed = Group.objects.create( + def _create_regressed_group(self): + group = Group.objects.create( project=self.project, status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=3), + first_seen=self.now - timedelta(days=3), ) - self.group_regressed.update(status=GroupStatus.RESOLVED, substatus=None) - self.group_regressed_resolution_activity = Activity.objects.create( - group=self.group_regressed, + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity = Activity.objects.create( + group=group, project=self.project, type=ActivityType.SET_RESOLVED.value, - datetime=now - timedelta(days=2), + datetime=self.now - timedelta(days=2), ) - self.group_regressed.update( - status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED - ) - Activity.objects.create( - group=self.group_regressed, + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED) + regressed_activity = Activity.objects.create( + group=group, project=self.project, type=ActivityType.SET_REGRESSION.value, - datetime=now - timedelta(days=1), + datetime=self.now - timedelta(days=1), ) - assert self.group_regressed.status == GroupStatus.UNRESOLVED - assert self.group_regressed.substatus == GroupSubStatus.REGRESSED - - # Create a new group that has never been resolved - self.group_new = Group.objects.create( - project=self.project, - status=GroupStatus.UNRESOLVED, - substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=3), + return ( + group, + [group.first_seen, regressed_activity.datetime], + [resolution_activity.datetime, None], + [resolution_activity, None], ) - assert self.group_new.status == GroupStatus.UNRESOLVED - assert self.group_new.substatus == GroupSubStatus.NEW - # Create a group that has been ignored until escalating - self.group_ignored = Group.objects.create( + def _create_ignored_group(self): + group = Group.objects.create( project=self.project, status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=3), - ) - self.group_ignored.update( - status=GroupStatus.IGNORED, substatus=GroupSubStatus.UNTIL_ESCALATING + first_seen=self.now - timedelta(days=3), ) + group.update(status=GroupStatus.IGNORED, substatus=GroupSubStatus.UNTIL_ESCALATING) Activity.objects.create( - group=self.group_ignored, + group=group, project=self.project, type=ActivityType.SET_IGNORED.value, - datetime=now - timedelta(days=2), + datetime=self.now - timedelta(days=2), + ) + return ( + group, + [group.first_seen], + [None], + [None], ) - assert self.group_ignored.status == GroupStatus.IGNORED - assert self.group_ignored.substatus == GroupSubStatus.UNTIL_ESCALATING - # Create an unresolved group that already has an open period - self.unresolved_group_with_open_period = Group.objects.create( + def _create_resolved_group_with_open_period(self): + group = Group.objects.create( project=self.project, status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=5), + first_seen=self.now - timedelta(days=3), ) - GroupOpenPeriod.objects.create( - group=self.unresolved_group_with_open_period, + gop = GroupOpenPeriod.objects.create( + group=group, project=self.project, - date_started=self.unresolved_group_with_open_period.first_seen, + date_started=group.first_seen, ) - - assert ( - GroupOpenPeriod.objects.filter(group=self.unresolved_group_with_open_period).count() - == 1 + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_RESOLVED.value, + datetime=self.now, + ) + gop.update( + resolution_activity=resolution_activity, + date_ended=resolution_activity.datetime, + ) + return ( + group, + [group.first_seen], + [resolution_activity.datetime], + [resolution_activity], ) - # Create a resolved group that already has an open period - self.resolved_group_with_open_period = Group.objects.create( + def setup_before_migration(self, app): + self.now = timezone.now() + self.organization = Organization.objects.create(name="test", slug="test") + self.project = self.create_project(organization=self.organization) + + self.test_cases = [] + + # Create a group that has been resolved and then regressed and resolved again + group, starts, ends, activities = self._create_resolved_group() + assert group.status == GroupStatus.RESOLVED + assert group.substatus is None + self.test_cases.append(("resolved_group", group, starts, ends, activities)) + + # Create a group that has been resolved and then regressed + group, starts, ends, activities = self._create_regressed_group() + assert group.status == GroupStatus.UNRESOLVED + assert group.substatus == GroupSubStatus.REGRESSED + self.test_cases.append(("regressed_group", group, starts, ends, activities)) + + # Create a new group that has never been resolved + group = Group.objects.create( project=self.project, status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.NEW, - first_seen=now - timedelta(days=3), + first_seen=self.now - timedelta(days=3), ) - gop = GroupOpenPeriod.objects.create( - group=self.resolved_group_with_open_period, + assert group.status == GroupStatus.UNRESOLVED + assert group.substatus == GroupSubStatus.NEW + self.test_cases.append(("new_group", group, [group.first_seen], [None], [None])) + + # Create a group that has been ignored until escalating + group, starts, ends, activities = self._create_ignored_group() + assert group.status == GroupStatus.IGNORED + assert group.substatus == GroupSubStatus.UNTIL_ESCALATING + self.test_cases.append(("ignored_group", group, starts, ends, activities)) + + # Create an unresolved group that already has an open period + group = Group.objects.create( project=self.project, - date_started=self.resolved_group_with_open_period.first_seen, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=self.now - timedelta(days=5), ) - self.resolved_group_with_open_period.update(status=GroupStatus.RESOLVED, substatus=None) - self.resolved_group_with_open_period_resolution_activity = Activity.objects.create( - group=self.resolved_group_with_open_period, + GroupOpenPeriod.objects.create( + group=group, project=self.project, - type=ActivityType.SET_RESOLVED.value, - datetime=now, - ) - gop.update( - resolution_activity=self.resolved_group_with_open_period_resolution_activity, - date_ended=self.resolved_group_with_open_period_resolution_activity.datetime, + date_started=group.first_seen, ) - assert ( - GroupOpenPeriod.objects.filter(group=self.resolved_group_with_open_period).count() == 1 + assert GroupOpenPeriod.objects.filter(group=group).count() == 1 + self.test_cases.append( + ("unresolved_group_with_open_period", group, [group.first_seen], [None], [None]) ) - assert self.resolved_group_with_open_period.status == GroupStatus.RESOLVED - assert self.resolved_group_with_open_period.substatus is None + + # Create a resolved group that already has an open period + group, starts, ends, activities = self._create_resolved_group_with_open_period() + assert GroupOpenPeriod.objects.filter(group=group).count() == 1 + assert group.status == GroupStatus.RESOLVED + assert group.substatus is None + self.test_cases.append(("resolved_group_with_open_period", group, starts, ends, activities)) def test(self): - self.group_resolved.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.group_resolved).order_by( - "date_started" - ) - assert len(open_periods) == 2 - assert open_periods[0].date_started == self.group_resolved.first_seen - assert open_periods[0].date_ended == self.group_resolved_resolution_activity_1.datetime - assert open_periods[0].resolution_activity == self.group_resolved_resolution_activity_1 - assert open_periods[1].date_started > open_periods[0].date_started - assert open_periods[1].date_ended == self.group_resolved_resolution_activity_2.datetime - assert open_periods[1].resolution_activity == self.group_resolved_resolution_activity_2 - - self.group_regressed.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.group_regressed).order_by( - "date_started" - ) - assert len(open_periods) == 2 - assert open_periods[0].date_started == self.group_regressed.first_seen - assert open_periods[0].date_ended == self.group_regressed_resolution_activity.datetime - assert open_periods[1].date_started > open_periods[0].date_started - assert open_periods[1].date_ended is None - assert open_periods[1].resolution_activity is None - - self.group_new.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.group_new) - assert len(open_periods) == 1 - assert open_periods[0].date_started == self.group_new.first_seen - assert open_periods[0].date_ended is None - assert open_periods[0].resolution_activity is None - - self.group_ignored.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.group_ignored) - assert len(open_periods) == 1 - assert open_periods[0].date_started == self.group_ignored.first_seen - assert open_periods[0].date_ended is None - assert open_periods[0].resolution_activity is None - - # Ensure that the existing open periods were not touched - self.unresolved_group_with_open_period.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.unresolved_group_with_open_period) - assert len(open_periods) == 1 - assert open_periods[0].date_started == self.unresolved_group_with_open_period.first_seen - assert open_periods[0].date_ended is None - assert open_periods[0].resolution_activity is None - - self.resolved_group_with_open_period.refresh_from_db() - open_periods = GroupOpenPeriod.objects.filter(group=self.resolved_group_with_open_period) - assert len(open_periods) == 1 - assert open_periods[0].date_started == self.resolved_group_with_open_period.first_seen - assert ( - open_periods[0].date_ended - == self.resolved_group_with_open_period_resolution_activity.datetime - ) - assert ( - open_periods[0].resolution_activity - == self.resolved_group_with_open_period_resolution_activity - ) + for description, group, starts, ends, activities in self.test_cases: + group.refresh_from_db() + open_periods = GroupOpenPeriod.objects.filter(group=group).order_by("date_started") + assert len(open_periods) == len( + starts + ), f"{description}: Expected {len(starts)} open periods, got {len(open_periods)}" + for i, open_period in enumerate(open_periods): + assert ( + open_period.date_started == starts[i] + ), f"{description}: Expected open period start date {starts[i]}, got {open_period.date_started}" + assert ( + open_period.date_ended == ends[i] + ), f"{description}: Expected open period end date {ends[i]}, got {open_period.date_ended}" + assert ( + open_period.resolution_activity == activities[i] + ), f"{description}: Expected resolution activity {activities[i]}, got {open_period.resolution_activity}" From 73d49a111fee21c96391fa6ae5254191354c9a94 Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Thu, 29 May 2025 22:37:58 -0700 Subject: [PATCH 3/6] Rework migration logic to account for successive activities of the same type --- .../migrations/0915_backfill_open_periods.py | 100 +++++++----------- 1 file changed, 37 insertions(+), 63 deletions(-) diff --git a/src/sentry/migrations/0915_backfill_open_periods.py b/src/sentry/migrations/0915_backfill_open_periods.py index ba701add92ca08..415f9fbe395d6d 100644 --- a/src/sentry/migrations/0915_backfill_open_periods.py +++ b/src/sentry/migrations/0915_backfill_open_periods.py @@ -4,7 +4,7 @@ from collections import defaultdict from datetime import datetime from enum import Enum -from typing import TYPE_CHECKING, Any +from typing import Any from django.conf import settings from django.db import IntegrityError, migrations, router, transaction @@ -16,10 +16,6 @@ from sentry.utils.iterators import chunked from sentry.utils.query import RangeQuerySetWrapperWithProgressBarApprox -if TYPE_CHECKING: - from sentry.models.activity import Activity as ActivityModelType - - logger = logging.getLogger(__name__) CHUNK_SIZE = 100 @@ -27,7 +23,6 @@ # copied constants and enums class ActivityType(Enum): - SET_UNRESOLVED = 2 SET_REGRESSION = 6 SET_RESOLVED = 1 SET_RESOLVED_IN_RELEASE = 13 @@ -62,70 +57,46 @@ def get_open_periods_for_group( activities: list[Any], GroupOpenPeriod: Any, ) -> list[Any]: - # Filter to REGRESSION and SET_RESOLVED_XX activties to find the bounds of each open period. - # The only UNRESOLVED activity we would care about is the first UNRESOLVED activity for the group creation, - # but we don't create an entry for that. + # No activities means the group has been open since the first_seen date + if not activities: + return [ + GroupOpenPeriod( + group_id=group_id, + project_id=project_id, + date_started=first_seen, + ) + ] + open_periods = [] - start: datetime | None = None - end: datetime | None = None - end_activity: ActivityModelType | None = None + regression_time: datetime | None = first_seen + for activity in activities: + if activity.type == ActivityType.SET_REGRESSION.value and regression_time is None: + regression_time = activity.datetime + + elif activity.type in RESOLVED_ACTIVITY_TYPES and regression_time is not None: + open_periods.append( + GroupOpenPeriod( + group_id=group_id, + project_id=project_id, + date_started=regression_time, + date_ended=activity.datetime, + resolution_activity=activity, + user_id=activity.user_id, + ) + ) + + regression_time = None - # Handle currently open period - if status == GroupStatus.UNRESOLVED and len(activities) > 0: + # Handle currently open period if the group is unresolved + if status == GroupStatus.UNRESOLVED and regression_time is not None: open_periods.append( GroupOpenPeriod( group_id=group_id, project_id=project_id, - date_started=activities[0].datetime, - date_ended=None, - resolution_activity=None, - user_id=None, + date_started=regression_time, ) ) - activities = activities[1:] - for activity in activities: - if activity.type in RESOLVED_ACTIVITY_TYPES: - end = activity.datetime - end_activity = activity - elif activity.type == ActivityType.SET_REGRESSION.value: - start = activity.datetime - if end is None: - logger.error( - "No end activity found for group open period backfill", - extra={"group_id": group_id, "starting_activity": activity.id}, - ) - if start is not None and end is not None: - if start > end: - logger.error( - "Open period has invalid start and end dates", - extra={"group_id": group_id}, - ) - return [] - - open_periods.append( - GroupOpenPeriod( - group_id=group_id, - project_id=project_id, - date_started=start, - date_ended=end, - resolution_activity=end_activity, - user_id=end_activity.user_id if end_activity else None, - ) - ) - end = None - end_activity = None - # Add the very first open period, which has no UNRESOLVED activity for the group creation - open_periods.append( - GroupOpenPeriod( - group_id=group_id, - project_id=project_id, - date_started=first_seen, - date_ended=end if end else None, - resolution_activity=end_activity, - user_id=end_activity.user_id if end_activity else None, - ) - ) return open_periods @@ -143,12 +114,15 @@ def _backfill_group_open_periods( ) group_ids = [group_id for group_id in group_ids if group_id not in groups_with_open_periods] - # Filter the relevant activities for groups that need to be backfilled. + # Filter to REGRESSION and SET_RESOLVED_XX activties to find the bounds of each open period. + # The only UNRESOLVED activity we would care about is the first UNRESOLVED activity for the group creation, + # but we don't create an entry for that. + activities = defaultdict(list) for activity in Activity.objects.filter( group_id__in=group_ids, type__in=[ActivityType.SET_REGRESSION.value, *RESOLVED_ACTIVITY_TYPES], - ).order_by("-datetime"): + ).order_by("datetime"): activities[activity.group_id].append(activity) open_periods = [] From a212a96b245f80075fad22d0dcf00f2904594202 Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Thu, 29 May 2025 22:38:11 -0700 Subject: [PATCH 4/6] Add more tests around the regressed/auto-resolve cycles --- .../test_0915_backfill_open_periods.py | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/sentry/migrations/test_0915_backfill_open_periods.py b/tests/sentry/migrations/test_0915_backfill_open_periods.py index f3e762141a42a9..f645f20a5c9ebc 100644 --- a/tests/sentry/migrations/test_0915_backfill_open_periods.py +++ b/tests/sentry/migrations/test_0915_backfill_open_periods.py @@ -131,6 +131,107 @@ def _create_resolved_group_with_open_period(self): [resolution_activity], ) + def _create_auto_resolved_group(self): + group = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=self.now - timedelta(days=3), + ) + + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_RESOLVED_BY_AGE.value, + datetime=self.now, + ) + + return ( + group, + [group.first_seen], + [resolution_activity.datetime], + [resolution_activity], + ) + + def _create_auto_resolved_regressed_group(self): + group = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=self.now - timedelta(days=3), + ) + + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_RESOLVED_BY_AGE.value, + datetime=self.now - timedelta(days=2), + ) + + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED) + regressed_activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_REGRESSION.value, + datetime=self.now - timedelta(days=1), + ) + + return ( + group, + [group.first_seen, regressed_activity.datetime], + [resolution_activity.datetime, None], + [resolution_activity, None], + ) + + def _create_regressed_group_with_auto_resolved_cycles(self): + group = Group.objects.create( + project=self.project, + status=GroupStatus.UNRESOLVED, + substatus=GroupSubStatus.NEW, + first_seen=self.now - timedelta(days=6), + ) + + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity_1 = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_RESOLVED_BY_AGE.value, + datetime=self.now - timedelta(days=5), + ) + + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED) + regressed_activity_1 = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_REGRESSION.value, + datetime=self.now - timedelta(days=4), + ) + + group.update(status=GroupStatus.RESOLVED, substatus=None) + resolution_activity_2 = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_RESOLVED_BY_AGE.value, + datetime=self.now - timedelta(days=3), + ) + + group.update(status=GroupStatus.UNRESOLVED, substatus=GroupSubStatus.REGRESSED) + regressed_activity_2 = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_REGRESSION.value, + datetime=self.now - timedelta(days=2), + ) + + return ( + group, + [group.first_seen, regressed_activity_1.datetime, regressed_activity_2.datetime], + [resolution_activity_1.datetime, resolution_activity_2.datetime, None], + [resolution_activity_1, resolution_activity_2, None], + ) + def setup_before_migration(self, app): self.now = timezone.now() self.organization = Organization.objects.create(name="test", slug="test") @@ -191,6 +292,26 @@ def setup_before_migration(self, app): assert group.substatus is None self.test_cases.append(("resolved_group_with_open_period", group, starts, ends, activities)) + # Create a group that has been auto-resolved + group, starts, ends, activities = self._create_auto_resolved_group() + assert group.status == GroupStatus.RESOLVED + assert group.substatus is None + self.test_cases.append(("auto_resolved_group", group, starts, ends, activities)) + + # Create a group that has been auto-resolved and then regressed + group, starts, ends, activities = self._create_auto_resolved_regressed_group() + assert group.status == GroupStatus.UNRESOLVED + assert group.substatus == GroupSubStatus.REGRESSED + self.test_cases.append(("auto_resolved_regressed_group", group, starts, ends, activities)) + + # Create a group that has been regressed and then auto-resolved and then regressed again + group, starts, ends, activities = self._create_regressed_group_with_auto_resolved_cycles() + assert group.status == GroupStatus.UNRESOLVED + assert group.substatus == GroupSubStatus.REGRESSED + self.test_cases.append( + ("regressed_group_with_auto_resolved_cycles", group, starts, ends, activities) + ) + def test(self): for description, group, starts, ends, activities in self.test_cases: group.refresh_from_db() From 26440c1e08b1c1362386149c983cff560386493e Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Thu, 5 Jun 2025 13:05:39 -0700 Subject: [PATCH 5/6] Regenerate migration --- migrations_lockfile.txt | 2 +- ...backfill_open_periods.py => 0924_backfill_open_periods.py} | 2 +- ...ill_open_periods.py => test_0924_backfill_open_periods.py} | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/sentry/migrations/{0915_backfill_open_periods.py => 0924_backfill_open_periods.py} (99%) rename tests/sentry/migrations/{test_0915_backfill_open_periods.py => test_0924_backfill_open_periods.py} (99%) diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index dbf6bd47e7ee2b..f335740bad9071 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -23,7 +23,7 @@ preprod: 0001_emerge_upload_models replays: 0001_squashed_0005_drop_replay_index -sentry: 0923_dashboard_starred_backfill_orgs +sentry: 0924_backfill_open_periods social_auth: 0001_squashed_0002_default_auto_field diff --git a/src/sentry/migrations/0915_backfill_open_periods.py b/src/sentry/migrations/0924_backfill_open_periods.py similarity index 99% rename from src/sentry/migrations/0915_backfill_open_periods.py rename to src/sentry/migrations/0924_backfill_open_periods.py index 415f9fbe395d6d..fc27d232d6d9d6 100644 --- a/src/sentry/migrations/0915_backfill_open_periods.py +++ b/src/sentry/migrations/0924_backfill_open_periods.py @@ -194,7 +194,7 @@ class Migration(CheckedMigration): is_post_deployment = True dependencies = [ - ("sentry", "0914_increase_orgmember_user_email_max_length"), + ("sentry", "0923_dashboard_starred_backfill_orgs"), ] operations = [ diff --git a/tests/sentry/migrations/test_0915_backfill_open_periods.py b/tests/sentry/migrations/test_0924_backfill_open_periods.py similarity index 99% rename from tests/sentry/migrations/test_0915_backfill_open_periods.py rename to tests/sentry/migrations/test_0924_backfill_open_periods.py index f645f20a5c9ebc..87439e64df91e3 100644 --- a/tests/sentry/migrations/test_0915_backfill_open_periods.py +++ b/tests/sentry/migrations/test_0924_backfill_open_periods.py @@ -12,8 +12,8 @@ class BackfillGroupOpenPeriodsTest(TestMigrations): - migrate_from = "0914_increase_orgmember_user_email_max_length" - migrate_to = "0915_backfill_open_periods" + migrate_from = "0923_dashboard_starred_backfill_orgs" + migrate_to = "0924_backfill_open_periods" def _create_resolved_group(self): group = Group.objects.create( From 39ca316f12194084b4c012377a1f7fc53d70cbf6 Mon Sep 17 00:00:00 2001 From: Snigdha Sharma Date: Mon, 9 Jun 2025 11:11:11 -0700 Subject: [PATCH 6/6] Regenerate migration --- migrations_lockfile.txt | 2 +- ...backfill_open_periods.py => 0925_backfill_open_periods.py} | 2 +- ...ill_open_periods.py => test_0925_backfill_open_periods.py} | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/sentry/migrations/{0924_backfill_open_periods.py => 0925_backfill_open_periods.py} (98%) rename tests/sentry/migrations/{test_0924_backfill_open_periods.py => test_0925_backfill_open_periods.py} (99%) diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 64539789d26f90..6f1a7bc1fd332b 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -23,7 +23,7 @@ preprod: 0001_emerge_upload_models replays: 0001_squashed_0005_drop_replay_index -sentry: 0924_dashboard_add_unique_constraint_for_user_org_position +sentry: 0925_backfill_open_periods social_auth: 0001_squashed_0002_default_auto_field diff --git a/src/sentry/migrations/0924_backfill_open_periods.py b/src/sentry/migrations/0925_backfill_open_periods.py similarity index 98% rename from src/sentry/migrations/0924_backfill_open_periods.py rename to src/sentry/migrations/0925_backfill_open_periods.py index fc27d232d6d9d6..0b890ed2d49135 100644 --- a/src/sentry/migrations/0924_backfill_open_periods.py +++ b/src/sentry/migrations/0925_backfill_open_periods.py @@ -194,7 +194,7 @@ class Migration(CheckedMigration): is_post_deployment = True dependencies = [ - ("sentry", "0923_dashboard_starred_backfill_orgs"), + ("sentry", "0924_dashboard_add_unique_constraint_for_user_org_position"), ] operations = [ diff --git a/tests/sentry/migrations/test_0924_backfill_open_periods.py b/tests/sentry/migrations/test_0925_backfill_open_periods.py similarity index 99% rename from tests/sentry/migrations/test_0924_backfill_open_periods.py rename to tests/sentry/migrations/test_0925_backfill_open_periods.py index 87439e64df91e3..f32c599ae3cb41 100644 --- a/tests/sentry/migrations/test_0924_backfill_open_periods.py +++ b/tests/sentry/migrations/test_0925_backfill_open_periods.py @@ -12,8 +12,8 @@ class BackfillGroupOpenPeriodsTest(TestMigrations): - migrate_from = "0923_dashboard_starred_backfill_orgs" - migrate_to = "0924_backfill_open_periods" + migrate_from = "0924_dashboard_add_unique_constraint_for_user_org_position" + migrate_to = "0925_backfill_open_periods" def _create_resolved_group(self): group = Group.objects.create(