Skip to content

feat(issues): Model Sentry App and org callers as distinct action-log actors#117354

Merged
yuvmen merged 4 commits into
masterfrom
yuvmen/action-log-actor-types
Jun 12, 2026
Merged

feat(issues): Model Sentry App and org callers as distinct action-log actors#117354
yuvmen merged 4 commits into
masterfrom
yuvmen/action-log-actor-types

Conversation

@yuvmen

@yuvmen yuvmen commented Jun 10, 2026

Copy link
Copy Markdown
Member

Problem

A token-authenticated request can come from a Sentry App (authenticating as its proxy user, a real User row with is_sentry_app=True) or an org-scoped token. The action-log boundary recorded all of these as USER(proxy_user_id) — a machine mislabeled as a human. The distinction between custom-integration vs public-integration vs org-token vs personal-user-token is about who made the call, not how (they all arrive over source=api), so it belongs on the actor axis, not source.

Change

  • Extend GroupActorType with SENTRY_APP (actor_id = SentryApp id) and ORG (actor_id = organization id), plus GroupActionActor.sentry_app(...) / .org(...) constructors.

  • Add a resolve_action_actor(request) boundary helper mirroring resolve_action_source(request). Region-side request.auth is an AuthenticatedToken whose kind drives classification:

    caller actor
    org_auth_token / legacy api_key ORG(organization_id)
    api_token with application_id SENTRY_APP(sentry_app_id) (resolved via app_service.get_by_application_id)
    api_token w/o application_id, or session USER(user_id)
    no authenticated caller SYSTEM
  • Endpoints now set actor=resolve_action_actor(request) instead of hand-rolling the user.is_authenticated ? USER : SYSTEM check (group_details, group_integration_details ×3, group_ai_autofix, bulk status update).

  • Add actor_type as a low-cardinality metric dimension on issues.action_log.

Notes

  • internal vs public app is the same SENTRY_APP type — internal is just SentryApp.status == INTERNAL, derived from actor_id at read time. No separate type.
  • The specific id stays in the row (actor_id), not in a metric tag.
  • This subsumes splitting the api source bucket — source stays the channel, the caller question is answered by actor_type.

Tests

TestResolveActionActor covers user (session + personal token), Sentry App token (real app_service resolution), org-auth-token, legacy api-key, and unauthenticated → SYSTEM.

Fixes ID-1620

… actors

A token-authenticated request can come from a Sentry App (its proxy user) or
an org-scoped token, but the boundary code recorded all of them as
USER(proxy_user_id) -- a machine mislabeled as a human. The real distinction
is *who* made the call, not *how* (all arrive over source=api), so it belongs
on the actor axis.

Extend GroupActorType with SENTRY_APP (actor_id = SentryApp id) and ORG
(actor_id = organization id), and add a resolve_action_actor(request) boundary
helper mirroring resolve_action_source: org/legacy tokens -> ORG, integration
tokens -> SENTRY_APP (resolved via application_id), everything else
authenticated -> USER, else SYSTEM. internal-vs-public app is derived from
SentryApp.status at read time, so both map to SENTRY_APP.

Endpoints now set actor=resolve_action_actor(request) instead of hand-rolling
the user/SYSTEM check. Add actor_type as a low-cardinality metric dimension.

Fixes ID-1620
@linear-code

linear-code Bot commented Jun 10, 2026

Copy link
Copy Markdown

ID-1620

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 10, 2026
Gate the app lookup on request.user.is_sentry_app rather than application_id
alone. An OAuth client acting on behalf of a user (e.g. the MCP) also carries
an application_id but authenticates as the real user, so it must resolve to
USER -- and gating on application_id would run an app lookup (uncached on a
None result) on every such request. is_sentry_app is true only for an app's
proxy user, so only genuine app-as-itself tokens pay the cached lookup.
@yuvmen yuvmen force-pushed the yuvmen/action-log-actor-types branch from 0b0eb91 to 4e3e916 Compare June 10, 2026 21:11
@yuvmen yuvmen marked this pull request as ready for review June 11, 2026 16:55
@yuvmen yuvmen requested review from a team as code owners June 11, 2026 16:55
@yuvmen yuvmen requested a review from kcons June 11, 2026 16:55

@ceorourke ceorourke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

makes sense 👍

return ActionSource.API


def resolve_action_actor(request: Request) -> GroupActionActor:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

big fan of this.

Comment thread src/sentry/issues/action_log/base.py Outdated
return GroupActionActor.user(user_id)

user = getattr(request, "user", None)
if user is not None and getattr(user, "is_authenticated", False):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we have to lean on getattr this much? I recall resolving some mypy confusion my just using the attribute directly, but maybe I was contextually lucky?
My brain just signals warning lights whenever I see getattr as it's as dynamic as possible, and mypy usually gives up and gives you an Any.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmm no, I realize now I just went with the AI but it went overboard here and I didnt notice. We dont need many of these, and actually even the places where type isnt obvious we can use isinstance which is prbably better, gonna refactor

yuvmen added 2 commits June 11, 2026 15:34
…ion_actor

Narrow request.auth to AuthenticatedToken and request.user to User/RpcUser so
mypy type-checks the field accesses instead of getting Any from getattr. No
behavior change. app_service stays a function-local import to avoid a circular
import.
…ctor-types

# Conflicts:
#	tests/sentry/issues/test_action_log.py
@yuvmen yuvmen merged commit 39af1c7 into master Jun 12, 2026
70 checks passed
@yuvmen yuvmen deleted the yuvmen/action-log-actor-types branch June 12, 2026 16:06
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.

3 participants