diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 47487a5cc7c85d..a46e72f3e8016a 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -1349,17 +1349,17 @@ name="sentry-api-0-organization-monitors", ), url( - r"^(?P[^\/]+)/monitors/(?P[^\/]+)/$", + r"^(?P[^\/]+)/monitors/(?P[^\/]+)/$", OrganizationMonitorDetailsEndpoint.as_view(), name="sentry-api-0-organization-monitor-details", ), url( - r"^(?P[^\/]+)/monitors/(?P[^\/]+)/stats/$", + r"^(?P[^\/]+)/monitors/(?P[^\/]+)/stats/$", OrganizationMonitorStatsEndpoint.as_view(), name="sentry-api-0-organization-monitor-stats", ), url( - r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/$", + r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/$", method_dispatch( GET=OrganizationMonitorCheckInIndexEndpoint.as_view(), POST=MonitorIngestCheckInIndexEndpoint.as_view(), # Legacy ingest endpoint @@ -1368,7 +1368,7 @@ name="sentry-api-0-organization-monitor-check-in-index", ), url( - r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/$", + r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/$", method_dispatch( PUT=MonitorIngestCheckInDetailsEndpoint.as_view(), # Legacy ingest endpoint csrf_exempt=True, @@ -1376,7 +1376,7 @@ name="sentry-api-0-organization-monitor-check-in-details", ), url( - r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/attachment/$", + r"^(?P[^\/]+)/monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/attachment/$", method_dispatch( GET=OrganizationMonitorCheckInAttachmentEndpoint.as_view(), POST=MonitorIngestCheckinAttachmentEndpoint.as_view(), # Legacy ingest endpoint @@ -2625,12 +2625,12 @@ # Top-level monitor checkin APIs. NOTE that there are also organization # level checkin ingest APIs. url( - r"^monitors/(?P[^\/]+)/checkins/$", + r"^monitors/(?P[^\/]+)/checkins/$", MonitorIngestCheckInIndexEndpoint.as_view(), name="sentry-api-0-monitor-ingest-check-in-index", ), url( - r"^monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/$", + r"^monitors/(?P[^\/]+)/checkins/(?P[^\/]+)/$", MonitorIngestCheckInDetailsEndpoint.as_view(), name="sentry-api-0-monitor-ingest-check-in-details", ), diff --git a/src/sentry/apidocs/parameters.py b/src/sentry/apidocs/parameters.py index 1fd9bb2a6b56e5..4bbf8fd3b40868 100644 --- a/src/sentry/apidocs/parameters.py +++ b/src/sentry/apidocs/parameters.py @@ -144,12 +144,12 @@ class CURSOR_QUERY_PARAM(serializers.Serializer): # type: ignore class MONITOR_PARAMS: - MONITOR_ID = OpenApiParameter( - name="monitor_id", + MONITOR_SLUG = OpenApiParameter( + name="monitor_slug", location="path", required=True, - type=OpenApiTypes.UUID, - description="The id of the monitor", + type=str, + description="The slug of the monitor", ) CHECKIN_ID = OpenApiParameter( name="checkin_id", diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 2b23f1176df00c..fc7cf0c430e402 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -753,7 +753,7 @@ def SOCIAL_AUTH_DEFAULT_USERNAME(): from celery.schedules import crontab -# XXX: Make sure to register the monitor_id for each job in `SENTRY_CELERYBEAT_MONITORS`! +# XXX: Make sure to register the monitor_slug for each job in `SENTRY_CELERYBEAT_MONITORS`! CELERYBEAT_SCHEDULE_FILENAME = os.path.join(tempfile.gettempdir(), "sentry-celerybeat") CELERYBEAT_SCHEDULE = { "check-auth": { diff --git a/src/sentry/monitors/endpoints/base.py b/src/sentry/monitors/endpoints/base.py index ae0b0904b2e547..1c0facc66567f0 100644 --- a/src/sentry/monitors/endpoints/base.py +++ b/src/sentry/monitors/endpoints/base.py @@ -47,7 +47,7 @@ def convert_args( self, request: Request, organization_slug: str, - monitor_id: str, + monitor_slug: str, checkin_id: str | None = None, *args, **kwargs, @@ -58,7 +58,7 @@ def convert_args( raise ResourceDoesNotExist try: - monitor = Monitor.objects.get(organization_id=organization.id, slug=monitor_id) + monitor = Monitor.objects.get(organization_id=organization.id, slug=monitor_slug) except Monitor.DoesNotExist: raise ResourceDoesNotExist @@ -109,7 +109,7 @@ class MonitorIngestEndpoint(Endpoint): allow_auto_create_monitors = False """ - Loosens the base endpoint such that a monitor with the provided monitor_id + Loosens the base endpoint such that a monitor with the provided monitor_slug does not need to exist. This is used for initial checkin creation with monitor upsert. @@ -121,7 +121,7 @@ class MonitorIngestEndpoint(Endpoint): def convert_args( self, request: Request, - monitor_id: str, + monitor_slug: str, checkin_id: str | None = None, organization_slug: str | None = None, *args, @@ -130,9 +130,9 @@ def convert_args( organization = None monitor = None - # Include monitor_id in kwargs when upsert is enabled + # Include monitor_slug in kwargs when upsert is enabled if self.allow_auto_create_monitors: - kwargs["monitor_id"] = monitor_id + kwargs["monitor_slug"] = monitor_slug using_dsn_auth = isinstance(request.auth, ProjectKey) @@ -147,7 +147,7 @@ def convert_args( organization = Organization.objects.get_from_cache(slug=organization_slug) # Try lookup by slug first. This requires organization context since # slugs are unique only to the organization - monitor = Monitor.objects.get(organization_id=organization.id, slug=monitor_id) + monitor = Monitor.objects.get(organization_id=organization.id, slug=monitor_slug) except (Organization.DoesNotExist, Monitor.DoesNotExist): pass @@ -155,12 +155,12 @@ def convert_args( if not monitor: # Validate GUIDs try: - UUID(monitor_id) + UUID(monitor_slug) # When looking up by guid we don't include the org conditional # (since GUID lookup allows orgless routes), we will validate # permissions later in this function try: - monitor = Monitor.objects.get(guid=monitor_id) + monitor = Monitor.objects.get(guid=monitor_slug) except Monitor.DoesNotExist: monitor = None except ValueError: diff --git a/src/sentry/monitors/endpoints/monitor_ingest_checkin_details.py b/src/sentry/monitors/endpoints/monitor_ingest_checkin_details.py index 9e2774a6f38da7..64af34045495cd 100644 --- a/src/sentry/monitors/endpoints/monitor_ingest_checkin_details.py +++ b/src/sentry/monitors/endpoints/monitor_ingest_checkin_details.py @@ -34,7 +34,7 @@ class MonitorIngestCheckInDetailsEndpoint(MonitorIngestEndpoint): operation_id="Update a check-in", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, MONITOR_PARAMS.CHECKIN_ID, ], request=MonitorCheckInValidator, diff --git a/src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py b/src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py index e7c02b1bd35135..dab2aae9a1ce31 100644 --- a/src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py +++ b/src/sentry/monitors/endpoints/monitor_ingest_checkin_index.py @@ -62,7 +62,7 @@ class MonitorIngestCheckInIndexEndpoint(MonitorIngestEndpoint): operation_id="Create a new check-in", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, ], request=MonitorCheckInValidator, responses={ @@ -82,7 +82,7 @@ def post( self, request: Request, project: Project, - monitor_id: str, + monitor_slug: str, monitor: Monitor | None, organization_slug: str | None = None, ) -> Response: @@ -104,13 +104,13 @@ def post( checkin_validator = MonitorCheckInValidator( data=request.data, - context={"project": project, "request": request, "monitor_id": monitor_id}, + context={"project": project, "request": request, "monitor_slug": monitor_slug}, ) if not checkin_validator.is_valid(): return self.respond(checkin_validator.errors, status=400) if not monitor: - ratelimit_key = monitor_id + ratelimit_key = monitor_slug else: ratelimit_key = monitor.id diff --git a/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py b/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py index 0ed358fda3fc56..2f7d7bf4d60391 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py @@ -32,7 +32,7 @@ class OrganizationMonitorCheckInIndexEndpoint(MonitorEndpoint): operation_id="Retrieve check-ins for a monitor", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, MONITOR_PARAMS.CHECKIN_ID, ], responses={ diff --git a/src/sentry/monitors/endpoints/organization_monitor_details.py b/src/sentry/monitors/endpoints/organization_monitor_details.py index 87b0dfa72bbdda..f492846384c9a4 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_details.py +++ b/src/sentry/monitors/endpoints/organization_monitor_details.py @@ -36,7 +36,7 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint): operation_id="Retrieve a monitor", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, ], responses={ 200: inline_sentry_response_serializer("Monitor", MonitorSerializerResponse), @@ -60,7 +60,7 @@ def get(self, request: Request, organization, project, monitor) -> Response: operation_id="Update a monitor", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, ], request=MonitorValidator, responses={ @@ -125,7 +125,7 @@ def put(self, request: Request, organization, project, monitor) -> Response: operation_id="Delete a monitor", parameters=[ GLOBAL_PARAMS.ORG_SLUG, - MONITOR_PARAMS.MONITOR_ID, + MONITOR_PARAMS.MONITOR_SLUG, ], request=MonitorValidator, responses={ diff --git a/src/sentry/monitors/validators.py b/src/sentry/monitors/validators.py index b3174678d8ae4a..2bcee033b6dedd 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -235,8 +235,8 @@ def validate(self, attrs): monitor_validator = MonitorValidator( data={ "type": "cron_job", - "name": self.context["monitor_id"], - "slug": self.context["monitor_id"], + "name": self.context["monitor_slug"], + "slug": self.context["monitor_slug"], "project": project.slug, "config": monitor_config, }, diff --git a/src/sentry/testutils/cases.py b/src/sentry/testutils/cases.py index 673fcde1e055c1..eecbc1f8ed5366 100644 --- a/src/sentry/testutils/cases.py +++ b/src/sentry/testutils/cases.py @@ -2365,8 +2365,8 @@ def _get_path_functions(self): # Because removing old urls takes time and consideration of the cost of breaking lingering references, a # decision to permanently remove either path schema is a TODO. return ( - lambda monitor_id: reverse(self.endpoint, args=[monitor_id]), - lambda monitor_id: reverse( - self.endpoint_with_org, args=[self.organization.slug, monitor_id] + lambda monitor_slug: reverse(self.endpoint, args=[monitor_slug]), + lambda monitor_slug: reverse( + self.endpoint_with_org, args=[self.organization.slug, monitor_slug] ), ) diff --git a/src/sentry/utils/monitors.py b/src/sentry/utils/monitors.py index 23e8f6334ec93c..8d494c0fc6a375 100644 --- a/src/sentry/utils/monitors.py +++ b/src/sentry/utils/monitors.py @@ -34,9 +34,9 @@ def connect(app): if hasattr(app.conf, "beat_schedule") else app.conf["CELERYBEAT_SCHEDULE"] ) - for schedule_name, monitor_id in settings.SENTRY_CELERYBEAT_MONITORS.items(): + for schedule_name, monitor_slug in settings.SENTRY_CELERYBEAT_MONITORS.items(): schedule[schedule_name].setdefault("options", {}).setdefault("headers", {}).setdefault( - "X-Sentry-Monitor", monitor_id + "X-Sentry-Monitor", monitor_slug ) @@ -63,16 +63,16 @@ def report_monitor_begin(task, **kwargs): if not headers: return - monitor_id = headers.get("X-Sentry-Monitor") - if not monitor_id: + monitor_slug = headers.get("X-Sentry-Monitor") + if not monitor_slug: return with configure_scope() as scope: - scope.set_context("monitor", {"id": monitor_id}) + scope.set_context("monitor", {"id": monitor_slug}) with SafeSession() as session: req = session.post( - f"{API_ROOT}/api/0/monitors/{monitor_id}/checkins/", + f"{API_ROOT}/api/0/monitors/{monitor_slug}/checkins/", headers={"Authorization": f"DSN {SENTRY_DSN}"}, json={"status": "in_progress"}, ) @@ -90,8 +90,8 @@ def report_monitor_complete(task, retval, **kwargs): if not headers: return - monitor_id = headers.get("X-Sentry-Monitor") - if not monitor_id: + monitor_slug = headers.get("X-Sentry-Monitor") + if not monitor_slug: return try: @@ -103,7 +103,7 @@ def report_monitor_complete(task, retval, **kwargs): with SafeSession() as session: session.put( - f"{API_ROOT}/api/0/monitors/{monitor_id}/checkins/{checkin_id}/", + f"{API_ROOT}/api/0/monitors/{monitor_slug}/checkins/{checkin_id}/", headers={"Authorization": f"DSN {SENTRY_DSN}"}, json={ "status": "error" if isinstance(retval, Exception) else "ok", diff --git a/tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_details.py b/tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_details.py index 13775ad568c068..1f57ce5fd3103e 100644 --- a/tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_details.py +++ b/tests/sentry/monitors/endpoints/test_monitor_ingest_checkin_details.py @@ -31,9 +31,11 @@ def _get_path_functions(self): # Because removing old urls takes time and consideration of the cost of breaking lingering references, a # decision to permanently remove either path schema is a TODO. return ( - lambda monitor_id, checkin_id: reverse(self.endpoint, args=[monitor_id, checkin_id]), - lambda monitor_id, checkin_id: reverse( - self.endpoint_with_org, args=[self.organization.slug, monitor_id, checkin_id] + lambda monitor_slug, checkin_id: reverse( + self.endpoint, args=[monitor_slug, checkin_id] + ), + lambda monitor_slug, checkin_id: reverse( + self.endpoint_with_org, args=[self.organization.slug, monitor_slug, checkin_id] ), )