Skip to content

Commit eb97ba0

Browse files
ChristinarlongChristinarlong
authored
chore(sentry apps): add and test slos for installation creation (#94485)
1 parent a979b24 commit eb97ba0

File tree

7 files changed

+68
-44
lines changed

7 files changed

+68
-44
lines changed

‎src/sentry/sentry_apps/installations.py

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
from sentry.sentry_apps.api.serializers.sentry_app_installation import (
1818
SentryAppInstallationSerializer,
1919
)
20+
from sentry.sentry_apps.metrics import (
21+
SentryAppEventType,
22+
SentryAppInteractionEvent,
23+
SentryAppInteractionType,
24+
)
2025
from sentry.sentry_apps.models.sentry_app import SentryApp
2126
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
2227
from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken
@@ -110,20 +115,25 @@ class SentryAppInstallationCreator:
110115
notify: bool = True
111116

112117
def run(self, *, user: User | RpcUser, request: HttpRequest | None) -> SentryAppInstallation:
113-
metrics.incr("sentry_apps.installation.attempt")
114-
with transaction.atomic(router.db_for_write(ApiGrant)):
115-
api_grant = self._create_api_grant()
116-
install = self._create_install(api_grant=api_grant)
117-
self.audit(request=request)
118+
with SentryAppInteractionEvent(
119+
operation_type=SentryAppInteractionType.MANAGEMENT,
120+
event_type=SentryAppEventType.INSTALLATION_CREATE,
121+
).capture() as lifecycle:
122+
with transaction.atomic(router.db_for_write(ApiGrant)):
123+
api_grant = self._create_api_grant()
124+
install = self._create_install(api_grant=api_grant)
125+
lifecycle.add_extra("installation_id", install.id)
126+
127+
self.audit(request=request)
118128

119-
self._create_service_hooks(install=install)
120-
install.is_new = True
129+
self._create_service_hooks(install=install)
130+
install.is_new = True
121131

122-
if self.notify:
123-
installation_webhook.delay(install.id, user.id)
132+
if self.notify:
133+
installation_webhook.delay(install.id, user.id)
124134

125-
self.record_analytics(user=user)
126-
return install
135+
self.record_analytics(user=user)
136+
return install
127137

128138
def _create_install(self, api_grant: ApiGrant) -> SentryAppInstallation:
129139
status = SentryAppInstallationStatus.PENDING

‎src/sentry/sentry_apps/metrics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,5 +131,5 @@ class SentryAppEventType(StrEnum):
131131
APP_CREATE = "app_create"
132132
APP_UPDATE = "app_update"
133133
REQUESTS = "requests"
134-
PUBLISH = "publish"
135134
WEBHOOK_UPDATE = "webhook_update"
135+
INSTALLATION_CREATE = "install_create"

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -610,13 +610,13 @@ def test_does_not_process_sentry_apps_without_issue_webhooks(self, mock_record,
610610
assert len(safe_urlopen.mock_calls) == 0
611611
assert_success_metric(mock_record)
612612

613-
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success)
613+
# APP_CREATE (success) -> UPDATE_WEBHOOK (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success)
614614
# Our SentryAppInstallation test fixture automatically runs GrantExchanger to get a valid token
615615
assert_count_of_metric(
616-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
616+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
617617
)
618618
assert_count_of_metric(
619-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
619+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=4
620620
)
621621

622622
@with_feature("organizations:integrations-event-hooks")
@@ -669,13 +669,13 @@ def test_error_created_sends_webhook(self, mock_record, safe_urlopen):
669669

670670
assert_success_metric(mock_record)
671671

672-
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) ->
672+
# APP_CREATE (success) -> UPDATE_WEBHOOK (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) ->
673673
# SEND_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
674674
assert_count_of_metric(
675-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=6
675+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=7
676676
)
677677
assert_count_of_metric(
678-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=6
678+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=7
679679
)
680680

681681
@responses.activate
@@ -959,13 +959,13 @@ def test_record_lifecycle_error_from_pubished_apps(self, mock_record, safe_urlop
959959
assert self.sentry_app_1.webhook_url in call_urls
960960
assert self.sentry_app_2.webhook_url in call_urls
961961

962-
# APP_CREATE (success) x 2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
962+
# APP_CREATE (success) x 2 -> UPDATE_WEBHOOK (success) x2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
963963
# -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (halt) x2
964964
assert_count_of_metric(
965-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=11
965+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=13
966966
)
967967
assert_count_of_metric(
968-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=9
968+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=11
969969
)
970970
assert_count_of_metric(
971971
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=2
@@ -1030,13 +1030,13 @@ def test_sends_webhooks_to_all_installs_success(self, mock_record, safe_urlopen)
10301030
assert self.sentry_app_2.webhook_url in call_urls
10311031

10321032
assert_success_metric(mock_record)
1033-
# APP_CREATE (success) x 2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
1033+
# APP_CREATE (success) x 2 -> UPDATE_WEBHOOK (success) x2 -> GRANT_EXCHANGER (success) x 2 -> PREPARE_WEBHOOK (success)
10341034
# -> SEND_WEBHOOK (success) x 2 -> SEND_WEBHOOK (success) x2 -> SEND_WEBHOOK (success) x2
10351035
assert_count_of_metric(
1036-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=11
1036+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=13
10371037
)
10381038
assert_count_of_metric(
1039-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=11
1039+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=13
10401040
)
10411041

10421042
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -1086,12 +1086,12 @@ def test_sends_webhooks_with_send_webhook_sentry_failure(self, mock_record):
10861086
assert_failure_metric(
10871087
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.EVENT_NOT_IN_SERVCEHOOK)
10881088
)
1089-
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x 1 -> SEND_WEBHOOK (failure)
1089+
# APP_CREATE (success) -> UPDATE_WEBHOOK (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) x 1 -> SEND_WEBHOOK (failure)
10901090
assert_count_of_metric(
1091-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=5
1091+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=6
10921092
)
10931093
assert_count_of_metric(
1094-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=4
1094+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=5
10951095
)
10961096
assert_count_of_metric(
10971097
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
@@ -1352,12 +1352,12 @@ def test_does_not_send_if_no_service_hook_exists(self, mock_record, safe_urlopen
13521352
assert_failure_metric(
13531353
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.MISSING_SERVICEHOOK)
13541354
)
1355-
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> send_webhook (error)
1355+
# APP_CREATE (success) -> UPDATE_WEBHOOK (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> send_webhook (error)
13561356
assert_count_of_metric(
1357-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
1357+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=5
13581358
)
13591359
assert_count_of_metric(
1360-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
1360+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=4
13611361
)
13621362
assert_count_of_metric(
13631363
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
@@ -1380,12 +1380,12 @@ def test_does_not_send_if_event_not_in_app_events(self, mock_record, safe_urlope
13801380
assert_failure_metric(
13811381
mock_record, SentryAppSentryError(SentryAppWebhookFailureReason.EVENT_NOT_IN_SERVCEHOOK)
13821382
)
1383-
# APP_CREATE (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (failure)
1383+
# APP_CREATE (success) -> UPDATE_WEBHOOK (success) -> GRANT_EXCHANGER (success) -> PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (failure)
13841384
assert_count_of_metric(
1385-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
1385+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=5
13861386
)
13871387
assert_count_of_metric(
1388-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
1388+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=4
13891389
)
13901390
assert_count_of_metric(
13911391
mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1

‎tests/sentry/sentry_apps/test_sentry_app_installation_creator.py

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

55
from sentry import audit_log
66
from sentry.constants import SentryAppInstallationStatus
7+
from sentry.integrations.types import EventLifecycleOutcome
78
from sentry.models.apigrant import ApiGrant
89
from sentry.models.auditlogentry import AuditLogEntry
910
from sentry.sentry_apps.installations import SentryAppInstallationCreator
1011
from sentry.sentry_apps.models.servicehook import ServiceHook, ServiceHookProject
1112
from sentry.silo.base import SiloMode
13+
from sentry.testutils.asserts import assert_count_of_metric, assert_success_metric
1214
from sentry.testutils.cases import TestCase
1315
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
1416
from sentry.users.models.user import User
@@ -39,11 +41,23 @@ def run_creator(self, **kwargs):
3941
).run(user=self.user, request=kwargs.pop("request", None))
4042

4143
@responses.activate
42-
def test_creates_installation(self):
44+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
45+
def test_creates_installation(self, mock_record):
4346
responses.add(responses.POST, "https://example.com/webhook")
4447
install = self.run_creator()
4548
assert install.pk
4649

50+
# SLO assertions
51+
assert_success_metric(mock_record=mock_record)
52+
53+
# INSTALLATION_CREATE (success)
54+
assert_count_of_metric(
55+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
56+
)
57+
assert_count_of_metric(
58+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=1
59+
)
60+
4761
@responses.activate
4862
def test_creates_installation__multiple_runs(self):
4963
responses.add(responses.POST, "https://example.com/webhook")

‎tests/sentry/sentry_apps/test_sentry_app_updater.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ def test_update_scopes_internal_integration(self, mock_record):
6060
# SLO assertions
6161
assert_success_metric(mock_record=mock_record)
6262

63-
# CREATE (success) -> UPDATE (success)
63+
# CREATE (success) -> INSTALLATION_CREATE (success) -> UPDATE (success)
6464
assert_count_of_metric(
65-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
65+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
6666
)
6767
assert_count_of_metric(
68-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
68+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
6969
)
7070

7171
def test_updates_unpublished_app_scopes(self):
@@ -233,12 +233,12 @@ def test_create_service_hook_on_update(self, mock_record):
233233
# SLO assertions
234234
assert_success_metric(mock_record=mock_record)
235235

236-
# APP_CREATE (success) -> WEBHOOK_UPDATE (success)
236+
# APP_CREATE (success) -> INSTALL_CREATE (success) -> WEBHOOK_UPDATE (success)
237237
assert_count_of_metric(
238-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
238+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
239239
)
240240
assert_count_of_metric(
241-
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
241+
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
242242
)
243243

244244
def test_delete_service_hook_on_update(self):

‎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-
# APP_CREATE (success) -> GRANT_EXCHANGER (halt)
83+
# APP_CREATE (success) -> INSTALLATION_CREATE (success) -> GRANT_EXCHANGER (halt)
8484
assert_count_of_metric(
85-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
85+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
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-
# APP_CREATE (success) -> TOKEN_EXCHANGE (success) -> REFRESHER (halt)
75+
# APP_CREATE (success) -> WEBHOOK_UPDATE (success) -> TOKEN_EXCHANGE (success) -> REFRESHER (halt)
7676
assert_count_of_metric(
77-
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
77+
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=4
7878
)
7979
assert_count_of_metric(
8080
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1

0 commit comments

Comments
 (0)