Repository: getsentry/sentry
Pull request: feat(monitors): Adds ability to search monitors by environment
Adds environment filter for endpoints.

Closes https://github.com/getsentry/sentry/issues/45019

Problem statement:
Crons: Update `monitor_index` + `monitor_details` endpoint to filter on environment
This updates the monitor endpoints to be queried on environment

1. If no environment is specified use the default environent
2. Status should come from the monitor environment and not the monitor itself

Hints from issue discussion:
Hmm going to start up on part 2 of the backfill which should make this cleaner

Commit message: removed serializer

Patch to review:
diff --git a/src/sentry/api/serializers/base.py b/src/sentry/api/serializers/base.py
index 20ca254bbeb011..ca14a8ea12c0d4 100644
--- a/src/sentry/api/serializers/base.py
+++ b/src/sentry/api/serializers/base.py
@@ -45,7 +45,7 @@ def serialize(
 
     :param objects: A list of objects
     :param user: The user who will be viewing the objects. Omit to view as `AnonymousUser`.
-    :param serializer: The `Serializer` class who's logic we'll use to serialize
+    :param serializer: The `Serializer` class whose logic we'll use to serialize
         `objects` (see below.) Omit to just look up the Serializer in the
         registry by the `objects`'s type.
     :param kwargs Any
diff --git a/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py b/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py
index 3b3a64811c8e29..ce49641e645d7d 100644
--- a/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py
+++ b/src/sentry/monitors/endpoints/organization_monitor_checkin_index.py
@@ -8,6 +8,7 @@
 from rest_framework.response import Response
 
 from sentry.api.base import region_silo_endpoint
+from sentry.api.helpers.environments import get_environments
 from sentry.api.paginator import OffsetPaginator
 from sentry.api.serializers import serialize
 from sentry.api.utils import get_date_range_from_params
@@ -15,7 +16,7 @@
 from sentry.apidocs.parameters import GLOBAL_PARAMS, MONITOR_PARAMS
 from sentry.apidocs.utils import inline_sentry_response_serializer
 from sentry.models import ProjectKey
-from sentry.monitors.models import MonitorCheckIn
+from sentry.monitors.models import MonitorCheckIn, MonitorEnvironment
 from sentry.monitors.serializers import MonitorCheckInSerializerResponse
 
 from .base import MonitorEndpoint
@@ -58,6 +59,17 @@ def get(self, request: Request, organization, project, monitor) -> Response:
             monitor_id=monitor.id, date_added__gte=start, date_added__lte=end
         )
 
+        environments = get_environments(request, organization)
+
+        if environments:
+            try:
+                monitor_environment = MonitorEnvironment.objects.get(
+                    environment=environments[0], monitor=monitor
+                )
+                queryset = queryset.filter(monitor_environment=monitor_environment)
+            except MonitorEnvironment.DoesNotExist:
+                queryset = queryset.none()
+
         return self.paginate(
             request=request,
             queryset=queryset,
diff --git a/src/sentry/monitors/endpoints/organization_monitor_details.py b/src/sentry/monitors/endpoints/organization_monitor_details.py
index 840fe39a95e016..13cd507a97a807 100644
--- a/src/sentry/monitors/endpoints/organization_monitor_details.py
+++ b/src/sentry/monitors/endpoints/organization_monitor_details.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from django.db import transaction
+from django.db.models import Prefetch
 from drf_spectacular.utils import extend_schema
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -8,6 +9,7 @@
 from sentry import audit_log
 from sentry.api.base import region_silo_endpoint
 from sentry.api.exceptions import ParameterValidationError
+from sentry.api.helpers.environments import get_environments
 from sentry.api.serializers import serialize
 from sentry.apidocs.constants import (
     RESPONSE_ACCEPTED,
@@ -19,7 +21,7 @@
 from sentry.apidocs.parameters import GLOBAL_PARAMS, MONITOR_PARAMS
 from sentry.apidocs.utils import inline_sentry_response_serializer
 from sentry.models import ScheduledDeletion
-from sentry.monitors.models import Monitor, MonitorStatus
+from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorStatus
 from sentry.monitors.serializers import MonitorSerializerResponse
 from sentry.monitors.validators import MonitorValidator
 
@@ -48,6 +50,19 @@ def get(self, request: Request, organization, project, monitor) -> Response:
         """
         Retrieves details for a monitor.
         """
+
+        environments = get_environments(request, organization)
+
+        if environments:
+            monitor_environment = MonitorEnvironment.objects.filter(
+                environment=environments[0], monitor=monitor
+            )
+            prefetch = Prefetch(
+                "monitorenvironment_set",
+                queryset=monitor_environment,
+                to_attr="selected_monitorenvironment",
+            )
+            monitor = Monitor.objects.prefetch_related(prefetch).get(id=monitor.id)
         return self.respond(serialize(monitor, request.user))
 
     @extend_schema(
diff --git a/src/sentry/monitors/endpoints/organization_monitors.py b/src/sentry/monitors/endpoints/organization_monitors.py
index 9a68e3cdab80e2..9759fc8e1635bd 100644
--- a/src/sentry/monitors/endpoints/organization_monitors.py
+++ b/src/sentry/monitors/endpoints/organization_monitors.py
@@ -1,4 +1,4 @@
-from django.db.models import Case, IntegerField, Q, Value, When
+from django.db.models import Case, IntegerField, Prefetch, Q, Value, When
 
 from sentry import audit_log
 from sentry.api.base import region_silo_endpoint
@@ -8,7 +8,7 @@
 from sentry.api.serializers import serialize
 from sentry.db.models.query import in_iexact
 from sentry.models import Organization, Project
-from sentry.monitors.models import Monitor, MonitorStatus, MonitorType
+from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorStatus, MonitorType
 from sentry.monitors.validators import MonitorValidator
 from sentry.search.utils import tokenize_query
 from sentry.signals import first_cron_monitor_created
@@ -69,6 +69,20 @@ def get(self, request: Request, organization: Organization) -> Response:
             )
         )
         query = request.GET.get("query")
+
+        if "environment" in filter_params:
+            monitor_environments = MonitorEnvironment.objects.filter(
+                environment=filter_params["environment_objects"][0]
+            )
+            prefetch = Prefetch(
+                "monitorenvironment_set",
+                queryset=monitor_environments,
+                to_attr="selected_monitorenvironment",
+            )
+            queryset = queryset.prefetch_related(prefetch).filter(
+                monitorenvironment__in=monitor_environments
+            )
+
         if query:
             tokens = tokenize_query(query)
             for key, value in tokens.items():
@@ -89,7 +103,7 @@ def get(self, request: Request, organization: Organization) -> Response:
                 elif key == "type":
                     try:
                         queryset = queryset.filter(
-                            status__in=map_value_to_constant(MonitorType, value)
+                            type__in=map_value_to_constant(MonitorType, value)
                         )
                     except ValueError:
                         queryset = queryset.none()
diff --git a/src/sentry/monitors/serializers.py b/src/sentry/monitors/serializers.py
index b89f955beb2afb..bf91f2d012dbb6 100644
--- a/src/sentry/monitors/serializers.py
+++ b/src/sentry/monitors/serializers.py
@@ -11,11 +11,11 @@
 
 @register(Monitor)
 class MonitorSerializer(Serializer):
-    def get_attrs(self, item_list, user):
+    def get_attrs(self, item_list, user, **kwargs):
         # TODO(dcramer): assert on relations
         projects = {
-            d["id"]: d
-            for d in serialize(
+            p["id"]: p
+            for p in serialize(
                 list(Project.objects.filter(id__in=[i.project_id for i in item_list])), user
             )
         }
@@ -29,15 +29,27 @@ def serialize(self, obj, attrs, user):
         config = obj.config.copy()
         if "schedule_type" in config:
             config["schedule_type"] = obj.get_schedule_type_display()
+        status, lastCheckIn, nextCheckIn = (
+            obj.get_status_display(),
+            obj.last_checkin,
+            obj.next_checkin,
+        )
+        if hasattr(obj, "selected_monitorenvironment") and obj.selected_monitorenvironment:
+            monitor_environment = obj.selected_monitorenvironment[0]
+            status, lastCheckIn, nextCheckIn = (
+                monitor_environment.get_status_display(),
+                monitor_environment.last_checkin,
+                monitor_environment.next_checkin,
+            )
         return {
             "id": str(obj.guid),
-            "status": obj.get_status_display(),
+            "status": status,
             "type": obj.get_type_display(),
             "name": obj.name,
             "slug": obj.slug,
             "config": config,
-            "lastCheckIn": obj.last_checkin,
-            "nextCheckIn": obj.next_checkin,
+            "lastCheckIn": lastCheckIn,
+            "nextCheckIn": nextCheckIn,
             "dateCreated": obj.date_added,
             "project": attrs["project"],
         }
diff --git a/src/sentry/testutils/cases.py b/src/sentry/testutils/cases.py
index 2f39f98732e7c5..a028ca93bd5039 100644
--- a/src/sentry/testutils/cases.py
+++ b/src/sentry/testutils/cases.py
@@ -95,6 +95,7 @@
     DashboardWidgetQuery,
     DeletedOrganization,
     Deploy,
+    Environment,
     File,
     GroupMeta,
     Identity,
@@ -109,7 +110,7 @@
     UserEmail,
     UserOption,
 )
-from sentry.monitors.models import Monitor, MonitorType, ScheduleType
+from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorType, ScheduleType
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
 from sentry.plugins.base import plugins
 from sentry.replays.models import ReplayRecordingSegment
@@ -2309,6 +2310,19 @@ def _create_monitor(self, **kwargs):
             **kwargs,
         )
 
+    def _create_monitor_environment(self, monitor, name="production"):
+        environment = Environment.get_or_create(project=self.project, name=name)
+
+        monitorenvironment_defaults = {
+            "status": monitor.status,
+            "next_checkin": monitor.next_checkin,
+            "last_checkin": monitor.last_checkin,
+        }
+
+        return MonitorEnvironment.objects.create(
+            monitor=monitor, environment=environment, **monitorenvironment_defaults
+        )
+
 
 class MonitorIngestTestCase(MonitorTestCase):
     """
diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_checkin_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_checkin_index.py
index e99e802f914846..8be97657c6deeb 100644
--- a/tests/sentry/monitors/endpoints/test_organization_monitor_checkin_index.py
+++ b/tests/sentry/monitors/endpoints/test_organization_monitor_checkin_index.py
@@ -3,6 +3,7 @@
 from django.utils import timezone
 from freezegun import freeze_time
 
+from sentry.models import Environment
 from sentry.monitors.models import CheckInStatus, MonitorCheckIn, MonitorStatus
 from sentry.testutils import MonitorTestCase
 from sentry.testutils.silo import region_silo_test
@@ -72,3 +73,81 @@ def test_statsperiod_constraints(self):
             **{"start": startOneDayAgo.isoformat(), "end": end.isoformat()},
         )
         assert resp.data[0]["id"] == str(checkin.guid)
+
+    def test_simple_environment(self):
+        self.login_as(self.user)
+
+        monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor, name="jungle")
+        checkin1 = MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=2),
+            status=CheckInStatus.OK,
+        )
+        checkin2 = MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=1),
+            status=CheckInStatus.OK,
+        )
+
+        resp = self.get_success_response(
+            self.organization.slug, monitor.slug, **{"statsPeriod": "1d", "environment": "jungle"}
+        )
+        assert len(resp.data) == 2
+
+        # Newest first
+        assert resp.data[0]["id"] == str(checkin2.guid)
+        assert resp.data[1]["id"] == str(checkin1.guid)
+
+    def test_bad_environment(self):
+        self.login_as(self.user)
+
+        monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor, name="jungle")
+        MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=2),
+            status=CheckInStatus.OK,
+        )
+        MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=1),
+            status=CheckInStatus.OK,
+        )
+
+        self.get_error_response(
+            self.organization.slug, monitor.slug, **{"statsPeriod": "1d", "environment": "desert"}
+        )
+
+    def test_bad_monitorenvironment(self):
+        self.login_as(self.user)
+
+        monitor = self._create_monitor()
+        monitor_environment = self._create_monitor_environment(monitor, name="jungle")
+        Environment.objects.create(name="volcano", organization_id=self.organization.id)
+        MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=2),
+            status=CheckInStatus.OK,
+        )
+        MonitorCheckIn.objects.create(
+            monitor=monitor,
+            monitor_environment=monitor_environment,
+            project_id=self.project.id,
+            date_added=monitor.date_added - timedelta(minutes=1),
+            status=CheckInStatus.OK,
+        )
+
+        self.get_error_response(
+            self.organization.slug, monitor.slug, **{"statsPeriod": "1d", "environment": "volcano"}
+        )


Code review comments:
diff_hunk: @@ -69,6 +69,20 @@ def get(self, request: Request, organization: Organization) -> Response:
             )
         )
         query = request.GET.get("query")
+
+        if "environment" in filter_params:
+            monitor_environments = MonitorEnvironment.objects.filter(
+                environment=filter_params["environment_objects"][0]
+            )
+            prefetch = Prefetch(
+                "monitorenvironment_set",
+                queryset=monitor_environments,
+                to_attr="selected_monitorenvironment",
+            )
+            queryset = queryset.prefetch_related(prefetch).filter(
+                monitorenvironment__in=monitor_environments
+            )
path: src/sentry/monitors/endpoints/organization_monitors.py
text: @author:
I'm not super confident in this approach, looking for some feedback

@user1:
Might be easier to do `.filter(monitorenvironment__environment=filter_params["environment_objects"][0])`

@author:
yeah...but i still want to pull the status from the `menv` object

diff_hunk: @@ -48,6 +50,19 @@ def get(self, request: Request, organization, project, monitor) -> Response:
         """
         Retrieves details for a monitor.
         """
+
+        environments = get_environments(request, organization)
+
+        if environments:
+            monitor_environment = MonitorEnvironment.objects.filter(
+                environment=environments[0], monitor=monitor
+            )
+            prefetch = Prefetch(
+                "monitorenvironment_set",
+                queryset=monitor_environment,
+                to_attr="selected_monitorenvironment",
+            )
+            monitor = Monitor.objects.prefetch_related(prefetch).get(id=monitor.id)
path: src/sentry/monitors/endpoints/organization_monitor_details.py
text: @author:
don't like having to get the object out of the db again, but need it to work with the serializer for the list endpoint

@user1:
You can use https://docs.djangoproject.com/en/2.2/ref/models/querysets/#django.db.models.prefetch_related_objects to avoid this. Should be able to use `Prefetch` with this

@author:
`prefetch_related` gives me the ability to set `attr` which I use for the serializer

@user1:
I thought that was part of the `Prefetch`? I haven't tested this tbh, but I assumed that `prefetch_related_object` would do the same thing.

@author:
`Prefetch` is only used for `prefect_related`, not for `prefetch_related_objects`

@user1:
The docs I linked to say it'll accept a `Prefetch`. Seems to work

```
n [9]: pf  = Prefetch("monitorenvironment_set", to_attr="selected_monitorenvironment")

In [10]: monitor = Monitor.objects.first()

In [11]: monitor
Out[11]: <Monitor at 0x11e317730: id=1, guid=UUID('b7239e50-6b2d-45b6-a6c6-836553ada51f'), project_id=2, name='check-auth'>

In [15]: prefetch_related_objects([monitor], pf)

In [16]: monitor
Out[16]: <Monitor at 0x11e317730: id=1, guid=UUID('b7239e50-6b2d-45b6-a6c6-836553ada51f'), project_id=2, name='check-auth'>

In [17]: monitor.selected_monitorenvironment
Out[17]: []
```

I don't have any envs in my local, but that seems to work ok?

@user1:
Could also just fetch the monitor and assign it, like `monitor.selected_monitorenvironment = me`, since we're only ever fetching one

@author:
for some reason i didn't think you couldn't assign arbitrary values like that...fixing. thanks