-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(issues): Record Seer events for display in the issue activity timeline #115486
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
|
|
||
| from sentry import features | ||
| from sentry.constants import DataCategory | ||
| from sentry.models.activity import Activity | ||
| from sentry.models.group import Group | ||
| from sentry.models.organization import Organization | ||
| from sentry.seer.agent.client import SeerAgentClient | ||
|
|
@@ -43,17 +44,18 @@ | |
| from sentry.sentry_apps.metrics import SentryAppEventType | ||
| from sentry.tasks.base import instrumented_task | ||
| from sentry.taskworker.namespaces import seer_tasks | ||
| from sentry.types.activity import ActivityType | ||
| from sentry.users.models.user import User | ||
| from sentry.users.services.user import RpcUser | ||
|
|
||
| SEER_OPERATOR_AUTOFIX_UPDATE_EVENTS = { | ||
| SentryAppEventType.SEER_ROOT_CAUSE_STARTED, | ||
| SentryAppEventType.SEER_ROOT_CAUSE_COMPLETED, | ||
| SentryAppEventType.SEER_SOLUTION_STARTED, | ||
| SentryAppEventType.SEER_SOLUTION_COMPLETED, | ||
| SentryAppEventType.SEER_CODING_STARTED, | ||
| SentryAppEventType.SEER_CODING_COMPLETED, | ||
| SentryAppEventType.SEER_PR_CREATED, | ||
| SEER_EVENT_TO_ACTIVITY_TYPE: dict[SentryAppEventType, ActivityType] = { | ||
| SentryAppEventType.SEER_ROOT_CAUSE_STARTED: ActivityType.SEER_RCA_STARTED, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kcons just a heads up on the naming -- easy enough to update these later, but wanted to give a heads up. |
||
| SentryAppEventType.SEER_ROOT_CAUSE_COMPLETED: ActivityType.SEER_RCA_COMPLETED, | ||
| SentryAppEventType.SEER_SOLUTION_STARTED: ActivityType.SEER_SOLUTION_STARTED, | ||
| SentryAppEventType.SEER_SOLUTION_COMPLETED: ActivityType.SEER_SOLUTION_COMPLETED, | ||
| SentryAppEventType.SEER_CODING_STARTED: ActivityType.SEER_CODING_STARTED, | ||
| SentryAppEventType.SEER_CODING_COMPLETED: ActivityType.SEER_CODING_COMPLETED, | ||
| SentryAppEventType.SEER_PR_CREATED: ActivityType.SEER_PR_CREATED, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want PR merged eventually but looks like we're generally missing the plumbing for that so it can come later. |
||
| } | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
@@ -690,6 +692,46 @@ def trigger_agent( | |
| return run_id | ||
|
|
||
|
|
||
| def _create_seer_activity( | ||
| group: Group, | ||
| event_type: SentryAppEventType, | ||
| event_payload: dict[str, Any], | ||
| ) -> None: | ||
| activity_type = SEER_EVENT_TO_ACTIVITY_TYPE.get(event_type) | ||
| if not activity_type: | ||
| return | ||
|
|
||
| organization = group.project.organization | ||
| if not features.has("organizations:seer-activity-timeline", organization): | ||
| return | ||
|
|
||
| run_id = event_payload.get("run_id") | ||
|
|
||
| activity_data: dict[str, Any] = {} | ||
| if run_id is not None: | ||
| activity_data["run_id"] = run_id | ||
|
|
||
| if event_type == SentryAppEventType.SEER_ROOT_CAUSE_COMPLETED: | ||
| if "root_cause" in event_payload: | ||
| activity_data["root_cause"] = event_payload["root_cause"] | ||
| elif event_type == SentryAppEventType.SEER_SOLUTION_COMPLETED: | ||
| if "solution" in event_payload: | ||
| activity_data["solution"] = event_payload["solution"] | ||
| elif event_type == SentryAppEventType.SEER_CODING_COMPLETED: | ||
| if "changes" in event_payload: | ||
| activity_data["changes"] = event_payload["changes"] | ||
| elif event_type == SentryAppEventType.SEER_PR_CREATED: | ||
| if "pull_requests" in event_payload: | ||
| activity_data["pull_requests"] = event_payload["pull_requests"] | ||
|
Comment on lines
+714
to
+725
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably also just do these in one line: Could we normalize the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean normalize the event payload Seer-side when the events get sent? I'm supportive of that - @chromy any thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, although i think we can reduce this down to 1 line of code per
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yea good point, will clean that up thanks |
||
|
|
||
| Activity.objects.create_group_activity( | ||
| group, | ||
| activity_type, | ||
| data=activity_data if activity_data else None, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Open to thoughts on exactly what metadata we want to include on the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would suggest to merge this as is then figure it out as we want things.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 ^^^ Also agree that having a referrer here would be nice.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah agreed - I was thinking something similar on @Jesse-Box's draft of activity types in order to collapse PR triggered / PR created (and similar states) into a single thing which is further differentiated by a referrer |
||
| send_notification=False, | ||
| ) | ||
|
|
||
|
|
||
| @instrumented_task( | ||
| name="sentry.seer.entrypoints.operator.process_autofix_updates", | ||
| namespace=seer_tasks, | ||
|
|
@@ -724,7 +766,7 @@ def process_autofix_updates( | |
| lifecycle.record_failure(failure_reason="missing_identifiers") | ||
| return | ||
|
|
||
| if event_type not in SEER_OPERATOR_AUTOFIX_UPDATE_EVENTS: | ||
| if event_type not in SEER_EVENT_TO_ACTIVITY_TYPE: | ||
| lifecycle.record_halt(halt_reason="skipped") | ||
| return | ||
|
|
||
|
|
@@ -740,6 +782,18 @@ def process_autofix_updates( | |
| lifecycle.record_halt(halt_reason="no_operator_access") | ||
| return | ||
|
|
||
| try: | ||
| _create_seer_activity(group, event_type, event_payload) | ||
| except Exception: | ||
| logger.exception( | ||
| "seer.activity_creation_failed", | ||
| extra={ | ||
| "group_id": group_id, | ||
| "run_id": run_id, | ||
| "event_type": str(event_type), | ||
| }, | ||
| ) | ||
|
|
||
| for entrypoint_key, entrypoint_cls in autofix_entrypoint_registry.registrations.items(): | ||
| logging_ctx = { | ||
| "organization_id": organization.id, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an org-level feature flag to gate for initial rollout