Skip to content

fix(aci): remove refs to open periods from unmerge task #102114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+1 −32
Open

fix(aci): remove refs to open periods from unmerge task #102114

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 1 addition & 32 deletions src/sentry/tasks/unmerge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@
from sentry.models.activity import Activity
from sentry.models.environment import Environment
from sentry.models.eventattachment import EventAttachment
from sentry.models.group import Group, GroupStatus
from sentry.models.group import Group
from sentry.models.groupenvironment import GroupEnvironment
from sentry.models.grouphash import GroupHash
from sentry.models.groupopenperiod import GroupOpenPeriod
from sentry.models.grouprelease import GroupRelease
from sentry.models.project import Project
from sentry.models.release import Release
Expand Down Expand Up @@ -203,7 +202,6 @@ def migrate_events(
destination = Group.objects.get(id=destination_id)
destination.update(**get_group_backfill_attributes(caches, destination, events))

update_open_periods(source, destination)
logger.info("migrate_events.migrate", extra={**extra, "destination_id": destination_id})

if isinstance(args, InitialUnmergeArgs) or opt_eventstream_state is None:
Comment on lines 202 to 207
Copy link

Choose a reason for hiding this comment

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

Bug: Removing update_open_periods during error group unmerge causes destination groups to retain inconsistent open period data.
Severity: MEDIUM | Confidence: 0.90

🔍 Detailed Analysis

When two error groups are unmerged, the removal of the update_open_periods function prevents the destination group's open period data from being synchronized with the source group's data. This occurs because ErrorGroupType issues typically have open periods created by default. Consequently, the destination group retains stale or inconsistent open period information, which could affect workflow decisions or queries relying on this data.

💡 Suggested Fix

Reintroduce the call to update_open_periods or implement equivalent logic to synchronize open period data for the destination group during error group unmerge operations.

🤖 Prompt for AI Agent 
Fix this bug. In src/sentry/tasks/unmerge.py at lines 202-207: When two error groups are
unmerged, the removal of the `update_open_periods` function prevents the destination
group's open period data from being synchronized with the source group's data. This
occurs because `ErrorGroupType` issues typically have open periods created by default.
Consequently, the destination group retains stale or inconsistent open period
information, which could affect workflow decisions or queries relying on this data.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open period data doesn't matter for these groups. They don't use it.

Expand Down Expand Up @@ -249,35 +247,6 @@ def migrate_events(
return (destination.id, eventstream_state)


def update_open_periods(source: Group, destination: Group) -> None:
# For groups that are not resolved, the open period created on group creation should have the necessary information
if destination.status != GroupStatus.RESOLVED:
return

try:
dest_open_period = GroupOpenPeriod.objects.get(group=destination)
except GroupOpenPeriod.DoesNotExist:
logger.exception("No open period found for group", extra={"group_id": destination.id})

source_open_period = (
GroupOpenPeriod.objects.filter(group=source).order_by("-date_started").first()
)
if not source_open_period:
logger.error("No open period found for group", extra={"group_id": destination.id})
return

if source_open_period.date_ended is None:
return

# If the destination group is resolved, set the open period fields to match the source's open period.
dest_open_period.update(
date_started=source_open_period.date_started,
date_ended=source_open_period.date_ended,
resolution_activity=source_open_period.resolution_activity,
user_id=source_open_period.user_id,
)


def truncate_denormalizations(project: Project, group: Group) -> None:
GroupRelease.objects.filter(group_id=group.id).delete()

Expand Down
Loading