From 428ecb1f74b669c9d8840820583106f6debf7a13 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Tue, 8 Sep 2026 14:26:07 -0700 Subject: [PATCH 1/3] Instrument the activity read path --- src/sentry/api/helpers/group_index/update.py | 19 ++++++++- src/sentry/issues/action_log/read_metrics.py | 42 +++++++++++++++++++ src/sentry/issues/derived/gate.py | 22 ++++++++-- .../issues/endpoints/group_activities.py | 19 ++++++++- src/sentry/issues/endpoints/group_details.py | 19 ++++++++- src/sentry/issues/endpoints/group_notes.py | 23 +++++++++- .../issues/endpoints/group_notes_details.py | 29 +++++++++---- .../testutils/helpers/test_action_log.py | 21 +++++++--- 8 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 src/sentry/issues/action_log/read_metrics.py diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index 7c9126de41af..ec2578d0639c 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -32,6 +32,12 @@ resolve_action_actor, resolve_action_source, ) +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.issues.action_log.types import MergeIntoOtherAction from sentry.issues.derived.gate import should_serve_action_log_activity from sentry.issues.grouptype import GroupCategory @@ -784,16 +790,27 @@ def prepare_response( if len(group_list) == 1: if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release): group = group_list[0] - if should_serve_action_log_activity(group.project, acting_user): + if should_serve_action_log_activity( + group.project, acting_user, endpoint=ActivityReadEndpoint.GROUP_INDEX_UPDATE + ): action_log = GroupActionLogEntry.objects.get_actions_for_group( group, ACTIVITIES_COUNT - 1 ) if action_log: + record_activity_read( + ActivityReadEndpoint.GROUP_INDEX_UPDATE, + ActivityReadResult.GALE, + ) result["activity"] = [ *serialize(action_log, acting_user), serialize_first_seen_entry(group), ] else: + record_activity_read( + ActivityReadEndpoint.GROUP_INDEX_UPDATE, + ActivityReadResult.FELL_BACK, + ActivityReadReason.EMPTY_LOG, + ) logger.info( "group_index.groupactionlogentry.not_found", extra={"group_id": group.id}, diff --git a/src/sentry/issues/action_log/read_metrics.py b/src/sentry/issues/action_log/read_metrics.py new file mode 100644 index 000000000000..fdbb8248e652 --- /dev/null +++ b/src/sentry/issues/action_log/read_metrics.py @@ -0,0 +1,42 @@ +from enum import StrEnum + +from sentry.utils import metrics + +ACTIVITY_READ_METRIC = "issues.action_log.activity_read" + + +class ActivityReadEndpoint(StrEnum): + """Read paths that can serve Activity-shaped responses from the action log.""" + + GROUP_DETAILS = "group_details" + GROUP_ACTIVITIES = "group_activities" + GROUP_INDEX_UPDATE = "group_index_update" + GROUP_NOTES = "group_notes" + GROUP_NOTES_DETAILS = "group_notes_details" + + +class ActivityReadResult(StrEnum): + #: The log backed the response. + GALE = "gale" + #: The gate was open, or the flag was on, but we served Activity anyway. + FELL_BACK = "fell_back" + #: The read flag is off for this project, so the log was never consulted. + FLAG_OFF = "flag_off" + + +class ActivityReadReason(StrEnum): + #: The project is enrolled but its backfill hasn't finished. + NOT_BACKFILLED = "not_backfilled" + #: The gate was open and the log came back empty. + EMPTY_LOG = "empty_log" + + +def record_activity_read( + endpoint: ActivityReadEndpoint, + result: ActivityReadResult, + reason: ActivityReadReason | None = None, +) -> None: + tags = {"endpoint": endpoint.value, "result": result.value} + if reason is not None: + tags["reason"] = reason.value + metrics.incr(ACTIVITY_READ_METRIC, sample_rate=1.0, tags=tags) diff --git a/src/sentry/issues/derived/gate.py b/src/sentry/issues/derived/gate.py index a3361ddd87d8..4aecc9952978 100644 --- a/src/sentry/issues/derived/gate.py +++ b/src/sentry/issues/derived/gate.py @@ -1,6 +1,12 @@ from django.contrib.auth.models import AnonymousUser from sentry import features +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.models.options.project_option import ProjectOption from sentry.models.project import Project from sentry.users.models.user import User @@ -29,8 +35,18 @@ def derived_should_be_correct(project: Project) -> bool: def should_serve_action_log_activity( project: Project, actor: User | RpcUser | AnonymousUser | None = None, + *, + endpoint: ActivityReadEndpoint, ) -> bool: """Whether the action log can back this project's Activity-shaped responses.""" - return features.has( - "projects:issue-action-log-activity", project, actor=actor - ) and derived_should_be_correct(project) + if not features.has("projects:issue-action-log-activity", project, actor=actor): + record_activity_read(endpoint, ActivityReadResult.FLAG_OFF) + return False + + if not derived_should_be_correct(project): + record_activity_read( + endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.NOT_BACKFILLED + ) + return False + + return True diff --git a/src/sentry/issues/endpoints/group_activities.py b/src/sentry/issues/endpoints/group_activities.py index a166790ae865..8bb7a97abde3 100644 --- a/src/sentry/issues/endpoints/group_activities.py +++ b/src/sentry/issues/endpoints/group_activities.py @@ -9,6 +9,12 @@ from sentry.api.serializers import serialize from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry from sentry.constants import CELL_API_DEPRECATION_DATE +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.issues.derived.gate import should_serve_action_log_activity from sentry.issues.endpoints.bases.group import GroupEndpoint from sentry.issues.models.groupactionlogentry import GroupActionLogEntry @@ -33,9 +39,15 @@ def get(self, request: Request, group: Group) -> Response: """ Retrieve all the Activities for a Group """ - if should_serve_action_log_activity(group.project, request.user): + if should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_ACTIVITIES + ): action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) if action_log: + record_activity_read( + ActivityReadEndpoint.GROUP_ACTIVITIES, + ActivityReadResult.GALE, + ) serialized = serialize(action_log, request.user) serialized.append(serialize_first_seen_entry(group)) return Response( @@ -43,6 +55,11 @@ def get(self, request: Request, group: Group) -> Response: "activity": serialized, } ) + record_activity_read( + ActivityReadEndpoint.GROUP_ACTIVITIES, + ActivityReadResult.FELL_BACK, + ActivityReadReason.EMPTY_LOG, + ) logger.info( "group_activities.groupactionlogentry.not_found", extra={"group_id": group.id} ) diff --git a/src/sentry/issues/endpoints/group_details.py b/src/sentry/issues/endpoints/group_details.py index 7b5878064166..01eac46d100f 100644 --- a/src/sentry/issues/endpoints/group_details.py +++ b/src/sentry/issues/endpoints/group_details.py @@ -47,6 +47,12 @@ resolve_action_actor, resolve_action_source, ) +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.issues.action_log.types import ViewAction from sentry.issues.constants import ( ISSUE_VIEW_CACHE_KEY_TTL, @@ -342,15 +348,26 @@ def get(self, request: Request, group: Group) -> Response[GroupDetailsResponse]: } ) - if should_serve_action_log_activity(group.project, request.user): + if should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ): action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) if action_log: + record_activity_read( + ActivityReadEndpoint.GROUP_DETAILS, + ActivityReadResult.GALE, + ) # swap action log data in under the activity name first_seen_entry = cast(dict[str, Any], serialize_first_seen_entry(group)) data.update( {"activity": [*serialize(action_log, request.user), first_seen_entry]} ) else: + record_activity_read( + ActivityReadEndpoint.GROUP_DETAILS, + ActivityReadResult.FELL_BACK, + ActivityReadReason.EMPTY_LOG, + ) logger.info( "group_details.groupactionlogentry.not_found", extra={"group_id": group.id} ) diff --git a/src/sentry/issues/endpoints/group_notes.py b/src/sentry/issues/endpoints/group_notes.py index b71087573e94..8cc701200912 100644 --- a/src/sentry/issues/endpoints/group_notes.py +++ b/src/sentry/issues/endpoints/group_notes.py @@ -18,6 +18,12 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import CELL_API_DEPRECATION_DATE from sentry.issues.action_log import action_context_scope, resolve_action_source +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.issues.action_log.types import ( CommentDeleteAction, CommentEditAction, @@ -58,7 +64,12 @@ class GroupNotesEndpoint(GroupEndpoint): url_names=["sentry-api-0-group-notes"], ) def get(self, request: Request, group: Group) -> Response: - if should_serve_action_log_activity(group.project, request.user): + if should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES + ): + # No empty-log fallback on this path: once the gate is open the log is + # authoritative for comments, including when the group has none. + record_activity_read(ActivityReadEndpoint.GROUP_NOTES, ActivityReadResult.GALE) edit_entries = GroupActionLogEntry.objects.filter( group_id=group.id, type=GroupActionType.COMMENT_EDIT.value ).order_by("-date_added", "-id") @@ -187,13 +198,21 @@ def post(self, request: Request, group: Group) -> Response: sender="post", ) - if should_serve_action_log_activity(group.project, request.user): + if should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES + ): entry = GroupActionLogEntry.objects.filter( group_id=group.id, idempotency_key=activity_action_idempotency_key(activity), ).first() if entry: + record_activity_read(ActivityReadEndpoint.GROUP_NOTES, ActivityReadResult.GALE) return Response(serialize(entry, request.user), status=201) + record_activity_read( + ActivityReadEndpoint.GROUP_NOTES, + ActivityReadResult.FELL_BACK, + ActivityReadReason.EMPTY_LOG, + ) logger.info("group_notes.groupactionlogentry.not_found", extra={"group_id": group.id}) return Response(serialize(activity, request.user), status=201) diff --git a/src/sentry/issues/endpoints/group_notes_details.py b/src/sentry/issues/endpoints/group_notes_details.py index b505d4dff682..201888f6121b 100644 --- a/src/sentry/issues/endpoints/group_notes_details.py +++ b/src/sentry/issues/endpoints/group_notes_details.py @@ -22,6 +22,11 @@ publish_action, resolve_action_source, ) +from sentry.issues.action_log.read_metrics import ( + ActivityReadEndpoint, + ActivityReadResult, + record_activity_read, +) from sentry.issues.action_log.types import CommentDeleteAction, CommentEditAction from sentry.issues.derived.gate import should_serve_action_log_activity from sentry.issues.endpoints.bases.group import GroupEndpoint @@ -80,9 +85,14 @@ def delete(self, request: Request, group: Group, note_id: str) -> Response: group_id=group.id, idempotency_key=activity_action_idempotency_key(note), ).first() - if original_comment_log_action is None and should_serve_action_log_activity( - group.project, request.user - ): + serve_from_log = should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES_DETAILS + ) + if serve_from_log: + # The log is authoritative for existence whether or not the entry is + # there: a missing one means the comment is already gone. + record_activity_read(ActivityReadEndpoint.GROUP_NOTES_DETAILS, ActivityReadResult.GALE) + if original_comment_log_action is None and serve_from_log: raise ResourceDoesNotExist webhook_data = { @@ -164,9 +174,14 @@ def put(self, request: Request, group: Group, note_id: str) -> Response: group_id=group.id, idempotency_key=activity_action_idempotency_key(note), ).first() - if original_comment_log_action is None and should_serve_action_log_activity( - group.project, request.user - ): + serve_from_log = should_serve_action_log_activity( + group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES_DETAILS + ) + if serve_from_log: + record_activity_read( + ActivityReadEndpoint.GROUP_NOTES_DETAILS, ActivityReadResult.GALE + ) + if original_comment_log_action is None and serve_from_log: raise ResourceDoesNotExist # Would be nice to have a last_modified timestamp we could bump here @@ -208,7 +223,7 @@ def put(self, request: Request, group: Group, note_id: str) -> Response: sender="put", ) - if should_serve_action_log_activity(group.project, request.user): + if serve_from_log: if original_comment_log_action is not None: # editing a note doesn't update its COMMENT entry (instead it # appends a separate COMMENT_EDIT entry), so patch in the fresh diff --git a/tests/sentry/testutils/helpers/test_action_log.py b/tests/sentry/testutils/helpers/test_action_log.py index 15bbc5394086..669c5a6eedd3 100644 --- a/tests/sentry/testutils/helpers/test_action_log.py +++ b/tests/sentry/testutils/helpers/test_action_log.py @@ -3,6 +3,7 @@ import pytest from sentry.issues.action_log import SYSTEM_ACTOR, GroupActionActor, publish_action +from sentry.issues.action_log.read_metrics import ActivityReadEndpoint from sentry.issues.action_log.types import ( ActionSource, ArchiveAction, @@ -116,17 +117,27 @@ def test_nested_captures(self) -> None: class TestActionLogActivityEnabled(TestCase): def test_opens_and_closes_the_gate(self) -> None: - assert not should_serve_action_log_activity(self.project, self.user) + assert not should_serve_action_log_activity( + self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ) with action_log_activity_enabled(): - assert should_serve_action_log_activity(self.project, self.user) + assert should_serve_action_log_activity( + self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ) - assert not should_serve_action_log_activity(self.project, self.user) + assert not should_serve_action_log_activity( + self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ) @action_log_activity_enabled() def test_works_as_a_decorator(self) -> None: - assert should_serve_action_log_activity(self.project, self.user) + assert should_serve_action_log_activity( + self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ) @action_log_activity_enabled() def test_applies_to_every_project(self) -> None: - assert should_serve_action_log_activity(self.create_project(), self.user) + assert should_serve_action_log_activity( + self.create_project(), self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS + ) From bb005daf2a46e811299a51a6c4bd511799d7f74b Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Wed, 9 Sep 2026 10:39:09 -0700 Subject: [PATCH 2/3] Document metric recording behavior + use endpoint URL as tag --- src/sentry/api/helpers/group_index/update.py | 11 ++++--- src/sentry/issues/action_log/read_metrics.py | 29 +++++++++++-------- src/sentry/issues/derived/gate.py | 11 +++++-- .../issues/endpoints/group_activities.py | 11 ++++--- src/sentry/issues/endpoints/group_details.py | 11 ++++--- src/sentry/issues/endpoints/group_notes.py | 18 +++++------- .../issues/endpoints/group_notes_details.py | 14 ++++----- .../testutils/helpers/test_action_log.py | 21 ++++---------- 8 files changed, 60 insertions(+), 66 deletions(-) diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index ec2578d0639c..c8f746836f3d 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -33,9 +33,9 @@ resolve_action_source, ) from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadReason, ActivityReadResult, + activity_read_endpoint, record_activity_read, ) from sentry.issues.action_log.types import MergeIntoOtherAction @@ -790,15 +790,14 @@ def prepare_response( if len(group_list) == 1: if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release): group = group_list[0] - if should_serve_action_log_activity( - group.project, acting_user, endpoint=ActivityReadEndpoint.GROUP_INDEX_UPDATE - ): + endpoint = activity_read_endpoint(request) + if should_serve_action_log_activity(group.project, acting_user, endpoint=endpoint): action_log = GroupActionLogEntry.objects.get_actions_for_group( group, ACTIVITIES_COUNT - 1 ) if action_log: record_activity_read( - ActivityReadEndpoint.GROUP_INDEX_UPDATE, + endpoint, ActivityReadResult.GALE, ) result["activity"] = [ @@ -807,7 +806,7 @@ def prepare_response( ] else: record_activity_read( - ActivityReadEndpoint.GROUP_INDEX_UPDATE, + endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.EMPTY_LOG, ) diff --git a/src/sentry/issues/action_log/read_metrics.py b/src/sentry/issues/action_log/read_metrics.py index fdbb8248e652..af8c0febc521 100644 --- a/src/sentry/issues/action_log/read_metrics.py +++ b/src/sentry/issues/action_log/read_metrics.py @@ -1,20 +1,12 @@ from enum import StrEnum +from rest_framework.request import Request + from sentry.utils import metrics ACTIVITY_READ_METRIC = "issues.action_log.activity_read" -class ActivityReadEndpoint(StrEnum): - """Read paths that can serve Activity-shaped responses from the action log.""" - - GROUP_DETAILS = "group_details" - GROUP_ACTIVITIES = "group_activities" - GROUP_INDEX_UPDATE = "group_index_update" - GROUP_NOTES = "group_notes" - GROUP_NOTES_DETAILS = "group_notes_details" - - class ActivityReadResult(StrEnum): #: The log backed the response. GALE = "gale" @@ -31,12 +23,25 @@ class ActivityReadReason(StrEnum): EMPTY_LOG = "empty_log" +def activity_read_endpoint(request: Request) -> str: + """The route being served, for the `endpoint` tag.""" + if request.resolver_match is None or request.resolver_match.url_name is None: + return "unknown" + return request.resolver_match.url_name + + def record_activity_read( - endpoint: ActivityReadEndpoint, + endpoint: str, result: ActivityReadResult, reason: ActivityReadReason | None = None, ) -> None: - tags = {"endpoint": endpoint.value, "result": result.value} + """ + Record the outcome of one attempt to serve activity from the action log. + + Called once per read: by ``should_serve_action_log_activity`` when the gate closes, + otherwise by the caller once it knows whether the read produced anything. + """ + tags = {"endpoint": endpoint, "result": result.value} if reason is not None: tags["reason"] = reason.value metrics.incr(ACTIVITY_READ_METRIC, sample_rate=1.0, tags=tags) diff --git a/src/sentry/issues/derived/gate.py b/src/sentry/issues/derived/gate.py index 4aecc9952978..4870b0e9cc55 100644 --- a/src/sentry/issues/derived/gate.py +++ b/src/sentry/issues/derived/gate.py @@ -2,7 +2,6 @@ from sentry import features from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadReason, ActivityReadResult, record_activity_read, @@ -36,9 +35,15 @@ def should_serve_action_log_activity( project: Project, actor: User | RpcUser | AnonymousUser | None = None, *, - endpoint: ActivityReadEndpoint, + endpoint: str, ) -> bool: - """Whether the action log can back this project's Activity-shaped responses.""" + """ + Whether the action log can back this project's Activity-shaped responses. + + Records the read outcome itself when it returns False, because only it knows which + condition closed the gate. Returning True records nothing: the caller goes on to read + the log, so the caller reports whether that produced anything. + """ if not features.has("projects:issue-action-log-activity", project, actor=actor): record_activity_read(endpoint, ActivityReadResult.FLAG_OFF) return False diff --git a/src/sentry/issues/endpoints/group_activities.py b/src/sentry/issues/endpoints/group_activities.py index 8bb7a97abde3..4a0771c2de0c 100644 --- a/src/sentry/issues/endpoints/group_activities.py +++ b/src/sentry/issues/endpoints/group_activities.py @@ -10,9 +10,9 @@ from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry from sentry.constants import CELL_API_DEPRECATION_DATE from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadReason, ActivityReadResult, + activity_read_endpoint, record_activity_read, ) from sentry.issues.derived.gate import should_serve_action_log_activity @@ -39,13 +39,12 @@ def get(self, request: Request, group: Group) -> Response: """ Retrieve all the Activities for a Group """ - if should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_ACTIVITIES - ): + endpoint = activity_read_endpoint(request) + if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) if action_log: record_activity_read( - ActivityReadEndpoint.GROUP_ACTIVITIES, + endpoint, ActivityReadResult.GALE, ) serialized = serialize(action_log, request.user) @@ -56,7 +55,7 @@ def get(self, request: Request, group: Group) -> Response: } ) record_activity_read( - ActivityReadEndpoint.GROUP_ACTIVITIES, + endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.EMPTY_LOG, ) diff --git a/src/sentry/issues/endpoints/group_details.py b/src/sentry/issues/endpoints/group_details.py index 01eac46d100f..30adfda18455 100644 --- a/src/sentry/issues/endpoints/group_details.py +++ b/src/sentry/issues/endpoints/group_details.py @@ -48,9 +48,9 @@ resolve_action_source, ) from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadReason, ActivityReadResult, + activity_read_endpoint, record_activity_read, ) from sentry.issues.action_log.types import ViewAction @@ -348,13 +348,12 @@ def get(self, request: Request, group: Group) -> Response[GroupDetailsResponse]: } ) - if should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ): + endpoint = activity_read_endpoint(request) + if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) if action_log: record_activity_read( - ActivityReadEndpoint.GROUP_DETAILS, + endpoint, ActivityReadResult.GALE, ) # swap action log data in under the activity name @@ -364,7 +363,7 @@ def get(self, request: Request, group: Group) -> Response[GroupDetailsResponse]: ) else: record_activity_read( - ActivityReadEndpoint.GROUP_DETAILS, + endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.EMPTY_LOG, ) diff --git a/src/sentry/issues/endpoints/group_notes.py b/src/sentry/issues/endpoints/group_notes.py index 8cc701200912..4efd806d38b5 100644 --- a/src/sentry/issues/endpoints/group_notes.py +++ b/src/sentry/issues/endpoints/group_notes.py @@ -19,9 +19,9 @@ from sentry.constants import CELL_API_DEPRECATION_DATE from sentry.issues.action_log import action_context_scope, resolve_action_source from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadReason, ActivityReadResult, + activity_read_endpoint, record_activity_read, ) from sentry.issues.action_log.types import ( @@ -64,12 +64,11 @@ class GroupNotesEndpoint(GroupEndpoint): url_names=["sentry-api-0-group-notes"], ) def get(self, request: Request, group: Group) -> Response: - if should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES - ): + endpoint = activity_read_endpoint(request) + if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): # No empty-log fallback on this path: once the gate is open the log is # authoritative for comments, including when the group has none. - record_activity_read(ActivityReadEndpoint.GROUP_NOTES, ActivityReadResult.GALE) + record_activity_read(endpoint, ActivityReadResult.GALE) edit_entries = GroupActionLogEntry.objects.filter( group_id=group.id, type=GroupActionType.COMMENT_EDIT.value ).order_by("-date_added", "-id") @@ -198,18 +197,17 @@ def post(self, request: Request, group: Group) -> Response: sender="post", ) - if should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES - ): + endpoint = activity_read_endpoint(request) + if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): entry = GroupActionLogEntry.objects.filter( group_id=group.id, idempotency_key=activity_action_idempotency_key(activity), ).first() if entry: - record_activity_read(ActivityReadEndpoint.GROUP_NOTES, ActivityReadResult.GALE) + record_activity_read(endpoint, ActivityReadResult.GALE) return Response(serialize(entry, request.user), status=201) record_activity_read( - ActivityReadEndpoint.GROUP_NOTES, + endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.EMPTY_LOG, ) diff --git a/src/sentry/issues/endpoints/group_notes_details.py b/src/sentry/issues/endpoints/group_notes_details.py index 201888f6121b..059e1fdc5e92 100644 --- a/src/sentry/issues/endpoints/group_notes_details.py +++ b/src/sentry/issues/endpoints/group_notes_details.py @@ -23,8 +23,8 @@ resolve_action_source, ) from sentry.issues.action_log.read_metrics import ( - ActivityReadEndpoint, ActivityReadResult, + activity_read_endpoint, record_activity_read, ) from sentry.issues.action_log.types import CommentDeleteAction, CommentEditAction @@ -85,13 +85,14 @@ def delete(self, request: Request, group: Group, note_id: str) -> Response: group_id=group.id, idempotency_key=activity_action_idempotency_key(note), ).first() + endpoint = activity_read_endpoint(request) serve_from_log = should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES_DETAILS + group.project, request.user, endpoint=endpoint ) if serve_from_log: # The log is authoritative for existence whether or not the entry is # there: a missing one means the comment is already gone. - record_activity_read(ActivityReadEndpoint.GROUP_NOTES_DETAILS, ActivityReadResult.GALE) + record_activity_read(endpoint, ActivityReadResult.GALE) if original_comment_log_action is None and serve_from_log: raise ResourceDoesNotExist @@ -174,13 +175,12 @@ def put(self, request: Request, group: Group, note_id: str) -> Response: group_id=group.id, idempotency_key=activity_action_idempotency_key(note), ).first() + endpoint = activity_read_endpoint(request) serve_from_log = should_serve_action_log_activity( - group.project, request.user, endpoint=ActivityReadEndpoint.GROUP_NOTES_DETAILS + group.project, request.user, endpoint=endpoint ) if serve_from_log: - record_activity_read( - ActivityReadEndpoint.GROUP_NOTES_DETAILS, ActivityReadResult.GALE - ) + record_activity_read(endpoint, ActivityReadResult.GALE) if original_comment_log_action is None and serve_from_log: raise ResourceDoesNotExist diff --git a/tests/sentry/testutils/helpers/test_action_log.py b/tests/sentry/testutils/helpers/test_action_log.py index 669c5a6eedd3..69b1f3d5e13b 100644 --- a/tests/sentry/testutils/helpers/test_action_log.py +++ b/tests/sentry/testutils/helpers/test_action_log.py @@ -3,7 +3,6 @@ import pytest from sentry.issues.action_log import SYSTEM_ACTOR, GroupActionActor, publish_action -from sentry.issues.action_log.read_metrics import ActivityReadEndpoint from sentry.issues.action_log.types import ( ActionSource, ArchiveAction, @@ -117,27 +116,17 @@ def test_nested_captures(self) -> None: class TestActionLogActivityEnabled(TestCase): def test_opens_and_closes_the_gate(self) -> None: - assert not should_serve_action_log_activity( - self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ) + assert not should_serve_action_log_activity(self.project, self.user, endpoint="test") with action_log_activity_enabled(): - assert should_serve_action_log_activity( - self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ) + assert should_serve_action_log_activity(self.project, self.user, endpoint="test") - assert not should_serve_action_log_activity( - self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ) + assert not should_serve_action_log_activity(self.project, self.user, endpoint="test") @action_log_activity_enabled() def test_works_as_a_decorator(self) -> None: - assert should_serve_action_log_activity( - self.project, self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ) + assert should_serve_action_log_activity(self.project, self.user, endpoint="test") @action_log_activity_enabled() def test_applies_to_every_project(self) -> None: - assert should_serve_action_log_activity( - self.create_project(), self.user, endpoint=ActivityReadEndpoint.GROUP_DETAILS - ) + assert should_serve_action_log_activity(self.create_project(), self.user, endpoint="test") From b70458428871caadf16aa2ed1896b0302ffab06c Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Wed, 9 Sep 2026 11:58:58 -0700 Subject: [PATCH 3/3] Implement `get_serialized_activity_items` --- src/sentry/api/helpers/group_index/update.py | 44 +++++-------------- .../serializers/models/groupactionlogentry.py | 39 ++++++++++++++++ .../issues/endpoints/group_activities.py | 39 +++------------- src/sentry/issues/endpoints/group_details.py | 40 ++++------------- tests/sentry/api/helpers/test_group_index.py | 14 +++--- 5 files changed, 74 insertions(+), 102 deletions(-) diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index c8f746836f3d..b725464c375e 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -22,7 +22,7 @@ from sentry.analytics.events.manual_issue_assignment import ManualIssueAssignment from sentry.api.serializers import serialize from sentry.api.serializers.models.actor import ActorSerializer, ActorSerializerResponse -from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry +from sentry.api.serializers.models.groupactionlogentry import get_serialized_activity_items from sentry.hybridcloud.rpc import coerce_id_from from sentry.integrations.tasks.kick_off_status_syncs import kick_off_status_syncs from sentry.issues.action_log import ( @@ -32,18 +32,11 @@ resolve_action_actor, resolve_action_source, ) -from sentry.issues.action_log.read_metrics import ( - ActivityReadReason, - ActivityReadResult, - activity_read_endpoint, - record_activity_read, -) +from sentry.issues.action_log.read_metrics import activity_read_endpoint from sentry.issues.action_log.types import MergeIntoOtherAction -from sentry.issues.derived.gate import should_serve_action_log_activity from sentry.issues.grouptype import GroupCategory from sentry.issues.ignored import handle_archived_until_escalating, handle_ignored from sentry.issues.merge import MergedGroup, handle_merge -from sentry.issues.models.groupactionlogentry import GroupActionLogEntry from sentry.issues.priority import update_priority from sentry.issues.status_change import handle_status_update, infer_substatus from sentry.issues.update_inbox import update_inbox @@ -790,31 +783,14 @@ def prepare_response( if len(group_list) == 1: if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release): group = group_list[0] - endpoint = activity_read_endpoint(request) - if should_serve_action_log_activity(group.project, acting_user, endpoint=endpoint): - action_log = GroupActionLogEntry.objects.get_actions_for_group( - group, ACTIVITIES_COUNT - 1 - ) - if action_log: - record_activity_read( - endpoint, - ActivityReadResult.GALE, - ) - result["activity"] = [ - *serialize(action_log, acting_user), - serialize_first_seen_entry(group), - ] - else: - record_activity_read( - endpoint, - ActivityReadResult.FELL_BACK, - ActivityReadReason.EMPTY_LOG, - ) - logger.info( - "group_index.groupactionlogentry.not_found", - extra={"group_id": group.id}, - ) - + activity_items = get_serialized_activity_items( + group, + acting_user, + endpoint=activity_read_endpoint(request), + limit=ACTIVITIES_COUNT - 1, + ) + if activity_items is not None: + result["activity"] = activity_items else: result["activity"] = serialize( Activity.objects.get_activities_for_group( diff --git a/src/sentry/api/serializers/models/groupactionlogentry.py b/src/sentry/api/serializers/models/groupactionlogentry.py index e1f7808068ce..5d21df088c3e 100644 --- a/src/sentry/api/serializers/models/groupactionlogentry.py +++ b/src/sentry/api/serializers/models/groupactionlogentry.py @@ -1,3 +1,4 @@ +import logging from collections.abc import Mapping, Sequence from datetime import datetime from typing import TYPE_CHECKING, Any, TypedDict @@ -7,6 +8,11 @@ from sentry.api.serializers import Serializer, register, serialize from sentry.api.serializers.models.activity import _ActivitySentryAppEmbed from sentry.api.serializers.models.commit import CommitWithReleaseSerializer +from sentry.issues.action_log.read_metrics import ( + ActivityReadReason, + ActivityReadResult, + record_activity_read, +) from sentry.issues.action_log.types import ( ACTION_TYPES_WITH_COMMIT_DATA, COMMIT_ACTION_TYPES, @@ -15,6 +21,7 @@ GroupActionType, GroupActorType, ) +from sentry.issues.derived.gate import should_serve_action_log_activity from sentry.issues.models.groupactionlogentry import GroupActionLogEntry from sentry.models.commit import Commit from sentry.models.pullrequest import PullRequest @@ -35,6 +42,8 @@ if TYPE_CHECKING: from sentry.models.group import Group +logger = logging.getLogger(__name__) + class GroupActionLogEntrySerializerResponse(TypedDict): id: str @@ -83,6 +92,36 @@ def _serialized_id(obj: GroupActionLogEntry) -> str: return str(obj.id) +def get_serialized_activity_items( + group: "Group", + user: User | RpcUser | AnonymousUser | None, + *, + endpoint: str, + limit: int = 99, +) -> list[Any] | None: + """ + Activity-shaped items for a group, read from the action log. + + Returns None when the log can't back the response — either the gate is closed or it's + open and the log is empty — and the caller should fall back to Activity. Reports the + read outcome in both cases, so callers don't have to. + """ + if not should_serve_action_log_activity(group.project, user, endpoint=endpoint): + return None + + action_log = GroupActionLogEntry.objects.get_actions_for_group(group, limit) + if not action_log: + record_activity_read(endpoint, ActivityReadResult.FELL_BACK, ActivityReadReason.EMPTY_LOG) + logger.info( + "issues.action_log.activity_read.not_found", + extra={"endpoint": endpoint, "group_id": group.id}, + ) + return None + + record_activity_read(endpoint, ActivityReadResult.GALE) + return [*serialize(action_log, user), serialize_first_seen_entry(group)] + + @register(GroupActionLogEntry) class GroupActionLogEntrySerializer(Serializer): def get_attrs( diff --git a/src/sentry/issues/endpoints/group_activities.py b/src/sentry/issues/endpoints/group_activities.py index 4a0771c2de0c..1dc4e3d649eb 100644 --- a/src/sentry/issues/endpoints/group_activities.py +++ b/src/sentry/issues/endpoints/group_activities.py @@ -7,17 +7,10 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.helpers.deprecation import deprecated from sentry.api.serializers import serialize -from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry +from sentry.api.serializers.models.groupactionlogentry import get_serialized_activity_items from sentry.constants import CELL_API_DEPRECATION_DATE -from sentry.issues.action_log.read_metrics import ( - ActivityReadReason, - ActivityReadResult, - activity_read_endpoint, - record_activity_read, -) -from sentry.issues.derived.gate import should_serve_action_log_activity +from sentry.issues.action_log.read_metrics import activity_read_endpoint from sentry.issues.endpoints.bases.group import GroupEndpoint -from sentry.issues.models.groupactionlogentry import GroupActionLogEntry from sentry.models.activity import Activity from sentry.models.group import Group @@ -39,29 +32,11 @@ def get(self, request: Request, group: Group) -> Response: """ Retrieve all the Activities for a Group """ - endpoint = activity_read_endpoint(request) - if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): - action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) - if action_log: - record_activity_read( - endpoint, - ActivityReadResult.GALE, - ) - serialized = serialize(action_log, request.user) - serialized.append(serialize_first_seen_entry(group)) - return Response( - { - "activity": serialized, - } - ) - record_activity_read( - endpoint, - ActivityReadResult.FELL_BACK, - ActivityReadReason.EMPTY_LOG, - ) - logger.info( - "group_activities.groupactionlogentry.not_found", extra={"group_id": group.id} - ) + activity_items = get_serialized_activity_items( + group, request.user, endpoint=activity_read_endpoint(request) + ) + if activity_items is not None: + return Response({"activity": activity_items}) activity = Activity.objects.get_activities_for_group(group, num=100) return Response( diff --git a/src/sentry/issues/endpoints/group_details.py b/src/sentry/issues/endpoints/group_details.py index 30adfda18455..22145766eb86 100644 --- a/src/sentry/issues/endpoints/group_details.py +++ b/src/sentry/issues/endpoints/group_details.py @@ -27,7 +27,7 @@ from sentry.api.helpers.group_index.validators import GroupValidator from sentry.api.serializers import GroupSerializer, GroupSerializerSnuba, serialize from sentry.api.serializers.models.group import BaseGroupSerializerResponse, GroupDetailsResponse -from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry +from sentry.api.serializers.models.groupactionlogentry import get_serialized_activity_items from sentry.apidocs.constants import ( RESPONSE_ACCEPTED, RESPONSE_BAD_REQUEST, @@ -47,12 +47,7 @@ resolve_action_actor, resolve_action_source, ) -from sentry.issues.action_log.read_metrics import ( - ActivityReadReason, - ActivityReadResult, - activity_read_endpoint, - record_activity_read, -) +from sentry.issues.action_log.read_metrics import activity_read_endpoint from sentry.issues.action_log.types import ViewAction from sentry.issues.constants import ( ISSUE_VIEW_CACHE_KEY_TTL, @@ -60,10 +55,9 @@ get_issue_tsdb_group_model, ) from sentry.issues.derived.check import record_status_consistency -from sentry.issues.derived.gate import derived_should_be_correct, should_serve_action_log_activity +from sentry.issues.derived.gate import derived_should_be_correct from sentry.issues.endpoints.bases.group import GroupEndpoint from sentry.issues.escalating.escalating_group_forecast import EscalatingGroupForecast -from sentry.issues.models.groupactionlogentry import GroupActionLogEntry from sentry.issues.models.groupderiveddata import GroupDerivedData from sentry.models.activity import Activity from sentry.models.eventattachment import EventAttachment @@ -348,28 +342,12 @@ def get(self, request: Request, group: Group) -> Response[GroupDetailsResponse]: } ) - endpoint = activity_read_endpoint(request) - if should_serve_action_log_activity(group.project, request.user, endpoint=endpoint): - action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99) - if action_log: - record_activity_read( - endpoint, - ActivityReadResult.GALE, - ) - # swap action log data in under the activity name - first_seen_entry = cast(dict[str, Any], serialize_first_seen_entry(group)) - data.update( - {"activity": [*serialize(action_log, request.user), first_seen_entry]} - ) - else: - record_activity_read( - endpoint, - ActivityReadResult.FELL_BACK, - ActivityReadReason.EMPTY_LOG, - ) - logger.info( - "group_details.groupactionlogentry.not_found", extra={"group_id": group.id} - ) + # swap action log data in under the activity name + activity_items = get_serialized_activity_items( + group, request.user, endpoint=activity_read_endpoint(request) + ) + if activity_items is not None: + data.update({"activity": activity_items}) if "stats" not in collapse: hourly_stats, daily_stats = self.__group_hourly_daily_stats(group, environment_ids) diff --git a/tests/sentry/api/helpers/test_group_index.py b/tests/sentry/api/helpers/test_group_index.py index d9a640b77846..750341a27a95 100644 --- a/tests/sentry/api/helpers/test_group_index.py +++ b/tests/sentry/api/helpers/test_group_index.py @@ -761,9 +761,10 @@ def test_resolve_in_next_release_activity_from_action_log(self) -> None: assert "set_resolved" in [entry["type"] for entry in activity] assert activity[-1]["id"] == "0" - def test_resolve_in_next_release_no_activity_when_action_log_is_empty(self) -> None: + def test_resolve_in_next_release_falls_back_when_action_log_is_empty(self) -> None: # A gated project can still read an empty log: the GALE write for this - # resolve goes through an outbox that may not have drained yet. + # resolve goes through an outbox that may not have drained yet. Fall back to + # Activity rather than omitting the key, matching the other feed endpoints. self.create_release(project=self.project, version="test@1.0.0.0") group = self.create_group(status=GroupStatus.UNRESOLVED) @@ -775,15 +776,18 @@ def test_resolve_in_next_release_no_activity_when_action_log_is_empty(self) -> N with ( action_log_activity_enabled(), patch.object(GroupActionLogEntry.objects, "get_actions_for_group", return_value=[]), - self.assertLogs("sentry.api.helpers.group_index.update", level="INFO") as logs, + self.assertLogs( + "sentry.api.serializers.models.groupactionlogentry", level="INFO" + ) as logs, ): response = update_groups(request, group_list) assert any( - record.message == "group_index.groupactionlogentry.not_found" for record in logs.records + record.message == "issues.action_log.activity_read.not_found" for record in logs.records ) assert response is not None - assert "activity" not in response.data + # the log read is patched to return nothing, so anything here came from Activity + assert "activity" in response.data def test_resolve_in_next_release_ignores_action_log_when_disabled(self) -> None: # With the gate closed the log may cover only part of this project's history,