From 99e689fb0e367ccf6e6aff14de8dfdb3afe979aa Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Tue, 3 Dec 2024 16:10:48 -0800 Subject: [PATCH 1/9] new control endpoint --- src/sentry/api/urls.py | 6 + .../api/endpoints/sentry_app_requests_v2.py | 168 ++++++++ .../sentry_apps/api/serializers/request_v2.py | 46 +++ .../sentry_apps/services/app_request/model.py | 4 +- .../utils/sentry_apps/request_buffer.py | 24 +- .../endpoints/test_sentry_app_requests_v2.py | 372 ++++++++++++++++++ 6 files changed, 607 insertions(+), 13 deletions(-) create mode 100644 src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py create mode 100644 src/sentry/sentry_apps/api/serializers/request_v2.py create mode 100644 tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index b3e00d4222cfc9..a9935a73a12506 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -300,6 +300,7 @@ SentryAppPublishRequestEndpoint, ) from sentry.sentry_apps.api.endpoints.sentry_app_requests import SentryAppRequestsEndpoint +from sentry.sentry_apps.api.endpoints.sentry_app_requests_v2 import SentryAppRequestsEndpointV2 from sentry.sentry_apps.api.endpoints.sentry_app_rotate_secret import SentryAppRotateSecretEndpoint from sentry.sentry_apps.api.endpoints.sentry_app_stats_details import SentryAppStatsEndpoint from sentry.sentry_apps.api.endpoints.sentry_apps import SentryAppsEndpoint @@ -2906,6 +2907,11 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: SentryAppPublishRequestEndpoint.as_view(), name="sentry-api-0-sentry-app-publish-request", ), + re_path( + r"^(?P[^\/]+)/requests-v2/$", + SentryAppRequestsEndpointV2.as_view(), + name="sentry-api-0-sentry-app-requests-v2", + ), # The following a region endpoints as interactions and request logs # are per-region. re_path( diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py new file mode 100644 index 00000000000000..d24c996dfa0860 --- /dev/null +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py @@ -0,0 +1,168 @@ +from dataclasses import dataclass +from datetime import datetime + +from dateutil.parser import parse as parse_date +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import control_silo_endpoint +from sentry.api.paginator import GenericOffsetPaginator +from sentry.api.serializers import serialize +from sentry.organizations.services.organization import RpcOrganizationSummary, organization_service +from sentry.sentry_apps.api.bases.sentryapps import SentryAppBaseEndpoint, SentryAppStatsPermission +from sentry.sentry_apps.api.serializers.request_v2 import RequestSerializer +from sentry.sentry_apps.models.sentry_app import SentryApp +from sentry.sentry_apps.services.app_request import RpcSentryAppRequest, SentryAppRequestFilterArgs +from sentry.sentry_apps.services.app_request.serial import serialize_rpc_sentry_app_request +from sentry.sentry_apps.services.app_request.service import app_request_service +from sentry.types.region import find_all_region_names +from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS, SentryAppWebhookRequestsBuffer + +INVALID_DATE_FORMAT_MESSAGE = "Invalid date format. Format must be YYYY-MM-DD HH:MM:SS." + + +def filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: + date_str = request.date + if not date_str: + return False + timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace(microsecond=0) + return start <= timestamp <= end + + +def filter_by_organization( + request: RpcSentryAppRequest, organization: RpcOrganizationSummary | None +) -> bool: + if not organization: + return True + return request.organization_id == organization.id + + +@dataclass +class BufferedRequest: + id: int + data: RpcSentryAppRequest + + def __hash__(self): + return self.id + + +@control_silo_endpoint +class SentryAppRequestsEndpointV2(SentryAppBaseEndpoint): + owner = ApiOwner.INTEGRATIONS + publish_status = { + "GET": ApiPublishStatus.UNKNOWN, + } + permission_classes = (SentryAppStatsPermission,) + + def get(self, request: Request, sentry_app: SentryApp) -> Response: + """ + :qparam string eventType: Optionally specify a specific event type to filter requests + :qparam bool errorsOnly: If this is true, only return error/warning requests (300-599) + :qparam string organizationSlug: Optionally specify an org slug to filter requests + :qparam string start: Optionally specify a date to begin at. Format must be YYYY-MM-DD HH:MM:SS + :qparam string end: Optionally specify a date to end at. Format must be YYYY-MM-DD HH:MM:SS + """ + date_format = "%Y-%m-%d %H:%M:%S" + start_time: datetime = datetime.strptime("2000-01-01 00:00:00", date_format) + end_time: datetime = datetime.now() + + event_type = request.GET.get("eventType") + errors_only = request.GET.get("errorsOnly") + org_slug = request.GET.get("organizationSlug") + start_parameter = request.GET.get("start", None) + end_parameter = request.GET.get("end", None) + + try: + start_time = ( + datetime.strptime(start_parameter, date_format) if start_parameter else start_time + ) + except ValueError: + return Response({"detail": INVALID_DATE_FORMAT_MESSAGE}, status=400) + + try: + + end_time = datetime.strptime(end_parameter, date_format) if end_parameter else end_time + except ValueError: + return Response({"detail": INVALID_DATE_FORMAT_MESSAGE}, status=400) + + filter: SentryAppRequestFilterArgs = {} + if event_type: + if event_type not in EXTENDED_VALID_EVENTS: + return Response({"detail": "Invalid event type."}, status=400) + filter["event"] = event_type + else: + filter["event"] = list( + set(EXTENDED_VALID_EVENTS) + - { + "installation.created", + "installation.deleted", + } + ) + if errors_only: + filter["errors_only"] = True + + organization = None + if org_slug: + organization = organization_service.get_org_by_slug(slug=org_slug) + if organization is None: + return Response({"detail": "Invalid organization."}, status=400) + + requests: list[RpcSentryAppRequest] = [] + + buffer = SentryAppWebhookRequestsBuffer(sentry_app) + control_errors_only = True if errors_only else False + + if event_type == "installation.created" or event_type == "installation.deleted": + + requests.extend( + [ + serialize_rpc_sentry_app_request(req) + for req in buffer.get_requests( + event=event_type, errors_only=control_errors_only + ) + ] + ) + elif event_type is None: + control_event_type = [ + "installation.created", + "installation.deleted", + ] + requests.extend( + [ + serialize_rpc_sentry_app_request(req) + for req in buffer.get_requests( + event=control_event_type, errors_only=control_errors_only + ) + ] + ) + + # If event type is installation.created or installation.deleted, we don't need to fetch requests from other regions + if event_type != "installation.created" and event_type != "installation.deleted": + for region_name in find_all_region_names(): + buffer_requests = app_request_service.get_buffer_requests_for_region( + sentry_app_id=sentry_app.id, + region_name=region_name, + filter=filter, + ) + if buffer_requests is not None: + requests.extend(buffer_requests) + + requests.sort(key=lambda x: parse_date(x.date), reverse=True) + filtered_requests: list[BufferedRequest] = [] + for i, req in enumerate(requests): + if filter_by_date(req, start_time, end_time) and filter_by_organization( + req, organization + ): + filtered_requests.append(BufferedRequest(id=i, data=req)) + + def data_fn(offset, limit): + page_offset = offset * limit + return filtered_requests[page_offset : page_offset + limit] + + return self.paginate( + request=request, + paginator=GenericOffsetPaginator(data_fn), + on_results=lambda x: serialize(x, request.user, RequestSerializer(sentry_app)), + ) diff --git a/src/sentry/sentry_apps/api/serializers/request_v2.py b/src/sentry/sentry_apps/api/serializers/request_v2.py new file mode 100644 index 00000000000000..54998101d4f88b --- /dev/null +++ b/src/sentry/sentry_apps/api/serializers/request_v2.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from collections.abc import Mapping, MutableMapping, Sequence +from typing import Any + +from sentry.api.serializers import Serializer +from sentry.hybridcloud.services.organization_mapping import organization_mapping_service +from sentry.sentry_apps.models.sentry_app import SentryApp + + +class RequestSerializer(Serializer): + def __init__(self, sentry_app: SentryApp) -> None: + self.sentry_app = sentry_app + + def get_attrs( + self, item_list: Sequence[Any], user: Any, **kwargs: Any + ) -> MutableMapping[Any, Any]: + organization_ids = {item.data.organization_id for item in item_list} + organizations = organization_mapping_service.get_many(organization_ids=organization_ids) + organizations_by_id = {organization.id: organization for organization in organizations} + + return { + item: { + "organization": organizations_by_id.get(item.data.organization_id), + } + for item in item_list + } + + def serialize( + self, obj: Any, attrs: Mapping[Any, Any], user: Any, **kwargs: Any + ) -> Mapping[str, Any]: + organization = attrs.get("organization") + response_code = obj.data.response_code + + data = { + "webhookUrl": obj.data.webhook_url, + "sentryAppSlug": self.sentry_app.slug, + "eventType": obj.data.event_type, + "date": obj.data.date, + "responseCode": response_code, + } + + if organization: + data["organization"] = {"name": organization.name, "slug": organization.slug} + + return data diff --git a/src/sentry/sentry_apps/services/app_request/model.py b/src/sentry/sentry_apps/services/app_request/model.py index c676f5eeb0af8e..3e98df2d0a417c 100644 --- a/src/sentry/sentry_apps/services/app_request/model.py +++ b/src/sentry/sentry_apps/services/app_request/model.py @@ -12,10 +12,10 @@ class RpcSentryAppRequest(RpcModel): date: str response_code: int webhook_url: str - organization_id: int + organization_id: int | None event_type: str class SentryAppRequestFilterArgs(TypedDict, total=False): - event: str + event: str | list[str] errors_only: bool diff --git a/src/sentry/utils/sentry_apps/request_buffer.py b/src/sentry/utils/sentry_apps/request_buffer.py index e7d60fa4393714..e27b6c3c68e3f2 100644 --- a/src/sentry/utils/sentry_apps/request_buffer.py +++ b/src/sentry/utils/sentry_apps/request_buffer.py @@ -100,18 +100,26 @@ def _get_all_from_buffer( else: return self.client.lrange(buffer_key, 0, BUFFER_SIZE - 1) - def _get_requests(self, event: str | None = None, error: bool = False) -> list[dict[str, Any]]: + def _get_requests( + self, event: str | list[str] | None = None, error: bool = False + ) -> list[dict[str, Any]]: + if isinstance(event, str): + return [ + self._convert_redis_request(request, event) + for request in self._get_all_from_buffer(self._get_redis_key(event, error=error)) + ] # If no event is specified, return the latest requests/errors for all event types - if event is None: + else: + event_types = event or EXTENDED_VALID_EVENTS pipe = self.client.pipeline() all_requests = [] - for evt in EXTENDED_VALID_EVENTS: + for evt in event_types: self._get_all_from_buffer(self._get_redis_key(evt, error=error), pipeline=pipe) values = pipe.execute() - for idx, evt in enumerate(EXTENDED_VALID_EVENTS): + for idx, evt in enumerate(event_types): event_requests = [ self._convert_redis_request(request, evt) for request in values[idx] ] @@ -120,14 +128,8 @@ def _get_requests(self, event: str | None = None, error: bool = False) -> list[d all_requests.sort(key=lambda x: parse_date(x["date"]), reverse=True) return all_requests[0:BUFFER_SIZE] - else: - return [ - self._convert_redis_request(request, event) - for request in self._get_all_from_buffer(self._get_redis_key(event, error=error)) - ] - def get_requests( - self, event: str | None = None, errors_only: bool = False + self, event: str | list[str] | None = None, errors_only: bool = False ) -> list[dict[str, Any]]: return self._get_requests(event=event, error=errors_only) diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py new file mode 100644 index 00000000000000..6c7218161f2d0c --- /dev/null +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py @@ -0,0 +1,372 @@ +from datetime import datetime, timedelta + +from django.urls import reverse + +from sentry.sentry_apps.api.endpoints.sentry_app_requests_v2 import INVALID_DATE_FORMAT_MESSAGE +from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers.datetime import before_now, freeze_time +from sentry.testutils.silo import control_silo_test, create_test_regions +from sentry.testutils.skips import requires_snuba +from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer + +pytestmark = [requires_snuba] + + +@control_silo_test(regions=create_test_regions("us")) +class SentryAppRequestsV2GetTest(APITestCase): + def setUp(self): + self.superuser = self.create_user(email="superuser@example.com", is_superuser=True) + self.user = self.create_user(email="user@example.com") + self.org = self.create_organization(owner=self.user, region="us") + self.project = self.create_project(organization=self.org) + self.event_id = "d5111da2c28645c5889d072017e3445d" + + self.published_app = self.create_sentry_app( + name="Published App", organization=self.org, published=True + ) + self.unowned_published_app = self.create_sentry_app( + name="Unowned Published App", organization=self.create_organization(), published=True + ) + + self.unpublished_app = self.create_sentry_app(name="Unpublished App", organization=self.org) + self.unowned_unpublished_app = self.create_sentry_app( + name="Unowned Unpublished App", organization=self.create_organization() + ) + + self.internal_app = self.create_internal_integration( + name="Internal app", organization=self.org + ) + + self.create_sentry_app_installation( + organization=self.org, slug=self.published_app.slug, prevent_token_exchange=True + ) + + def test_superuser_sees_unowned_published_requests(self): + self.login_as(user=self.superuser, superuser=True) + + buffer = SentryAppWebhookRequestsBuffer(self.unowned_published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.unowned_published_app.webhook_url, + ) + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="issue.assigned", + url=self.unowned_published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unowned_published_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 2 + assert response.data[0]["organization"]["slug"] == self.org.slug + assert response.data[0]["sentryAppSlug"] == self.unowned_published_app.slug + assert response.data[0]["responseCode"] == 500 + + def test_superuser_sees_unpublished_stats(self): + self.login_as(user=self.superuser, superuser=True) + + buffer = SentryAppWebhookRequestsBuffer(self.unowned_unpublished_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.unowned_unpublished_app.webhook_url, + ) + + url = reverse( + "sentry-api-0-sentry-app-requests-v2", args=[self.unowned_unpublished_app.slug] + ) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 1 + assert response.data[0]["sentryAppSlug"] == self.unowned_unpublished_app.slug + + def test_user_sees_owned_published_requests(self): + self.login_as(user=self.user) + + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 1 + assert response.data[0]["organization"]["slug"] == self.org.slug + assert response.data[0]["sentryAppSlug"] == self.published_app.slug + assert response.data[0]["responseCode"] == 200 + + def test_user_does_not_see_unowned_published_requests(self): + self.login_as(user=self.user) + + buffer = SentryAppWebhookRequestsBuffer(self.unowned_published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.unowned_published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unowned_published_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 403 + assert response.data["detail"] == "You do not have permission to perform this action." + + def test_user_sees_owned_unpublished_requests(self): + self.login_as(user=self.user) + + buffer = SentryAppWebhookRequestsBuffer(self.unpublished_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.unpublished_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unpublished_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 1 + + def test_internal_app_requests_does_not_have_organization_field(self): + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.internal_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.internal_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.internal_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 1 + assert "organization" not in response.data[0] + assert response.data[0]["sentryAppSlug"] == self.internal_app.slug + assert response.data[0]["responseCode"] == 200 + + def test_event_type_filter(self): + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="installation.created", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + response1 = self.client.get(f"{url}?eventType=issue.created", format="json") + assert response1.status_code == 200 + assert len(response1.data) == 0 + + response2 = self.client.get(f"{url}?eventType=issue.assigned", format="json") + assert response2.status_code == 200 + assert len(response2.data) == 1 + + response3 = self.client.get(f"{url}?eventType=installation.created", format="json") + assert response3.status_code == 200 + assert len(response3.data) == 1 + + def test_invalid_event_type(self): + self.login_as(user=self.user) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + response = self.client.get(f"{url}?eventType=invalid_type", format="json") + + assert response.status_code == 400 + + def test_errors_only_filter(self): + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + errors_only_response = self.client.get(f"{url}?errorsOnly=true", format="json") + assert errors_only_response.status_code == 200 + assert len(errors_only_response.data) == 1 + + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 2 + + def test_linked_error_not_returned_if_project_does_not_exist(self): + self.login_as(user=self.user) + + self.store_event( + data={"event_id": self.event_id, "timestamp": before_now(minutes=1).isoformat()}, + project_id=self.project.id, + ) + + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.unpublished_app.webhook_url, + error_id=self.event_id, + project_id=1000, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 1 + assert response.data[0]["organization"]["slug"] == self.org.slug + assert response.data[0]["sentryAppSlug"] == self.published_app.slug + assert "errorUrl" not in response.data[0] + + def test_org_slug_filter(self): + """Test that filtering by the qparam organizationSlug properly filters results""" + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + made_up_org_response = self.client.get(f"{url}?organizationSlug=madeUpOrg", format="json") + assert made_up_org_response.status_code == 400 + assert made_up_org_response.data["detail"] == "Invalid organization." + + org_response = self.client.get(f"{url}?organizationSlug={self.org.slug}", format="json") + assert org_response.status_code == 200 + assert len(org_response.data) == 2 + + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 2 + + def test_date_filter(self): + """Test that filtering by the qparams start and end properly filters results""" + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + now = datetime.now() - timedelta(hours=1) + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + with freeze_time(now + timedelta(seconds=1)): + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + with freeze_time(now + timedelta(seconds=2)): + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 3 + + # test adding a start time + start_date = now.strftime("%Y-%m-%d %H:%M:%S") + start_date_response = self.client.get(f"{url}?start={start_date}", format="json") + assert start_date_response.status_code == 200 + assert len(start_date_response.data) == 3 + + # test adding an end time + end_date = (now + timedelta(seconds=2)).strftime("%Y-%m-%d %H:%M:%S") + end_date_response = self.client.get(f"{url}?end={end_date}", format="json") + assert end_date_response.status_code == 200 + assert len(end_date_response.data) == 2 + + # test adding a start and end time + new_start_date = (now + timedelta(seconds=1)).strftime("%Y-%m-%d %H:%M:%S") + new_end_date = (now + timedelta(seconds=2)).strftime("%Y-%m-%d %H:%M:%S") + start_end_date_response = self.client.get( + f"{url}?start={new_start_date}&end={new_end_date}", format="json" + ) + assert start_end_date_response.status_code == 200 + assert len(start_end_date_response.data) == 2 + + # test adding an improperly formatted end time + bad_date_format_response = self.client.get(f"{url}?end=2000-01- 00:00:00", format="json") + assert bad_date_format_response.data["detail"] == INVALID_DATE_FORMAT_MESSAGE + + def test_get_includes_installation_requests(self): + self.login_as(user=self.user) + buffer = SentryAppWebhookRequestsBuffer(self.published_app) + now = datetime.now() - timedelta(hours=1) + with freeze_time(now): + buffer.add_request( + response_code=200, + org_id=self.org.id, + event="issue.created", + url=self.published_app.webhook_url, + ) + with freeze_time(now + timedelta(seconds=1)): + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="installation.created", + url=self.published_app.webhook_url, + ) + with freeze_time(now + timedelta(seconds=2)): + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="issue.assigned", + url=self.published_app.webhook_url, + ) + with freeze_time(now + timedelta(seconds=3)): + buffer.add_request( + response_code=500, + org_id=self.org.id, + event="installation.deleted", + url=self.published_app.webhook_url, + ) + + url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + + response = self.client.get(url, format="json") + assert response.status_code == 200 + assert len(response.data) == 4 + assert response.data[0]["eventType"] == "installation.deleted" + assert response.data[1]["eventType"] == "issue.assigned" + assert response.data[2]["eventType"] == "installation.created" + assert response.data[3]["eventType"] == "issue.created" From baa6ceb9edc2f5ffa997dca4a243959962214105 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Thu, 5 Dec 2024 10:30:23 -0800 Subject: [PATCH 2/9] BORKED --- .../api/endpoints/sentry_app_requests_v2.py | 122 +++++++++++------- .../endpoints/test_sentry_app_requests_v2.py | 12 +- 2 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py index d24c996dfa0860..30e0fbd61c14ba 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py @@ -1,7 +1,12 @@ from dataclasses import dataclass from datetime import datetime +from collections.abc import Mapping +from typing import Any + from dateutil.parser import parse as parse_date +from responses import start +from rest_framework import serializers, status from rest_framework.request import Request from rest_framework.response import Response @@ -9,7 +14,6 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.paginator import GenericOffsetPaginator -from sentry.api.serializers import serialize from sentry.organizations.services.organization import RpcOrganizationSummary, organization_service from sentry.sentry_apps.api.bases.sentryapps import SentryAppBaseEndpoint, SentryAppStatsPermission from sentry.sentry_apps.api.serializers.request_v2 import RequestSerializer @@ -19,10 +23,58 @@ from sentry.sentry_apps.services.app_request.service import app_request_service from sentry.types.region import find_all_region_names from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS, SentryAppWebhookRequestsBuffer +from sentry.api.serializers import Serializer, serialize +from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer INVALID_DATE_FORMAT_MESSAGE = "Invalid date format. Format must be YYYY-MM-DD HH:MM:SS." +class IncomingRequestSerializer(CamelSnakeSerializer, serializers.Serializer): + date_format = "%Y-%m-%d %H:%M:%S" + event_type = serializers.ChoiceField( + choices=EXTENDED_VALID_EVENTS, + required=False, + ) + errors_only = serializers.BooleanField(required=False) + organization_slug = serializers.CharField(required=False) + start = serializers.DateTimeField( + format=date_format, + default=datetime.strptime("2000-01-01 00:00:00", date_format), + required=False, + ) + end = serializers.DateTimeField(format=date_format, default=datetime.now(), required=False) + + def validate(self, data): + start_time = data.get("start") + end_time = data.get("end") + + if start_time and end_time and start_time >= end_time: + raise serializers.ValidationError("Invalid timestamp (start must be before end).") + return data + + # def validate_organization_slug( + # self, organization_slug: str | None + # ) -> RpcOrganizationSummary | None: + # breakpoint() + # if organization_slug: + # organization = organization_service.get_org_by_slug(slug=organization_slug) + # if organization is None: + # raise serializers.ValidationError("Invalid organization slug.") + # return organization + # return None + + # def serialize( + # self, obj: Any, attrs: Mapping[Any, Any], user: Any, **kwargs: Any + # ) -> Mapping[str, Any]: + # return { + # "event_type": obj.event_type, + # "errors_only": obj.errors_only, + # "org_slug": obj.org_slug, + # "start_time": obj.start_time, + # "end_time": obj.end_time, + # } + + def filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: date_str = request.date if not date_str: @@ -52,7 +104,7 @@ def __hash__(self): class SentryAppRequestsEndpointV2(SentryAppBaseEndpoint): owner = ApiOwner.INTEGRATIONS publish_status = { - "GET": ApiPublishStatus.UNKNOWN, + "GET": ApiPublishStatus.EXPERIMENTAL, } permission_classes = (SentryAppStatsPermission,) @@ -64,28 +116,17 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: :qparam string start: Optionally specify a date to begin at. Format must be YYYY-MM-DD HH:MM:SS :qparam string end: Optionally specify a date to end at. Format must be YYYY-MM-DD HH:MM:SS """ - date_format = "%Y-%m-%d %H:%M:%S" - start_time: datetime = datetime.strptime("2000-01-01 00:00:00", date_format) - end_time: datetime = datetime.now() - - event_type = request.GET.get("eventType") - errors_only = request.GET.get("errorsOnly") - org_slug = request.GET.get("organizationSlug") - start_parameter = request.GET.get("start", None) - end_parameter = request.GET.get("end", None) - - try: - start_time = ( - datetime.strptime(start_parameter, date_format) if start_parameter else start_time - ) - except ValueError: - return Response({"detail": INVALID_DATE_FORMAT_MESSAGE}, status=400) - - try: - - end_time = datetime.strptime(end_parameter, date_format) if end_parameter else end_time - except ValueError: - return Response({"detail": INVALID_DATE_FORMAT_MESSAGE}, status=400) + breakpoint() + serializer = IncomingRequestSerializer(data=request.GET) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + print("VALIDATED", serializer.validated_data) + event_type = serializer.validated_data.get("event_type") + errors_only = serializer.validated_data.get("errors_only") + organization = serializer.validated_data.get("organization") + start_time = serializer.validated_data.get("start_time") + end_time = serializer.validated_data.get("end_time") filter: SentryAppRequestFilterArgs = {} if event_type: @@ -103,40 +144,25 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: if errors_only: filter["errors_only"] = True - organization = None - if org_slug: - organization = organization_service.get_org_by_slug(slug=org_slug) - if organization is None: - return Response({"detail": "Invalid organization."}, status=400) - requests: list[RpcSentryAppRequest] = [] - buffer = SentryAppWebhookRequestsBuffer(sentry_app) + control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) control_errors_only = True if errors_only else False - if event_type == "installation.created" or event_type == "installation.deleted": + def get_buffer_requests_for_control(event) -> list[RpcSentryAppRequest]: + return [ + serialize_rpc_sentry_app_request(req) + for req in control_buffer.get_requests(event=event, errors_only=control_errors_only) + ] - requests.extend( - [ - serialize_rpc_sentry_app_request(req) - for req in buffer.get_requests( - event=event_type, errors_only=control_errors_only - ) - ] - ) + if event_type == "installation.created" or event_type == "installation.deleted": + requests.extend(get_buffer_requests_for_control(event_type)) elif event_type is None: control_event_type = [ "installation.created", "installation.deleted", ] - requests.extend( - [ - serialize_rpc_sentry_app_request(req) - for req in buffer.get_requests( - event=control_event_type, errors_only=control_errors_only - ) - ] - ) + requests.extend(get_buffer_requests_for_control(control_event_type)) # If event type is installation.created or installation.deleted, we don't need to fetch requests from other regions if event_type != "installation.created" and event_type != "installation.deleted": diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py index 6c7218161f2d0c..2df6b5376adf86 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py @@ -2,6 +2,9 @@ from django.urls import reverse +from rest_framework.exceptions import ErrorDetail + + from sentry.sentry_apps.api.endpoints.sentry_app_requests_v2 import INVALID_DATE_FORMAT_MESSAGE from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import before_now, freeze_time @@ -262,7 +265,14 @@ def test_org_slug_filter(self): url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) made_up_org_response = self.client.get(f"{url}?organizationSlug=madeUpOrg", format="json") assert made_up_org_response.status_code == 400 - assert made_up_org_response.data["detail"] == "Invalid organization." + assert made_up_org_response.data == { + "organization_slug": [ + ErrorDetail( + "Invalid organization slug.", + code="invalid", + ) + ] + } org_response = self.client.get(f"{url}?organizationSlug={self.org.slug}", format="json") assert org_response.status_code == 200 From 7c02538ce482fae57471ef9d17c66e28c24516ca Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Thu, 5 Dec 2024 15:18:26 -0800 Subject: [PATCH 3/9] omg its working --- .../api/endpoints/sentry_app_requests_v2.py | 159 ++++++++---------- .../endpoints/test_sentry_app_requests_v2.py | 25 ++- 2 files changed, 83 insertions(+), 101 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py index 30e0fbd61c14ba..1f7bab8bfaff06 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py @@ -1,11 +1,7 @@ from dataclasses import dataclass -from datetime import datetime - -from collections.abc import Mapping -from typing import Any +from datetime import datetime, timezone from dateutil.parser import parse as parse_date -from responses import start from rest_framework import serializers, status from rest_framework.request import Request from rest_framework.response import Response @@ -14,7 +10,8 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.paginator import GenericOffsetPaginator -from sentry.organizations.services.organization import RpcOrganizationSummary, organization_service +from sentry.api.serializers import serialize +from sentry.models.organizationmapping import OrganizationMapping from sentry.sentry_apps.api.bases.sentryapps import SentryAppBaseEndpoint, SentryAppStatsPermission from sentry.sentry_apps.api.serializers.request_v2 import RequestSerializer from sentry.sentry_apps.models.sentry_app import SentryApp @@ -23,72 +20,53 @@ from sentry.sentry_apps.services.app_request.service import app_request_service from sentry.types.region import find_all_region_names from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS, SentryAppWebhookRequestsBuffer -from sentry.api.serializers import Serializer, serialize -from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer - -INVALID_DATE_FORMAT_MESSAGE = "Invalid date format. Format must be YYYY-MM-DD HH:MM:SS." -class IncomingRequestSerializer(CamelSnakeSerializer, serializers.Serializer): +class IncomingRequestSerializer(serializers.Serializer): date_format = "%Y-%m-%d %H:%M:%S" - event_type = serializers.ChoiceField( + eventType = serializers.ChoiceField( choices=EXTENDED_VALID_EVENTS, required=False, ) - errors_only = serializers.BooleanField(required=False) - organization_slug = serializers.CharField(required=False) + errorsOnly = serializers.BooleanField(required=False) + organizationSlug = serializers.CharField(required=False) start = serializers.DateTimeField( format=date_format, - default=datetime.strptime("2000-01-01 00:00:00", date_format), + default=datetime.strptime("2000-01-01 00:00:00", date_format).replace(tzinfo=timezone.utc), + default_timezone=timezone.utc, required=False, ) - end = serializers.DateTimeField(format=date_format, default=datetime.now(), required=False) + end = serializers.DateTimeField( + format=date_format, default=None, default_timezone=timezone.utc, required=False + ) def validate(self, data): - start_time = data.get("start") - end_time = data.get("end") - - if start_time and end_time and start_time >= end_time: + if "start" in data and "end" in data and data["start"] > data["end"]: raise serializers.ValidationError("Invalid timestamp (start must be before end).") return data - # def validate_organization_slug( - # self, organization_slug: str | None - # ) -> RpcOrganizationSummary | None: - # breakpoint() - # if organization_slug: - # organization = organization_service.get_org_by_slug(slug=organization_slug) - # if organization is None: - # raise serializers.ValidationError("Invalid organization slug.") - # return organization - # return None - - # def serialize( - # self, obj: Any, attrs: Mapping[Any, Any], user: Any, **kwargs: Any - # ) -> Mapping[str, Any]: - # return { - # "event_type": obj.event_type, - # "errors_only": obj.errors_only, - # "org_slug": obj.org_slug, - # "start_time": obj.start_time, - # "end_time": obj.end_time, - # } + def validate_end(self, end): + if end is None: + end = datetime.now(tz=timezone.utc) + return end def filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: date_str = request.date if not date_str: return False - timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace(microsecond=0) + timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( + microsecond=0, tzinfo=timezone.utc + ) return start <= timestamp <= end def filter_by_organization( - request: RpcSentryAppRequest, organization: RpcOrganizationSummary | None + request: RpcSentryAppRequest, organization: OrganizationMapping | None ) -> bool: if not organization: return True - return request.organization_id == organization.id + return request.organization_id == organization.organization_id @dataclass @@ -116,65 +94,72 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: :qparam string start: Optionally specify a date to begin at. Format must be YYYY-MM-DD HH:MM:SS :qparam string end: Optionally specify a date to end at. Format must be YYYY-MM-DD HH:MM:SS """ - breakpoint() serializer = IncomingRequestSerializer(data=request.GET) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + serialized = serializer.validated_data - print("VALIDATED", serializer.validated_data) - event_type = serializer.validated_data.get("event_type") - errors_only = serializer.validated_data.get("errors_only") - organization = serializer.validated_data.get("organization") - start_time = serializer.validated_data.get("start_time") - end_time = serializer.validated_data.get("end_time") - - filter: SentryAppRequestFilterArgs = {} - if event_type: - if event_type not in EXTENDED_VALID_EVENTS: - return Response({"detail": "Invalid event type."}, status=400) - filter["event"] = event_type - else: - filter["event"] = list( - set(EXTENDED_VALID_EVENTS) - - { - "installation.created", - "installation.deleted", - } - ) - if errors_only: - filter["errors_only"] = True + event_type = serialized.get("eventType") + errors_only = serialized.get("errorsOnly") + org_slug = serialized.get("organizationSlug") + start_time = serialized.get("start") + end_time = serialized.get("end") - requests: list[RpcSentryAppRequest] = [] + organization = None + if org_slug: + try: + organization = OrganizationMapping.objects.get(slug=org_slug) + except OrganizationMapping.DoesNotExist: + return Response({"detail": "Invalid organization."}, status=400) + requests: list[RpcSentryAppRequest] = [] + region_filter: SentryAppRequestFilterArgs = {} control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) - control_errors_only = True if errors_only else False - - def get_buffer_requests_for_control(event) -> list[RpcSentryAppRequest]: - return [ - serialize_rpc_sentry_app_request(req) - for req in control_buffer.get_requests(event=event, errors_only=control_errors_only) - ] - - if event_type == "installation.created" or event_type == "installation.deleted": - requests.extend(get_buffer_requests_for_control(event_type)) - elif event_type is None: - control_event_type = [ - "installation.created", - "installation.deleted", - ] - requests.extend(get_buffer_requests_for_control(control_event_type)) + control_errors_only = region_filter["errors_only"] = errors_only + + def get_buffer_requests_for_control(event_type) -> list[RpcSentryAppRequest]: + control_buffer.get_requests(event=event_type, errors_only=control_errors_only) + requests.extend( + [ + serialize_rpc_sentry_app_request(req) + for req in control_buffer.get_requests( + event=event_type, errors_only=control_errors_only + ) + ] + ) - # If event type is installation.created or installation.deleted, we don't need to fetch requests from other regions - if event_type != "installation.created" and event_type != "installation.deleted": + def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: for region_name in find_all_region_names(): buffer_requests = app_request_service.get_buffer_requests_for_region( sentry_app_id=sentry_app.id, region_name=region_name, - filter=filter, + filter=region_filter, ) if buffer_requests is not None: requests.extend(buffer_requests) + # If event type is installation.created or installation.deleted, we only need to fetch requests from the control buffer + if event_type == "installation.created" or event_type == "installation.deleted": + get_buffer_requests_for_control(event_type) + # If event type has been specified, we only need to fetch requests from region buffers + elif event_type: + region_filter["event"] = event_type + get_buffer_requests_for_regions() + else: + control_event_type = [ + "installation.created", + "installation.deleted", + ] + get_buffer_requests_for_control(control_event_type) + region_filter["event"] = list( + set(EXTENDED_VALID_EVENTS) + - { + "installation.created", + "installation.deleted", + } + ) + get_buffer_requests_for_regions() + requests.sort(key=lambda x: parse_date(x.date), reverse=True) filtered_requests: list[BufferedRequest] = [] for i, req in enumerate(requests): diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py index 2df6b5376adf86..02d39f39dea32a 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py @@ -2,10 +2,6 @@ from django.urls import reverse -from rest_framework.exceptions import ErrorDetail - - -from sentry.sentry_apps.api.endpoints.sentry_app_requests_v2 import INVALID_DATE_FORMAT_MESSAGE from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import before_now, freeze_time from sentry.testutils.silo import control_silo_test, create_test_regions @@ -20,7 +16,7 @@ class SentryAppRequestsV2GetTest(APITestCase): def setUp(self): self.superuser = self.create_user(email="superuser@example.com", is_superuser=True) self.user = self.create_user(email="user@example.com") - self.org = self.create_organization(owner=self.user, region="us") + self.org = self.create_organization(owner=self.user, region="us", slug="test-org") self.project = self.create_project(organization=self.org) self.event_id = "d5111da2c28645c5889d072017e3445d" @@ -265,14 +261,7 @@ def test_org_slug_filter(self): url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) made_up_org_response = self.client.get(f"{url}?organizationSlug=madeUpOrg", format="json") assert made_up_org_response.status_code == 400 - assert made_up_org_response.data == { - "organization_slug": [ - ErrorDetail( - "Invalid organization slug.", - code="invalid", - ) - ] - } + assert made_up_org_response.data["detail"] == "Invalid organization." org_response = self.client.get(f"{url}?organizationSlug={self.org.slug}", format="json") assert org_response.status_code == 200 @@ -336,7 +325,15 @@ def test_date_filter(self): # test adding an improperly formatted end time bad_date_format_response = self.client.get(f"{url}?end=2000-01- 00:00:00", format="json") - assert bad_date_format_response.data["detail"] == INVALID_DATE_FORMAT_MESSAGE + assert bad_date_format_response.status_code == 400 + + # test adding a start and end time + late_start_date = (now + timedelta(seconds=2)).strftime("%Y-%m-%d %H:%M:%S") + early_end_date = (now + timedelta(seconds=1)).strftime("%Y-%m-%d %H:%M:%S") + start_after_end_response = self.client.get( + f"{url}?start={late_start_date}&end={early_end_date}", format="json" + ) + assert start_after_end_response.status_code == 400 def test_get_includes_installation_requests(self): self.login_as(user=self.user) From a117f7433257602b7ee0bfbfc0a8abd0a0380c05 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Thu, 5 Dec 2024 15:44:20 -0800 Subject: [PATCH 4/9] filter and append --- .../api/endpoints/sentry_app_requests_v2.py | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py index 1f7bab8bfaff06..808cc6a5178a43 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py @@ -117,16 +117,21 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) control_errors_only = region_filter["errors_only"] = errors_only + def filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: + for i, req in enumerate(unfiltered_requests): + if filter_by_date(req, start_time, end_time) and filter_by_organization( + req, organization + ): + requests.append(BufferedRequest(id=i, data=req)) + def get_buffer_requests_for_control(event_type) -> list[RpcSentryAppRequest]: - control_buffer.get_requests(event=event_type, errors_only=control_errors_only) - requests.extend( - [ - serialize_rpc_sentry_app_request(req) - for req in control_buffer.get_requests( - event=event_type, errors_only=control_errors_only - ) - ] - ) + control_requests = [ + serialize_rpc_sentry_app_request(req) + for req in control_buffer.get_requests( + event=event_type, errors_only=control_errors_only + ) + ] + filter_and_append_requests(control_requests) def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: for region_name in find_all_region_names(): @@ -136,7 +141,7 @@ def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: filter=region_filter, ) if buffer_requests is not None: - requests.extend(buffer_requests) + filter_and_append_requests(buffer_requests) # If event type is installation.created or installation.deleted, we only need to fetch requests from the control buffer if event_type == "installation.created" or event_type == "installation.deleted": @@ -160,17 +165,11 @@ def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: ) get_buffer_requests_for_regions() - requests.sort(key=lambda x: parse_date(x.date), reverse=True) - filtered_requests: list[BufferedRequest] = [] - for i, req in enumerate(requests): - if filter_by_date(req, start_time, end_time) and filter_by_organization( - req, organization - ): - filtered_requests.append(BufferedRequest(id=i, data=req)) + requests.sort(key=lambda x: parse_date(x.data.date), reverse=True) def data_fn(offset, limit): page_offset = offset * limit - return filtered_requests[page_offset : page_offset + limit] + return requests[page_offset : page_offset + limit] return self.paginate( request=request, From c2d8951ad39a154a855b6b03534509be04eac91b Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 6 Dec 2024 14:20:08 -0800 Subject: [PATCH 5/9] helper methods --- src/sentry/api/urls.py | 10 +- .../api_pagination_allowlist_do_not_modify.py | 1 + ...s_v2.py => sentry_app_webhook_requests.py} | 121 +++++++----------- ...st_v2.py => sentry_app_webhook_request.py} | 13 +- src/sentry/sentry_apps/api/utils/__init__.py | 0 .../sentry_apps/api/utils/webhook_requests.py | 46 +++++++ static/app/data/controlsiloUrlPatterns.ts | 1 + ...py => test_sentry_app_webhook_requests.py} | 30 +++-- 8 files changed, 129 insertions(+), 93 deletions(-) rename src/sentry/sentry_apps/api/endpoints/{sentry_app_requests_v2.py => sentry_app_webhook_requests.py} (57%) rename src/sentry/sentry_apps/api/serializers/{request_v2.py => sentry_app_webhook_request.py} (73%) create mode 100644 src/sentry/sentry_apps/api/utils/__init__.py create mode 100644 src/sentry/sentry_apps/api/utils/webhook_requests.py rename tests/sentry/sentry_apps/api/endpoints/{test_sentry_app_requests_v2.py => test_sentry_app_webhook_requests.py} (91%) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index a9935a73a12506..8f4ccc74c8032b 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -300,9 +300,11 @@ SentryAppPublishRequestEndpoint, ) from sentry.sentry_apps.api.endpoints.sentry_app_requests import SentryAppRequestsEndpoint -from sentry.sentry_apps.api.endpoints.sentry_app_requests_v2 import SentryAppRequestsEndpointV2 from sentry.sentry_apps.api.endpoints.sentry_app_rotate_secret import SentryAppRotateSecretEndpoint from sentry.sentry_apps.api.endpoints.sentry_app_stats_details import SentryAppStatsEndpoint +from sentry.sentry_apps.api.endpoints.sentry_app_webhook_requests import ( + SentryAppWebhookRequestsEndpoint, +) from sentry.sentry_apps.api.endpoints.sentry_apps import SentryAppsEndpoint from sentry.sentry_apps.api.endpoints.sentry_apps_stats import SentryAppsStatsEndpoint from sentry.sentry_apps.api.endpoints.sentry_internal_app_token_details import ( @@ -2908,9 +2910,9 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: name="sentry-api-0-sentry-app-publish-request", ), re_path( - r"^(?P[^\/]+)/requests-v2/$", - SentryAppRequestsEndpointV2.as_view(), - name="sentry-api-0-sentry-app-requests-v2", + r"^(?P[^\/]+)/webhook-requests/$", + SentryAppWebhookRequestsEndpoint.as_view(), + name="sentry-api-0-sentry-app-webhook-requests", ), # The following a region endpoints as interactions and request logs # are per-region. diff --git a/src/sentry/conf/api_pagination_allowlist_do_not_modify.py b/src/sentry/conf/api_pagination_allowlist_do_not_modify.py index d3ad9bce77bc6a..1655bf1e2657b3 100644 --- a/src/sentry/conf/api_pagination_allowlist_do_not_modify.py +++ b/src/sentry/conf/api_pagination_allowlist_do_not_modify.py @@ -92,6 +92,7 @@ "ProjectUsersEndpoint", "ReleaseThresholdEndpoint", "SentryAppRequestsEndpoint", + "SentryAppWebhookRequestsEndpoint", "SentryAppsStatsEndpoint", "SentryInternalAppTokensEndpoint", "TeamGroupsOldEndpoint", diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py similarity index 57% rename from src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py rename to src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py index 808cc6a5178a43..66b68f064ad489 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_requests_v2.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py @@ -1,4 +1,3 @@ -from dataclasses import dataclass from datetime import datetime, timezone from dateutil.parser import parse as parse_date @@ -9,17 +8,20 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint -from sentry.api.paginator import GenericOffsetPaginator from sentry.api.serializers import serialize from sentry.models.organizationmapping import OrganizationMapping from sentry.sentry_apps.api.bases.sentryapps import SentryAppBaseEndpoint, SentryAppStatsPermission -from sentry.sentry_apps.api.serializers.request_v2 import RequestSerializer +from sentry.sentry_apps.api.serializers.sentry_app_webhook_request import ( + SentryAppWebhookRequestSerializer, +) +from sentry.sentry_apps.api.utils.webhook_requests import ( + BufferedRequest, + get_buffer_requests_from_control, + get_buffer_requests_from_regions, +) from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.sentry_apps.services.app_request import RpcSentryAppRequest, SentryAppRequestFilterArgs -from sentry.sentry_apps.services.app_request.serial import serialize_rpc_sentry_app_request -from sentry.sentry_apps.services.app_request.service import app_request_service -from sentry.types.region import find_all_region_names -from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS, SentryAppWebhookRequestsBuffer +from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS class IncomingRequestSerializer(serializers.Serializer): @@ -28,7 +30,7 @@ class IncomingRequestSerializer(serializers.Serializer): choices=EXTENDED_VALID_EVENTS, required=False, ) - errorsOnly = serializers.BooleanField(required=False) + errorsOnly = serializers.BooleanField(default=False, required=False) organizationSlug = serializers.CharField(required=False) start = serializers.DateTimeField( format=date_format, @@ -51,35 +53,8 @@ def validate_end(self, end): return end -def filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: - date_str = request.date - if not date_str: - return False - timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( - microsecond=0, tzinfo=timezone.utc - ) - return start <= timestamp <= end - - -def filter_by_organization( - request: RpcSentryAppRequest, organization: OrganizationMapping | None -) -> bool: - if not organization: - return True - return request.organization_id == organization.organization_id - - -@dataclass -class BufferedRequest: - id: int - data: RpcSentryAppRequest - - def __hash__(self): - return self.id - - @control_silo_endpoint -class SentryAppRequestsEndpointV2(SentryAppBaseEndpoint): +class SentryAppWebhookRequestsEndpoint(SentryAppBaseEndpoint): owner = ApiOwner.INTEGRATIONS publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, @@ -112,50 +87,54 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: except OrganizationMapping.DoesNotExist: return Response({"detail": "Invalid organization."}, status=400) - requests: list[RpcSentryAppRequest] = [] + requests: list[BufferedRequest] = [] + control_filter: SentryAppRequestFilterArgs = {} region_filter: SentryAppRequestFilterArgs = {} - control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) - control_errors_only = region_filter["errors_only"] = errors_only + control_filter["errors_only"] = region_filter["errors_only"] = errors_only + + def _filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: + date_str = request.date + if not date_str: + return False + timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( + microsecond=0, tzinfo=timezone.utc + ) + return start <= timestamp <= end - def filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: + def _filter_by_organization( + request: RpcSentryAppRequest, organization: OrganizationMapping | None + ) -> bool: + if not organization: + return True + return request.organization_id == organization.organization_id + + def _filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: for i, req in enumerate(unfiltered_requests): - if filter_by_date(req, start_time, end_time) and filter_by_organization( + if _filter_by_date(req, start_time, end_time) and _filter_by_organization( req, organization ): requests.append(BufferedRequest(id=i, data=req)) - def get_buffer_requests_for_control(event_type) -> list[RpcSentryAppRequest]: - control_requests = [ - serialize_rpc_sentry_app_request(req) - for req in control_buffer.get_requests( - event=event_type, errors_only=control_errors_only - ) - ] - filter_and_append_requests(control_requests) - - def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: - for region_name in find_all_region_names(): - buffer_requests = app_request_service.get_buffer_requests_for_region( - sentry_app_id=sentry_app.id, - region_name=region_name, - filter=region_filter, - ) - if buffer_requests is not None: - filter_and_append_requests(buffer_requests) - # If event type is installation.created or installation.deleted, we only need to fetch requests from the control buffer if event_type == "installation.created" or event_type == "installation.deleted": - get_buffer_requests_for_control(event_type) + control_filter["event"] = event_type + _filter_and_append_requests( + get_buffer_requests_from_control(sentry_app, control_filter) + ) # If event type has been specified, we only need to fetch requests from region buffers elif event_type: region_filter["event"] = event_type - get_buffer_requests_for_regions() + _filter_and_append_requests( + get_buffer_requests_from_regions(sentry_app.id, region_filter) + ) else: - control_event_type = [ + control_filter["event"] = [ "installation.created", "installation.deleted", ] - get_buffer_requests_for_control(control_event_type) + _filter_and_append_requests( + get_buffer_requests_from_control(sentry_app, control_filter) + ) region_filter["event"] = list( set(EXTENDED_VALID_EVENTS) - { @@ -163,16 +142,12 @@ def get_buffer_requests_for_regions() -> list[RpcSentryAppRequest]: "installation.deleted", } ) - get_buffer_requests_for_regions() + _filter_and_append_requests( + get_buffer_requests_from_regions(sentry_app.id, region_filter) + ) requests.sort(key=lambda x: parse_date(x.data.date), reverse=True) - def data_fn(offset, limit): - page_offset = offset * limit - return requests[page_offset : page_offset + limit] - - return self.paginate( - request=request, - paginator=GenericOffsetPaginator(data_fn), - on_results=lambda x: serialize(x, request.user, RequestSerializer(sentry_app)), + return Response( + serialize(requests, request.user, SentryAppWebhookRequestSerializer(sentry_app)) ) diff --git a/src/sentry/sentry_apps/api/serializers/request_v2.py b/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py similarity index 73% rename from src/sentry/sentry_apps/api/serializers/request_v2.py rename to src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py index 54998101d4f88b..06a539e4ae9686 100644 --- a/src/sentry/sentry_apps/api/serializers/request_v2.py +++ b/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py @@ -5,15 +5,18 @@ from sentry.api.serializers import Serializer from sentry.hybridcloud.services.organization_mapping import organization_mapping_service +from sentry.sentry_apps.api.utils.webhook_requests import BufferedRequest from sentry.sentry_apps.models.sentry_app import SentryApp +from sentry.users.models.user import User +from sentry.users.services.user import RpcUser -class RequestSerializer(Serializer): +class SentryAppWebhookRequestSerializer(Serializer): def __init__(self, sentry_app: SentryApp) -> None: self.sentry_app = sentry_app def get_attrs( - self, item_list: Sequence[Any], user: Any, **kwargs: Any + self, item_list: Sequence[BufferedRequest], user: User | RpcUser, **kwargs: Any ) -> MutableMapping[Any, Any]: organization_ids = {item.data.organization_id for item in item_list} organizations = organization_mapping_service.get_many(organization_ids=organization_ids) @@ -21,7 +24,11 @@ def get_attrs( return { item: { - "organization": organizations_by_id.get(item.data.organization_id), + "organization": ( + organizations_by_id.get(item.data.organization_id) + if item.data.organization_id + else None + ) } for item in item_list } diff --git a/src/sentry/sentry_apps/api/utils/__init__.py b/src/sentry/sentry_apps/api/utils/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/src/sentry/sentry_apps/api/utils/webhook_requests.py b/src/sentry/sentry_apps/api/utils/webhook_requests.py new file mode 100644 index 00000000000000..a9ffa0b99a7f12 --- /dev/null +++ b/src/sentry/sentry_apps/api/utils/webhook_requests.py @@ -0,0 +1,46 @@ +from dataclasses import dataclass + +from sentry.sentry_apps.models.sentry_app import SentryApp +from sentry.sentry_apps.services.app_request import RpcSentryAppRequest, SentryAppRequestFilterArgs +from sentry.sentry_apps.services.app_request.serial import serialize_rpc_sentry_app_request +from sentry.sentry_apps.services.app_request.service import app_request_service +from sentry.types.region import find_all_region_names +from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer + + +@dataclass +class BufferedRequest: + id: int + data: RpcSentryAppRequest + + def __hash__(self): + return self.id + + +def get_buffer_requests_from_control( + sentry_app: SentryApp, filter: SentryAppRequestFilterArgs +) -> list[RpcSentryAppRequest]: + control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) + + event = filter.get("event", None) if filter else None + errors_only = filter.get("errors_only", False) if filter else False + + return [ + serialize_rpc_sentry_app_request(req) + for req in control_buffer.get_requests(event=event, errors_only=errors_only) + ] + + +def get_buffer_requests_from_regions( + sentry_app_id: int, filter: SentryAppRequestFilterArgs +) -> list[RpcSentryAppRequest]: + requests: list[RpcSentryAppRequest] = [] + for region_name in find_all_region_names(): + buffer_requests = app_request_service.get_buffer_requests_for_region( + sentry_app_id=sentry_app_id, + region_name=region_name, + filter=filter, + ) + if buffer_requests: + requests.extend(buffer_requests) + return requests diff --git a/static/app/data/controlsiloUrlPatterns.ts b/static/app/data/controlsiloUrlPatterns.ts index f0e1959803be80..d5586ad241d0d9 100644 --- a/static/app/data/controlsiloUrlPatterns.ts +++ b/static/app/data/controlsiloUrlPatterns.ts @@ -98,6 +98,7 @@ const patterns: RegExp[] = [ new RegExp('^api/0/sentry-apps/[^/]+/rotate-secret/$'), new RegExp('^api/0/sentry-apps/[^/]+/stats/$'), new RegExp('^api/0/sentry-apps/[^/]+/publish-request/$'), + new RegExp('^api/0/sentry-apps/[^/]+/webhook-requests/$'), new RegExp('^api/0/sentry-app-installations/[^/]+/$'), new RegExp('^api/0/sentry-app-installations/[^/]+/authorizations/$'), new RegExp('^api/0/auth/$'), diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py similarity index 91% rename from tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py rename to tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py index 02d39f39dea32a..88ab5c4f7f64e0 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_requests_v2.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py @@ -57,7 +57,9 @@ def test_superuser_sees_unowned_published_requests(self): url=self.unowned_published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unowned_published_app.slug]) + url = reverse( + "sentry-api-0-sentry-app-webhook-requests", args=[self.unowned_published_app.slug] + ) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 2 @@ -77,7 +79,7 @@ def test_superuser_sees_unpublished_stats(self): ) url = reverse( - "sentry-api-0-sentry-app-requests-v2", args=[self.unowned_unpublished_app.slug] + "sentry-api-0-sentry-app-webhook-requests", args=[self.unowned_unpublished_app.slug] ) response = self.client.get(url, format="json") assert response.status_code == 200 @@ -95,7 +97,7 @@ def test_user_sees_owned_published_requests(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 1 @@ -114,7 +116,9 @@ def test_user_does_not_see_unowned_published_requests(self): url=self.unowned_published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unowned_published_app.slug]) + url = reverse( + "sentry-api-0-sentry-app-webhook-requests", args=[self.unowned_published_app.slug] + ) response = self.client.get(url, format="json") assert response.status_code == 403 assert response.data["detail"] == "You do not have permission to perform this action." @@ -130,7 +134,7 @@ def test_user_sees_owned_unpublished_requests(self): url=self.unpublished_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.unpublished_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.unpublished_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 1 @@ -145,7 +149,7 @@ def test_internal_app_requests_does_not_have_organization_field(self): url=self.internal_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.internal_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.internal_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 1 @@ -169,7 +173,7 @@ def test_event_type_filter(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response1 = self.client.get(f"{url}?eventType=issue.created", format="json") assert response1.status_code == 200 assert len(response1.data) == 0 @@ -185,7 +189,7 @@ def test_event_type_filter(self): def test_invalid_event_type(self): self.login_as(user=self.user) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response = self.client.get(f"{url}?eventType=invalid_type", format="json") assert response.status_code == 400 @@ -206,7 +210,7 @@ def test_errors_only_filter(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) errors_only_response = self.client.get(f"{url}?errorsOnly=true", format="json") assert errors_only_response.status_code == 200 assert len(errors_only_response.data) == 1 @@ -233,7 +237,7 @@ def test_linked_error_not_returned_if_project_does_not_exist(self): project_id=1000, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 1 @@ -258,7 +262,7 @@ def test_org_slug_filter(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) made_up_org_response = self.client.get(f"{url}?organizationSlug=madeUpOrg", format="json") assert made_up_org_response.status_code == 400 assert made_up_org_response.data["detail"] == "Invalid organization." @@ -297,7 +301,7 @@ def test_date_filter(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 assert len(response.data) == 3 @@ -368,7 +372,7 @@ def test_get_includes_installation_requests(self): url=self.published_app.webhook_url, ) - url = reverse("sentry-api-0-sentry-app-requests-v2", args=[self.published_app.slug]) + url = reverse("sentry-api-0-sentry-app-webhook-requests", args=[self.published_app.slug]) response = self.client.get(url, format="json") assert response.status_code == 200 From 135705ae8c1bbec387a2b93ece54b65f962cd6af Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 6 Dec 2024 15:46:51 -0800 Subject: [PATCH 6/9] move filter methods --- .../endpoints/sentry_app_webhook_requests.py | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py index 66b68f064ad489..5904442d753e20 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py @@ -61,6 +61,22 @@ class SentryAppWebhookRequestsEndpoint(SentryAppBaseEndpoint): } permission_classes = (SentryAppStatsPermission,) + def _filter_by_date(self, request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: + date_str = request.date + if not date_str: + return False + timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( + microsecond=0, tzinfo=timezone.utc + ) + return start <= timestamp <= end + + def _filter_by_organization( + self, request: RpcSentryAppRequest, organization: OrganizationMapping | None + ) -> bool: + if not organization: + return True + return request.organization_id == organization.organization_id + def get(self, request: Request, sentry_app: SentryApp) -> Response: """ :qparam string eventType: Optionally specify a specific event type to filter requests @@ -92,25 +108,9 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: region_filter: SentryAppRequestFilterArgs = {} control_filter["errors_only"] = region_filter["errors_only"] = errors_only - def _filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: - date_str = request.date - if not date_str: - return False - timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( - microsecond=0, tzinfo=timezone.utc - ) - return start <= timestamp <= end - - def _filter_by_organization( - request: RpcSentryAppRequest, organization: OrganizationMapping | None - ) -> bool: - if not organization: - return True - return request.organization_id == organization.organization_id - - def _filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: + def filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: for i, req in enumerate(unfiltered_requests): - if _filter_by_date(req, start_time, end_time) and _filter_by_organization( + if self._filter_by_date(req, start_time, end_time) and self._filter_by_organization( req, organization ): requests.append(BufferedRequest(id=i, data=req)) @@ -118,13 +118,11 @@ def _filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) # If event type is installation.created or installation.deleted, we only need to fetch requests from the control buffer if event_type == "installation.created" or event_type == "installation.deleted": control_filter["event"] = event_type - _filter_and_append_requests( - get_buffer_requests_from_control(sentry_app, control_filter) - ) + filter_and_append_requests(get_buffer_requests_from_control(sentry_app, control_filter)) # If event type has been specified, we only need to fetch requests from region buffers elif event_type: region_filter["event"] = event_type - _filter_and_append_requests( + filter_and_append_requests( get_buffer_requests_from_regions(sentry_app.id, region_filter) ) else: @@ -132,9 +130,7 @@ def _filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) "installation.created", "installation.deleted", ] - _filter_and_append_requests( - get_buffer_requests_from_control(sentry_app, control_filter) - ) + filter_and_append_requests(get_buffer_requests_from_control(sentry_app, control_filter)) region_filter["event"] = list( set(EXTENDED_VALID_EVENTS) - { @@ -142,7 +138,7 @@ def _filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) "installation.deleted", } ) - _filter_and_append_requests( + filter_and_append_requests( get_buffer_requests_from_regions(sentry_app.id, region_filter) ) From e1aa6413e52e38cff270421675fabbfe9b2a3942 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 20 Dec 2024 04:40:10 +0800 Subject: [PATCH 7/9] typing + new filtering --- .../endpoints/sentry_app_webhook_requests.py | 47 ++++++--------- .../serializers/sentry_app_webhook_request.py | 33 ++++++++-- .../sentry_apps/api/utils/webhook_requests.py | 60 +++++++++++++++++-- 3 files changed, 98 insertions(+), 42 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py index 5904442d753e20..0fd9e3650cc0e9 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py @@ -16,11 +16,12 @@ ) from sentry.sentry_apps.api.utils.webhook_requests import ( BufferedRequest, + DatetimeOrganizationFilterArgs, get_buffer_requests_from_control, get_buffer_requests_from_regions, ) from sentry.sentry_apps.models.sentry_app import SentryApp -from sentry.sentry_apps.services.app_request import RpcSentryAppRequest, SentryAppRequestFilterArgs +from sentry.sentry_apps.services.app_request import SentryAppRequestFilterArgs from sentry.utils.sentry_apps import EXTENDED_VALID_EVENTS @@ -61,22 +62,6 @@ class SentryAppWebhookRequestsEndpoint(SentryAppBaseEndpoint): } permission_classes = (SentryAppStatsPermission,) - def _filter_by_date(self, request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: - date_str = request.date - if not date_str: - return False - timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( - microsecond=0, tzinfo=timezone.utc - ) - return start <= timestamp <= end - - def _filter_by_organization( - self, request: RpcSentryAppRequest, organization: OrganizationMapping | None - ) -> bool: - if not organization: - return True - return request.organization_id == organization.organization_id - def get(self, request: Request, sentry_app: SentryApp) -> Response: """ :qparam string eventType: Optionally specify a specific event type to filter requests @@ -107,30 +92,32 @@ def get(self, request: Request, sentry_app: SentryApp) -> Response: control_filter: SentryAppRequestFilterArgs = {} region_filter: SentryAppRequestFilterArgs = {} control_filter["errors_only"] = region_filter["errors_only"] = errors_only - - def filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) -> None: - for i, req in enumerate(unfiltered_requests): - if self._filter_by_date(req, start_time, end_time) and self._filter_by_organization( - req, organization - ): - requests.append(BufferedRequest(id=i, data=req)) + datetime_org_filter: DatetimeOrganizationFilterArgs = { + "start_time": start_time, + "end_time": end_time, + "organization": organization, + } # If event type is installation.created or installation.deleted, we only need to fetch requests from the control buffer if event_type == "installation.created" or event_type == "installation.deleted": control_filter["event"] = event_type - filter_and_append_requests(get_buffer_requests_from_control(sentry_app, control_filter)) + requests.extend( + get_buffer_requests_from_control(sentry_app, control_filter, datetime_org_filter) + ) # If event type has been specified, we only need to fetch requests from region buffers elif event_type: region_filter["event"] = event_type - filter_and_append_requests( - get_buffer_requests_from_regions(sentry_app.id, region_filter) + requests.extend( + get_buffer_requests_from_regions(sentry_app.id, region_filter, datetime_org_filter) ) else: control_filter["event"] = [ "installation.created", "installation.deleted", ] - filter_and_append_requests(get_buffer_requests_from_control(sentry_app, control_filter)) + requests.extend( + get_buffer_requests_from_control(sentry_app, control_filter, datetime_org_filter) + ) region_filter["event"] = list( set(EXTENDED_VALID_EVENTS) - { @@ -138,8 +125,8 @@ def filter_and_append_requests(unfiltered_requests: list[RpcSentryAppRequest]) - "installation.deleted", } ) - filter_and_append_requests( - get_buffer_requests_from_regions(sentry_app.id, region_filter) + requests.extend( + get_buffer_requests_from_regions(sentry_app.id, region_filter, datetime_org_filter) ) requests.sort(key=lambda x: parse_date(x.data.date), reverse=True) diff --git a/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py b/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py index 06a539e4ae9686..cb784624e5a188 100644 --- a/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py +++ b/src/sentry/sentry_apps/api/serializers/sentry_app_webhook_request.py @@ -1,23 +1,44 @@ from __future__ import annotations from collections.abc import Mapping, MutableMapping, Sequence -from typing import Any +from typing import Any, NotRequired, TypedDict from sentry.api.serializers import Serializer -from sentry.hybridcloud.services.organization_mapping import organization_mapping_service +from sentry.hybridcloud.services.organization_mapping import ( + RpcOrganizationMapping, + organization_mapping_service, +) from sentry.sentry_apps.api.utils.webhook_requests import BufferedRequest from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.users.models.user import User from sentry.users.services.user import RpcUser +class _BufferedRequestAttrs(TypedDict): + organization: RpcOrganizationMapping | None + + +class OrganizationResponse(TypedDict): + name: str + slug: str + + +class SentryAppWebhookRequestSerializerResponse(TypedDict): + webhookUrl: str + sentryAppSlug: str + eventType: str + date: str + responseCode: int + organization: NotRequired[OrganizationResponse] + + class SentryAppWebhookRequestSerializer(Serializer): def __init__(self, sentry_app: SentryApp) -> None: self.sentry_app = sentry_app def get_attrs( self, item_list: Sequence[BufferedRequest], user: User | RpcUser, **kwargs: Any - ) -> MutableMapping[Any, Any]: + ) -> MutableMapping[BufferedRequest, _BufferedRequestAttrs]: organization_ids = {item.data.organization_id for item in item_list} organizations = organization_mapping_service.get_many(organization_ids=organization_ids) organizations_by_id = {organization.id: organization for organization in organizations} @@ -34,12 +55,12 @@ def get_attrs( } def serialize( - self, obj: Any, attrs: Mapping[Any, Any], user: Any, **kwargs: Any - ) -> Mapping[str, Any]: + self, obj: BufferedRequest, attrs: Mapping[Any, Any], user: Any, **kwargs: Any + ) -> SentryAppWebhookRequestSerializerResponse: organization = attrs.get("organization") response_code = obj.data.response_code - data = { + data: SentryAppWebhookRequestSerializerResponse = { "webhookUrl": obj.data.webhook_url, "sentryAppSlug": self.sentry_app.slug, "eventType": obj.data.event_type, diff --git a/src/sentry/sentry_apps/api/utils/webhook_requests.py b/src/sentry/sentry_apps/api/utils/webhook_requests.py index a9ffa0b99a7f12..93d97dff70219d 100644 --- a/src/sentry/sentry_apps/api/utils/webhook_requests.py +++ b/src/sentry/sentry_apps/api/utils/webhook_requests.py @@ -1,5 +1,8 @@ from dataclasses import dataclass +from datetime import datetime, timezone +from typing import TypedDict +from sentry.models.organizationmapping import OrganizationMapping from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.sentry_apps.services.app_request import RpcSentryAppRequest, SentryAppRequestFilterArgs from sentry.sentry_apps.services.app_request.serial import serialize_rpc_sentry_app_request @@ -17,23 +20,68 @@ def __hash__(self): return self.id +class DatetimeOrganizationFilterArgs(TypedDict): + start_time: datetime + end_time: datetime + organization: OrganizationMapping | None + + +def _filter_by_date(request: RpcSentryAppRequest, start: datetime, end: datetime) -> bool: + date_str = request.date + if not date_str: + return False + timestamp = datetime.strptime(date_str, "%Y-%m-%d %H:%M:%S.%f+00:00").replace( + microsecond=0, tzinfo=timezone.utc + ) + return start <= timestamp <= end + + +def _filter_by_organization( + request: RpcSentryAppRequest, organization: OrganizationMapping | None +) -> bool: + if not organization: + return True + return request.organization_id == organization.organization_id + + +def filter_requests( + unfiltered_requests: list[RpcSentryAppRequest], + filter: DatetimeOrganizationFilterArgs, +) -> list[BufferedRequest]: + requests: list[BufferedRequest] = [] + for i, req in enumerate(unfiltered_requests): + if _filter_by_date( + req, filter.get("start_time"), filter.get("end_time") + ) and _filter_by_organization(req, organization=filter.get("organization")): + requests.append(BufferedRequest(id=i, data=req)) + return requests + + def get_buffer_requests_from_control( - sentry_app: SentryApp, filter: SentryAppRequestFilterArgs -) -> list[RpcSentryAppRequest]: + sentry_app: SentryApp, + filter: SentryAppRequestFilterArgs, + datetime_org_filter: DatetimeOrganizationFilterArgs, +) -> list[BufferedRequest]: control_buffer = SentryAppWebhookRequestsBuffer(sentry_app) event = filter.get("event", None) if filter else None errors_only = filter.get("errors_only", False) if filter else False - return [ + unfiltered_requests = [ serialize_rpc_sentry_app_request(req) for req in control_buffer.get_requests(event=event, errors_only=errors_only) ] + return filter_requests( + unfiltered_requests, + datetime_org_filter, + ) def get_buffer_requests_from_regions( - sentry_app_id: int, filter: SentryAppRequestFilterArgs -) -> list[RpcSentryAppRequest]: + sentry_app_id: int, + filter: SentryAppRequestFilterArgs, + datetime_org_filter: DatetimeOrganizationFilterArgs, +) -> list[BufferedRequest]: requests: list[RpcSentryAppRequest] = [] for region_name in find_all_region_names(): buffer_requests = app_request_service.get_buffer_requests_for_region( @@ -43,4 +91,4 @@ def get_buffer_requests_from_regions( ) if buffer_requests: requests.extend(buffer_requests) - return requests + return filter_requests(requests, datetime_org_filter) From e2a69d4d5a89c43f6243864809dab9cb65aa785b Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 20 Dec 2024 04:55:19 +0800 Subject: [PATCH 8/9] more detailed tests --- .../api/endpoints/sentry_app_webhook_requests.py | 2 +- .../api/endpoints/test_sentry_app_webhook_requests.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py index 0fd9e3650cc0e9..1c5c9abcbd96db 100644 --- a/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/sentry_app_webhook_requests.py @@ -56,7 +56,7 @@ def validate_end(self, end): @control_silo_endpoint class SentryAppWebhookRequestsEndpoint(SentryAppBaseEndpoint): - owner = ApiOwner.INTEGRATIONS + owner = ApiOwner.ECOSYSTEM publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, } diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py index 88ab5c4f7f64e0..0260eded0a4546 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_webhook_requests.py @@ -12,7 +12,7 @@ @control_silo_test(regions=create_test_regions("us")) -class SentryAppRequestsV2GetTest(APITestCase): +class SentryAppWebhookRequestsGetTest(APITestCase): def setUp(self): self.superuser = self.create_user(email="superuser@example.com", is_superuser=True) self.user = self.create_user(email="user@example.com") @@ -167,7 +167,7 @@ def test_event_type_filter(self): url=self.published_app.webhook_url, ) buffer.add_request( - response_code=200, + response_code=400, org_id=self.org.id, event="installation.created", url=self.published_app.webhook_url, @@ -181,10 +181,14 @@ def test_event_type_filter(self): response2 = self.client.get(f"{url}?eventType=issue.assigned", format="json") assert response2.status_code == 200 assert len(response2.data) == 1 + assert response2.data[0]["sentryAppSlug"] == self.published_app.slug + assert response2.data[0]["responseCode"] == 200 response3 = self.client.get(f"{url}?eventType=installation.created", format="json") assert response3.status_code == 200 assert len(response3.data) == 1 + assert response3.data[0]["sentryAppSlug"] == self.published_app.slug + assert response3.data[0]["responseCode"] == 400 def test_invalid_event_type(self): self.login_as(user=self.user) @@ -214,6 +218,8 @@ def test_errors_only_filter(self): errors_only_response = self.client.get(f"{url}?errorsOnly=true", format="json") assert errors_only_response.status_code == 200 assert len(errors_only_response.data) == 1 + assert errors_only_response.data[0]["sentryAppSlug"] == self.published_app.slug + assert errors_only_response.data[0]["responseCode"] == 500 response = self.client.get(url, format="json") assert response.status_code == 200 From d0122779bf116a651bc1418e66d13e491952ac91 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 20 Dec 2024 05:03:29 +0800 Subject: [PATCH 9/9] fix mypy error smh --- src/sentry/sentry_apps/api/utils/webhook_requests.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/sentry/sentry_apps/api/utils/webhook_requests.py b/src/sentry/sentry_apps/api/utils/webhook_requests.py index 93d97dff70219d..af424e4948e601 100644 --- a/src/sentry/sentry_apps/api/utils/webhook_requests.py +++ b/src/sentry/sentry_apps/api/utils/webhook_requests.py @@ -50,9 +50,14 @@ def filter_requests( ) -> list[BufferedRequest]: requests: list[BufferedRequest] = [] for i, req in enumerate(unfiltered_requests): - if _filter_by_date( - req, filter.get("start_time"), filter.get("end_time") - ) and _filter_by_organization(req, organization=filter.get("organization")): + start_time = filter.get("start_time") + end_time = filter.get("end_time") + if ( + start_time + and end_time + and _filter_by_date(req, start_time, end_time) + and _filter_by_organization(req, organization=filter.get("organization")) + ): requests.append(BufferedRequest(id=i, data=req)) return requests