Skip to content

Commit

Permalink
fix(hc): Silo fixes for alert rule actions (#58185)
Browse files Browse the repository at this point in the history
Restore #57949, which was reverted by #58091. Fix
SENTRY-16NH (https://sentry.sentry.io/issues/4544284664/).

Adapt OrganizationAlertRuleAvailableActionIndexEndpoint and
get_available_action_integrations_for_org to run in the region silo.

Introduce a prepare_sentry_app_components method to AppService. Remove
the method of the same name from the SentryAppInstallation model. (It
previously dispatched to the same module-level function that AppService
now calls. The model method was called nowhere else.)

Mark OrganizationAlertRuleAvailableActionIndexEndpointTest as stable.
Change setup to pass RPC models as needed.
  • Loading branch information
RyanSkonnord committed Oct 16, 2023
1 parent 650777e commit 9b30768
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 52 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from __future__ import annotations

from collections import defaultdict
from typing import Any, DefaultDict, List, Mapping

from rest_framework import status
from rest_framework.request import Request
Expand All @@ -9,20 +12,24 @@
from sentry.api.base import region_silo_endpoint
from sentry.api.bases.organization import OrganizationEndpoint
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.constants import SentryAppStatus
from sentry.incidents.logic import (
get_available_action_integrations_for_org,
get_opsgenie_teams,
get_pagerduty_services,
)
from sentry.incidents.models import AlertRuleTriggerAction
from sentry.incidents.serializers import ACTION_TARGET_TYPE_TO_STRING
from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
from sentry.models.organization import Organization
from sentry.services.hybrid_cloud.app import RpcSentryAppInstallation, app_service
from sentry.services.hybrid_cloud.integration import RpcIntegration


def build_action_response(
registered_type, integration=None, organization=None, sentry_app_installation=None
):
registered_type,
integration: RpcIntegration | None = None,
organization: Organization | None = None,
sentry_app_installation: RpcSentryAppInstallation | None = None,
) -> Mapping[str, Any]:
"""
Build the "available action" objects for the API. Each one can have different fields.
Expand All @@ -45,28 +52,32 @@ def build_action_response(
action_response["integrationId"] = integration.id

if registered_type.type == AlertRuleTriggerAction.Type.PAGERDUTY:
if organization is None:
raise Exception("Organization is required for PAGERDUTY actions")
action_response["options"] = [
{"value": id, "label": service_name}
for id, service_name in get_pagerduty_services(organization.id, integration.id)
]
elif registered_type.type == AlertRuleTriggerAction.Type.OPSGENIE:
if organization is None:
raise Exception("Organization is required for OPSGENIE actions")
action_response["options"] = [
{"value": id, "label": team}
for id, team in get_opsgenie_teams(organization.id, integration.id)
]

elif sentry_app_installation:
action_response["sentryAppName"] = sentry_app_installation.sentry_app.name
action_response["sentryAppId"] = sentry_app_installation.sentry_app_id
action_response["sentryAppId"] = sentry_app_installation.sentry_app.id
action_response["sentryAppInstallationUuid"] = sentry_app_installation.uuid
action_response["status"] = SentryAppStatus.as_str(
sentry_app_installation.sentry_app.status
)
action_response["status"] = sentry_app_installation.sentry_app.status

# Sentry Apps can be alertable but not have an Alert Rule UI Component
component = sentry_app_installation.prepare_sentry_app_components("alert-rule-action")
component = app_service.prepare_sentry_app_components(
installation_id=sentry_app_installation.id, component_type="alert-rule-action"
)
if component:
action_response["settings"] = component.schema.get("settings", {})
action_response["settings"] = component.app_schema.get("settings", {})

return action_response

Expand All @@ -87,7 +98,7 @@ def get(self, request: Request, organization) -> Response:
actions = []

# Cache Integration objects in this data structure to save DB calls.
provider_integrations = defaultdict(list)
provider_integrations: DefaultDict[str, List[RpcIntegration]] = defaultdict(list)
for integration in get_available_action_integrations_for_org(organization):
provider_integrations[integration.provider].append(integration)

Expand All @@ -103,13 +114,13 @@ def get(self, request: Request, organization) -> Response:

# Add all alertable SentryApps to the list.
elif registered_type.type == AlertRuleTriggerAction.Type.SENTRY_APP:
installs = app_service.get_installed_for_organization(
organization_id=organization.id
)
actions += [
build_action_response(registered_type, sentry_app_installation=install)
for install in SentryAppInstallation.objects.get_installed_for_organization(
organization.id
).filter(
sentry_app__is_alertable=True,
)
for install in installs
if install.sentry_app.is_alertable
]

else:
Expand Down
12 changes: 7 additions & 5 deletions src/sentry/incidents/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from sentry import analytics, audit_log, features, quotas
from sentry.auth.access import SystemAccess
from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS
from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS, ObjectStatus
from sentry.incidents import tasks
from sentry.incidents.models import (
AlertRule,
Expand All @@ -38,7 +38,6 @@
TriggerStatus,
)
from sentry.models.actor import Actor
from sentry.models.integrations.integration import Integration
from sentry.models.integrations.organization_integration import OrganizationIntegration
from sentry.models.notificationaction import ActionService, ActionTarget
from sentry.models.project import Project
Expand Down Expand Up @@ -1458,7 +1457,7 @@ def get_actions_for_trigger(trigger):
return AlertRuleTriggerAction.objects.filter(alert_rule_trigger=trigger)


def get_available_action_integrations_for_org(organization):
def get_available_action_integrations_for_org(organization) -> List[RpcIntegration]:
"""
Returns a list of integrations that the organization has installed. Integrations are
filtered by the list of registered providers.
Expand All @@ -1472,8 +1471,11 @@ def get_available_action_integrations_for_org(organization):
if registration.type != AlertRuleTriggerAction.Type.DISCORD
or features.has("organizations:integrations-discord-metric-alerts", organization)
]
return Integration.objects.get_active_integrations(organization.id).filter(
provider__in=providers
return integration_service.get_integrations(
status=ObjectStatus.ACTIVE,
org_integration_status=ObjectStatus.ACTIVE,
organization_id=organization.id,
providers=providers,
)


Expand Down
28 changes: 11 additions & 17 deletions src/sentry/models/integrations/sentry_app_installation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import uuid
from itertools import chain
from typing import TYPE_CHECKING, Any, List
from typing import TYPE_CHECKING, Any, List, Mapping

from django.db import models, router, transaction
from django.db.models import OuterRef, QuerySet, Subquery
Expand All @@ -19,6 +19,7 @@
)
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
from sentry.services.hybrid_cloud.auth import AuthenticatedToken
from sentry.services.hybrid_cloud.project import RpcProject
from sentry.types.region import find_regions_for_orgs

if TYPE_CHECKING:
Expand Down Expand Up @@ -195,19 +196,12 @@ def outboxes_for_update(self) -> List[ControlOutbox]:
for region_name in find_regions_for_orgs([self.organization_id])
]

def prepare_sentry_app_components(self, component_type, project=None, values=None):
from sentry.models.integrations.sentry_app_component import SentryAppComponent

try:
component = SentryAppComponent.objects.get(
sentry_app_id=self.sentry_app_id, type=component_type
)
except SentryAppComponent.DoesNotExist:
return None

return self.prepare_ui_component(component, project, values)

def prepare_ui_component(self, component, project=None, values=None):
def prepare_ui_component(
self,
component: SentryAppComponent,
project: Project | RpcProject | None = None,
values: Any = None,
) -> SentryAppComponent | None:
return prepare_ui_component(
self, component, project_slug=project.slug if project else None, values=values
)
Expand All @@ -217,8 +211,8 @@ def prepare_sentry_app_components(
installation: SentryAppInstallation,
component_type: str,
project_slug: str | None = None,
values: Any = None,
):
values: List[Mapping[str, Any]] | None = None,
) -> SentryAppComponent | None:
from sentry.models.integrations.sentry_app_component import SentryAppComponent

try:
Expand All @@ -235,7 +229,7 @@ def prepare_ui_component(
installation: SentryAppInstallation,
component: SentryAppComponent,
project_slug: str | None = None,
values: Any = None,
values: List[Mapping[str, Any]] | None = None,
) -> SentryAppComponent | None:
from sentry.coreapi import APIError
from sentry.sentry_apps.components import SentryAppComponentPreparer
Expand Down
17 changes: 11 additions & 6 deletions src/sentry/services/hybrid_cloud/app/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
)
from sentry.services.hybrid_cloud.app.serial import (
serialize_sentry_app,
serialize_sentry_app_component,
serialize_sentry_app_installation,
)
from sentry.services.hybrid_cloud.auth import AuthenticationContext
Expand Down Expand Up @@ -55,12 +56,7 @@ def get_many(

def find_app_components(self, *, app_id: int) -> List[RpcSentryAppComponent]:
return [
RpcSentryAppComponent(
uuid=str(c.uuid),
sentry_app_id=c.sentry_app_id,
type=c.type,
app_schema=c.schema,
)
serialize_sentry_app_component(c)
for c in SentryAppComponent.objects.filter(sentry_app_id=app_id)
]

Expand Down Expand Up @@ -263,3 +259,12 @@ def create_internal_integration_for_channel_request(
installation = SentryAppInstallation.objects.get(sentry_app=sentry_app)

return serialize_sentry_app_installation(installation=installation, app=sentry_app)

def prepare_sentry_app_components(
self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
) -> Optional[RpcSentryAppComponent]:
from sentry.models.integrations.sentry_app_installation import prepare_sentry_app_components

installation = SentryAppInstallation.objects.get(id=installation_id)
component = prepare_sentry_app_components(installation, component_type, project_slug)
return serialize_sentry_app_component(component) if component else None
1 change: 1 addition & 0 deletions src/sentry/services/hybrid_cloud/app/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class RpcSentryApp(RpcModel):
uuid: str = ""
events: List[str] = Field(default_factory=list)
webhook_url: Optional[str] = None
is_alertable: bool = False
is_published: bool = False
is_unpublished: bool = False
is_internal: bool = True
Expand Down
14 changes: 13 additions & 1 deletion src/sentry/services/hybrid_cloud/app/serial.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

from sentry.constants import SentryAppStatus
from sentry.models.apiapplication import ApiApplication
from sentry.models.integrations import SentryAppComponent
from sentry.models.integrations.sentry_app import SentryApp
from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
from sentry.services.hybrid_cloud.app import (
RpcApiApplication,
RpcSentryApp,
RpcSentryAppComponent,
RpcSentryAppInstallation,
)

Expand Down Expand Up @@ -34,11 +36,12 @@ def serialize_sentry_app(app: SentryApp) -> RpcSentryApp:
uuid=app.uuid,
events=app.events,
webhook_url=app.webhook_url,
is_alertable=app.is_alertable,
is_published=app.status == SentryAppStatus.PUBLISHED,
is_unpublished=app.status == SentryAppStatus.UNPUBLISHED,
is_internal=app.status == SentryAppStatus.INTERNAL,
is_publish_request_inprogress=app.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS,
status=app.status,
status=SentryAppStatus.as_str(app.status),
)


Expand All @@ -58,3 +61,12 @@ def serialize_sentry_app_installation(
uuid=installation.uuid,
api_token=installation.api_token.token if installation.api_token else None,
)


def serialize_sentry_app_component(component: SentryAppComponent) -> RpcSentryAppComponent:
return RpcSentryAppComponent(
uuid=str(component.uuid),
sentry_app_id=component.sentry_app_id,
type=component.type,
app_schema=component.schema,
)
7 changes: 7 additions & 0 deletions src/sentry/services/hybrid_cloud/app/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,12 @@ def create_internal_integration_for_channel_request(
) -> RpcSentryAppInstallation:
pass

@rpc_method
@abc.abstractmethod
def prepare_sentry_app_components(
self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
) -> Optional[RpcSentryAppComponent]:
pass


app_service = cast(AppService, AppService.create_delegation())
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from typing import Any, Mapping

from sentry.constants import ObjectStatus, SentryAppStatus
from sentry.incidents.endpoints.organization_alert_rule_available_action_index import (
build_action_response,
)
from sentry.incidents.models import AlertRuleTriggerAction
from sentry.models.integrations import SentryAppComponent, SentryAppInstallation
from sentry.models.integrations.integration import Integration
from sentry.models.integrations.organization_integration import OrganizationIntegration
from sentry.services.hybrid_cloud.app.serial import serialize_sentry_app_installation
from sentry.silo import SiloMode
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
Expand All @@ -25,7 +29,7 @@
}


@region_silo_test
@region_silo_test(stable=True)
class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
endpoint = "sentry-api-0-organization-alert-rule-available-actions"
email = AlertRuleTriggerAction.get_registered_type(AlertRuleTriggerAction.Type.EMAIL)
Expand All @@ -38,7 +42,7 @@ def setUp(self):
super().setUp()
self.login_as(self.user)

def install_new_sentry_app(self, name, **kwargs):
def install_new_sentry_app(self, name, **kwargs) -> SentryAppInstallation:
kwargs.update(
name=name, organization=self.organization, is_alertable=True, verify_install=False
)
Expand Down Expand Up @@ -109,12 +113,30 @@ def test_build_action_response_pagerduty(self):
def test_build_action_response_sentry_app(self):
installation = self.install_new_sentry_app("foo")

data = build_action_response(self.sentry_app, sentry_app_installation=installation)
data = build_action_response(
self.sentry_app, sentry_app_installation=serialize_sentry_app_installation(installation)
)

assert data["type"] == "sentry_app"
assert data["allowedTargetTypes"] == ["sentry_app"]
assert data["status"] == SentryAppStatus.UNPUBLISHED_STR

def test_build_action_response_sentry_app_with_component(self):
installation = self.install_new_sentry_app("foo")
test_settings: Mapping[str, Any] = {"test-settings": []}
with assume_test_silo_mode(SiloMode.CONTROL):
SentryAppComponent.objects.create(
sentry_app=installation.sentry_app,
type="alert-rule-action",
schema={"settings": test_settings},
)

data = build_action_response(
self.sentry_app, sentry_app_installation=serialize_sentry_app_installation(installation)
)

assert data["settings"] == test_settings

def test_no_integrations(self):
with self.feature("organizations:incidents"):
response = self.get_success_response(self.organization.slug)
Expand Down Expand Up @@ -180,7 +202,10 @@ def test_sentry_apps(self):
assert len(response.data) == 2
assert build_action_response(self.email) in response.data
assert (
build_action_response(self.sentry_app, sentry_app_installation=installation)
build_action_response(
self.sentry_app,
sentry_app_installation=serialize_sentry_app_installation(installation),
)
in response.data
)

Expand All @@ -193,7 +218,10 @@ def test_published_sentry_apps(self):

assert len(response.data) == 2
assert (
build_action_response(self.sentry_app, sentry_app_installation=installation)
build_action_response(
self.sentry_app,
sentry_app_installation=serialize_sentry_app_installation(installation),
)
in response.data
)

Expand Down
Loading

0 comments on commit 9b30768

Please sign in to comment.