From 6fa4e0c9b664428a5d7a91056d995a453b1add52 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 7 Jan 2025 13:41:41 +0100 Subject: [PATCH] Addressed first bunch of feedbacks --- src/sentry/conf/server.py | 1 - src/sentry/tempest/tasks.py | 26 ++++++++++++++++++-------- tests/sentry/tempest/test_tempest.py | 9 ++++++--- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index af8ff132519797..e27fde25d9b731 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -952,7 +952,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str: "dynamicsampling", routing_key="dynamicsampling", ), - Queue("tempest", routing_key="tempest"), Queue("incidents", routing_key="incidents"), Queue("incident_snapshots", routing_key="incident_snapshots"), Queue("incidents", routing_key="incidents"), diff --git a/src/sentry/tempest/tasks.py b/src/sentry/tempest/tasks.py index 7ce350538f0e61..3a690f2cf2b2ce 100644 --- a/src/sentry/tempest/tasks.py +++ b/src/sentry/tempest/tasks.py @@ -2,6 +2,7 @@ from sentry import http from sentry.models.projectkey import ProjectKey, UseCase +from sentry.silo.base import SiloMode from sentry.tasks.base import instrumented_task from sentry.tempest.models import TempestCredentials from sentry.utils import json @@ -16,6 +17,9 @@ @instrumented_task( name="sentry.tempest.tasks.poll_tempest", queue="tempest", + silo_mode=SiloMode.REGION, + soft_time_limit=4 * 60, + time_limit=4 * 60 + 5, ) def poll_tempest(**kwargs): # FIXME: Once we have more traffic this needs to be done smarter. @@ -29,6 +33,9 @@ def poll_tempest(**kwargs): @instrumented_task( name="sentry.tempest.tasks.fetch_latest_item_id", queue="tempest", + silo_mode=SiloMode.REGION, + soft_time_limit=1 * 60, + time_limit=1 * 60 + 5, ) def fetch_latest_item_id(credentials_id: int) -> None: # FIXME: Try catch this later @@ -72,8 +79,11 @@ def fetch_latest_item_id(credentials_id: int) -> None: @instrumented_task( - name="sentry.tempest.tasks.poll_tempest", + name="sentry.tempest.tasks.poll_tempest_crashes", queue="tempest", + silo_mode=SiloMode.REGION, + soft_time_limit=4 * 60, + time_limit=4 * 60 + 5, ) def poll_tempest_crashes(credentials_id: int) -> None: # FIXME: Try catch this later @@ -83,16 +93,16 @@ def poll_tempest_crashes(credentials_id: int) -> None: client_id = credentials.client_id try: - response_text = fetch_crashes_from_tempest( + # This does generate a dsn not sure if it will work in the grand scheme of things though. + dsn = ProjectKey.objects.get_or_create( + use_case=UseCase.TEMPEST, project=credentials.project + )[0].get_dsn() + response_text = fetch_items_from_tempest( org_id=org_id, project_id=project_id, client_id=client_id, client_secret=credentials.client_secret, - dsn=ProjectKey.objects.get_or_create( - use_case=UseCase.TEMPEST, project=credentials.project - )[ - 0 - ].get_dsn(), # This does generate a dsn not sure if it will work in the grand scheme of things though. + dsn=dsn, offset=int( credentials.latest_fetched_item_id ), # Need to convert here because it is a char in the DB @@ -146,7 +156,7 @@ def fetch_latest_id_from_tempest( return response.text -def fetch_crashes_from_tempest( +def fetch_items_from_tempest( org_id: int, project_id: int, client_id: str, diff --git a/tests/sentry/tempest/test_tempest.py b/tests/sentry/tempest/test_tempest.py index 0941a92495d0ef..1b1e71a5a2b784 100644 --- a/tests/sentry/tempest/test_tempest.py +++ b/tests/sentry/tempest/test_tempest.py @@ -1,5 +1,6 @@ from unittest.mock import patch +from sentry.tempest.models import MessageType from sentry.tempest.tasks import fetch_latest_item_id, poll_tempest_crashes from sentry.testutils.cases import TestCase from sentry.utils import json @@ -30,6 +31,7 @@ def test_fetch_latest_item_id_error(self, mock_fetch): self.credentials.refresh_from_db() assert self.credentials.message == "Seems like the provided credentials are invalid" + assert self.credentials.message_type == MessageType.ERROR assert self.credentials.latest_fetched_item_id is None @patch("sentry.tempest.tasks.fetch_latest_id_from_tempest") @@ -40,6 +42,7 @@ def test_fetch_latest_item_id_ip_not_allowlisted(self, mock_fetch): self.credentials.refresh_from_db() assert self.credentials.message == "Seems like our IP is not allow-listed" + assert self.credentials.message_type == MessageType.ERROR assert self.credentials.latest_fetched_item_id is None @patch("sentry.tempest.tasks.fetch_latest_id_from_tempest") @@ -66,7 +69,7 @@ def test_fetch_latest_item_id_connection_error(self, mock_fetch): mock_fetch.assert_called_once() assert "Fetching the latest item id failed." in cm.output[0] - @patch("sentry.tempest.tasks.fetch_crashes_from_tempest") + @patch("sentry.tempest.tasks.fetch_items_from_tempest") def test_poll_tempest_crashes_task(self, mock_fetch): mock_fetch.return_value = json.dumps({"latest_id": 20002}) @@ -80,7 +83,7 @@ def test_poll_tempest_crashes_task(self, mock_fetch): assert self.credentials.latest_fetched_item_id == "20002" mock_fetch.assert_called_once() - @patch("sentry.tempest.tasks.fetch_crashes_from_tempest") + @patch("sentry.tempest.tasks.fetch_items_from_tempest") def test_poll_tempest_crashes_invalid_json(self, mock_fetch): mock_fetch.return_value = "not valid json" @@ -97,7 +100,7 @@ def test_poll_tempest_crashes_invalid_json(self, mock_fetch): mock_fetch.assert_called_once() assert "Fetching the crashes failed." in cm.output[0] - @patch("sentry.tempest.tasks.fetch_crashes_from_tempest") + @patch("sentry.tempest.tasks.fetch_items_from_tempest") def test_poll_tempest_crashes_error(self, mock_fetch): mock_fetch.side_effect = Exception("Connection error")