Skip to content

fix(activity): Include all group activities #100881

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

Closed
wants to merge 2 commits into from
Closed

fix(activity): Include all group activities #100881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f2b530
fix(activity): Include all group activities
armenzg Oct 3, 2025
0aca129
Fix tests
armenzg Oct 3, 2025
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
3 changes: 0 additions & 3 deletions src/sentry/api/helpers/group_index/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,13 @@
# complicated right now.
BULK_MUTATION_LIMIT = 1000

ACTIVITIES_COUNT = 100

# XXX: The 1000 magic number for `max_hits` is an abstraction leak from
# `sentry.api.paginator.BasePaginator.get_result`.
SEARCH_MAX_HITS = 1000

SearchFunction = Callable[[Mapping[str, Any]], tuple[CursorResult[Group], Mapping[str, Any]]]

__all__ = (
"ACTIVITIES_COUNT",
"BULK_MUTATION_LIMIT",
"SEARCH_MAX_HITS",
"delete_group_list",
Expand Down
6 changes: 2 additions & 4 deletions src/sentry/api/helpers/group_index/update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
from sentry.users.services.user_option import user_option_service
from sentry.utils import metrics

from . import ACTIVITIES_COUNT, BULK_MUTATION_LIMIT, SearchFunction, delete_group_list
from . import BULK_MUTATION_LIMIT, SearchFunction, delete_group_list
from .validators import GroupValidator, ValidationError

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -752,9 +752,7 @@ def prepare_response(
if len(group_list) == 1:
if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release):
result["activity"] = serialize(
Activity.objects.get_activities_for_group(
group=group_list[0], num=ACTIVITIES_COUNT
),
Activity.objects.get_activities_for_group(group=group_list[0]),
acting_user,
)
except UnboundLocalError:
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/issues/endpoints/group_activities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def get(self, request: Request, group: Group) -> Response:
"""
Retrieve all the Activities for a Group
"""
activity = Activity.objects.get_activities_for_group(group, num=100)
activities = Activity.objects.get_activities_for_group(group)
return Response(
{
"activity": serialize(activity, request.user),
"activity": serialize(activities, request.user),
}
)
2 changes: 1 addition & 1 deletion src/sentry/issues/endpoints/group_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def get(self, request: Request, group: Group) -> Response:
)

# TODO: these probably should be another endpoint
activity = Activity.objects.get_activities_for_group(group, 100)
activity = Activity.objects.get_activities_for_group(group)
seen_by = self._get_seen_by(request, group)

if "release" not in collapse:
Expand Down
12 changes: 9 additions & 3 deletions src/sentry/models/activity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@


class ActivityManager(BaseManager["Activity"]):
def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity]:
def get_activities_for_group(self, group: Group) -> Sequence[Activity]:
Copy link
Member Author

Choose a reason for hiding this comment

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

This function is called by (link to code search):

  • the group_activities API
  • the group_details API
  • calling update_groups is called by:
    • the project group index API
    • the organization group index API

"""
Returns a list of Activity objects for the given group, ordered by most recent first,
with duplicate consecutive activities (by type, ident, user_id, and data) filtered out,
except for notes which are always included. Also appends a synthetic FIRST_SEEN activity
at the end, using the group's initial priority if available.
"""
activities = []
activity_qs = self.filter(group=group).order_by("-datetime")
Copy link
Member Author

@armenzg armenzg Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This has always done a full table scan. We may want to add a new index.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can get metrics on the query here:
https://sentry.sentry.io/insights/backend/database/spans/span/35fcb71003a016d1/?project=1&span.domain=%2A%2Csentry_activity%2C%2A&statsPeriod=7d


Expand All @@ -56,7 +62,7 @@ def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity]
prev_sig = None
sig = None
# we select excess so we can filter dupes
for item in activity_qs[: num * 2]:
for item in activity_qs:
prev_sig = sig
sig = (item.type, item.ident, item.user_id, item.data)

Comment on lines 62 to 68
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential bug: Removing the limit in get_activities_for_group on a query filtering by the unindexed group field may cause slow database queries and high memory usage.

  • Description: The removal of the num limit in get_activities_for_group causes the method to fetch all activities for a given group. The query filters on the group field, but the Activity model is only indexed on (project, datetime). Without an index that includes group, the database may perform a slow table scan for groups with many activities. This change loads an unbounded number of activities into memory, risking high memory consumption and slow API responses for long-lived or high-activity groups.

  • Suggested fix: Reintroduce a reasonable upper limit to the number of activities fetched to prevent unbounded queries. For a more robust solution, add a database index on the group field in the Activity model to improve query performance. Implementing pagination for activity loading is the ideal long-term fix.
    severity: 0.75, confidence: 0.85

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

Expand All @@ -78,7 +84,7 @@ def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity]
)
)

return activities[:num]
return activities

def create_group_activity(
self,
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/testutils/factories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,9 @@ def create_group(project, create_open_period=True, **kwargs):
if "status" not in kwargs:
kwargs["status"] = GroupStatus.UNRESOLVED
kwargs["substatus"] = GroupSubStatus.NEW
if "status" in kwargs:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an unrelated change but I always see an error when executing tests locally.

if kwargs["status"] == GroupStatus.UNRESOLVED and "substatus" not in kwargs:
kwargs["substatus"] = GroupSubStatus.NEW

group = Group.objects.create(project=project, **kwargs)
if create_open_period:
Expand Down
9 changes: 3 additions & 6 deletions tests/sentry/issues/endpoints/test_group_activities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def test_endpoint_with_no_group_activities(self) -> None:
def test_endpoint_with_group_activities(self) -> None:
group = self.create_group(status=GroupStatus.UNRESOLVED)

for i in range(0, 4):
for _ in range(0, 100):
Activity.objects.create(
group=group,
project=group.project,
Expand All @@ -33,10 +33,7 @@ def test_endpoint_with_group_activities(self) -> None:
self.login_as(user=self.user)

url = f"/api/0/issues/{group.id}/activities/"
response = self.client.get(
url,
format="json",
)
response = self.client.get(url, format="json")

assert "activity" in response.data
assert len(response.data["activity"]) == 5
assert len(response.data["activity"]) == 101
12 changes: 6 additions & 6 deletions tests/sentry/models/test_activity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def test_get_activities_for_group_none(self) -> None:
project = self.create_project(name="test_activities_group")
group = self.create_group(project)

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)
assert len(act_for_group) == 1
assert act_for_group[0].type == ActivityType.FIRST_SEEN.value

Expand Down Expand Up @@ -47,7 +47,7 @@ def test_get_activities_for_group_priority(self) -> None:
),
]

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)
assert len(act_for_group) == 3
assert act_for_group[0] == activities[-1]
assert act_for_group[1] == activities[-2]
Expand Down Expand Up @@ -87,7 +87,7 @@ def test_get_activities_for_group_simple_priority_ff_on_dups(self) -> None:
),
]

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)

assert len(act_for_group) == 3
assert act_for_group[0] == activities[-1]
Expand Down Expand Up @@ -117,7 +117,7 @@ def test_get_activities_for_group_simple(self) -> None:
),
]

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)
assert len(act_for_group) == 3
assert act_for_group[0] == activities[-1]
assert act_for_group[1] == activities[-2]
Expand Down Expand Up @@ -210,7 +210,7 @@ def test_get_activities_for_group_collapse_same(self) -> None:
),
]

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)
assert len(act_for_group) == 7
assert act_for_group[0] == activities[-1]
assert act_for_group[1] == activities[-2]
Expand Down Expand Up @@ -314,7 +314,7 @@ def test_get_activities_for_group_flip_flop(self) -> None:
),
]

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)

assert len(act_for_group) == len(activities) + 1
assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value
Expand Down
2 changes: 1 addition & 1 deletion tests/snuba/api/endpoints/test_group_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def test_group_post_priority(self) -> None:
assert response.status_code == 200, response.content
assert response.data["priority"] == "high"

act_for_group = Activity.objects.get_activities_for_group(group=group, num=100)
act_for_group = Activity.objects.get_activities_for_group(group=group)
assert len(act_for_group) == 2
assert act_for_group[0].type == ActivityType.SET_PRIORITY.value
assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value
Expand Down
Loading