Skip to content

feat(issues): Add action log instrumentation with source attribution#116244

Closed
yuvmen wants to merge 10 commits into
masterfrom
yuvmen/issue-action-log-instrumentation
Closed

feat(issues): Add action log instrumentation with source attribution#116244
yuvmen wants to merge 10 commits into
masterfrom
yuvmen/issue-action-log-instrumentation

Conversation

@yuvmen
Copy link
Copy Markdown
Member

@yuvmen yuvmen commented May 26, 2026

This sets up the instrumentation that currently doesnt actually write anywhere other than log and metrics, but lays the groundwork making it easy to just swap in the actual recording when we are ready.

  • Adds resolve_action_source(request) to detect caller surface: MCP agents (gated on application_id), Seer (via X-Seer-Referrer or RPC auth), web UI, sentry-cli, generic API
  • Adds publish_action() no-op logger for issue mutations — will be wired to record_group_action() when feat(issues): Add GroupActionLogEntry #115771 lands
  • Instruments core write paths at mutation sites (not request intent), following the same gates as Activity creation:
    • resolve, unresolve, archive (status handlers)
    • assign, priority, bookmark, subscribe, merge, mark_reviewed (prepare_response handlers)
    • view (GroupDetailsEndpoint.get)
    • trigger_autofix (GroupAutofixEndpoint.post)

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 26, 2026
@yuvmen yuvmen force-pushed the yuvmen/issue-action-log-instrumentation branch from eff9ad6 to f068622 Compare May 26, 2026 21:44
Add `resolve_action_source(request)` to detect the caller surface (MCP
agents, Seer, web UI, sentry-cli, API) and `publish_action()` as a
no-op logger for issue mutations. Instrument the core write paths:

- resolve/unresolve/archive via update_groups handlers
- assign, priority, bookmark, subscribe, merge, mark_reviewed
- view (GroupDetailsEndpoint.get)
- trigger_autofix (GroupAutofixEndpoint.post)

Actions are only emitted when a mutation actually occurs, following the
same gates as Activity creation. Source is resolved once per request and
threaded through to each handler.
@yuvmen yuvmen force-pushed the yuvmen/issue-action-log-instrumentation branch from f068622 to a50d729 Compare May 26, 2026 22:22
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 26, 2026

ID-1570

Import ApiClient and ApiError directly instead of using the client
module as a namespace, which mypy cannot resolve attributes on.
from sentry.snuba.referrer import Referrer

logger = logging.getLogger(__name__)
client = ApiClient()
Copy link
Copy Markdown
Member Author

@yuvmen yuvmen May 26, 2026

Choose a reason for hiding this comment

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

was getting typing errors on all these tool files, have no idea why when main has them go through fine. Eventually gave up and fixed the typing issue here, seems like import was too ambigious
https://github.com/getsentry/sentry/actions/runs/26478542633/job/77970203553

yuvmen added 2 commits May 26, 2026 16:05
Derive ActorType (USER/SYSTEM) from actor_id to align with Kyle's
GroupActionLogEntry model which stores actor_type as a separate field.
Aligns with the Issue Action Log spec requirement that every event has a
stable client-supplied id for deduplication.
Comment thread src/sentry/issues/action_log.py
@yuvmen yuvmen marked this pull request as ready for review May 26, 2026 23:43
@yuvmen yuvmen requested review from a team as code owners May 26, 2026 23:43
Comment thread src/sentry/api/helpers/group_index/update.py
Comment thread src/sentry/api/helpers/group_index/update.py
Comment thread src/sentry/api/helpers/group_index/update.py Outdated
Comment thread src/sentry/api/helpers/group_index/update.py
- Priority: only emit SET_PRIORITY when group priority actually differs
- Assign: snapshot assignees before/after, skip when unchanged
- Merge: emit MERGE_FROM_OTHER on survivor, MERGE_INTO_OTHER on each
  absorbed group with counterpart metadata
- Mark reviewed: only emit for groups that were actually in inbox
- Add integration tests for all mutation paths including no-op cases
Comment thread src/sentry/api/helpers/group_index/update.py Outdated
Read parent/children from handle_merge return value instead of assuming
group_list[0] is the primary — handle_merge sorts by first_seen, event
count, and id to pick the survivor.
Comment thread src/sentry/issues/action_log.py
Comment thread src/sentry/api/helpers/group_index/update.py Outdated
- Cache negative _get_mcp_application_id result to avoid DB query per
  request when no MCP app exists
- Only emit unresolve/archive actions for groups whose status actually
  changed, matching the queryset.exclude(status=new_status) gate
Comment thread src/sentry/api/helpers/group_index/update.py Outdated
Comment thread src/sentry/issues/action_log.py Outdated
- Remove unused MCP_CLIENT_ID_HEADER constant
- Move changed_group_ids query inside transaction to avoid race
- Add tests for archive/unresolve already-at-target-status skipping
- Add test for unassign with existing assignee emitting action
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 87798be. Configure here.

Comment thread src/sentry/api/helpers/group_index/update.py Outdated
Comment thread src/sentry/api/helpers/group_index/update.py
yuvmen added 2 commits May 27, 2026 10:06
…dler

- Bookmark: snapshot existing bookmarks, only log for newly created
- Subscribe: snapshot current subscription state, skip when unchanged
- Merge: guard against mocked merge result in tests (isinstance check)
- Add tests for bookmark/subscribe idempotency
@yuvmen
Copy link
Copy Markdown
Member Author

yuvmen commented May 27, 2026

Did some rethinking and I think this ended up being kind of clusmy, I tried to avoid getting tied to the way Activity is being created and try to unify some paths, but ended up needing a bunch of logic to avoid duplication of reporting. Going to open a new PR to clean up all the noise here and start fresh sticking closer to where we create Activity.

@yuvmen yuvmen closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant