Browse Source

feat(api): Don't filter out releases without session data when sorting by sessions (WOR-1178) (#28234)

This changes the behaviour of the session sorts in the `organization_releases` endpoint to include
`Release` rows that don't have any session data. The sort will prioritize all rows that have session
data, and then sort all rows that don't have session data by `date_added` afterwards.

It was painful to do this - we basically have to do a sort across two tables. So when we've
exhausted rows from the session table, we then fetch <total number of rows so far> + <number of rows
left on page> versions from the database. These are passed to the sessions dataset to allow us to
validate which rows have data. We then filter out all rows without session data, and query postgres
for the full `Release` rows.

Not particularly efficient, but this is mostly an edge case for orgs that are switching over to use
session data. Most of the time they won't hit the extra path, since there will be enough rows in the
sessions table to begin with.
Dan Fuller 3 years ago
parent
commit
9778851bc9

+ 42 - 3
src/sentry/api/endpoints/organization_releases.py

@@ -1,4 +1,5 @@
 import re
+from datetime import datetime, timedelta
 
 from django.db import IntegrityError
 from django.db.models import F, Q
@@ -41,9 +42,11 @@ from sentry.search.events.filter import parse_semver
 from sentry.signals import release_created
 from sentry.snuba.sessions import (
     STATS_PERIODS,
+    check_releases_have_health_data,
     get_changed_project_release_model_adoptions,
     get_oldest_health_data_for_releases,
     get_project_releases_by_stability,
+    get_project_releases_count,
 )
 from sentry.utils.cache import cache
 from sentry.utils.compat import zip as izip
@@ -282,8 +285,7 @@ class OrganizationReleasesEndpoint(
         if flatten:
             select_extra["_for_project_id"] = "sentry_release_project.project_id"
 
-        if sort not in self.SESSION_SORTS:
-            queryset = queryset.filter(projects__id__in=filter_params["project_id"])
+        queryset = queryset.filter(projects__id__in=filter_params["project_id"])
 
         if sort == "date":
             queryset = queryset.order_by("-date")
@@ -312,6 +314,35 @@ class OrganizationReleasesEndpoint(
                     status=400,
                 )
 
+            def qs_load_func(queryset, total_offset, qs_offset, limit):
+                # We want to fetch at least total_offset + limit releases to check, to make sure
+                # we're not fetching only releases that were on previous pages.
+                release_versions = list(
+                    queryset.order_by_recent().values_list("version", flat=True)[
+                        : total_offset + limit
+                    ]
+                )
+                releases_with_session_data = check_releases_have_health_data(
+                    organization.id,
+                    filter_params["project_id"],
+                    release_versions,
+                    filter_params["start"]
+                    if filter_params["start"]
+                    else datetime.utcnow() - timedelta(days=90),
+                    filter_params["end"] if filter_params["end"] else datetime.utcnow(),
+                )
+                valid_versions = [
+                    rv for rv in release_versions if rv not in releases_with_session_data
+                ]
+
+                results = list(
+                    Release.objects.filter(
+                        organization_id=organization.id,
+                        version__in=valid_versions,
+                    ).order_by_recent()[qs_offset : qs_offset + limit]
+                )
+                return results
+
             paginator_cls = MergingOffsetPaginator
             paginator_kwargs.update(
                 data_load_func=lambda offset, limit: get_project_releases_by_stability(
@@ -322,9 +353,17 @@ class OrganizationReleasesEndpoint(
                     stats_period=summary_stats_period,
                     limit=limit,
                 ),
+                data_count_func=lambda: get_project_releases_count(
+                    organization_id=organization.id,
+                    project_ids=filter_params["project_id"],
+                    environments=filter_params.get("environment"),
+                    scope=sort,
+                    stats_period=summary_stats_period,
+                ),
                 apply_to_queryset=lambda queryset, rows: queryset.filter(
-                    projects__id__in=list(x[0] for x in rows), version__in=list(x[1] for x in rows)
+                    version__in=list(x[1] for x in rows)
                 ),
+                queryset_load_func=qs_load_func,
                 key_from_model=lambda x: (x._for_project_id, x.version),
             )
         else:

+ 23 - 4
src/sentry/api/paginator.py

@@ -286,12 +286,16 @@ class MergingOffsetPaginator(OffsetPaginator):
         key_from_data=None,
         max_limit=MAX_LIMIT,
         on_results=None,
+        data_count_func=None,
+        queryset_load_func=None,
     ):
         super().__init__(queryset, max_limit=max_limit, on_results=on_results)
         self.data_load_func = data_load_func
         self.apply_to_queryset = apply_to_queryset
         self.key_from_model = key_from_model or (lambda x: x.id)
         self.key_from_data = key_from_data or (lambda x: x)
+        self.data_count_func = data_count_func
+        self.queryset_load_func = queryset_load_func
 
     def get_result(self, limit=100, cursor=None):
         if cursor is None:
@@ -301,14 +305,14 @@ class MergingOffsetPaginator(OffsetPaginator):
 
         page = cursor.offset
         offset = cursor.offset * cursor.value
-        limit = (cursor.value or limit) + 1
+        limit = cursor.value or limit
 
         if self.max_offset is not None and offset >= self.max_offset:
             raise BadPaginationError("Pagination offset too large")
         if offset < 0:
             raise BadPaginationError("Pagination offset cannot be negative")
 
-        primary_results = self.data_load_func(offset=offset, limit=self.max_limit)
+        primary_results = self.data_load_func(offset=offset, limit=self.max_limit + 1)
 
         queryset = self.apply_to_queryset(self.queryset, primary_results)
 
@@ -322,9 +326,24 @@ class MergingOffsetPaginator(OffsetPaginator):
             if model is not None:
                 results.append(model)
 
-        next_cursor = Cursor(limit, page + 1, False, len(primary_results) > limit)
+        if self.queryset_load_func and self.data_count_func and len(results) < limit:
+            # If we hit the end of the results from the data load func, check whether there are
+            # any additional results in the queryset_load_func, if one is provided.
+            extra_limit = limit - len(results) + 1
+            total_data_count = self.data_count_func()
+            total_offset = offset + len(results)
+            qs_offset = total_offset - total_data_count
+            qs_results = self.queryset_load_func(
+                self.queryset, total_offset, qs_offset, extra_limit
+            )
+            results.extend(qs_results)
+            has_more = len(qs_results) == extra_limit
+        else:
+            has_more = len(primary_results) > limit
+
+        results = results[:limit]
+        next_cursor = Cursor(limit, page + 1, False, has_more)
         prev_cursor = Cursor(limit, page - 1, True, page > 0)
-        results = list(results[:limit])
 
         if self.on_results:
             results = self.on_results(results)

+ 6 - 0
src/sentry/models/release.py

@@ -257,6 +257,9 @@ class ReleaseQuerySet(models.QuerySet):
         qs = self.filter(id__in=Subquery(rpes.filter(query).values_list("release_id", flat=True)))
         return qs
 
+    def order_by_recent(self):
+        return self.order_by("-date_added", "-id")
+
 
 class ReleaseModelManager(models.Manager):
     def get_queryset(self):
@@ -299,6 +302,9 @@ class ReleaseModelManager(models.Manager):
             organization_id, operator, value, project_ids, environments
         )
 
+    def order_by_recent(self):
+        return self.get_queryset().order_by_recent()
+
     @staticmethod
     def _convert_build_code_to_build_number(build_code):
         """

+ 79 - 0
src/sentry/snuba/sessions.py

@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta
+from typing import List, Optional, Set
 
 import pytz
 from snuba_sdk.column import Column
@@ -9,6 +10,7 @@ from snuba_sdk.orderby import Direction, OrderBy
 from snuba_sdk.query import Query
 
 from sentry.snuba.dataset import Dataset
+from sentry.utils import snuba
 from sentry.utils.dates import to_datetime, to_timestamp
 from sentry.utils.snuba import (
     QueryOutsideRetentionError,
@@ -147,6 +149,38 @@ def check_has_health_data(projects_list):
     return {data_tuple(x) for x in raw_query(**raw_query_args)["data"]}
 
 
+def check_releases_have_health_data(
+    organization_id: int,
+    project_ids: List[int],
+    release_versions: List[str],
+    start: datetime,
+    end: datetime,
+) -> Set[str]:
+    """
+    Returns a set of all release versions that have health data within a given period of time.
+    """
+    if not release_versions:
+        return set()
+
+    query = Query(
+        dataset="sessions",
+        match=Entity("sessions"),
+        select=[Column("release")],
+        groupby=[Column("release")],
+        where=[
+            Condition(Column("started"), Op.GTE, start),
+            Condition(Column("started"), Op.LT, end),
+            Condition(Column("org_id"), Op.EQ, organization_id),
+            Condition(Column("project_id"), Op.IN, project_ids),
+            Condition(Column("release"), Op.IN, release_versions),
+        ],
+    )
+    data = snuba.raw_snql_query(query, referrer="snuba.sessions.check_releases_have_health_data")[
+        "data"
+    ]
+    return {row["release"] for row in data}
+
+
 def get_project_releases_by_stability(
     project_ids, offset, limit, scope, stats_period=None, environments=None
 ):
@@ -199,6 +233,51 @@ def get_project_releases_by_stability(
     return rv
 
 
+def get_project_releases_count(
+    organization_id: int,
+    project_ids: List[int],
+    scope: str,
+    stats_period: Optional[str] = None,
+    environments: Optional[str] = None,
+) -> int:
+    """
+    Fetches the total count of releases/project combinations
+    """
+    if stats_period is None:
+        stats_period = "24h"
+
+    # Special rule that we support sorting by the last 24h only.
+    if scope.endswith("_24h"):
+        stats_period = "24h"
+
+    _, stats_start, _ = get_rollup_starts_and_buckets(stats_period)
+
+    where = [
+        Condition(Column("started"), Op.GTE, stats_start),
+        Condition(Column("started"), Op.LT, datetime.now()),
+        Condition(Column("project_id"), Op.IN, project_ids),
+        Condition(Column("org_id"), Op.EQ, organization_id),
+    ]
+    if environments is not None:
+        where.append(Condition(Column("environment"), Op.IN, environments))
+
+    having = []
+    # Filter out releases with zero users when sorting by either `users` or `crash_free_users`
+    if scope in ["users", "crash_free_users"]:
+        having.append(Condition(Column("users"), Op.GT, 0))
+
+    query = Query(
+        dataset="sessions",
+        match=Entity("sessions"),
+        select=[Function("uniq", [Column("release"), Column("project_id")], alias="count")],
+        where=where,
+        having=having,
+    )
+    return snuba.raw_snql_query(query, referrer="snuba.sessions.check_releases_have_health_data")[
+        "data"
+    ][0]["count"]
+
+
 def _make_stats(start, rollup, buckets, default=0):
     rv = []
     start = int(to_timestamp(start) // rollup + 1) * rollup

+ 23 - 0
src/sentry/testutils/cases.py

@@ -815,6 +815,29 @@ class SnubaTestCase(BaseTestCase):
             == 200
         )
 
+    def session_dict(self, project=None, release=None, environment_name=None):
+        if project is None:
+            project = self.project
+
+        release_version = release.version if release else None
+        received = time.time()
+        session_started = received // 60 * 60
+        return dict(
+            distinct_id=uuid4().hex,
+            session_id=uuid4().hex,
+            org_id=project.organization_id,
+            project_id=project.id,
+            status="ok",
+            seq=0,
+            release=release_version,
+            environment=environment_name,
+            retention_days=90,
+            duration=None,
+            errors=0,
+            started=session_started,
+            received=received,
+        )
+
     def store_session(self, session):
         self.bulk_store_sessions([session])
 

+ 149 - 131
tests/sentry/api/endpoints/test_organization_releases.py

@@ -36,13 +36,22 @@ from sentry.search.events.constants import (
     SEMVER_BUILD_ALIAS,
     SEMVER_PACKAGE_ALIAS,
 )
-from sentry.testutils import APITestCase, ReleaseCommitPatchTest, SetRefsTestCase, TestCase
+from sentry.testutils import (
+    APITestCase,
+    ReleaseCommitPatchTest,
+    SetRefsTestCase,
+    SnubaTestCase,
+    TestCase,
+)
 from sentry.utils.compat.mock import patch
 
 
-class OrganizationReleaseListTest(APITestCase):
+class OrganizationReleaseListTest(APITestCase, SnubaTestCase):
     endpoint = "sentry-api-0-organization-releases"
 
+    def assert_expected_versions(self, response, expected):
+        assert [item["version"] for item in response.data] == [e.version for e in expected]
+
     def test_simple(self):
         user = self.create_user(is_staff=False, is_superuser=False)
         org = self.organization
@@ -84,14 +93,8 @@ class OrganizationReleaseListTest(APITestCase):
         )
         release4.add_project(project3)
 
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 3
-        assert response.data[0]["version"] == release4.version
-        assert response.data[1]["version"] == release1.version
-        assert response.data[2]["version"] == release3.version
+        response = self.get_valid_response(org.slug)
+        self.assert_expected_versions(response, [release4, release1, release3])
 
     def test_release_list_order_by_date_added(self):
         """
@@ -136,14 +139,89 @@ class OrganizationReleaseListTest(APITestCase):
         )
         release8.add_project(project)
 
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url, format="json")
+        response = self.get_valid_response(org.slug)
+        self.assert_expected_versions(response, [release8, release7, release6])
 
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 3
-        assert response.data[0]["version"] == release8.version
-        assert response.data[1]["version"] == release7.version
-        assert response.data[2]["version"] == release6.version
+    def test_release_list_order_by_sessions_empty(self):
+        self.login_as(user=self.user)
+
+        release_1 = self.create_release(version="1")
+        release_2 = self.create_release(version="2")
+        release_3 = self.create_release(version="3")
+        release_4 = self.create_release(version="4")
+        release_5 = self.create_release(version="5")
+
+        #  Make sure ordering works fine when we have no session data at all
+        response = self.get_valid_response(self.organization.slug, sort="sessions", flatten="1")
+        self.assert_expected_versions(
+            response, [release_5, release_4, release_3, release_2, release_1]
+        )
+
+    def test_release_list_order_by_sessions(self):
+        self.login_as(user=self.user)
+
+        release_1 = self.create_release(version="1")
+        self.store_session(self.session_dict(release=release_1))
+        release_2 = self.create_release(version="2")
+        release_3 = self.create_release(version="3")
+        release_4 = self.create_release(version="4")
+        release_5 = self.create_release(version="5")
+        self.bulk_store_sessions([self.session_dict(release=release_5) for _ in range(2)])
+
+        response = self.get_valid_response(self.organization.slug, sort="sessions", flatten="1")
+        self.assert_expected_versions(
+            response, [release_5, release_1, release_4, release_3, release_2]
+        )
+
+        response = self.get_valid_response(
+            self.organization.slug, sort="sessions", flatten="1", per_page=1
+        )
+        self.assert_expected_versions(response, [release_5])
+        response = self.get_valid_response(
+            self.organization.slug,
+            sort="sessions",
+            flatten="1",
+            per_page=1,
+            cursor=self.get_cursor_headers(response)[1],
+        )
+        self.assert_expected_versions(response, [release_1])
+        response = self.get_valid_response(
+            self.organization.slug,
+            sort="sessions",
+            flatten="1",
+            per_page=1,
+            cursor=self.get_cursor_headers(response)[1],
+        )
+        self.assert_expected_versions(response, [release_4])
+        response = self.get_valid_response(
+            self.organization.slug,
+            sort="sessions",
+            flatten="1",
+            per_page=1,
+            cursor=self.get_cursor_headers(response)[1],
+        )
+        self.assert_expected_versions(response, [release_3])
+        response = self.get_valid_response(
+            self.organization.slug,
+            sort="sessions",
+            flatten="1",
+            per_page=1,
+            cursor=self.get_cursor_headers(response)[1],
+        )
+        self.assert_expected_versions(response, [release_2])
+
+        response = self.get_valid_response(
+            self.organization.slug, sort="sessions", flatten="1", per_page=3
+        )
+        self.assert_expected_versions(response, [release_5, release_1, release_4])
+        response = self.get_valid_response(
+            self.organization.slug,
+            sort="sessions",
+            flatten="1",
+            per_page=3,
+            cursor=self.get_cursor_headers(response)[1],
+        )
+        self.assert_expected_versions(response, [release_3, release_2])
 
     def test_release_list_order_by_build_number(self):
         self.login_as(user=self.user)
@@ -154,11 +232,7 @@ class OrganizationReleaseListTest(APITestCase):
         self.create_release(version="test@1.2+500alpha")
 
         response = self.get_valid_response(self.organization.slug, sort="build")
-        assert [r["version"] for r in response.data] == [
-            release_1.version,
-            release_3.version,
-            release_2.version,
-        ]
+        self.assert_expected_versions(response, [release_1, release_3, release_2])
 
     def test_release_list_order_by_semver(self):
         self.login_as(user=self.user)
@@ -173,17 +247,20 @@ class OrganizationReleaseListTest(APITestCase):
         release_9 = self.create_release(version="random_junk")
 
         response = self.get_valid_response(self.organization.slug, sort="semver")
-        assert [r["version"] for r in response.data] == [
-            release_7.version,
-            release_2.version,
-            release_6.version,
-            release_5.version,
-            release_4.version,
-            release_1.version,
-            release_3.version,
-            release_9.version,
-            release_8.version,
-        ]
+        self.assert_expected_versions(
+            response,
+            [
+                release_7,
+                release_2,
+                release_6,
+                release_5,
+                release_4,
+                release_1,
+                release_3,
+                release_9,
+                release_8,
+            ],
+        )
 
     def test_query_filter(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -213,17 +290,11 @@ class OrganizationReleaseListTest(APITestCase):
         )
         release2.add_project(project)
 
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url + "?query=oob", format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["version"] == release.version
-
-        response = self.client.get(url + "?query=baz", format="json")
+        response = self.get_valid_response(org.slug, query="oob")
+        self.assert_expected_versions(response, [release])
 
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 0
+        response = self.get_valid_response(org.slug, query="baz")
+        self.assert_expected_versions(response, [])
 
     def test_release_filter(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -254,21 +325,13 @@ class OrganizationReleaseListTest(APITestCase):
         release2.add_project(project)
 
         response = self.get_valid_response(self.organization.slug, query=f"{RELEASE_ALIAS}:foobar")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["version"] == release.version
+        self.assert_expected_versions(response, [release])
 
         response = self.get_valid_response(self.organization.slug, query=f"{RELEASE_ALIAS}:foo*")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["version"] == release.version
+        self.assert_expected_versions(response, [release])
 
         response = self.get_valid_response(self.organization.slug, query=f"{RELEASE_ALIAS}:baz")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 0
+        self.assert_expected_versions(response, [])
 
     def test_query_filter_suffix(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -314,53 +377,41 @@ class OrganizationReleaseListTest(APITestCase):
         self.create_release(version="some.release")
 
         response = self.get_valid_response(self.organization.slug, query=f"{SEMVER_ALIAS}:>1.2.3")
-        assert [r["version"] for r in response.data] == [release_3.version, release_1.version]
+        self.assert_expected_versions(response, [release_3, release_1])
 
         response = self.get_valid_response(self.organization.slug, query=f"{SEMVER_ALIAS}:>=1.2.3")
-        assert [r["version"] for r in response.data] == [
-            release_3.version,
-            release_2.version,
-            release_1.version,
-        ]
+        self.assert_expected_versions(response, [release_3, release_2, release_1])
 
         response = self.get_valid_response(self.organization.slug, query=f"{SEMVER_ALIAS}:1.2.*")
-        assert [r["version"] for r in response.data] == [
-            release_3.version,
-            release_2.version,
-            release_1.version,
-        ]
+        self.assert_expected_versions(response, [release_3, release_2, release_1])
 
         response = self.get_valid_response(
             self.organization.slug, query=f"{SEMVER_ALIAS}:>=1.2.3", sort="semver"
         )
-        assert [r["version"] for r in response.data] == [
-            release_3.version,
-            release_1.version,
-            release_2.version,
-        ]
+        self.assert_expected_versions(response, [release_3, release_1, release_2])
 
         response = self.get_valid_response(self.organization.slug, query=f"{SEMVER_ALIAS}:2.2.1")
-        assert [r["version"] for r in response.data] == []
+        self.assert_expected_versions(response, [])
 
         response = self.get_valid_response(
             self.organization.slug, query=f"{SEMVER_PACKAGE_ALIAS}:test2"
         )
-        assert [r["version"] for r in response.data] == [release_3.version]
+        self.assert_expected_versions(response, [release_3])
 
         response = self.get_valid_response(
             self.organization.slug, query=f"{SEMVER_PACKAGE_ALIAS}:test"
         )
-        assert [r["version"] for r in response.data] == [release_2.version, release_1.version]
+        self.assert_expected_versions(response, [release_2, release_1])
 
         response = self.get_valid_response(
             self.organization.slug, query=f"{SEMVER_BUILD_ALIAS}:>124"
         )
-        assert [r["version"] for r in response.data] == [release_3.version]
+        self.assert_expected_versions(response, [release_3])
 
         response = self.get_valid_response(
             self.organization.slug, query=f"{SEMVER_BUILD_ALIAS}:<125"
         )
-        assert [r["version"] for r in response.data] == [release_2.version, release_1.version]
+        self.assert_expected_versions(response, [release_2, release_1])
 
     def test_release_stage_filter(self):
         self.login_as(user=self.user)
@@ -399,31 +450,28 @@ class OrganizationReleaseListTest(APITestCase):
             query=f"{RELEASE_STAGE_ALIAS}:{ReleaseStages.ADOPTED}",
             environment=self.environment.name,
         )
-        assert [r["version"] for r in response.data] == [adopted_release.version]
+        self.assert_expected_versions(response, [adopted_release])
 
         response = self.get_valid_response(
             self.organization.slug,
             query=f"{RELEASE_STAGE_ALIAS}:{ReleaseStages.LOW_ADOPTION}",
             environment=self.environment.name,
         )
-        assert [r["version"] for r in response.data] == [not_adopted_release.version]
+        self.assert_expected_versions(response, [not_adopted_release])
 
         response = self.get_valid_response(
             self.organization.slug,
             query=f"{RELEASE_STAGE_ALIAS}:{ReleaseStages.REPLACED}",
             environment=self.environment.name,
         )
-        assert [r["version"] for r in response.data] == [replaced_release.version]
+        self.assert_expected_versions(response, [replaced_release])
 
         response = self.get_valid_response(
             self.organization.slug,
             query=f"{RELEASE_STAGE_ALIAS}:[{ReleaseStages.ADOPTED},{ReleaseStages.REPLACED}]",
             environment=self.environment.name,
         )
-        assert [r["version"] for r in response.data] == [
-            adopted_release.version,
-            replaced_release.version,
-        ]
+        self.assert_expected_versions(response, [adopted_release, replaced_release])
 
         response = self.get_valid_response(
             self.organization.slug,
@@ -431,17 +479,15 @@ class OrganizationReleaseListTest(APITestCase):
             environment=self.environment.name,
         )
 
-        assert [r["version"] for r in response.data] == [not_adopted_release.version]
+        self.assert_expected_versions(response, [not_adopted_release])
 
         response = self.get_valid_response(
             self.organization.slug,
             sort="adoption",
         )
-        assert [r["version"] for r in response.data] == [
-            adopted_release.version,
-            replaced_release.version,
-            not_adopted_release.version,
-        ]
+        self.assert_expected_versions(
+            response, [adopted_release, replaced_release, not_adopted_release]
+        )
         adopted_rpe.update(adopted=timezone.now() - timedelta(minutes=15))
 
         # Replaced should come first now.
@@ -449,16 +495,12 @@ class OrganizationReleaseListTest(APITestCase):
             self.organization.slug,
             sort="adoption",
         )
-        assert [r["version"] for r in response.data] == [
-            replaced_release.version,
-            adopted_release.version,
-            not_adopted_release.version,
-        ]
+        self.assert_expected_versions(
+            response, [replaced_release, adopted_release, not_adopted_release]
+        )
 
         response = self.get_valid_response(self.organization.slug, sort="adoption", per_page=1)
-        assert [r["version"] for r in response.data] == [
-            replaced_release.version,
-        ]
+        self.assert_expected_versions(response, [replaced_release])
         next_cursor = self.get_cursor_headers(response)[1]
         response = self.get_valid_response(
             self.organization.slug,
@@ -466,9 +508,7 @@ class OrganizationReleaseListTest(APITestCase):
             per_page=1,
             cursor=next_cursor,
         )
-        assert [r["version"] for r in response.data] == [
-            adopted_release.version,
-        ]
+        self.assert_expected_versions(response, [adopted_release])
         next_cursor = self.get_cursor_headers(response)[1]
         response = self.get_valid_response(
             self.organization.slug,
@@ -477,9 +517,7 @@ class OrganizationReleaseListTest(APITestCase):
             cursor=next_cursor,
         )
         prev_cursor = self.get_cursor_headers(response)[0]
-        assert [r["version"] for r in response.data] == [
-            not_adopted_release.version,
-        ]
+        self.assert_expected_versions(response, [not_adopted_release])
         response = self.get_valid_response(
             self.organization.slug,
             sort="adoption",
@@ -487,9 +525,7 @@ class OrganizationReleaseListTest(APITestCase):
             cursor=prev_cursor,
         )
         prev_cursor = self.get_cursor_headers(response)[0]
-        assert [r["version"] for r in response.data] == [
-            adopted_release.version,
-        ]
+        self.assert_expected_versions(response, [adopted_release])
         response = self.get_valid_response(
             self.organization.slug,
             sort="adoption",
@@ -497,9 +533,7 @@ class OrganizationReleaseListTest(APITestCase):
             cursor=prev_cursor,
         )
         prev_cursor = self.get_cursor_headers(response)[0]
-        assert [r["version"] for r in response.data] == [
-            replaced_release.version,
-        ]
+        self.assert_expected_versions(response, [replaced_release])
 
         adopted_rpe.update(adopted=timezone.now() - timedelta(minutes=15))
 
@@ -509,10 +543,7 @@ class OrganizationReleaseListTest(APITestCase):
             sort="adoption",
             environment=self.environment.name,
         )
-        assert [r["version"] for r in response.data] == [
-            replaced_release.version,
-            not_adopted_release.version,
-        ]
+        self.assert_expected_versions(response, [replaced_release, not_adopted_release])
 
         response = self.get_response(
             self.organization.slug,
@@ -561,13 +592,8 @@ class OrganizationReleaseListTest(APITestCase):
         )
         release3.add_project(project1)
 
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 2
-        assert response.data[0]["version"] == release1.version
-        assert response.data[1]["version"] == release3.version
+        response = self.get_valid_response(org.slug)
+        self.assert_expected_versions(response, [release1, release3])
 
     def test_all_projects_parameter(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -594,13 +620,8 @@ class OrganizationReleaseListTest(APITestCase):
         )
         release2.add_project(project2)
 
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url, data={"project": [-1]}, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 2
-        assert response.data[0]["version"] == release2.version
-        assert response.data[1]["version"] == release1.version
+        response = self.get_valid_response(org.slug, project=[-1])
+        self.assert_expected_versions(response, [release2, release1])
 
     def test_new_org(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -608,11 +629,8 @@ class OrganizationReleaseListTest(APITestCase):
         team = self.create_team(organization=org)
         self.create_member(teams=[team], user=user, organization=org)
         self.login_as(user=user)
-        url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
-        response = self.client.get(url, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 0
+        response = self.get_valid_response(org.slug)
+        self.assert_expected_versions(response, [])
 
     def test_archive_release(self):
         self.login_as(user=self.user)

+ 28 - 56
tests/sentry/tasks/test_releasemonitor.py

@@ -1,5 +1,4 @@
 import time
-from uuid import uuid4
 
 from django.db.models import F
 from django.utils import timezone
@@ -89,41 +88,21 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
             group_id=self.event.group.id, project_id=self.project.id, release_id=self.release.id
         )
 
-    def session_dict(self, i, project, release_version, environment_name):
-        received = time.time()
-        session_started = received // 60 * 60
-        return dict(
-            distinct_id=uuid4().hex,
-            session_id=uuid4().hex,
-            org_id=project.organization_id,
-            project_id=project.id,
-            status="ok",
-            seq=0,
-            release=release_version,
-            environment=environment_name,
-            retention_days=90,
-            duration=None,
-            errors=0,
-            started=session_started,
-            received=received,
-        )
-
     def test_simple(self):
         self.bulk_store_sessions(
             [
                 self.session_dict(
-                    i,
                     self.project1,
-                    self.release.version,
+                    self.release,
                     self.environment.name,
                 )
-                for i in range(11)
+                for _ in range(11)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project2, self.release.version, self.environment2.name)
-                for i in range(1)
+                self.session_dict(self.project2, self.release, self.environment2.name)
+                for _ in range(1)
             ]
         )
         assert self.project1.flags.has_sessions.is_set is False
@@ -247,24 +226,23 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
         self.bulk_store_sessions(
             [
                 self.session_dict(
-                    i,
                     self.project1,
-                    self.release.version,
+                    self.release,
                     self.environment.name,
                 )
-                for i in range(1)
+                for _ in range(1)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project2, self.release.version, self.environment2.name)
-                for i in range(11)
+                self.session_dict(self.project2, self.release, self.environment2.name)
+                for _ in range(11)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project1, self.release2.version, self.environment.name)
-                for i in range(20)
+                self.session_dict(self.project1, self.release2, self.environment.name)
+                for _ in range(20)
             ]
         )
         now = timezone.now()
@@ -316,8 +294,8 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
         # Make sure re-adopting works
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project1, self.release.version, self.environment.name)
-                for i in range(50)
+                self.session_dict(self.project1, self.release, self.environment.name)
+                for _ in range(50)
             ]
         )
         time.sleep(1)
@@ -362,30 +340,29 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
         self.bulk_store_sessions(
             [
                 self.session_dict(
-                    i,
                     self.project1,
-                    self.release.version,
+                    self.release,
                     self.environment.name,
                 )
-                for i in range(11)
+                for _ in range(11)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project2, self.release.version, self.environment2.name)
-                for i in range(1)
+                self.session_dict(self.project2, self.release, self.environment2.name)
+                for _ in range(1)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project1, self.release2.version, self.environment.name)
-                for i in range(1)
+                self.session_dict(self.project1, self.release2, self.environment.name)
+                for _ in range(1)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project1, self.release3.version, self.environment.name)
-                for i in range(1)
+                self.session_dict(self.project1, self.release3, self.environment.name)
+                for _ in range(1)
             ]
         )
         now = timezone.now()
@@ -459,30 +436,28 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
         self.bulk_store_sessions(
             [
                 self.session_dict(
-                    i,
                     self.org2_project,
-                    self.org2_release.version,
+                    self.org2_release,
                     self.org2_environment.name,
                 )
-                for i in range(20)
+                for _ in range(20)
             ]
         )
         # Tests the scheduled task to ensure it properly processes each org
         self.bulk_store_sessions(
             [
                 self.session_dict(
-                    i,
                     self.project1,
-                    self.release.version,
+                    self.release,
                     self.environment.name,
                 )
-                for i in range(11)
+                for _ in range(11)
             ]
         )
         self.bulk_store_sessions(
             [
-                self.session_dict(i, self.project2, self.release.version, self.environment2.name)
-                for i in range(1)
+                self.session_dict(self.project2, self.release, self.environment2.name)
+                for _ in range(1)
             ]
         )
 
@@ -507,13 +482,10 @@ class TestReleaseMonitor(TestCase, SnubaTestCase):
 
     def test_missing_rpe_is_created(self):
         self.bulk_store_sessions(
-            [
-                self.session_dict(i, self.project1, self.release2.version, "somenvname")
-                for i in range(20)
-            ]
+            [self.session_dict(self.project1, self.release2, "somenvname") for _ in range(20)]
         )
         self.bulk_store_sessions(
-            [self.session_dict(i, self.project1, self.release2.version, "") for i in range(20)]
+            [self.session_dict(self.project1, self.release2, "") for _ in range(20)]
         )
         now = timezone.now()
         assert not ReleaseProjectEnvironment.objects.filter(

+ 125 - 0
tests/snuba/sessions/test_sessions.py

@@ -8,10 +8,12 @@ from django.utils import timezone
 from sentry.snuba.sessions import (
     _make_stats,
     check_has_health_data,
+    check_releases_have_health_data,
     get_adjacent_releases_based_on_adoption,
     get_current_and_previous_crash_free_rates,
     get_oldest_health_data_for_releases,
     get_project_releases_by_stability,
+    get_project_releases_count,
     get_release_adoption,
     get_release_health_data_overview,
     get_release_sessions_time_bounds,
@@ -1775,3 +1777,126 @@ class GetCrashFreeRateTestCase(TestCase, SnubaTestCase):
                 "previousCrashFreeRate": None,
             },
         }
+
+
+class GetProjectReleasesCountTest(TestCase, SnubaTestCase):
+    def test_empty(self):
+        # Test no errors when no session data
+        org = self.create_organization()
+        proj = self.create_project(organization=org)
+        assert (
+            get_project_releases_count(
+                org.id,
+                [proj.id],
+                "",
+            )
+            == 0
+        )
+
+    def test(self):
+        project_release_1 = self.create_release(self.project)
+        other_project = self.create_project()
+        other_project_release_1 = self.create_release(other_project)
+        self.bulk_store_sessions(
+            [
+                generate_session_default_args(
+                    {
+                        "org_id": self.organization.id,
+                        "environment": "prod",
+                        "project_id": self.project.id,
+                        "release": project_release_1.version,
+                    }
+                ),
+                generate_session_default_args(
+                    {
+                        "org_id": self.organization.id,
+                        "environment": "staging",
+                        "project_id": other_project.id,
+                        "release": other_project_release_1.version,
+                    }
+                ),
+            ]
+        )
+        assert get_project_releases_count(self.organization.id, [self.project.id], "sessions") == 1
+        assert get_project_releases_count(self.organization.id, [self.project.id], "users") == 1
+        assert (
+            get_project_releases_count(
+                self.organization.id, [self.project.id, other_project.id], "sessions"
+            )
+            == 2
+        )
+        assert (
+            get_project_releases_count(
+                self.organization.id, [self.project.id, other_project.id], "users"
+            )
+            == 2
+        )
+        assert (
+            get_project_releases_count(
+                self.organization.id,
+                [self.project.id, other_project.id],
+                "sessions",
+                environments=["prod"],
+            )
+            == 1
+        )
+
+
+class CheckReleasesHaveHealthDataTest(TestCase, SnubaTestCase):
+    def run_test(self, expected, projects, releases, start=None, end=None):
+        if not start:
+            start = datetime.now() - timedelta(days=1)
+        if not end:
+            end = datetime.now()
+        assert (
+            check_releases_have_health_data(
+                self.organization.id,
+                [p.id for p in projects],
+                [r.version for r in releases],
+                start,
+                end,
+            )
+            == {v.version for v in expected}
+        )
+
+    def test_empty(self):
+        # Test no errors when no session data
+        project_release_1 = self.create_release(self.project)
+        self.run_test([], [self.project], [project_release_1])
+
+    def test(self):
+        other_project = self.create_project()
+        release_1 = self.create_release(
+            self.project, version="1", additional_projects=[other_project]
+        )
+        release_2 = self.create_release(other_project, version="2")
+        self.bulk_store_sessions(
+            [
+                generate_session_default_args(
+                    {
+                        "org_id": self.organization.id,
+                        "project_id": self.project.id,
+                        "release": release_1.version,
+                    }
+                ),
+                generate_session_default_args(
+                    {
+                        "org_id": self.organization.id,
+                        "project_id": other_project.id,
+                        "release": release_1.version,
+                    }
+                ),
+                generate_session_default_args(
+                    {
+                        "org_id": self.organization.id,
+                        "project_id": other_project.id,
+                        "release": release_2.version,
+                    }
+                ),
+            ]
+        )
+        self.run_test([release_1], [self.project], [release_1])
+        self.run_test([release_1], [self.project], [release_1, release_2])
+        self.run_test([release_1], [other_project], [release_1])
+        self.run_test([release_1, release_2], [other_project], [release_1, release_2])
+        self.run_test([release_1, release_2], [self.project, other_project], [release_1, release_2])