Skip to content

Commit 1d757ec

Browse files
leeandherandrewshie-sentry
authored andcommitted
feat(pagerduty): Issue alerts will include alert name in summary (#87786)
For issue alerts that include a pagerduty notification action, the name of the alert rule will be included in the title. Also ensures test notifications have a label and can include one from the alert. This will enable users to see what the actual Pagerduty Incident title will be from the `Send Test Notification` button. If they are setting up a notif for the first time and haven't added a name `Test Alert` will be used instead. Will require a followup FE PR to send the notif title along with the preview alert payload. <img width="1402" alt="image" src="https://github.com/user-attachments/assets/fb481f48-03c5-4213-93fb-d2c6293f0617" /> **todo** - [x] Add screenshots - [x] Add tests
1 parent 60d9097 commit 1d757ec

File tree

5 files changed

+116
-17
lines changed

5 files changed

+116
-17
lines changed

src/sentry/api/endpoints/project_rule_actions.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from sentry.api.api_publish_status import ApiPublishStatus
1010
from sentry.api.base import region_silo_endpoint
1111
from sentry.api.bases import ProjectAlertRulePermission, ProjectEndpoint
12-
from sentry.api.serializers.rest_framework import RuleActionSerializer
12+
from sentry.api.serializers.rest_framework import DummyRuleSerializer
1313
from sentry.eventstore.models import GroupEvent
1414
from sentry.models.rule import Rule
1515
from sentry.rules.processing.processor import activate_downstream_actions
@@ -33,10 +33,11 @@ def post(self, request: Request, project) -> Response:
3333
{method} {path}
3434
{{
3535
"actions": []
36+
"name": string
3637
}}
3738
3839
"""
39-
serializer = RuleActionSerializer(
40+
serializer = DummyRuleSerializer(
4041
context={"project": project, "organization": project.organization}, data=request.data
4142
)
4243

@@ -58,7 +59,7 @@ def post(self, request: Request, project) -> Response:
5859
"frequency": 30,
5960
}
6061
)
61-
rule = Rule(id=-1, project=project, data=data)
62+
rule = Rule(id=-1, project=project, data=data, label=data.get("name"))
6263

6364
test_event = create_sample_event(
6465
project, platform=project.platform, default="javascript", tagged=True

src/sentry/api/serializers/rest_framework/rule.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,17 @@ class RulePreviewSerializer(RuleSetSerializer):
140140
endpoint = serializers.DateTimeField(required=False, allow_null=True)
141141

142142

143-
class RuleActionSerializer(serializers.Serializer):
143+
class DummyRuleSerializer(serializers.Serializer):
144+
name = serializers.CharField(
145+
max_length=256, required=False, allow_null=True, allow_blank=True, default="Test Alert"
146+
)
144147
actions = serializers.ListField(child=RuleNodeField(type="action/event"), required=False)
145148

149+
def validate_name(self, name):
150+
if name == "" or name is None:
151+
return "Test Alert"
152+
return name
153+
146154
def validate(self, attrs):
147155
return validate_actions(attrs)
148156

src/sentry/integrations/pagerduty/actions/notification.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
PagerdutySeverity,
1313
build_pagerduty_event_payload,
1414
)
15+
from sentry.models.rule import Rule
1516
from sentry.rules.actions import IntegrationEventAction
1617
from sentry.shared_integrations.exceptions import ApiError
1718

@@ -90,6 +91,12 @@ def send_notification(event, futures):
9091
severity=severity,
9192
)
9293

94+
rules: list[Rule] = [f.rule for f in futures]
95+
rule = rules[0] if rules else None
96+
97+
if rule and rule.label:
98+
data["payload"]["summary"] = f"[{rule.label}]: {data['payload']['summary']}"
99+
93100
try:
94101
resp = client.send_trigger(data=data)
95102
except ApiError as e:
@@ -104,8 +111,7 @@ def send_notification(event, futures):
104111
},
105112
)
106113
raise
107-
rules = [f.rule for f in futures]
108-
rule = rules[0] if rules else None
114+
109115
self.record_notification_sent(event, str(service["id"]), rule, notification_uuid)
110116

111117
# TODO(meredith): Maybe have a generic success log statements for

tests/sentry/api/endpoints/test_project_rule_actions.py

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import sentry_sdk
44

55
from sentry.integrations.jira.integration import JiraIntegration
6+
from sentry.integrations.pagerduty.client import PagerDutyClient
7+
from sentry.integrations.pagerduty.utils import PagerDutyServiceDict, add_service
68
from sentry.rules.actions.notify_event import NotifyEventAction
79
from sentry.shared_integrations.exceptions import IntegrationFormError
810
from sentry.silo.base import SiloMode
@@ -20,6 +22,27 @@ class ProjectRuleActionsEndpointTest(APITestCase):
2022
def setUp(self):
2123
self.login_as(self.user)
2224

25+
def setup_pd_service(self) -> PagerDutyServiceDict:
26+
service_info = {
27+
"type": "service",
28+
"integration_key": "PND123",
29+
"service_name": "Sentry Service",
30+
}
31+
_integration, org_integration = self.create_provider_integration_for(
32+
provider="pagerduty",
33+
name="Example PagerDuty",
34+
external_id="example-pd",
35+
metadata={"services": [service_info]},
36+
organization=self.organization,
37+
user=self.user,
38+
)
39+
with assume_test_silo_mode(SiloMode.CONTROL):
40+
return add_service(
41+
org_integration,
42+
service_name=service_info["service_name"],
43+
integration_key=service_info["integration_key"],
44+
)
45+
2346
@mock.patch.object(NotifyEventAction, "after")
2447
def test_actions(self, action):
2548
action_data = [
@@ -29,9 +52,53 @@ def test_actions(self, action):
2952
]
3053

3154
self.get_success_response(self.organization.slug, self.project.slug, actions=action_data)
32-
3355
assert action.called
3456

57+
@mock.patch.object(PagerDutyClient, "send_trigger")
58+
def test_name_action_default(self, mock_send_trigger):
59+
"""
60+
Tests that label will be used as 'Test Alert' if not present. Uses PagerDuty since those
61+
notifications will differ based on the name of the alert.
62+
"""
63+
service_info = self.setup_pd_service()
64+
action_data = [
65+
{
66+
"account": service_info["integration_id"],
67+
"service": service_info["id"],
68+
"severity": "info",
69+
"id": "sentry.integrations.pagerduty.notify_action.PagerDutyNotifyServiceAction",
70+
}
71+
]
72+
self.get_success_response(self.organization.slug, self.project.slug, actions=action_data)
73+
74+
assert mock_send_trigger.call_count == 1
75+
pagerduty_data = mock_send_trigger.call_args.kwargs.get("data")
76+
assert pagerduty_data["payload"]["summary"].startswith("[Test Alert]:")
77+
78+
@mock.patch.object(PagerDutyClient, "send_trigger")
79+
def test_name_action_with_custom_name(self, mock_send_trigger):
80+
"""
81+
Tests that custom names can be provided to the test notification. Uses PagerDuty since those
82+
notifications will differ based on the name of the alert.
83+
"""
84+
service_info = self.setup_pd_service()
85+
action_data = [
86+
{
87+
"account": service_info["integration_id"],
88+
"service": service_info["id"],
89+
"severity": "info",
90+
"id": "sentry.integrations.pagerduty.notify_action.PagerDutyNotifyServiceAction",
91+
}
92+
]
93+
custom_alert_name = "Check #feed-issues"
94+
95+
self.get_success_response(
96+
self.organization.slug, self.project.slug, actions=action_data, name=custom_alert_name
97+
)
98+
assert mock_send_trigger.call_count == 1
99+
pagerduty_data = mock_send_trigger.call_args.kwargs.get("data")
100+
assert pagerduty_data["payload"]["summary"].startswith(f"[{custom_alert_name}]:")
101+
35102
@mock.patch.object(JiraIntegration, "create_issue")
36103
@mock.patch.object(sentry_sdk, "capture_exception")
37104
def test_sample_event_raises_exceptions(self, mock_sdk_capture, mock_create_issue):

tests/sentry/integrations/pagerduty/test_notification.py

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE
1616
from sentry.testutils.silo import assume_test_silo_mode
1717
from sentry.testutils.skips import requires_snuba
18+
from sentry.types.rules import RuleFuture
1819

1920
pytestmark = [requires_snuba]
2021

@@ -35,6 +36,7 @@ class PagerDutyNotifyActionTest(RuleTestCase, PerformanceIssueTestCase):
3536
rule_cls = PagerDutyNotifyServiceAction
3637

3738
def setUp(self):
39+
self.project_rule = self.create_project_rule(name="Check #project-channel")
3840
with assume_test_silo_mode(SiloMode.CONTROL):
3941
self.integration, org_integration = self.create_provider_integration_for(
4042
self.organization,
@@ -66,12 +68,21 @@ def test_applies_correctly(self, mock_record):
6668
)
6769

6870
rule = self.get_rule(
69-
data={"account": self.integration.id, "service": str(self.service["id"])}
71+
data={
72+
"account": self.integration.id,
73+
"service": str(self.service["id"]),
74+
"name": "Test Alert",
75+
}
7076
)
7177

7278
notification_uuid = "123e4567-e89b-12d3-a456-426614174000"
7379

74-
results = list(rule.after(event=event, notification_uuid=notification_uuid))
80+
results = list(
81+
rule.after(
82+
event=event,
83+
notification_uuid=notification_uuid,
84+
)
85+
)
7586
assert len(results) == 1
7687

7788
responses.add(
@@ -83,11 +94,12 @@ def test_applies_correctly(self, mock_record):
8394
)
8495

8596
# Trigger rule callback
86-
results[0].callback(event, futures=[])
97+
rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs)
98+
results[0].callback(event, futures=[rule_future])
8799
data = orjson.loads(responses.calls[0].request.body)
88100

89101
assert data["event_action"] == "trigger"
90-
assert data["payload"]["summary"] == event.message
102+
assert data["payload"]["summary"] == f"[{self.project_rule.label}]: {event.message}"
91103
assert data["payload"]["custom_details"]["message"] == event.message
92104
assert event.group is not None
93105
assert data["links"][0]["href"] == event.group.get_absolute_url(
@@ -97,7 +109,7 @@ def test_applies_correctly(self, mock_record):
97109
mock_record.assert_called_with(
98110
"alert.sent",
99111
provider="pagerduty",
100-
alert_id="",
112+
alert_id=self.project_rule.id,
101113
alert_type="issue_alert",
102114
organization_id=self.organization.id,
103115
project_id=event.project_id,
@@ -111,7 +123,7 @@ def test_applies_correctly(self, mock_record):
111123
project_id=event.project_id,
112124
group_id=event.group_id,
113125
notification_uuid=notification_uuid,
114-
alert_id=None,
126+
alert_id=self.project_rule.id,
115127
)
116128

117129
@responses.activate
@@ -130,13 +142,14 @@ def test_applies_correctly_performance_issue(self):
130142
)
131143

132144
# Trigger rule callback
133-
results[0].callback(event, futures=[])
145+
rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs)
146+
results[0].callback(event, futures=[rule_future])
134147
data = orjson.loads(responses.calls[0].request.body)
135148

136149
perf_issue_title = "N+1 Query"
137150

138151
assert data["event_action"] == "trigger"
139-
assert data["payload"]["summary"] == perf_issue_title
152+
assert data["payload"]["summary"] == f"[{self.project_rule.label}]: {perf_issue_title}"
140153
assert data["payload"]["custom_details"]["title"] == perf_issue_title
141154

142155
@responses.activate
@@ -165,11 +178,15 @@ def test_applies_correctly_generic_issue(self):
165178
)
166179

167180
# Trigger rule callback
168-
results[0].callback(group_event, futures=[])
181+
rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs)
182+
results[0].callback(group_event, futures=[rule_future])
169183
data = orjson.loads(responses.calls[0].request.body)
170184

171185
assert data["event_action"] == "trigger"
172-
assert data["payload"]["summary"] == group_event.occurrence.issue_title
186+
assert (
187+
data["payload"]["summary"]
188+
== f"[{self.project_rule.label}]: {group_event.occurrence.issue_title}"
189+
)
173190
assert data["payload"]["custom_details"]["title"] == group_event.occurrence.issue_title
174191

175192
def test_render_label_without_severity(self):

0 commit comments

Comments
 (0)