Skip to content

Commit 4dd52a3

Browse files
ChristinarlongChristinarlong
authored
chore(sentry apps): Add SLOs for sentry app creation & updating (#93762)
1 parent 81e2427 commit 4dd52a3

File tree

7 files changed

+123
-62
lines changed

7 files changed

+123
-62
lines changed

‎src/sentry/sentry_apps/logic.py

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@
2727
SentryAppInstallationCreator,
2828
SentryAppInstallationTokenCreator,
2929
)
30+
from sentry.sentry_apps.metrics import (
31+
SentryAppEventType,
32+
SentryAppInteractionEvent,
33+
SentryAppInteractionType,
34+
)
3035
from sentry.sentry_apps.models.sentry_app import (
3136
EVENT_EXPANSION,
3237
REQUIRED_EVENT_PERMISSIONS,
@@ -105,25 +110,29 @@ class SentryAppUpdater:
105110
features: list[int] | None = None
106111

107112
def run(self, user: User | RpcUser) -> SentryApp:
108-
with transaction.atomic(router.db_for_write(User)):
109-
self._update_name()
110-
self._update_author()
111-
self._update_features(user=user)
112-
self._update_status(user=user)
113-
self._update_scopes()
114-
self._update_events()
115-
self._update_webhook_url()
116-
self._update_redirect_url()
117-
self._update_is_alertable()
118-
self._update_verify_install()
119-
self._update_overview()
120-
self._update_allowed_origins()
121-
new_schema_elements = self._update_schema()
122-
self._update_popularity(user=user)
123-
self.sentry_app.save()
124-
self._update_service_hooks()
125-
self.record_analytics(user, new_schema_elements)
126-
return self.sentry_app
113+
with SentryAppInteractionEvent(
114+
operation_type=SentryAppInteractionType.MANAGEMENT,
115+
event_type=SentryAppEventType.APP_UPDATE,
116+
).capture():
117+
with transaction.atomic(router.db_for_write(User)):
118+
self._update_name()
119+
self._update_author()
120+
self._update_features(user=user)
121+
self._update_status(user=user)
122+
self._update_scopes()
123+
self._update_events()
124+
self._update_webhook_url()
125+
self._update_redirect_url()
126+
self._update_is_alertable()
127+
self._update_verify_install()
128+
self._update_overview()
129+
self._update_allowed_origins()
130+
new_schema_elements = self._update_schema()
131+
self._update_popularity(user=user)
132+
self.sentry_app.save()
133+
self._update_service_hooks()
134+
self.record_analytics(user, new_schema_elements)
135+
return self.sentry_app
127136

128137
def _update_features(self, user: User | RpcUser) -> None:
129138
if self.features is not None:
@@ -313,22 +322,29 @@ def run(
313322
request: HttpRequest | None = None,
314323
skip_default_auth_token: bool = False,
315324
) -> SentryApp:
316-
with transaction.atomic(router.db_for_write(User)), in_test_hide_transaction_boundary():
317-
slug = self._generate_and_validate_slug()
318-
proxy = self._create_proxy_user(slug=slug)
319-
api_app = self._create_api_application(proxy=proxy)
320-
sentry_app = self._create_sentry_app(user=user, slug=slug, proxy=proxy, api_app=api_app)
321-
self._create_ui_components(sentry_app=sentry_app)
322-
self._create_integration_feature(sentry_app=sentry_app)
323325

324-
if self.is_internal:
325-
install = self._install(slug=slug, user=user, request=request)
326-
if not skip_default_auth_token:
327-
self._create_access_token(user=user, install=install, request=request)
326+
with SentryAppInteractionEvent(
327+
operation_type=SentryAppInteractionType.MANAGEMENT,
328+
event_type=SentryAppEventType.APP_CREATE,
329+
).capture():
330+
with transaction.atomic(router.db_for_write(User)), in_test_hide_transaction_boundary():
331+
slug = self._generate_and_validate_slug()
332+
proxy = self._create_proxy_user(slug=slug)
333+
api_app = self._create_api_application(proxy=proxy)
334+
sentry_app = self._create_sentry_app(
335+
user=user, slug=slug, proxy=proxy, api_app=api_app
336+
)
337+
self._create_ui_components(sentry_app=sentry_app)
338+
self._create_integration_feature(sentry_app=sentry_app)
339+
340+
if self.is_internal:
341+
install = self._install(slug=slug, user=user, request=request)
342+
if not skip_default_auth_token:
343+
self._create_access_token(user=user, install=install, request=request)
328344

329-
self.audit(request=request, sentry_app=sentry_app)
330-
self.record_analytics(user=user, sentry_app=sentry_app)
331-
return sentry_app
345+
self.audit(request=request, sentry_app=sentry_app)
346+
self.record_analytics(user=user, sentry_app=sentry_app)
347+
return sentry_app
332348

333349
def _generate_and_validate_slug(self) -> str:
334350
# sentry_slugify ensures the slug is not entirely numeric

‎src/sentry/sentry_apps/metrics.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ class SentryAppInteractionType(StrEnum):
2020
# Authorizations
2121
AUTHORIZATIONS = "authorizations"
2222

23+
# Managing Sentry Apps
24+
MANAGEMENT = "management"
25+
2326

2427
@dataclass
2528
class SentryAppInteractionEvent(EventLifecycleMetric):
@@ -123,3 +126,10 @@ class SentryAppEventType(StrEnum):
123126
# authorizations
124127
GRANT_EXCHANGER = "grant_exchanger"
125128
REFRESHER = "refresher"
129+
130+
# management
131+
APP_CREATE = "app_create"
132+
APP_UPDATE = "app_update"
133+
REQUESTS = "requests"
134+
PUBLISH = "publish"
135+
WEBHOOK_UPDATE = "webhook_update"

‎tests/sentry/sentry_apps/tasks/test_sentry_apps.py

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,12 @@ def test_no_installation(self, mock_record, safe_urlopen):
192192
assert_halt_metric(
193193
mock_record=mock_record, error_msg=SentryAppWebhookHaltReason.MISSING_INSTALLATION
194194
)
195-
# PREPARE_WEBHOOK (failure)
195+
# APP_CREATE (success) -> PREPARE_WEBHOOK (failure)
196196
assert_count_of_metric(
197-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
197+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
198+
)
199+
assert_count_of_metric(
200+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=1
198201
)
199202
assert_count_of_metric(
200203
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
@@ -607,13 +610,13 @@ def test_does_not_process_sentry_apps_without_issue_webhooks(self, mock_record,
607610
assert len(safe_urlopen.mock_calls) == 0
608611
assert_success_metric(mock_record)
609612

610-
# GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success)
613+
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success)
611614
# Our SentryAppInstallation test fixture automatically runs GrantExchanger to get a valid token
612615
assert_count_of_metric(
613-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
616+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
614617
)
615618
assert_count_of_metric(
616-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
619+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
617620
)
618621

619622
@with_feature("organizations:integrations-event-hooks")
@@ -666,12 +669,13 @@ def test_error_created_sends_webhook(self, mock_record, safe_urlopen):
666669

667670
assert_success_metric(mock_record)
668671

669-
# GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
672+
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) ->
673+
# SEND_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
670674
assert_count_of_metric(
671-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=5
675+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=6
672676
)
673677
assert_count_of_metric(
674-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=5
678+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=6
675679
)
676680

677681
@responses.activate
@@ -943,12 +947,13 @@ def test_record_lifecycle_error_from_pubished_apps(self, mock_record, safe_urlop
943947
assert self.sentry_app_1.webhook_url in call_urls
944948
assert self.sentry_app_2.webhook_url in call_urls
945949

946-
# GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (halt) x2
950+
# APP_CREATE (success) x 2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
951+
# -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (halt) x2
947952
assert_count_of_metric(
948-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=9
953+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=11
949954
)
950955
assert_count_of_metric(
951-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=7
956+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=9
952957
)
953958
assert_count_of_metric(
954959
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=2
@@ -1013,12 +1018,13 @@ def test_sends_webhooks_to_all_installs_success(self, mock_record, safe_urlopen)
10131018
assert self.sentry_app_2.webhook_url in call_urls
10141019

10151020
assert_success_metric(mock_record)
1016-
# GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x 2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2
1021+
# APP_CREATE (success) x 2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
1022+
# -> SEND_WEBHOOK (success) x 2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2
10171023
assert_count_of_metric(
1018-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=9
1024+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=11
10191025
)
10201026
assert_count_of_metric(
1021-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=9
1027+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=11
10221028
)
10231029

10241030
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -1068,12 +1074,12 @@ def test_sends_webhooks_with_send_webhook_sentry_failure(self, mock_record):
10681074
assert_failure_metric(
10691075
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.EVENT_NOT_IN_SERVCEHOOK)
10701076
)
1071-
# GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x 1 -> SEND_WEBHOOK (failure)
1077+
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x 1 -> SEND_WEBHOOK (failure)
10721078
assert_count_of_metric(
1073-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
1079+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=5
10741080
)
10751081
assert_count_of_metric(
1076-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
1082+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=4
10771083
)
10781084
assert_count_of_metric(
10791085
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
@@ -1334,12 +1340,12 @@ def test_does_not_send_if_no_service_hook_exists(self, mock_record, safe_urlopen
13341340
assert_failure_metric(
13351341
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.MISSING_SERVICEHOOK)
13361342
)
1337-
# GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> send_webhook (error)
1343+
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> send_webhook (error)
13381344
assert_count_of_metric(
1339-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
1345+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
13401346
)
13411347
assert_count_of_metric(
1342-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
1348+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
13431349
)
13441350
assert_count_of_metric(
13451351
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
@@ -1362,12 +1368,12 @@ def test_does_not_send_if_event_not_in_app_events(self, mock_record, safe_urlope
13621368
assert_failure_metric(
13631369
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.EVENT_NOT_IN_SERVCEHOOK)
13641370
)
1365-
# GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (failure)
1371+
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (failure)
13661372
assert_count_of_metric(
1367-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
1373+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
13681374
)
13691375
assert_count_of_metric(
1370-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
1376+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
13711377
)
13721378
assert_count_of_metric(
13731379
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1

‎tests/sentry/sentry_apps/test_sentry_app_creator.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44

55
from sentry import audit_log
66
from sentry.integrations.models.integration_feature import IntegrationFeature, IntegrationTypes
7+
from sentry.integrations.types import EventLifecycleOutcome
78
from sentry.models.apiapplication import ApiApplication
89
from sentry.models.auditlogentry import AuditLogEntry
910
from sentry.sentry_apps.logic import SentryAppCreator
1011
from sentry.sentry_apps.models.sentry_app import SentryApp
1112
from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
1213
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
14+
from sentry.testutils.asserts import assert_count_of_metric, assert_success_metric
1315
from sentry.testutils.cases import TestCase
1416
from sentry.testutils.silo import control_silo_test
1517
from sentry.users.models.user import User
@@ -55,7 +57,8 @@ def test_creates_api_application(self):
5557

5658
assert ApiApplication.objects.get(owner=proxy)
5759

58-
def test_creates_sentry_app(self):
60+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
61+
def test_creates_sentry_app(self, mock_record):
5962
self.creator.run(user=self.user)
6063

6164
proxy = User.objects.get(username__contains="nulldb")
@@ -71,6 +74,17 @@ def test_creates_sentry_app(self):
7174
assert sentry_app.creator_user == self.user
7275
assert sentry_app.creator_label == "foo@bar.com"
7376

77+
# SLO assertions
78+
assert_success_metric(mock_record=mock_record)
79+
80+
# CREATE (success)
81+
assert_count_of_metric(
82+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
83+
)
84+
assert_count_of_metric(
85+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=1
86+
)
87+
7488
def test_creator_label_no_email(self):
7589
self.user.email = ""
7690
self.user.save()

‎tests/sentry/sentry_apps/test_sentry_app_updater.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
from datetime import timedelta
2+
from unittest.mock import patch
23

34
import pytest
45
from django.utils import timezone
56

67
from sentry.constants import SentryAppStatus
78
from sentry.coreapi import APIError
9+
from sentry.integrations.types import EventLifecycleOutcome
810
from sentry.models.apitoken import ApiToken
911
from sentry.sentry_apps.logic import SentryAppUpdater, expand_events
1012
from sentry.sentry_apps.models.sentry_app import SentryApp
1113
from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
1214
from sentry.sentry_apps.models.servicehook import ServiceHook
1315
from sentry.silo.base import SiloMode
16+
from sentry.testutils.asserts import assert_count_of_metric, assert_success_metric
1417
from sentry.testutils.cases import TestCase
1518
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
1619

@@ -33,7 +36,8 @@ def test_updates_name(self):
3336
self.updater.run(user=self.user)
3437
assert self.sentry_app.name == "A New Thing"
3538

36-
def test_update_scopes_internal_integration(self):
39+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
40+
def test_update_scopes_internal_integration(self, mock_record):
3741
self.create_project(organization=self.org)
3842
sentry_app = self.create_internal_integration(
3943
scopes=("project:read",), organization=self.org
@@ -53,6 +57,17 @@ def test_update_scopes_internal_integration(self):
5357
"project:write",
5458
]
5559

60+
# SLO assertions
61+
assert_success_metric(mock_record=mock_record)
62+
63+
# CREATE (success) -> UPDATE (success)
64+
assert_count_of_metric(
65+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
66+
)
67+
assert_count_of_metric(
68+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
69+
)
70+
5671
def test_updates_unpublished_app_scopes(self):
5772
# create both expired token and not expired tokens
5873
ApiToken.objects.create(

‎tests/sentry/sentry_apps/token_exchange/test_grant_exchanger.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ def test_grant_must_belong_to_installations(self, mock_record):
8080
mock_record=mock_record, error_msg=SentryAppIntegratorError(message="Forbidden grant")
8181
)
8282

83-
# GRANT_EXCHANGER (halt)
83+
# APP_CREATE (success) -> GRANT_EXCHANGER (halt)
8484
assert_count_of_metric(
85-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
85+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
8686
)
8787
assert_count_of_metric(
8888
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1

‎tests/sentry/sentry_apps/token_exchange/test_refresher.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ def test_validates_token_belongs_to_sentry_app(self, mock_record):
7272
error_msg=SentryAppIntegratorError(message="Token does not belong to the application"),
7373
)
7474

75-
# REFRESHER (halt)
75+
# APP_CREATE (success) -> TOKEN_EXCHANGE (success) -> REFRESHER (halt)
7676
assert_count_of_metric(
77-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
77+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
7878
)
7979
assert_count_of_metric(
8080
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1

0 commit comments

Comments
 (0)