From b04bc42d38f23e2a2206e73d8689034706c169a1 Mon Sep 17 00:00:00 2001 From: Lyn Nagara Date: Wed, 3 Apr 2024 14:55:56 -0700 Subject: [PATCH] feat(release-health): Sessions should not be default (#67353) Sessions does not even exist anymore, the metrics pipeline should be used for release health. Flip the default as all environments have been cut over. --- src/sentry/conf/server.py | 6 +- src/sentry/release_health/metrics.py | 3 +- src/sentry/testutils/pytest/metrics.py | 4 +- .../test_organization_dashboards.py | 3 + tests/acceptance/test_organization_switch.py | 3 + .../releases/test_organization_releases.py | 3 + .../releases/test_organization_sessions.py | 3 + .../test_organization_release_details.py | 3 +- .../endpoints/test_organization_releases.py | 7 +- .../endpoints/test_project_release_details.py | 3 +- .../endpoints/test_project_release_stats.py | 3 +- .../sentry/integrations/slack/test_unfurl.py | 2 +- .../test_metrics_sessions_v2.py | 21 ---- .../rules/filters/test_latest_release.py | 4 +- .../endpoints/test_organization_sessions.py | 5 +- .../api/serializers/test_group_stream.py | 2 + .../rules/conditions/test_event_frequency.py | 15 ++- tests/snuba/sessions/test_sessions.py | 111 ------------------ 18 files changed, 48 insertions(+), 153 deletions(-) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index ed57244b121bfa..2b9111e5e74fc2 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -2332,13 +2332,11 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: SENTRY_METRICS_INDEXER_ENABLE_SLICED_PRODUCER = False # Release Health -SENTRY_RELEASE_HEALTH = "sentry.release_health.sessions.SessionsReleaseHealthBackend" +SENTRY_RELEASE_HEALTH = "sentry.release_health.metrics.MetricsReleaseHealthBackend" SENTRY_RELEASE_HEALTH_OPTIONS: dict[str, Any] = {} # Release Monitor -SENTRY_RELEASE_MONITOR = ( - "sentry.release_health.release_monitor.sessions.SessionReleaseMonitorBackend" -) +SENTRY_RELEASE_MONITOR = "sentry.release_health.release_monitor.metrics.MetricReleaseMonitorBackend" SENTRY_RELEASE_MONITOR_OPTIONS: dict[str, Any] = {} # Render charts on the backend. This uses the Chartcuterie external service. diff --git a/src/sentry/release_health/metrics.py b/src/sentry/release_health/metrics.py index fe6e898be18870..a35b38745fe380 100644 --- a/src/sentry/release_health/metrics.py +++ b/src/sentry/release_health/metrics.py @@ -5,7 +5,7 @@ from typing import Any, Literal, TypeVar from snuba_sdk import Column, Condition, Direction, Op -from snuba_sdk.expressions import Granularity, Limit +from snuba_sdk.expressions import Granularity, Limit, Offset from sentry.models.environment import Environment from sentry.models.project import Project @@ -1716,6 +1716,7 @@ def get_project_releases_by_stability( orderby=orderby, groupby=groupby, granularity=Granularity(LEGACY_SESSIONS_DEFAULT_ROLLUP), + offset=Offset(offset) if offset is not None else None, limit=Limit(limit) if limit is not None else None, include_series=False, include_totals=True, diff --git a/src/sentry/testutils/pytest/metrics.py b/src/sentry/testutils/pytest/metrics.py index ff9e29bd79afbe..dee9813fb4fe9c 100644 --- a/src/sentry/testutils/pytest/metrics.py +++ b/src/sentry/testutils/pytest/metrics.py @@ -66,7 +66,7 @@ def new_build_results(*args, **kwargs): is_performance_metrics = False is_metrics = False if not isinstance(query, MetricsQuery) and not isinstance(query.match, Join): - is_performance_metrics = query.match.name.startswith("generic_metrics") + is_performance_metrics = query.match.name.startswith("generic") is_metrics = "metrics" in query.match.name if is_performance_metrics: @@ -85,7 +85,7 @@ def new_create_snql_in_snuba(subscription, snuba_query, snql_query, entity_subsc is_performance_metrics = False is_metrics = False if isinstance(query.match, Entity): - is_performance_metrics = query.match.name.startswith("generic_metrics") + is_performance_metrics = query.match.name.startswith("generic") is_metrics = "metrics" in query.match.name if is_performance_metrics: diff --git a/tests/acceptance/test_organization_dashboards.py b/tests/acceptance/test_organization_dashboards.py index 180e03da503e7c..694c615834d2b2 100644 --- a/tests/acceptance/test_organization_dashboards.py +++ b/tests/acceptance/test_organization_dashboards.py @@ -33,6 +33,9 @@ EDIT_FEATURE = ["organizations:dashboards-edit"] +pytestmark = pytest.mark.sentry_metrics + + @no_silo_test class OrganizationDashboardsAcceptanceTest(AcceptanceTestCase): def setUp(self): diff --git a/tests/acceptance/test_organization_switch.py b/tests/acceptance/test_organization_switch.py index e06617d626bdc2..ec958225f789c1 100644 --- a/tests/acceptance/test_organization_switch.py +++ b/tests/acceptance/test_organization_switch.py @@ -1,3 +1,4 @@ +import pytest from selenium.common.exceptions import TimeoutException from selenium.webdriver.common.by import By @@ -5,6 +6,8 @@ from sentry.testutils.silo import no_silo_test from sentry.utils.retries import TimedRetryPolicy +pytestmark = pytest.mark.sentry_metrics + @no_silo_test class OrganizationSwitchTest(AcceptanceTestCase, SnubaTestCase): diff --git a/tests/apidocs/endpoints/releases/test_organization_releases.py b/tests/apidocs/endpoints/releases/test_organization_releases.py index fb5eba6ad68790..b096f46c2becd6 100644 --- a/tests/apidocs/endpoints/releases/test_organization_releases.py +++ b/tests/apidocs/endpoints/releases/test_organization_releases.py @@ -1,11 +1,14 @@ from datetime import UTC, datetime +import pytest from django.test.client import RequestFactory from django.urls import reverse from fixtures.apidocs_test_case import APIDocsTestCase from sentry.models.release import Release +pytestmark = pytest.mark.sentry_metrics + class OrganizationReleasesDocsTest(APIDocsTestCase): def setUp(self): diff --git a/tests/apidocs/endpoints/releases/test_organization_sessions.py b/tests/apidocs/endpoints/releases/test_organization_sessions.py index 5ceec5517d134f..2373c954ebc581 100644 --- a/tests/apidocs/endpoints/releases/test_organization_sessions.py +++ b/tests/apidocs/endpoints/releases/test_organization_sessions.py @@ -1,9 +1,12 @@ +import pytest from django.test.client import RequestFactory from django.urls import reverse from fixtures.apidocs_test_case import APIDocsTestCase from sentry.testutils.cases import SnubaTestCase +pytestmark = pytest.mark.sentry_metrics + class OrganizationSessionsDocsTest(APIDocsTestCase, SnubaTestCase): def setUp(self): diff --git a/tests/sentry/api/endpoints/test_organization_release_details.py b/tests/sentry/api/endpoints/test_organization_release_details.py index 9667fcf2dc82aa..e8dc1a1f1f746b 100644 --- a/tests/sentry/api/endpoints/test_organization_release_details.py +++ b/tests/sentry/api/endpoints/test_organization_release_details.py @@ -2,6 +2,7 @@ from datetime import UTC, datetime, timedelta from unittest.mock import patch +import pytest from django.urls import reverse from sentry.api.endpoints.organization_release_details import OrganizationReleaseSerializer @@ -24,7 +25,7 @@ from sentry.types.activity import ActivityType from sentry.utils.security.orgauthtoken_token import generate_token, hash_token -pytestmark = [requires_snuba] +pytestmark = [pytest.mark.sentry_metrics, requires_snuba] class ReleaseDetailsTest(APITestCase): diff --git a/tests/sentry/api/endpoints/test_organization_releases.py b/tests/sentry/api/endpoints/test_organization_releases.py index 8e15eebac52baa..007e8dd60a8fbe 100644 --- a/tests/sentry/api/endpoints/test_organization_releases.py +++ b/tests/sentry/api/endpoints/test_organization_releases.py @@ -3,6 +3,7 @@ from functools import cached_property from unittest.mock import patch +import pytest from django.urls import reverse from django.utils import timezone @@ -36,9 +37,9 @@ from sentry.silo import SiloMode from sentry.testutils.cases import ( APITestCase, + BaseMetricsTestCase, ReleaseCommitPatchTest, SetRefsTestCase, - SnubaTestCase, TestCase, ) from sentry.testutils.outbox import outbox_runner @@ -47,10 +48,10 @@ from sentry.types.activity import ActivityType from sentry.utils.security.orgauthtoken_token import generate_token, hash_token -pytestmark = [requires_snuba] +pytestmark = [requires_snuba, pytest.mark.sentry_metrics] -class OrganizationReleaseListTest(APITestCase, SnubaTestCase): +class OrganizationReleaseListTest(APITestCase, BaseMetricsTestCase): endpoint = "sentry-api-0-organization-releases" def assert_expected_versions(self, response, expected): diff --git a/tests/sentry/api/endpoints/test_project_release_details.py b/tests/sentry/api/endpoints/test_project_release_details.py index a1dfbfbef06add..855d2e893ea38c 100644 --- a/tests/sentry/api/endpoints/test_project_release_details.py +++ b/tests/sentry/api/endpoints/test_project_release_details.py @@ -1,6 +1,7 @@ import unittest from datetime import UTC, datetime +import pytest from django.urls import reverse from sentry.api.serializers.rest_framework.release import ReleaseSerializer @@ -16,7 +17,7 @@ from sentry.types.activity import ActivityType from sentry.utils.security.orgauthtoken_token import generate_token, hash_token -pytestmark = [requires_snuba] +pytestmark = [pytest.mark.sentry_metrics, requires_snuba] class ReleaseDetailsTest(APITestCase): diff --git a/tests/sentry/api/endpoints/test_project_release_stats.py b/tests/sentry/api/endpoints/test_project_release_stats.py index 0a34cd28eb5ee8..dd1fa974756aa7 100644 --- a/tests/sentry/api/endpoints/test_project_release_stats.py +++ b/tests/sentry/api/endpoints/test_project_release_stats.py @@ -1,12 +1,13 @@ from datetime import UTC, datetime +import pytest from django.urls import reverse from sentry.models.release import Release from sentry.testutils.cases import APITestCase from sentry.testutils.skips import requires_snuba -pytestmark = [requires_snuba] +pytestmark = [requires_snuba, pytest.mark.sentry_metrics] class ProjectReleaseStatsTest(APITestCase): diff --git a/tests/sentry/integrations/slack/test_unfurl.py b/tests/sentry/integrations/slack/test_unfurl.py index 4196b6ba38c409..6125a035cf67a9 100644 --- a/tests/sentry/integrations/slack/test_unfurl.py +++ b/tests/sentry/integrations/slack/test_unfurl.py @@ -21,7 +21,7 @@ from sentry.testutils.helpers.features import with_feature from sentry.testutils.skips import requires_snuba -pytestmark = [requires_snuba] +pytestmark = [requires_snuba, pytest.mark.sentry_metrics] INTERVAL_COUNT = 300 INTERVALS_PER_DAY = int(60 * 60 * 24 / INTERVAL_COUNT) diff --git a/tests/sentry/release_health/test_metrics_sessions_v2.py b/tests/sentry/release_health/test_metrics_sessions_v2.py index db68c29e1c8a13..e860ca2e429534 100644 --- a/tests/sentry/release_health/test_metrics_sessions_v2.py +++ b/tests/sentry/release_health/test_metrics_sessions_v2.py @@ -61,27 +61,6 @@ def get_sessions_data(self, groupby: list[str], interval): assert response.status_code == 200 return response.data - def test_sessions_metrics_with_metrics_only_field(self): - """ - Tests whether the request of a metrics-only field forwarded to the SessionsReleaseHealthBackend - is handled with an empty response. - - This test is designed to show an edge-case that can happen in case the duplexer makes the wrong - decision with respect to which backend to choose for satisfying the query. - """ - response = self.do_request( - { - "organization_slug": [self.organization1], - "project": [self.project1.id], - "field": ["crash_free_rate(session)"], - "groupBy": [], - "interval": "1d", - } - ) - - assert len(response.data["groups"]) == 0 - assert response.status_code == 200 - @pytest.mark.parametrize( "input, expected_output, expected_status_filter", diff --git a/tests/sentry/rules/filters/test_latest_release.py b/tests/sentry/rules/filters/test_latest_release.py index 39365f6655b2c1..8bc4e1ca663ff3 100644 --- a/tests/sentry/rules/filters/test_latest_release.py +++ b/tests/sentry/rules/filters/test_latest_release.py @@ -1,5 +1,7 @@ from datetime import UTC, datetime +import pytest + from sentry.models.release import Release from sentry.models.rule import Rule from sentry.rules.filters.latest_release import LatestReleaseFilter, get_project_release_cache_key @@ -7,7 +9,7 @@ from sentry.testutils.skips import requires_snuba from sentry.utils.cache import cache -pytestmark = [requires_snuba] +pytestmark = [requires_snuba, pytest.mark.sentry_metrics] class LatestReleaseFilterTest(RuleTestCase): diff --git a/tests/snuba/api/endpoints/test_organization_sessions.py b/tests/snuba/api/endpoints/test_organization_sessions.py index 9ce62e8655b7a4..d82fc3749c0119 100644 --- a/tests/snuba/api/endpoints/test_organization_sessions.py +++ b/tests/snuba/api/endpoints/test_organization_sessions.py @@ -86,6 +86,9 @@ def adjust_end(end: datetime.datetime, interval: int) -> datetime.datetime: return end +@pytest.mark.xfail( + reason="Deprecated test. Will be faded out once SessionsReleaseHealthBackend will be removed." +) class OrganizationSessionsEndpointTest(APITestCase, SnubaTestCase): def setUp(self): super().setUp() @@ -1313,7 +1316,7 @@ def test_release_stage_filter(self): @patch("sentry.release_health.backend", MetricsReleaseHealthBackend()) class OrganizationSessionsEndpointMetricsTest( - BaseMetricsTestCase, OrganizationSessionsEndpointTest + OrganizationSessionsEndpointTest, BaseMetricsTestCase ): """Repeat all tests with metrics backend""" diff --git a/tests/snuba/api/serializers/test_group_stream.py b/tests/snuba/api/serializers/test_group_stream.py index ed76836009f718..a6d805bbc0a24b 100644 --- a/tests/snuba/api/serializers/test_group_stream.py +++ b/tests/snuba/api/serializers/test_group_stream.py @@ -2,6 +2,7 @@ from datetime import timedelta from unittest import mock +import pytest from django.utils import timezone from sentry.api.event_search import SearchFilter, SearchKey, SearchValue @@ -54,6 +55,7 @@ def test_environment(self): for args, kwargs in get_range.call_args_list: assert kwargs["environment_ids"] is None + @pytest.mark.xfail(reason="Does not work with the metrics release health backend") def test_session_count(self): group = self.group organization_id = group.project.organization_id diff --git a/tests/snuba/rules/conditions/test_event_frequency.py b/tests/snuba/rules/conditions/test_event_frequency.py index 64332f066cba4b..1ea81bfef90d23 100644 --- a/tests/snuba/rules/conditions/test_event_frequency.py +++ b/tests/snuba/rules/conditions/test_event_frequency.py @@ -15,12 +15,17 @@ EventUniqueUserFrequencyCondition, ) from sentry.testutils.abstract import Abstract -from sentry.testutils.cases import PerformanceIssueTestCase, RuleTestCase, SnubaTestCase +from sentry.testutils.cases import ( + BaseMetricsTestCase, + PerformanceIssueTestCase, + RuleTestCase, + SnubaTestCase, +) from sentry.testutils.helpers.datetime import before_now, freeze_time, iso_format from sentry.testutils.skips import requires_snuba from sentry.utils.samples import load_data -pytestmark = [requires_snuba] +pytestmark = [pytest.mark.sentry_metrics, requires_snuba] class ErrorEventMixin(SnubaTestCase): @@ -281,7 +286,7 @@ def increment(self, event, count, environment=None, timestamp=None): ) -class EventFrequencyPercentConditionTestCase(SnubaTestCase, RuleTestCase): +class EventFrequencyPercentConditionTestCase(BaseMetricsTestCase, RuleTestCase): __test__ = Abstract(__module__, __qualname__) rule_cls = EventFrequencyPercentCondition @@ -306,7 +311,7 @@ def make_session(i): duration=None, errors=0, # The line below is crucial to spread sessions throughout the time period. - started=received - i, + started=received - i - 1, received=received, ) @@ -422,7 +427,7 @@ def test_comparison(self): # Test data is 2 events in the current period and 1 events in the comparison period. # Number of sessions is 20 in each period, so current period is 20% of sessions, prev - # is 10%. Overall a 100% increase comparitively. + # is 10%. Overall a 100% increase comparatively. event = self.add_event( data={"fingerprint": ["something_random"]}, project_id=self.project.id, diff --git a/tests/snuba/sessions/test_sessions.py b/tests/snuba/sessions/test_sessions.py index 078ee1f0bfc729..800b3a05e3b355 100644 --- a/tests/snuba/sessions/test_sessions.py +++ b/tests/snuba/sessions/test_sessions.py @@ -309,115 +309,6 @@ def test_get_release_adoption_lowered(self): }, } - def test_get_release_health_data_overview_users(self): - data = self.backend.get_release_health_data_overview( - [ - (self.project.id, self.session_release), - (self.project.id, self.session_crashed_release), - ], - summary_stats_period="24h", - health_stats_period="24h", - stat="users", - ) - - stats = make_24h_stats(self.received - (24 * 3600), adjust_start=self.adjust_interval) - stats[-1] = [stats[-1][0], 1] - stats_ok = stats_crash = stats - - assert data == { - (self.project.id, self.session_crashed_release): { - "total_sessions": 1, - "sessions_errored": 0, - "total_sessions_24h": 1, - "total_users": 1, - "duration_p90": None, - "sessions_crashed": 1, - "total_users_24h": 1, - "stats": {"24h": stats_crash}, - "crash_free_users": 0.0, - "adoption": 100.0, - "sessions_adoption": 33.33333333333333, - "has_health_data": True, - "crash_free_sessions": 0.0, - "duration_p50": None, - "total_project_sessions_24h": 3, - "total_project_users_24h": 1, - }, - (self.project.id, self.session_release): { - "total_sessions": 2, - "sessions_errored": 0, - "total_sessions_24h": 2, - "total_users": 1, - "duration_p90": 57.0, - "sessions_crashed": 0, - "total_users_24h": 1, - "stats": {"24h": stats_ok}, - "crash_free_users": 100.0, - "adoption": 100.0, - "sessions_adoption": 66.66666666666666, - "has_health_data": True, - "crash_free_sessions": 100.0, - "duration_p50": 45.0, - "total_project_sessions_24h": 3, - "total_project_users_24h": 1, - }, - } - - def test_get_release_health_data_overview_sessions(self): - data = self.backend.get_release_health_data_overview( - [ - (self.project.id, self.session_release), - (self.project.id, self.session_crashed_release), - ], - summary_stats_period="24h", - health_stats_period="24h", - stat="sessions", - ) - - stats = make_24h_stats(self.received - (24 * 3600), adjust_start=self.adjust_interval) - - stats_ok = stats[:-1] + [[stats[-1][0], 2]] - stats_crash = stats[:-1] + [[stats[-1][0], 1]] - - assert data == { - (self.project.id, self.session_crashed_release): { - "total_sessions": 1, - "sessions_errored": 0, - "total_sessions_24h": 1, - "total_users": 1, - "duration_p90": None, - "sessions_crashed": 1, - "total_users_24h": 1, - "stats": {"24h": stats_crash}, - "crash_free_users": 0.0, - "adoption": 100.0, - "sessions_adoption": 33.33333333333333, - "has_health_data": True, - "crash_free_sessions": 0.0, - "duration_p50": None, - "total_project_sessions_24h": 3, - "total_project_users_24h": 1, - }, - (self.project.id, self.session_release): { - "total_sessions": 2, - "sessions_errored": 0, - "total_sessions_24h": 2, - "total_users": 1, - "duration_p90": 57.0, - "sessions_crashed": 0, - "total_users_24h": 1, - "stats": {"24h": stats_ok}, - "crash_free_users": 100.0, - "sessions_adoption": 66.66666666666666, - "adoption": 100.0, - "has_health_data": True, - "crash_free_sessions": 100.0, - "duration_p50": 45.0, - "total_project_sessions_24h": 3, - "total_project_users_24h": 1, - }, - } - def test_fetching_release_sessions_time_bounds_for_different_release(self): """ Test that ensures only session bounds for releases are calculated according @@ -1636,9 +1527,7 @@ def test_get_release_health_data_overview_users(self): inner = data[(self.project.id, self.session_release)] assert inner["total_users"] == 3 - assert inner["total_users_24h"] == 3 assert inner["crash_free_users"] == 66.66666666666667 - assert inner["total_project_users_24h"] == 3 def test_get_crash_free_breakdown(self): start = timezone.now() - timedelta(days=4)