Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ def register_temporary_features(manager: FeatureManager) -> None:
manager.add("organizations:seer-slack-explorer", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Show Seer run ID in Slack notification footers
manager.add("organizations:seer-run-id-in-slack", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable Seer activity events in the issue activity timeline
manager.add("organizations:seer-activity-timeline", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
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.

Using an org-level feature flag to gate for initial rollout

# Gate outbox-based mirroring of SeerRun records to Seer
manager.add("organizations:seer-run-mirror", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable search query attribute validation
Expand Down
72 changes: 63 additions & 9 deletions src/sentry/seer/entrypoints/operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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__)
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should probably also just do these in one line:

    elif event_type == SentryAppEventType.SEER_SOLUTION_COMPLETED:
        activity_data["solution"] = event_payload.get("solution")

Could we normalize the event_payload here so that these are all a generic data or seer_result or something? that way we can just have this be one line of code: activity_data["result"] = event_payload.get("seer_result") kind of thing.

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.

You mean normalize the event payload Seer-side when the events get sent? I'm supportive of that - @chromy any thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 if statement w/o normalizing from seer.

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.

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,
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.

Open to thoughts on exactly what metadata we want to include on the Activitys for each of these event types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.
One thing that might be useful is the referrer (what caused Autofix to run) but off the top of my head that's the only thing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 ^^^

Also agree that having a referrer here would be nice.

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.

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,
Expand Down Expand Up @@ -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

Expand All @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions src/sentry/types/activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ class ActivityType(Enum):
DELETED_ATTACHMENT = 27
REFERENCED_IN_COMMIT = 28

SEER_RCA_STARTED = 29
SEER_RCA_COMPLETED = 30
SEER_SOLUTION_STARTED = 31
SEER_SOLUTION_COMPLETED = 32
SEER_CODING_STARTED = 33
SEER_CODING_COMPLETED = 34
SEER_PR_CREATED = 35


# Warning: This must remain in this EXACT order.
CHOICES = tuple(
Expand Down Expand Up @@ -67,6 +75,13 @@ class ActivityType(Enum):
ActivityType.SET_PRIORITY, # 26
ActivityType.DELETED_ATTACHMENT, # 27
ActivityType.REFERENCED_IN_COMMIT, # 28
ActivityType.SEER_RCA_STARTED, # 29
ActivityType.SEER_RCA_COMPLETED, # 30
ActivityType.SEER_SOLUTION_STARTED, # 31
ActivityType.SEER_SOLUTION_COMPLETED, # 32
ActivityType.SEER_CODING_STARTED, # 33
ActivityType.SEER_CODING_COMPLETED, # 34
ActivityType.SEER_PR_CREATED, # 35
]
)

Expand Down
99 changes: 99 additions & 0 deletions tests/sentry/seer/entrypoints/test_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from rest_framework.response import Response

from fixtures.seer.webhooks import MOCK_RUN_ID
from sentry.models.activity import Activity
from sentry.models.organization import Organization
from sentry.seer.agent.client_models import (
CodingAgentState,
Expand All @@ -24,6 +25,7 @@
from sentry.seer.autofix.utils import CodingAgentState as LegacyCodingAgentState
from sentry.seer.entrypoints.operator import (
AUTOFIX_FALLBACK_CAUSE_ID,
SEER_EVENT_TO_ACTIVITY_TYPE,
SeerAgentOperator,
SeerAutofixOperator,
SeerOperatorCompletionHook,
Expand All @@ -41,6 +43,7 @@
from sentry.sentry_apps.metrics import SentryAppEventType
from sentry.testutils.asserts import assert_failure_metric
from sentry.testutils.cases import TestCase
from sentry.types.activity import ActivityType


class MockCachePayload(TypedDict):
Expand Down Expand Up @@ -812,6 +815,102 @@ def test_can_trigger_autofix_returns_false_without_quota(self, mock_quota):
):
assert SeerAutofixOperator.can_trigger_autofix(group=self.group) is False

@patch.object(SeerAutofixOperator, "has_access", return_value=True)
def test_seer_event_creates_activity(self, _mock_has_access):
event_payload = {
"run_id": MOCK_RUN_ID,
"group_id": self.group.id,
"root_cause": {
"description": "Test root cause",
"steps": [{"title": "Step 1"}],
},
}

with self.feature("organizations:seer-activity-timeline"):
process_autofix_updates(
event_type=SentryAppEventType.SEER_ROOT_CAUSE_COMPLETED,
event_payload=event_payload,
organization_id=self.organization.id,
)

activity = Activity.objects.get(
group=self.group, type=ActivityType.SEER_RCA_COMPLETED.value
)
assert activity.data["run_id"] == MOCK_RUN_ID
assert activity.data["root_cause"]["description"] == "Test root cause"

@patch.object(SeerAutofixOperator, "has_access", return_value=True)
def test_create_seer_activity_all_mapped_event_types(self, _mock_has_access):
for seer_event, expected_activity_type in SEER_EVENT_TO_ACTIVITY_TYPE.items():
event_payload = {"run_id": MOCK_RUN_ID, "group_id": self.group.id}
with self.feature("organizations:seer-activity-timeline"):
process_autofix_updates(
event_type=seer_event,
event_payload=event_payload,
organization_id=self.organization.id,
)
assert Activity.objects.filter(
group=self.group, type=expected_activity_type.value
).exists(), f"Activity not created for {seer_event}"

@patch.object(SeerAutofixOperator, "has_access", return_value=True)
def test_create_seer_activity_skips_non_seer_events(self, _mock_has_access):
event_payload = {"run_id": MOCK_RUN_ID, "group_id": self.group.id}

with self.feature("organizations:seer-activity-timeline"):
process_autofix_updates(
event_type=SentryAppEventType.ISSUE_CREATED,
event_payload=event_payload,
organization_id=self.organization.id,
)

seer_type_values = [t.value for t in SEER_EVENT_TO_ACTIVITY_TYPE.values()]
assert not Activity.objects.filter(group=self.group, type__in=seer_type_values).exists()

@patch.object(SeerAutofixOperator, "has_access", return_value=True)
def test_create_seer_activity_feature_flag_disabled(self, _mock_has_access):
event_payload = {"run_id": MOCK_RUN_ID, "group_id": self.group.id}

process_autofix_updates(
event_type=SentryAppEventType.SEER_ROOT_CAUSE_STARTED,
event_payload=event_payload,
organization_id=self.organization.id,
)

seer_type_values = [t.value for t in SEER_EVENT_TO_ACTIVITY_TYPE.values()]
assert not Activity.objects.filter(group=self.group, type__in=seer_type_values).exists()

@patch.object(SeerAutofixOperator, "has_access", return_value=True)
def test_create_seer_activity_pr_created_with_pull_requests(self, _mock_has_access):
event_payload = {
"run_id": MOCK_RUN_ID,
"group_id": self.group.id,
"pull_requests": [
{
"pull_request": {
"pr_number": 42,
"pr_url": "https://github.com/owner/repo/pull/42",
},
"repo_name": "owner/repo",
"provider": "github",
}
],
}

with self.feature("organizations:seer-activity-timeline"):
process_autofix_updates(
event_type=SentryAppEventType.SEER_PR_CREATED,
event_payload=event_payload,
organization_id=self.organization.id,
)

activity = Activity.objects.get(group=self.group, type=ActivityType.SEER_PR_CREATED.value)
assert activity.data["pull_requests"][0]["repo_name"] == "owner/repo"
assert (
activity.data["pull_requests"][0]["pull_request"]["pr_url"]
== "https://github.com/owner/repo/pull/42"
)


class TestGetAutofixExplorerStatus(TestCase):
@staticmethod
Expand Down
Loading