Skip to content

Commit 24dc9c7

Browse files
authored
feat(investigations): Store action requests from Seer events (#128440)
Part of a series that adds follow-up actions to Seer investigations: when a run completes, Seer suggests one next step, and the user can apply a typed action, such as a monitor change, with one click. Save the requests Seer sends during and after a run, and keep them current as Seer revises, withdraws, or replaces them, so Sentry always shows Seer's latest suggestions. Depends on #128421.
1 parent 15769f9 commit 24dc9c7

11 files changed

Lines changed: 1090 additions & 4 deletions
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
from __future__ import annotations
2+
3+
import logging
4+
from collections.abc import Sequence
5+
6+
from django.db.models import Q, QuerySet
7+
from django.utils import timezone
8+
9+
from sentry.investigations.contracts import ActionRequestItem
10+
from sentry.investigations.models import (
11+
Investigation,
12+
InvestigationActionRequest,
13+
InvestigationActionRequestAnchorType,
14+
InvestigationActionRequestState,
15+
InvestigationOrchestrationRun,
16+
)
17+
18+
logger = logging.getLogger(__name__)
19+
20+
21+
LIVE_STATES = (
22+
InvestigationActionRequestState.SUGGESTED,
23+
InvestigationActionRequestState.FAILED,
24+
)
25+
26+
27+
def _supersede(queryset: QuerySet[InvestigationActionRequest]) -> int:
28+
return queryset.filter(state__in=LIVE_STATES).update(
29+
state=InvestigationActionRequestState.SUPERSEDED, date_updated=timezone.now()
30+
)
31+
32+
33+
def supersede_action_requests_before(investigation: Investigation, report_revision: int) -> int:
34+
"""Supersede requests that belonged to an older report.
35+
36+
Only the primary and block-anchored requests go away with the old report. A re-run can
37+
ask for hypothesis and unanchored requests before its new report starts, so we keep
38+
those until Seer re-sends, withdraws, or leaves them out of a snapshot.
39+
"""
40+
41+
return _supersede(
42+
InvestigationActionRequest.objects.filter(
43+
investigation=investigation, report_revision__lt=report_revision
44+
).filter(Q(primary=True) | Q(anchor_type=InvestigationActionRequestAnchorType.BLOCK))
45+
)
46+
47+
48+
def _replace_primary(investigation: Investigation, row: InvestigationActionRequest) -> None:
49+
"""Make `row` the only live primary.
50+
51+
An older primary with an anchor stays as a normal request under that anchor. One without
52+
an anchor is superseded.
53+
"""
54+
55+
others = InvestigationActionRequest.objects.filter(
56+
investigation=investigation, primary=True, state__in=LIVE_STATES
57+
).exclude(id=row.id)
58+
others.filter(anchor_type__isnull=False).update(primary=False, date_updated=timezone.now())
59+
_supersede(others.filter(anchor_type__isnull=True))
60+
61+
62+
def upsert_action_request(
63+
run: InvestigationOrchestrationRun, item: ActionRequestItem, *, replace_primary: bool = True
64+
) -> InvestigationActionRequest:
65+
"""Create or update the request with this key. Callers must hold the investigation and
66+
run locks.
67+
68+
Applied requests are never touched. Failed, withdrawn, or errored requests only come back
69+
if Seer changes the action, so replaying an event can't bring them back.
70+
"""
71+
72+
investigation = run.investigation
73+
anchor = item["anchor"]
74+
action = item["action"]
75+
display: dict[str, object] = {
76+
"report_revision": item["reportRevision"],
77+
"primary": item["primary"],
78+
"anchor_type": anchor["type"] if anchor else None,
79+
"anchor_id": anchor["id"] if anchor else None,
80+
"title": item["title"],
81+
"rationale": item["rationale"],
82+
"impact": item["impact"],
83+
}
84+
row = InvestigationActionRequest.objects.filter(
85+
investigation=investigation, key=item["key"]
86+
).first()
87+
if row is None:
88+
row = InvestigationActionRequest.objects.create(
89+
investigation=investigation,
90+
key=item["key"],
91+
action_kind=action["kind"] if action is not None else None,
92+
action_payload=action,
93+
**display,
94+
)
95+
elif row.state != InvestigationActionRequestState.APPLIED:
96+
update_fields = [*display, "date_updated"]
97+
for field, value in display.items():
98+
setattr(row, field, value)
99+
if row.action_payload != action:
100+
row.action_kind = action["kind"] if action is not None else None
101+
row.action_payload = action
102+
row.state = InvestigationActionRequestState.SUGGESTED
103+
row.error = None
104+
row.reason = None
105+
update_fields.extend(["action_kind", "action_payload", "state", "error", "reason"])
106+
elif row.state == InvestigationActionRequestState.SUPERSEDED and row.error is None:
107+
row.state = InvestigationActionRequestState.SUGGESTED
108+
update_fields.append("state")
109+
row.save(update_fields=update_fields)
110+
111+
if item["primary"]:
112+
if row.state not in LIVE_STATES:
113+
# Seer should have used a new key. A finished request can't be the next step again.
114+
logger.warning(
115+
"investigations.action_requests.terminal_key_reused",
116+
extra={"action_request_id": row.id, "state": row.state},
117+
)
118+
if replace_primary:
119+
_replace_primary(investigation, row)
120+
return row
121+
122+
123+
def withdraw_action_request(run: InvestigationOrchestrationRun, key: str, reason: str) -> bool:
124+
"""Withdraw a suggested request that Seer no longer stands behind. Applied and failed
125+
requests are left alone.
126+
"""
127+
128+
return bool(
129+
InvestigationActionRequest.objects.filter(
130+
investigation=run.investigation,
131+
key=key,
132+
state=InvestigationActionRequestState.SUGGESTED,
133+
).update(
134+
state=InvestigationActionRequestState.WITHDRAWN,
135+
reason=reason,
136+
date_updated=timezone.now(),
137+
)
138+
)
139+
140+
141+
def sync_snapshot_action_requests(
142+
run: InvestigationOrchestrationRun, items: Sequence[ActionRequestItem]
143+
) -> None:
144+
"""Treat the snapshot as the full list: any live request it leaves out is superseded."""
145+
146+
primaries: list[InvestigationActionRequest] = []
147+
for item in items:
148+
row = upsert_action_request(run, item, replace_primary=False)
149+
if item["primary"]:
150+
primaries.append(row)
151+
if primaries:
152+
# Pick the newest revision, then a suggested request over a failed one, then the key,
153+
# so the list order never decides which primary wins.
154+
_replace_primary(
155+
run.investigation,
156+
max(
157+
primaries,
158+
key=lambda row: (
159+
row.report_revision,
160+
row.state == InvestigationActionRequestState.SUGGESTED,
161+
row.key,
162+
),
163+
),
164+
)
165+
_supersede(
166+
InvestigationActionRequest.objects.filter(investigation=run.investigation).exclude(
167+
key__in=[item["key"] for item in items]
168+
)
169+
)
170+
171+
172+
def supersede_block_action_requests(investigation: Investigation, block_keys: Sequence[str]) -> int:
173+
"""Supersede the requests anchored to blocks that were just removed, since their anchor is
174+
gone and the UI would have nowhere to show them.
175+
"""
176+
177+
return _supersede(
178+
InvestigationActionRequest.objects.filter(
179+
investigation=investigation,
180+
anchor_type=InvestigationActionRequestAnchorType.BLOCK,
181+
anchor_id__in=block_keys,
182+
)
183+
)

‎src/sentry/investigations/services/orchestration_events.py‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
EVENT_PAYLOAD_SERIALIZERS,
1818
MAX_MARKDOWN_CHARS,
1919
OrchestrationProjectionSerializer,
20+
valid_action_requests,
2021
validate_text_result,
2122
)
2223
from sentry.investigations.models import (
@@ -35,6 +36,13 @@
3536
InvestigationProject,
3637
InvestigationStatus,
3738
)
39+
from sentry.investigations.services.action_requests import (
40+
supersede_action_requests_before,
41+
supersede_block_action_requests,
42+
sync_snapshot_action_requests,
43+
upsert_action_request,
44+
withdraw_action_request,
45+
)
3846
from sentry.models.project import Project
3947
from sentry.seer.models.run import SeerRun, SeerRunMirrorStatus, SeerRunType
4048
from sentry.utils import json
@@ -54,6 +62,8 @@
5462
"report_failed",
5563
"title_delta",
5664
"metadata_completed",
65+
"action_requested",
66+
"action_withdrawn",
5767
}
5868

5969

@@ -403,10 +413,12 @@ def _adopt_preserved_report_revision(
403413
)
404414
if discarded_ids:
405415
_cancel_report_executions(run, block_ids=discarded_ids)
406-
InvestigationBlock.objects.filter(id__in=discarded_ids).update(
407-
deleted_at=now,
408-
date_updated=now,
416+
discarded = InvestigationBlock.objects.filter(id__in=discarded_ids)
417+
supersede_block_action_requests(
418+
run.investigation,
419+
[key for key in discarded.values_list("stable_agent_key", flat=True) if key],
409420
)
421+
discarded.update(deleted_at=now, date_updated=now)
410422
_control(run).update({"reportRevision": revision, "reportCleared": False})
411423
return bool(discarded_ids)
412424

@@ -707,6 +719,7 @@ def _clear_report(
707719
block_ids=list(live_blocks.values_list("id", flat=True)),
708720
)
709721
live_blocks.update(deleted_at=now, date_updated=now)
722+
supersede_action_requests_before(run.investigation, revision)
710723
control.update(
711724
{
712725
"reportRevision": revision,
@@ -772,10 +785,13 @@ def _apply_snapshot(
772785
generation = _event_generation(event)
773786
projection = payload["projection"]
774787
_set_projection(run, projection, event_generation=generation)
788+
revision = payload.get("reportRevision", projection.get("report", {}).get("revision", 0))
775789
if "blocks" not in payload:
790+
# Re-checking the action requests shouldn't change the notebook.
791+
if "actionRequests" in payload and revision == _report_revision(run):
792+
_sync_snapshot_action_requests(run, payload["actionRequests"])
776793
return
777794
blocks = payload["blocks"]
778-
revision = payload.get("reportRevision", projection.get("report", {}).get("revision", 0))
779795
_clear_report(run, revision, force=True, bump=False, mutate_projection=False)
780796
seen_keys: set[str] = set()
781797
for position, raw_block in enumerate(blocks):
@@ -798,9 +814,16 @@ def _apply_snapshot(
798814
metadata = {**metadata, "reportRevision": revision}
799815
metadata = _validate_event_payload("metadata_completed", metadata)
800816
_complete_metadata(run, metadata, bump=False)
817+
# An older snapshot doesn't clear the report, so its list isn't the full current one.
818+
if "actionRequests" in payload and revision == _report_revision(run):
819+
_sync_snapshot_action_requests(run, payload["actionRequests"])
801820
_bump_notebook(run, run.investigation)
802821

803822

823+
def _sync_snapshot_action_requests(run: InvestigationOrchestrationRun, items: list[object]) -> None:
824+
sync_snapshot_action_requests(run, valid_action_requests(items, log=True))
825+
826+
804827
def _event_carries_projection(event: InvestigationOrchestrationEvent) -> bool:
805828
return event.type in {"workflow_updated", "state_snapshot"} or (
806829
event.type == "workflow_failed" and isinstance(_event_data(event).get("projection"), dict)
@@ -919,6 +942,7 @@ def _apply_event(
919942
deleted_at__isnull=True,
920943
).update(deleted_at=now, date_updated=now)
921944
if updated:
945+
supersede_block_action_requests(run.investigation, [payload["stableAgentKey"]])
922946
_bump_notebook(run, run.investigation)
923947
elif event.type == "report_block_moved":
924948
block = InvestigationBlock.objects.filter(
@@ -967,6 +991,15 @@ def _apply_event(
967991
_bump_notebook(run, investigation)
968992
elif event.type == "metadata_completed":
969993
_complete_metadata(run, payload)
994+
elif event.type == "action_requested":
995+
request = payload["request"]
996+
if request is None:
997+
return False, "invalid_action_request"
998+
if request["reportRevision"] < _report_revision(run):
999+
return False, "stale_report_revision"
1000+
upsert_action_request(run, request)
1001+
elif event.type == "action_withdrawn":
1002+
withdraw_action_request(run, payload["key"], payload["reason"])
9701003
elif event.type == "workflow_failed":
9711004
# A delayed failure still fails the run, but it must not replace state
9721005
# that a newer projection has already established.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"request": {
3+
"action": null,
4+
"anchor": null,
5+
"impact": null,
6+
"key": "add-retry-spans",
7+
"primary": true,
8+
"rationale": "The retries hide where the time goes.",
9+
"reportRevision": 2,
10+
"title": "Add spans around retries"
11+
}
12+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"request": {
3+
"action": {
4+
"kind": "connect_telemetry",
5+
"provider": "datadog"
6+
},
7+
"anchor": null,
8+
"impact": null,
9+
"key": "connect-telemetry-datadog-r2",
10+
"primary": true,
11+
"rationale": "Host metrics would show whether the pods were throttled.",
12+
"reportRevision": 2,
13+
"title": "Connect Datadog"
14+
}
15+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"request": {
3+
"action": {
4+
"kind": "connect_telemetry",
5+
"provider": "datadog"
6+
},
7+
"anchor": {
8+
"id": "hypothesis-0",
9+
"type": "hypothesis"
10+
},
11+
"impact": null,
12+
"key": "connect-telemetry-datadog-r2",
13+
"primary": false,
14+
"rationale": "Host metrics would show whether the pods were throttled.",
15+
"reportRevision": 2,
16+
"title": "Connect Datadog"
17+
}
18+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"request": {
3+
"action": {
4+
"before": {
5+
"query": "transaction:/checkout",
6+
"timeWindowSeconds": 300
7+
},
8+
"changes": {
9+
"query": "transaction:/checkout !error.handled:true",
10+
"timeWindowSeconds": 900
11+
},
12+
"detectorId": "4821",
13+
"kind": "update_detector"
14+
},
15+
"anchor": null,
16+
"impact": "Alerts on 3 of the last 14 spikes instead of all 14.",
17+
"key": "widen-time-window",
18+
"primary": true,
19+
"rationale": "Short spikes recover within a minute.",
20+
"reportRevision": 2,
21+
"title": "Widen the time window"
22+
}
23+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"key": "add-retry-spans",
3+
"reason": "The new evidence shows the retries are not slow."
4+
}

0 commit comments

Comments
 (0)