diff --git a/src/sentry/api/endpoints/project_release_stats.py b/src/sentry/api/endpoints/project_release_stats.py index f205a4d8d02645..b50dcecf8c59c5 100644 --- a/src/sentry/api/endpoints/project_release_stats.py +++ b/src/sentry/api/endpoints/project_release_stats.py @@ -5,7 +5,7 @@ from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize from sentry.models import Release, ReleaseProject -from sentry.snuba.sessions import get_crash_free_breakdown, get_project_release_stats +from sentry.snuba.sessions import get_project_release_stats from sentry.utils.dates import get_rollup_from_request @@ -74,7 +74,7 @@ def get(self, request, project, version): ) users_breakdown = [] - for data in get_crash_free_breakdown( + for data in release_health.get_crash_free_breakdown( project_id=params["project_id"][0], release=version, environments=params.get("environment"), diff --git a/src/sentry/release_health/base.py b/src/sentry/release_health/base.py index 5e892b698fe3f1..ea41449ec16f52 100644 --- a/src/sentry/release_health/base.py +++ b/src/sentry/release_health/base.py @@ -54,6 +54,14 @@ class ReleaseAdoption(TypedDict): ReleasesAdoption = Mapping[Tuple[ProjectId, ReleaseName], ReleaseAdoption] +class CrashFreeBreakdown(TypedDict): + date: datetime + total_users: int + crash_free_users: Optional[float] + total_sessions: int + crash_free_sessions: Optional[float] + + class ReleaseHealthBackend(Service): # type: ignore """Abstraction layer for all release health related queries""" @@ -63,6 +71,7 @@ class ReleaseHealthBackend(Service): # type: ignore "check_has_health_data", "get_release_sessions_time_bounds", "check_releases_have_health_data", + "get_crash_free_breakdown", "get_changed_project_release_model_adoptions", "get_oldest_health_data_for_releases", ) @@ -178,6 +187,15 @@ def check_releases_have_health_data( """ raise NotImplementedError() + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Sequence[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + """Get stats about crash free sessions and stats for the last 1, 2, 7, 14 and 30 days""" + def get_changed_project_release_model_adoptions( self, project_ids: Sequence[ProjectId], diff --git a/src/sentry/release_health/metrics.py b/src/sentry/release_health/metrics.py index 910fb285e4d130..c7c3eab986ee61 100644 --- a/src/sentry/release_health/metrics.py +++ b/src/sentry/release_health/metrics.py @@ -8,6 +8,7 @@ from sentry.models.project import Project from sentry.release_health.base import ( + CrashFreeBreakdown, CurrentAndPreviousCrashFreeRates, EnvironmentName, OrganizationId, @@ -22,13 +23,17 @@ ) from sentry.sentry_metrics import indexer from sentry.snuba.dataset import Dataset, EntityKey -from sentry.utils.snuba import raw_snql_query +from sentry.utils.snuba import QueryOutsideRetentionError, raw_snql_query class MetricIndexNotFound(Exception): pass +def get_tag_values_list(org_id: int, values: Sequence[str]) -> Sequence[int]: + return [x for x in [try_get_string_index(org_id, x) for x in values] if x is not None] + + def metric_id(org_id: int, name: str) -> int: index = indexer.resolve(org_id, name) # type: ignore if index is None: @@ -50,7 +55,7 @@ def tag_value(org_id: int, name: str) -> int: return index # type: ignore -def try_get_tag_value(org_id: int, name: str) -> Optional[int]: +def try_get_string_index(org_id: int, name: str) -> Optional[int]: return indexer.resolve(org_id, name) # type: ignore @@ -382,13 +387,7 @@ def get_release_sessions_time_bounds( ] if environments is not None: - env_filter = [ - x - for x in [ - try_get_tag_value(org_id, environment) for environment in environments - ] - if x is not None - ] + env_filter = get_tag_values_list(org_id, environments) if not env_filter: raise MetricIndexNotFound() @@ -524,11 +523,7 @@ def check_has_health_data( if includes_releases: releases = [x[1] for x in projects_list] # type: ignore release_column_name = tag_key(org_id, "release") - releases_ids = [ - release_id - for release_id in [try_get_tag_value(org_id, release) for release in releases] - if release_id is not None - ] + releases_ids = get_tag_values_list(org_id, releases) where_clause.append(Condition(Column(release_column_name), Op.IN, releases_ids)) column_names = ["project_id", release_column_name] @@ -578,7 +573,7 @@ def check_releases_have_health_data( releases_ids = [ release_id for release_id in [ - try_get_tag_value(organization_id, release) for release in release_versions + try_get_string_index(organization_id, release) for release in release_versions ] if release_id is not None ] @@ -608,6 +603,138 @@ def extract_row_info(row: Mapping[str, Union[OrganizationId, str]]) -> ReleaseNa return {extract_row_info(row) for row in result["data"]} + def _get_crash_free_breakdown_fn( + self, + org_id: int, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Sequence[EnvironmentName]] = None, + ) -> Callable[[datetime], CrashFreeBreakdown]: + def generate_defaults(end: datetime) -> CrashFreeBreakdown: + """Function to use if querying snuba is not necessary""" + return { + "crash_free_sessions": None, + "crash_free_users": None, + "date": end, + "total_sessions": 0, + "total_users": 0, + } + + # 1) Get required string indexes + try: + release_key = tag_key(org_id, "release") + release_value = tag_value(org_id, release) + environment_key = tag_key(org_id, "environment") + status_key = tag_key(org_id, "session.status") + except MetricIndexNotFound: + # No need to query snuba if any of these is missing + return generate_defaults + + environment_values = None + if environments is not None: + environment_values = get_tag_values_list(org_id, environments) + + if environment_values == []: + # No need to query snuba with an empty list + return generate_defaults + + status_init = tag_value(org_id, "init") + status_crashed = tag_value(org_id, "crashed") + + conditions = [ + Condition(Column("org_id"), Op.EQ, org_id), + Condition(Column("project_id"), Op.EQ, project_id), + Condition(Column(release_key), Op.EQ, release_value), + Condition(Column("timestamp"), Op.GTE, start), + Condition(Column(status_key), Op.IN, [status_init, status_crashed]), + ] + if environment_values is not None: + conditions.append(Condition(Column(environment_key), Op.IN, environment_values)) + + def query_stats(end: datetime) -> CrashFreeBreakdown: + def _get_data(entity_key: EntityKey, metric_name: str) -> Tuple[int, int]: + total = 0 + crashed = 0 + metric_id = try_get_string_index(org_id, metric_name) + if metric_id is not None: + where = conditions + [ + Condition(Column("metric_id"), Op.EQ, metric_id), + Condition(Column("timestamp"), Op.LT, end), + ] + data = raw_snql_query( + Query( + dataset=Dataset.Metrics.value, + match=Entity(entity_key.value), + select=[Column("value")], + where=where, + groupby=[Column(status_key)], + ), + referrer="release_health.metrics.crash-free-breakdown.session", + )["data"] + for row in data: + if row[status_key] == status_init: + total = int(row["value"]) + elif row[status_key] == status_crashed: + crashed = int(row["value"]) + + return total, crashed + + sessions_total, sessions_crashed = _get_data(EntityKey.MetricsCounters, "session") + users_total, users_crashed = _get_data(EntityKey.MetricsSets, "user") + + return { + "date": end, + "total_users": users_total, + "crash_free_users": 100 - users_crashed / float(users_total) * 100 + if users_total + else None, + "total_sessions": sessions_total, + "crash_free_sessions": 100 - sessions_crashed / float(sessions_total) * 100 + if sessions_total + else None, + } + + return query_stats + + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Sequence[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + + org_id = self._get_org_id([project_id]) + + now = datetime.now(pytz.utc) + query_fn = self._get_crash_free_breakdown_fn( + org_id, project_id, release, start, environments + ) + + last: Optional[datetime] = None + rv = [] + for offset in ( + timedelta(days=1), + timedelta(days=2), + timedelta(days=7), + timedelta(days=14), + timedelta(days=30), + ): + try: + end = start + offset + if end > now: + if last is None or (end - last).days > 1: + rv.append(query_fn(now)) + break + rv.append(query_fn(end)) + last = end + except QueryOutsideRetentionError: + # cannot query for these + pass + + return rv + def get_changed_project_release_model_adoptions( self, project_ids: Sequence[ProjectId], @@ -667,7 +794,7 @@ def get_oldest_health_data_for_releases( releases = [x[1] for x in project_releases] releases_ids = [ release_id - for release_id in [try_get_tag_value(org_id, release) for release in releases] + for release_id in [try_get_string_index(org_id, release) for release in releases] if release_id is not None ] diff --git a/src/sentry/release_health/sessions.py b/src/sentry/release_health/sessions.py index c0ca42904acf04..d3240bd752db12 100644 --- a/src/sentry/release_health/sessions.py +++ b/src/sentry/release_health/sessions.py @@ -2,6 +2,7 @@ from typing import Mapping, Optional, Sequence, Set, Tuple from sentry.release_health.base import ( + CrashFreeBreakdown, CurrentAndPreviousCrashFreeRates, EnvironmentName, OrganizationId, @@ -17,6 +18,7 @@ _check_has_health_data, _check_releases_have_health_data, _get_changed_project_release_model_adoptions, + _get_crash_free_breakdown, _get_oldest_health_data_for_releases, _get_release_adoption, _get_release_sessions_time_bounds, @@ -89,6 +91,17 @@ def check_releases_have_health_data( end, ) + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Sequence[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + return _get_crash_free_breakdown( # type: ignore + project_id=project_id, release=release, start=start, environments=environments + ) + def get_changed_project_release_model_adoptions( self, project_ids: Sequence[ProjectId], diff --git a/src/sentry/snuba/sessions.py b/src/sentry/snuba/sessions.py index a326af38a31e52..d5f566ad0f96e8 100644 --- a/src/sentry/snuba/sessions.py +++ b/src/sentry/snuba/sessions.py @@ -480,7 +480,7 @@ def get_release_health_data_overview( return rv -def get_crash_free_breakdown(project_id, release, start, environments=None): +def _get_crash_free_breakdown(project_id, release, start, environments=None): filter_keys = {"project_id": [project_id]} conditions = [["release", "=", release]] if environments is not None: diff --git a/tests/sentry/api/endpoints/test_project_release_stats.py b/tests/sentry/api/endpoints/test_project_release_stats.py new file mode 100644 index 00000000000000..c373f0f8d0a99f --- /dev/null +++ b/tests/sentry/api/endpoints/test_project_release_stats.py @@ -0,0 +1,32 @@ +from datetime import datetime + +from django.urls import reverse + +from sentry.models import Release +from sentry.testutils import APITestCase + + +class ProjectReleaseStatsTest(APITestCase): + def test_simple(self): + """Minimal test to ensure code coverage of the endpoint""" + self.login_as(user=self.user) + + project = self.create_project(name="foo") + release = Release.objects.create( + organization_id=project.organization_id, + version="1", + date_added=datetime(2013, 8, 13, 3, 8, 24, 880386), + ) + release.add_project(project) + + url = reverse( + "sentry-api-0-project-release-stats", + kwargs={ + "organization_slug": project.organization.slug, + "project_slug": project.slug, + "version": "1", + }, + ) + response = self.client.get(url, format="json") + + assert response.status_code == 200, response.content diff --git a/tests/snuba/sessions/test_sessions.py b/tests/snuba/sessions/test_sessions.py index c7048106ec3a20..920dbd20470e0a 100644 --- a/tests/snuba/sessions/test_sessions.py +++ b/tests/snuba/sessions/test_sessions.py @@ -569,6 +569,101 @@ def test_fetching_release_sessions_time_bounds_for_different_release_with_no_ses "sessions_upper_bound": None, } + def test_get_crash_free_breakdown(self): + start = timezone.now() - timedelta(days=4) + data = self.backend.get_crash_free_breakdown( + project_id=self.project.id, + release=self.session_release, + start=start, + environments=["prod"], + ) + + # Last returned date is generated within function, should be close to now: + last_date = data[-1].pop("date") + assert timezone.now() - last_date < timedelta(seconds=1) + + assert data == [ + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=1), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=2), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": 100.0, + "crash_free_users": 100.0, + "total_sessions": 2, + "total_users": 1, + }, + ] + + data = self.backend.get_crash_free_breakdown( + project_id=self.project.id, + release=self.session_crashed_release, + start=start, + environments=["prod"], + ) + data[-1].pop("date") + assert data == [ + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=1), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=2), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": 0.0, + "crash_free_users": 0.0, + "total_sessions": 1, + "total_users": 1, + }, + ] + data = self.backend.get_crash_free_breakdown( + project_id=self.project.id, + release="non-existing", + start=start, + environments=["prod"], + ) + data[-1].pop("date") + assert data == [ + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=1), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": None, + "crash_free_users": None, + "date": start + timedelta(days=2), + "total_sessions": 0, + "total_users": 0, + }, + { + "crash_free_sessions": None, + "crash_free_users": None, + "total_sessions": 0, + "total_users": 0, + }, + ] + def test_basic_release_model_adoptions(self): """ Test that the basic (project,release) data is returned