Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
8 changes: 8 additions & 0 deletions src/sentry/models/groupopenperiodactivity.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from collections.abc import Mapping
from enum import IntEnum
from uuid import UUID, uuid4

Expand All @@ -14,6 +15,13 @@ class OpenPeriodActivityType(IntEnum):
CLOSED = 3


OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING: Mapping[int, str] = {
OpenPeriodActivityType.OPENED: "opened",
OpenPeriodActivityType.STATUS_CHANGE: "status_change",
OpenPeriodActivityType.CLOSED: "closed",
}


def generate_random_uuid() -> UUID:
return uuid4()

Expand Down
7 changes: 7 additions & 0 deletions src/sentry/types/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,10 @@ def from_str(self, name: str) -> "PriorityLevel | None":
"""
name = name.upper()
return self[name] if name in self.__members__ else None


PRIORITY_LEVEL_TO_STRING: Mapping[int, str] = {
PriorityLevel.LOW: "low",
PriorityLevel.MEDIUM: "medium",
PriorityLevel.HIGH: "high",
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
from collections import defaultdict
from collections.abc import Mapping
from datetime import datetime, timedelta
from typing import Any, TypedDict

from sentry.api.serializers import Serializer, register
from sentry.api.serializers import Serializer, register, serialize
from sentry.models.groupopenperiod import GroupOpenPeriod, get_last_checked_for_open_period
from sentry.models.groupopenperiodactivity import (
OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING,
GroupOpenPeriodActivity,
)
from sentry.types.group import PRIORITY_LEVEL_TO_STRING


class GroupOpenPeriodActivityResponse(TypedDict):
id: str
type: str
value: str | None


class GroupOpenPeriodResponse(TypedDict):
Expand All @@ -13,10 +25,41 @@ class GroupOpenPeriodResponse(TypedDict):
duration: timedelta | None
isOpen: bool
lastChecked: datetime
activities: GroupOpenPeriodActivityResponse | None


@register(GroupOpenPeriodActivity)
class GroupOpenPeriodActivitySerializer(Serializer):
def serialize(
self, obj: GroupOpenPeriodActivity, attrs: Mapping[str, Any], user, **kwargs
) -> GroupOpenPeriodActivityResponse:
return {
"id": str(obj.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[obj.type],
"value": PRIORITY_LEVEL_TO_STRING.get(obj.value) if obj.value else None,
}


@register(GroupOpenPeriod)
class GroupOpenPeriodSerializer(Serializer):
def get_attrs(self, item_list, user, **kwargs):
result: defaultdict[GroupOpenPeriod, dict[str, list[GroupOpenPeriodActivityResponse]]] = (
defaultdict(dict)
)
activities = GroupOpenPeriodActivity.objects.filter(
group_open_period__in=item_list
).order_by("id")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also order by date_added, but this works too :)


gopas = defaultdict(list)
for activity, serialized_activity in zip(
activities, serialize(list(activities), user=user, **kwargs)
):
gopas[activity.group_open_period].append(serialized_activity)
for item in item_list:
result[item]["activities"] = gopas[item][:100]
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Overfetching and Serialization Issues

The get_attrs method fetches and serializes all GroupOpenPeriodActivity records for each GroupOpenPeriod, even though only the first 100 are used. This can cause significant memory and processing overhead. Additionally, zipping the activities queryset with its serialized output risks an ordering mismatch, potentially leading to incorrect data associations.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

django lazy loads them


return result

def serialize(
self, obj: GroupOpenPeriod, attrs: Mapping[str, Any], user, **kwargs
) -> GroupOpenPeriodResponse:
Expand All @@ -27,4 +70,5 @@ def serialize(
"duration": obj.date_ended - obj.date_started if obj.date_ended else None,
"isOpen": obj.date_ended is None,
"lastChecked": get_last_checked_for_open_period(obj.group),
"activities": attrs.get("activities"),
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@
get_open_periods_for_group,
update_group_open_period,
)
from sentry.models.groupopenperiodactivity import (
OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING,
GroupOpenPeriodActivity,
OpenPeriodActivityType,
)
from sentry.testutils.cases import APITestCase
from sentry.types.activity import ActivityType
from sentry.types.group import PRIORITY_LEVEL_TO_STRING
from sentry.workflow_engine.models.detector_group import DetectorGroup


Expand All @@ -35,6 +41,15 @@ def setUp(self) -> None:
# Link detector to group
DetectorGroup.objects.create(detector=self.detector, group=self.group)

self.group_open_period = GroupOpenPeriod.objects.get(group=self.group)

self.opened_gopa = GroupOpenPeriodActivity.objects.create(
date_added=self.group_open_period.date_added,
group_open_period=self.group_open_period,
type=OpenPeriodActivityType.OPENED,
value=self.group.priority,
)

def get_url_args(self):
return [self.organization.slug]

Expand All @@ -58,6 +73,12 @@ def test_open_period_linked_to_group(self) -> None:
assert open_period["end"] is None
assert open_period["duration"] is None
assert open_period["isOpen"] is True
assert len(open_period["activities"]) == 1
assert open_period["activities"][0] == {
"id": str(self.opened_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.OPENED.value],
"value": PRIORITY_LEVEL_TO_STRING.get(self.group.priority),
}

def test_open_periods_group_id(self) -> None:
response = self.get_success_response(
Expand All @@ -82,13 +103,17 @@ def test_open_periods_new_group_with_last_checked(self) -> None:
assert response.status_code == 200, response.content
assert len(response.data) == 1
resp = response.data[0]
open_period = GroupOpenPeriod.objects.get(group=self.group)
assert resp["id"] == str(open_period.id)
assert resp["id"] == str(self.group_open_period.id)
assert resp["start"] == self.group.first_seen
assert resp["end"] is None
assert resp["duration"] is None
assert resp["isOpen"] is True
assert resp["lastChecked"] >= last_checked
assert resp["activities"][0] == {
"id": str(self.opened_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.OPENED.value],
"value": PRIORITY_LEVEL_TO_STRING.get(self.group.priority),
}

def test_open_periods_resolved_group(self) -> None:
self.group.status = GroupStatus.RESOLVED
Expand All @@ -114,6 +139,9 @@ def test_open_periods_resolved_group(self) -> None:
assert response.status_code == 200, response.content
resp = response.data[0]
open_period = GroupOpenPeriod.objects.get(group=self.group, date_ended=resolved_time)
closed_gopa = GroupOpenPeriodActivity.objects.get(
group_open_period=open_period, type=OpenPeriodActivityType.CLOSED
)
assert resp["id"] == str(open_period.id)
assert resp["start"] == self.group.first_seen
assert resp["end"] == resolved_time
Expand All @@ -122,6 +150,17 @@ def test_open_periods_resolved_group(self) -> None:
assert resp["lastChecked"].replace(second=0, microsecond=0) == activity.datetime.replace(
second=0, microsecond=0
)
assert len(resp["activities"]) == 2
assert resp["activities"][0] == {
"id": str(self.opened_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.OPENED.value],
"value": PRIORITY_LEVEL_TO_STRING.get(self.group.priority),
}
assert resp["activities"][1] == {
"id": str(closed_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.CLOSED.value],
"value": None,
}

def test_open_periods_unresolved_group(self) -> None:
self.group.status = GroupStatus.RESOLVED
Expand All @@ -140,6 +179,9 @@ def test_open_periods_unresolved_group(self) -> None:
resolution_activity=resolve_activity,
)
open_period = GroupOpenPeriod.objects.get(group=self.group, date_ended=resolved_time)
closed_gopa = GroupOpenPeriodActivity.objects.get(
group_open_period=open_period, type=OpenPeriodActivityType.CLOSED
)

unresolved_time = timezone.now()
self.group.status = GroupStatus.UNRESOLVED
Expand Down Expand Up @@ -170,6 +212,12 @@ def test_open_periods_unresolved_group(self) -> None:
open_period2 = GroupOpenPeriod.objects.get(
group=self.group, date_ended=second_resolved_time
)
opened_gopa2 = GroupOpenPeriodActivity.objects.get(
group_open_period=open_period2, type=OpenPeriodActivityType.OPENED
)
closed_gopa2 = GroupOpenPeriodActivity.objects.get(
group_open_period=open_period2, type=OpenPeriodActivityType.CLOSED
)

response = self.get_success_response(
*self.get_url_args(), qs_params={"groupId": self.group.id}
Expand All @@ -186,6 +234,17 @@ def test_open_periods_unresolved_group(self) -> None:
assert resp["lastChecked"].replace(second=0, microsecond=0) == second_resolved_time.replace(
second=0, microsecond=0
)
assert len(resp["activities"]) == 2
assert resp["activities"][0] == {
"id": str(opened_gopa2.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.OPENED.value],
"value": PRIORITY_LEVEL_TO_STRING.get(self.group.priority),
}
assert resp["activities"][1] == {
"id": str(closed_gopa2.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.CLOSED.value],
"value": None,
}

assert resp2["id"] == str(open_period.id)
assert resp2["start"] == self.group.first_seen
Expand All @@ -195,6 +254,17 @@ def test_open_periods_unresolved_group(self) -> None:
assert resp2["lastChecked"].replace(second=0, microsecond=0) == resolved_time.replace(
second=0, microsecond=0
)
assert len(resp2["activities"]) == 2
assert resp2["activities"][0] == {
"id": str(self.opened_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.OPENED.value],
"value": PRIORITY_LEVEL_TO_STRING.get(self.group.priority),
}
assert resp2["activities"][1] == {
"id": str(closed_gopa.id),
"type": OPEN_PERIOD_ACTIVITY_TYPE_TO_STRING[OpenPeriodActivityType.CLOSED.value],
"value": None,
}

def test_open_periods_limit(self) -> None:
self.group.status = GroupStatus.RESOLVED
Expand Down
Loading