From 6e376cdce13049ce611f9ee3c6592a82ed91831a Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Wed, 29 Sep 2021 13:22:10 +0200 Subject: [PATCH 1/4] Boilerplate and test --- src/sentry/release_health/base.py | 19 +++++- src/sentry/release_health/metrics.py | 24 ++++++- src/sentry/release_health/sessions.py | 15 ++++- tests/snuba/sessions/test_sessions.py | 93 +++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 3 deletions(-) diff --git a/src/sentry/release_health/base.py b/src/sentry/release_health/base.py index 0405d10d87eb71..10786474b34fbb 100644 --- a/src/sentry/release_health/base.py +++ b/src/sentry/release_health/base.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Mapping, Optional, Sequence, Set, Tuple, TypeVar, Union +from typing import Collection, Mapping, Optional, Sequence, Set, Tuple, TypeVar, Union from typing_extensions import TypedDict @@ -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[int] + total_sessions: int + crash_free_sessions: Optional[int] + + class ReleaseHealthBackend(Service): # type: ignore """Abstraction layer for all release health related queries""" @@ -176,3 +184,12 @@ def check_releases_have_health_data( Returns a set of all release versions that have health data within a given period of time. """ raise NotImplementedError() + + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Collection[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + raise NotImplementedError() diff --git a/src/sentry/release_health/metrics.py b/src/sentry/release_health/metrics.py index 6fc9b7ce4bcf4f..ba467c17785fa7 100644 --- a/src/sentry/release_health/metrics.py +++ b/src/sentry/release_health/metrics.py @@ -1,5 +1,17 @@ from datetime import datetime, timedelta -from typing import Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple, Union +from typing import ( + Any, + Callable, + Collection, + Dict, + List, + Mapping, + Optional, + Sequence, + Set, + Tuple, + Union, +) import pytz from snuba_sdk import BooleanCondition, Column, Condition, Entity, Function, Op, Query @@ -8,6 +20,7 @@ from sentry.models.project import Project from sentry.release_health.base import ( + CrashFreeBreakdown, CurrentAndPreviousCrashFreeRates, EnvironmentName, OrganizationId, @@ -606,3 +619,12 @@ def extract_row_info(row: Mapping[str, Union[OrganizationId, str]]) -> ReleaseNa return reverse_tag_value(organization_id, row.get(release_column_name)) # type: ignore return {extract_row_info(row) for row in result["data"]} + + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Collection[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + pass diff --git a/src/sentry/release_health/sessions.py b/src/sentry/release_health/sessions.py index 50e629a7c079b8..ee111d71cacebf 100644 --- a/src/sentry/release_health/sessions.py +++ b/src/sentry/release_health/sessions.py @@ -1,7 +1,8 @@ from datetime import datetime -from typing import Optional, Sequence, Set, Tuple +from typing import Collection, Optional, Sequence, Set, Tuple from sentry.release_health.base import ( + CrashFreeBreakdown, CurrentAndPreviousCrashFreeRates, EnvironmentName, OrganizationId, @@ -17,6 +18,7 @@ _check_releases_have_health_data, _get_release_adoption, _get_release_sessions_time_bounds, + get_crash_free_breakdown, get_current_and_previous_crash_free_rates, ) @@ -85,3 +87,14 @@ def check_releases_have_health_data( start, end, ) + + def get_crash_free_breakdown( + self, + project_id: ProjectId, + release: ReleaseName, + start: datetime, + environments: Optional[Collection[EnvironmentName]] = None, + ) -> Sequence[CrashFreeBreakdown]: + return get_crash_free_breakdown( + project_id=project_id, release=release, start=start, environments=environments + ) diff --git a/tests/snuba/sessions/test_sessions.py b/tests/snuba/sessions/test_sessions.py index cabf3c09e1ebd8..d5b7c3a2917149 100644 --- a/tests/snuba/sessions/test_sessions.py +++ b/tests/snuba/sessions/test_sessions.py @@ -4,6 +4,7 @@ import pytz from django.utils import timezone +from freezegun import freeze_time from sentry.release_health.metrics import MetricsReleaseHealthBackend from sentry.release_health.sessions import SessionsReleaseHealthBackend @@ -53,6 +54,7 @@ class ReleaseHealthMetricsTestCase(SessionMetricsTestCase): backend = MetricsReleaseHealthBackend() +@freeze_time("2021-05-01 12:59") class SnubaSessionsTest(TestCase, SnubaTestCase): backend = SessionsReleaseHealthBackend() @@ -568,6 +570,97 @@ 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"], + ) + 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, + "date": start + timedelta(days=4), + "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"], + ) + 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, + "date": start + timedelta(days=4), + "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"], + ) + 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, + "date": start + timedelta(days=4), + "total_sessions": 0, + "total_users": 0, + }, + ] + class SnubaSessionsTestMetrics(ReleaseHealthMetricsTestCase, SnubaSessionsTest): """ From ab07ea42646bb58b67ff7fdd33691101f00e33bb Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Wed, 29 Sep 2021 16:05:34 +0200 Subject: [PATCH 2/4] Impl crash free breakdown --- src/sentry/release_health/base.py | 8 +- src/sentry/release_health/metrics.py | 151 ++++++++++++++++++++++---- src/sentry/release_health/sessions.py | 6 +- tests/snuba/sessions/test_sessions.py | 12 +- 4 files changed, 145 insertions(+), 32 deletions(-) diff --git a/src/sentry/release_health/base.py b/src/sentry/release_health/base.py index 10786474b34fbb..bc2e0493dbaf6e 100644 --- a/src/sentry/release_health/base.py +++ b/src/sentry/release_health/base.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Collection, Mapping, Optional, Sequence, Set, Tuple, TypeVar, Union +from typing import Mapping, Optional, Sequence, Set, Tuple, TypeVar, Union from typing_extensions import TypedDict @@ -57,9 +57,9 @@ class ReleaseAdoption(TypedDict): class CrashFreeBreakdown(TypedDict): date: datetime total_users: int - crash_free_users: Optional[int] + crash_free_users: Optional[float] total_sessions: int - crash_free_sessions: Optional[int] + crash_free_sessions: Optional[float] class ReleaseHealthBackend(Service): # type: ignore @@ -190,6 +190,6 @@ def get_crash_free_breakdown( project_id: ProjectId, release: ReleaseName, start: datetime, - environments: Optional[Collection[EnvironmentName]] = None, + environments: Optional[Sequence[EnvironmentName]] = None, ) -> Sequence[CrashFreeBreakdown]: raise NotImplementedError() diff --git a/src/sentry/release_health/metrics.py b/src/sentry/release_health/metrics.py index ba467c17785fa7..66783a835aaa8a 100644 --- a/src/sentry/release_health/metrics.py +++ b/src/sentry/release_health/metrics.py @@ -1,17 +1,5 @@ from datetime import datetime, timedelta -from typing import ( - Any, - Callable, - Collection, - Dict, - List, - Mapping, - Optional, - Sequence, - Set, - Tuple, - Union, -) +from typing import Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple, Union import pytz from snuba_sdk import BooleanCondition, Column, Condition, Entity, Function, Op, Query @@ -34,13 +22,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: @@ -62,7 +54,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 @@ -397,7 +389,7 @@ def get_release_sessions_time_bounds( env_filter = [ x for x in [ - try_get_tag_value(org_id, environment) for environment in environments + try_get_string_index(org_id, environment) for environment in environments ] if x is not None ] @@ -538,7 +530,7 @@ def check_has_health_data( 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] + for release_id in [try_get_string_index(org_id, release) for release in releases] if release_id is not None ] where_clause.append(Condition(Column(release_column_name), Op.IN, releases_ids)) @@ -590,7 +582,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 ] @@ -620,11 +612,130 @@ 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 dummy_fn(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 dummy_fn + + 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 dummy_fn + + 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), + ] + 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: 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.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] == tag_value(org_id, "init"): + total = int(row["value"]) + elif row[status_key] == tag_value(org_id, "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[Collection[EnvironmentName]] = None, + environments: Optional[Sequence[EnvironmentName]] = None, ) -> Sequence[CrashFreeBreakdown]: - pass + + 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: + item_start = start + offset + if item_start > now: + if last is None or (item_start - last).days > 1: + rv.append(query_fn(now)) + break + rv.append(query_fn(item_start)) + last = item_start + except QueryOutsideRetentionError: + # cannot query for these + pass + + return rv diff --git a/src/sentry/release_health/sessions.py b/src/sentry/release_health/sessions.py index ee111d71cacebf..169f417022c94b 100644 --- a/src/sentry/release_health/sessions.py +++ b/src/sentry/release_health/sessions.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Collection, Optional, Sequence, Set, Tuple +from typing import Optional, Sequence, Set, Tuple from sentry.release_health.base import ( CrashFreeBreakdown, @@ -93,8 +93,8 @@ def get_crash_free_breakdown( project_id: ProjectId, release: ReleaseName, start: datetime, - environments: Optional[Collection[EnvironmentName]] = None, + environments: Optional[Sequence[EnvironmentName]] = None, ) -> Sequence[CrashFreeBreakdown]: - return get_crash_free_breakdown( + return get_crash_free_breakdown( # type: ignore project_id=project_id, release=release, start=start, environments=environments ) diff --git a/tests/snuba/sessions/test_sessions.py b/tests/snuba/sessions/test_sessions.py index d5b7c3a2917149..b528b353300156 100644 --- a/tests/snuba/sessions/test_sessions.py +++ b/tests/snuba/sessions/test_sessions.py @@ -4,7 +4,6 @@ import pytz from django.utils import timezone -from freezegun import freeze_time from sentry.release_health.metrics import MetricsReleaseHealthBackend from sentry.release_health.sessions import SessionsReleaseHealthBackend @@ -54,7 +53,6 @@ class ReleaseHealthMetricsTestCase(SessionMetricsTestCase): backend = MetricsReleaseHealthBackend() -@freeze_time("2021-05-01 12:59") class SnubaSessionsTest(TestCase, SnubaTestCase): backend = SessionsReleaseHealthBackend() @@ -578,6 +576,11 @@ def test_get_crash_free_breakdown(self): 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, @@ -596,7 +599,6 @@ def test_get_crash_free_breakdown(self): { "crash_free_sessions": 100.0, "crash_free_users": 100.0, - "date": start + timedelta(days=4), "total_sessions": 2, "total_users": 1, }, @@ -608,6 +610,7 @@ def test_get_crash_free_breakdown(self): start=start, environments=["prod"], ) + data[-1].pop("date") assert data == [ { "crash_free_sessions": None, @@ -626,7 +629,6 @@ def test_get_crash_free_breakdown(self): { "crash_free_sessions": 0.0, "crash_free_users": 0.0, - "date": start + timedelta(days=4), "total_sessions": 1, "total_users": 1, }, @@ -637,6 +639,7 @@ def test_get_crash_free_breakdown(self): start=start, environments=["prod"], ) + data[-1].pop("date") assert data == [ { "crash_free_sessions": None, @@ -655,7 +658,6 @@ def test_get_crash_free_breakdown(self): { "crash_free_sessions": None, "crash_free_users": None, - "date": start + timedelta(days=4), "total_sessions": 0, "total_users": 0, }, From 343739745495d5ac1926fb28a0367c15e61e3620 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 1 Oct 2021 11:28:03 +0200 Subject: [PATCH 3/4] ref: Apply review feedback --- .../api/endpoints/project_release_stats.py | 9 ++-- src/sentry/release_health/base.py | 2 + src/sentry/release_health/metrics.py | 42 ++++++++----------- src/sentry/release_health/sessions.py | 4 +- src/sentry/snuba/sessions.py | 2 +- 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/sentry/api/endpoints/project_release_stats.py b/src/sentry/api/endpoints/project_release_stats.py index 9bfa437fc8fafc..a045470c1d329b 100644 --- a/src/sentry/api/endpoints/project_release_stats.py +++ b/src/sentry/api/endpoints/project_release_stats.py @@ -1,14 +1,11 @@ from rest_framework.response import Response +from sentry import release_health from sentry.api.bases.project import ProjectEndpoint, ProjectEventsError, ProjectReleasePermission 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_oldest_health_data_for_releases, - get_project_release_stats, -) +from sentry.snuba.sessions import get_oldest_health_data_for_releases, get_project_release_stats from sentry.utils.dates import get_rollup_from_request @@ -77,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 bc2e0493dbaf6e..83f16ee887ed86 100644 --- a/src/sentry/release_health/base.py +++ b/src/sentry/release_health/base.py @@ -71,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", ) def get_current_and_previous_crash_free_rates( @@ -192,4 +193,5 @@ def get_crash_free_breakdown( 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""" raise NotImplementedError() diff --git a/src/sentry/release_health/metrics.py b/src/sentry/release_health/metrics.py index 66783a835aaa8a..0e0451ecc8bfa7 100644 --- a/src/sentry/release_health/metrics.py +++ b/src/sentry/release_health/metrics.py @@ -386,13 +386,7 @@ def get_release_sessions_time_bounds( ] if environments is not None: - env_filter = [ - x - for x in [ - try_get_string_index(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() @@ -528,11 +522,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_string_index(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] @@ -620,7 +610,7 @@ def _get_crash_free_breakdown_fn( start: datetime, environments: Optional[Sequence[EnvironmentName]] = None, ) -> Callable[[datetime], CrashFreeBreakdown]: - def dummy_fn(end: datetime) -> CrashFreeBreakdown: + def generate_defaults(end: datetime) -> CrashFreeBreakdown: """Function to use if querying snuba is not necessary""" return { "crash_free_sessions": None, @@ -638,7 +628,7 @@ def dummy_fn(end: datetime) -> CrashFreeBreakdown: status_key = tag_key(org_id, "session.status") except MetricIndexNotFound: # No need to query snuba if any of these is missing - return dummy_fn + return generate_defaults environment_values = None if environments is not None: @@ -646,19 +636,23 @@ def dummy_fn(end: datetime) -> CrashFreeBreakdown: if environment_values == []: # No need to query snuba with an empty list - return dummy_fn + 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: EntityKey, metric_name: str) -> Tuple[int, int]: + 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) @@ -670,7 +664,7 @@ def _get_data(entity: EntityKey, metric_name: str) -> Tuple[int, int]: data = raw_snql_query( Query( dataset=Dataset.Metrics.value, - match=Entity(entity.value), + match=Entity(entity_key.value), select=[Column("value")], where=where, groupby=[Column(status_key)], @@ -678,9 +672,9 @@ def _get_data(entity: EntityKey, metric_name: str) -> Tuple[int, int]: referrer="release_health.metrics.crash-free-breakdown.session", )["data"] for row in data: - if row[status_key] == tag_value(org_id, "init"): + if row[status_key] == status_init: total = int(row["value"]) - elif row[status_key] == tag_value(org_id, "crashed"): + elif row[status_key] == status_crashed: crashed = int(row["value"]) return total, crashed @@ -727,13 +721,13 @@ def get_crash_free_breakdown( timedelta(days=30), ): try: - item_start = start + offset - if item_start > now: - if last is None or (item_start - last).days > 1: + 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(item_start)) - last = item_start + rv.append(query_fn(end)) + last = end except QueryOutsideRetentionError: # cannot query for these pass diff --git a/src/sentry/release_health/sessions.py b/src/sentry/release_health/sessions.py index 169f417022c94b..61379f7da2dd75 100644 --- a/src/sentry/release_health/sessions.py +++ b/src/sentry/release_health/sessions.py @@ -16,9 +16,9 @@ from sentry.snuba.sessions import ( _check_has_health_data, _check_releases_have_health_data, + _get_crash_free_breakdown, _get_release_adoption, _get_release_sessions_time_bounds, - get_crash_free_breakdown, get_current_and_previous_crash_free_rates, ) @@ -95,6 +95,6 @@ def get_crash_free_breakdown( start: datetime, environments: Optional[Sequence[EnvironmentName]] = None, ) -> Sequence[CrashFreeBreakdown]: - return get_crash_free_breakdown( # type: ignore + return _get_crash_free_breakdown( # type: ignore project_id=project_id, release=release, start=start, environments=environments ) diff --git a/src/sentry/snuba/sessions.py b/src/sentry/snuba/sessions.py index 57c2c12b916603..41c9ee4dd20c22 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: From 1293d88c0530b0b7a97091fa8ba0e53b197bb874 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 4 Oct 2021 12:56:59 +0200 Subject: [PATCH 4/4] test: Minimal test for endpoint that uses ported function --- .../endpoints/test_project_release_stats.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 tests/sentry/api/endpoints/test_project_release_stats.py 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