Browse Source

ref(sessions): Remove sessions subscriptions (#68494)

Lyn Nagara 11 months ago
parent
commit
b2ed6307a1

+ 0 - 4
src/sentry/incidents/logic.py

@@ -533,10 +533,6 @@ def create_alert_rule(
         # Since comparison alerts make twice as many queries, run the queries less frequently.
         resolution = DEFAULT_CMP_ALERT_RULE_RESOLUTION
         comparison_delta = int(timedelta(minutes=comparison_delta).total_seconds())
-    if dataset == Dataset.Sessions and features.has(
-        "organizations:alert-crash-free-metrics", organization, actor=user
-    ):
-        dataset = Dataset.Metrics
 
     actor = None
     if owner and not isinstance(owner, Actor):

+ 0 - 17
src/sentry/incidents/subscription_processor.py

@@ -470,23 +470,6 @@ class SubscriptionProcessor:
 
         aggregation_value = self.get_aggregation_value(subscription_update)
 
-        if self.subscription.snuba_query.dataset == Dataset.Sessions.value:
-            try:
-                # Temporarily logging results from session updates for comparison with data from metric
-                # updates
-                logger.info(
-                    "subscription_processor.message",
-                    extra={
-                        "subscription_id": self.subscription.id,
-                        "dataset": self.subscription.snuba_query.dataset,
-                        "snuba_subscription_id": self.subscription.subscription_id,
-                        "result": subscription_update,
-                        "aggregation_value": aggregation_value,
-                    },
-                )
-            except Exception:
-                logger.exception("Failed to log subscription results for session subscription")
-
         # Trigger callbacks for any AlertRules that may need to know about the subscription update
         # TODO: register over/under triggers as alert rule callbacks as well
         invoke_alert_subscription_callback(

+ 1 - 6
src/sentry/search/events/builder/__init__.py

@@ -22,11 +22,7 @@ from .profile_functions import (  # NOQA
     ProfileTopFunctionsTimeseriesQueryBuilder,
 )
 from .profiles import ProfilesQueryBuilder, ProfilesTimeseriesQueryBuilder  # NOQA
-from .sessions import (  # NOQA
-    SessionsQueryBuilder,
-    SessionsV2QueryBuilder,
-    TimeseriesSessionsV2QueryBuilder,
-)
+from .sessions import SessionsV2QueryBuilder, TimeseriesSessionsV2QueryBuilder  # NOQA
 from .spans_indexed import (  # NOQA
     SpansIndexedQueryBuilder,
     TimeseriesSpanIndexedQueryBuilder,
@@ -58,7 +54,6 @@ __all__ = [
     "ProfileFunctionsQueryBuilder",
     "ProfileFunctionsTimeseriesQueryBuilder",
     "ProfileTopFunctionsTimeseriesQueryBuilder",
-    "SessionsQueryBuilder",
     "SessionsV2QueryBuilder",
     "SpansIndexedQueryBuilder",
     "TimeseriesSpanIndexedQueryBuilder",

+ 0 - 5
src/sentry/search/events/builder/sessions.py

@@ -9,11 +9,6 @@ from sentry.search.events.builder import QueryBuilder
 from sentry.search.events.types import SelectType, WhereType
 
 
-class SessionsQueryBuilder(QueryBuilder):
-    requires_organization_condition = True
-    organization_column: str = "org_id"
-
-
 class SessionsV2QueryBuilder(QueryBuilder):
     filter_allowlist_fields = {"project", "project_id", "environment", "release"}
     requires_organization_condition = True

+ 5 - 88
src/sentry/snuba/entity_subscription.py

@@ -9,7 +9,7 @@ from typing import TYPE_CHECKING, Any, TypedDict, Union
 from snuba_sdk import Column, Condition, Entity, Join, Op, Request
 
 from sentry import features
-from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS, CRASH_RATE_ALERT_SESSION_COUNT_ALIAS
+from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS
 from sentry.exceptions import InvalidQuerySubscription, UnsupportedQuerySubscription
 from sentry.models.environment import Environment
 from sentry.models.organization import Organization
@@ -223,85 +223,6 @@ class PerformanceTransactionsEntitySubscription(BaseEventsAndTransactionEntitySu
     dataset = Dataset.Transactions
 
 
-class SessionsEntitySubscription(BaseEntitySubscription):
-    query_type = SnubaQuery.Type.CRASH_RATE
-    dataset = Dataset.Sessions
-
-    def __init__(
-        self, aggregate: str, time_window: int, extra_fields: _EntitySpecificParams | None = None
-    ):
-        super().__init__(aggregate, time_window, extra_fields)
-        self.aggregate = aggregate
-        if not extra_fields or "org_id" not in extra_fields:
-            raise InvalidQuerySubscription(
-                "org_id is a required param when "
-                "building snuba filter for a metrics subscription"
-            )
-        self.org_id = extra_fields["org_id"]
-
-    def get_entity_extra_params(self) -> Mapping[str, Any]:
-        return {"organization": self.org_id}
-
-    def aggregate_query_results(
-        self, data: list[dict[str, Any]], alias: str | None = None
-    ) -> list[dict[str, Any]]:
-        assert len(data) == 1
-        col_name = alias if alias else CRASH_RATE_ALERT_AGGREGATE_ALIAS
-        if data[0][col_name] is not None:
-            data[0][col_name] = round((1 - data[0][col_name]) * 100, 3)
-        else:
-            metrics.incr(
-                "incidents.entity_subscription.sessions.aggregate_query_results.no_session_data"
-            )
-        return data
-
-    def build_query_builder(
-        self,
-        query: str,
-        project_ids: list[int],
-        environment: Environment | None,
-        params: ParamsType | None = None,
-        skip_field_validation_for_entity_subscription_deletion: bool = False,
-    ) -> QueryBuilder:
-        from sentry.search.events.builder import SessionsQueryBuilder
-
-        aggregations = [self.aggregate]
-        # This aggregation is added to return the total number of sessions in crash
-        # rate alerts that is used to identify if we are below a general minimum alert threshold
-        count_col = re.search(r"(sessions|users)", self.aggregate)
-        if not count_col:
-            raise UnsupportedQuerySubscription(
-                "Only crash free percentage queries are supported for subscriptions"
-                "over the sessions dataset"
-            )
-        count_col_matched = count_col.group()
-
-        aggregations += [f"identity({count_col_matched}) AS {CRASH_RATE_ALERT_SESSION_COUNT_ALIAS}"]
-
-        if params is None:
-            params = {}
-
-        params["project_id"] = project_ids
-
-        if environment:
-            params["environment"] = environment.name
-
-        return SessionsQueryBuilder(
-            dataset=Dataset(self.dataset.value),
-            query=query,
-            selected_columns=aggregations,
-            params=params,
-            offset=None,
-            limit=None,
-            config=QueryBuilderConfig(
-                functions_acl=["identity"],
-                skip_time_conditions=True,
-                parser_config_overrides={"blocked_keys": ALERT_BLOCKED_FIELDS},
-                skip_field_validation_for_entity_subscription_deletion=skip_field_validation_for_entity_subscription_deletion,
-            ),
-        )
-
-
 class BaseMetricsEntitySubscription(BaseEntitySubscription, ABC):
     def __init__(
         self, aggregate: str, time_window: int, extra_fields: _EntitySpecificParams | None = None
@@ -606,7 +527,6 @@ EntitySubscription = Union[
     MetricsSetsEntitySubscription,
     PerformanceTransactionsEntitySubscription,
     PerformanceMetricsEntitySubscription,
-    SessionsEntitySubscription,
 ]
 
 
@@ -632,13 +552,10 @@ def get_entity_subscription(
             entity_subscription_cls = PerformanceMetricsEntitySubscription
     if query_type == SnubaQuery.Type.CRASH_RATE:
         entity_key = determine_crash_rate_alert_entity(aggregate)
-        if dataset == Dataset.Metrics:
-            if entity_key == EntityKey.MetricsCounters:
-                entity_subscription_cls = MetricsCountersEntitySubscription
-            if entity_key == EntityKey.MetricsSets:
-                entity_subscription_cls = MetricsSetsEntitySubscription
-        else:
-            entity_subscription_cls = SessionsEntitySubscription
+        if entity_key == EntityKey.MetricsCounters:
+            entity_subscription_cls = MetricsCountersEntitySubscription
+        if entity_key == EntityKey.MetricsSets:
+            entity_subscription_cls = MetricsSetsEntitySubscription
 
     if entity_subscription_cls is None:
         raise UnsupportedQuerySubscription(

+ 0 - 54
tests/sentry/incidents/test_logic.py

@@ -699,38 +699,6 @@ class CreateAlertRuleTest(TestCase, BaseIncidentsTest):
         assert alert_rule.comparison_delta == comparison_delta * 60
         assert alert_rule.snuba_query.resolution == DEFAULT_CMP_ALERT_RULE_RESOLUTION * 60
 
-    def test_session_to_metric_alert(self):
-        alert_rule = create_alert_rule(
-            self.organization,
-            [self.project],
-            "session alert rule",
-            "",
-            "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate",
-            1,
-            AlertRuleThresholdType.ABOVE,
-            1,
-            query_type=SnubaQuery.Type.CRASH_RATE,
-            dataset=Dataset.Sessions,
-        )
-        assert alert_rule.snuba_query.type == SnubaQuery.Type.CRASH_RATE.value
-        assert alert_rule.snuba_query.dataset == Dataset.Sessions.value
-
-        with self.feature("organizations:alert-crash-free-metrics"):
-            alert_rule = create_alert_rule(
-                self.organization,
-                [self.project],
-                "session converted alert rule",
-                "",
-                "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate",
-                1,
-                AlertRuleThresholdType.ABOVE,
-                1,
-                query_type=SnubaQuery.Type.CRASH_RATE,
-                dataset=Dataset.Sessions,
-            )
-        assert alert_rule.snuba_query.type == SnubaQuery.Type.CRASH_RATE.value
-        assert alert_rule.snuba_query.dataset == Dataset.Metrics.value
-
     def test_performance_metric_alert(self):
         alert_rule = create_alert_rule(
             self.organization,
@@ -1037,28 +1005,6 @@ class UpdateAlertRuleTest(TestCase, BaseIncidentsTest):
         assert self.alert_rule.comparison_delta is None
         assert self.alert_rule.snuba_query.resolution == DEFAULT_ALERT_RULE_RESOLUTION * 60
 
-    def test_session_to_metric_alert(self):
-        alert_rule = create_alert_rule(
-            self.organization,
-            [self.project],
-            "session alert rule",
-            "",
-            "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate",
-            1,
-            AlertRuleThresholdType.ABOVE,
-            1,
-            query_type=SnubaQuery.Type.CRASH_RATE,
-            dataset=Dataset.Sessions,
-        )
-        alert_rule = update_alert_rule(alert_rule, dataset=Dataset.Sessions)
-        assert alert_rule.snuba_query.type == SnubaQuery.Type.CRASH_RATE.value
-        assert alert_rule.snuba_query.dataset == Dataset.Sessions.value
-
-        with self.feature("organizations:alert-crash-free-metrics"):
-            alert_rule = update_alert_rule(alert_rule, dataset=Dataset.Sessions)
-        assert alert_rule.snuba_query.type == SnubaQuery.Type.CRASH_RATE.value
-        assert alert_rule.snuba_query.dataset == Dataset.Metrics.value
-
     def test_performance_metric_alert(self):
         alert_rule = create_alert_rule(
             self.organization,

+ 1 - 1
tests/sentry/integrations/test_metric_alerts.py

@@ -264,7 +264,7 @@ class IncidentAttachmentInfoTestForCrashRateAlerts(TestCase, BaseMetricsTestCase
         alert_rule = self.create_alert_rule(
             query="",
             aggregate="percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate",
-            dataset=Dataset.Sessions,
+            dataset=Dataset.Metrics,
             time_window=60,
         )
         trigger = self.create_alert_rule_trigger(alert_rule, CRITICAL_TRIGGER_LABEL, 95)

+ 4 - 108
tests/sentry/snuba/test_entity_subscriptions.py

@@ -6,7 +6,6 @@ from snuba_sdk import And, Column, Condition, Entity, Function, Join, Op, Relati
 from sentry.exceptions import (
     IncompatibleMetricsQuery,
     InvalidQuerySubscription,
-    InvalidSearchQuery,
     UnsupportedQuerySubscription,
 )
 from sentry.models.group import GroupStatus
@@ -21,7 +20,6 @@ from sentry.snuba.entity_subscription import (
     MetricsSetsEntitySubscription,
     PerformanceMetricsEntitySubscription,
     PerformanceTransactionsEntitySubscription,
-    SessionsEntitySubscription,
     get_entity_key_from_snuba_query,
     get_entity_subscription,
     get_entity_subscription_from_snuba_query,
@@ -53,7 +51,7 @@ class EntitySubscriptionTestCase(TestCase):
         with pytest.raises(UnsupportedQuerySubscription):
             get_entity_subscription(
                 query_type=SnubaQuery.Type.CRASH_RATE,
-                dataset=Dataset.Sessions,
+                dataset=Dataset.Metrics,
                 aggregate=aggregate,
                 time_window=3600,
                 extra_fields={"org_id": self.organization.id},
@@ -64,7 +62,7 @@ class EntitySubscriptionTestCase(TestCase):
         with pytest.raises(InvalidQuerySubscription):
             get_entity_subscription(
                 query_type=SnubaQuery.Type.CRASH_RATE,
-                dataset=Dataset.Sessions,
+                dataset=Dataset.Metrics,
                 aggregate=aggregate,
                 time_window=3600,
             )
@@ -73,7 +71,7 @@ class EntitySubscriptionTestCase(TestCase):
         entities = [
             get_entity_subscription(
                 query_type=SnubaQuery.Type.CRASH_RATE,
-                dataset=Dataset.Sessions,
+                dataset=Dataset.Metrics,
                 aggregate="percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate",
                 time_window=3600,
                 extra_fields={"org_id": self.organization.id},
@@ -86,50 +84,9 @@ class EntitySubscriptionTestCase(TestCase):
             ),
         ]
         for entity in entities:
-            with pytest.raises(InvalidSearchQuery, match="Invalid key for this search: timestamp"):
+            with pytest.raises(Exception):
                 entity.build_query_builder("timestamp:-24h", [self.project.id], None)
 
-    def test_get_entity_subscriptions_for_sessions_dataset(self) -> None:
-        aggregate = "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate"
-        entity_subscription = get_entity_subscription(
-            query_type=SnubaQuery.Type.CRASH_RATE,
-            dataset=Dataset.Sessions,
-            aggregate=aggregate,
-            time_window=3600,
-            extra_fields={"org_id": self.organization.id},
-        )
-        assert isinstance(entity_subscription, SessionsEntitySubscription)
-        assert entity_subscription.aggregate == aggregate
-        assert entity_subscription.get_entity_extra_params() == {
-            "organization": self.organization.id
-        }
-        assert entity_subscription.dataset == Dataset.Sessions
-        snql_query = entity_subscription.build_query_builder(
-            "", [self.project.id], None
-        ).get_snql_query()
-        snql_query.query.select.sort(key=lambda q: q.function)
-        assert snql_query.query.select == [
-            Function(
-                function="identity", parameters=[Column(name="sessions")], alias="_total_count"
-            ),
-            Function(
-                function="if",
-                parameters=[
-                    Function(function="greater", parameters=[Column(name="sessions"), 0]),
-                    Function(
-                        function="divide",
-                        parameters=[Column(name="sessions_crashed"), Column(name="sessions")],
-                    ),
-                    None,
-                ],
-                alias="_crash_rate_alert_aggregate",
-            ),
-        ]
-        assert snql_query.query.where == [
-            Condition(Column(name="project_id"), Op.IN, [self.project.id]),
-            Condition(Column(name="org_id"), Op.EQ, self.organization.id),
-        ]
-
     def test_get_entity_subscription_for_metrics_dataset_non_supported_aggregate(self) -> None:
         aggregate = "count(sessions)"
         with pytest.raises(UnsupportedQuerySubscription):
@@ -151,67 +108,6 @@ class EntitySubscriptionTestCase(TestCase):
                 time_window=3600,
             )
 
-    # This test has been kept in order to validate whether the old queries through metrics are supported, in the future
-    # this should be removed.
-    def test_get_entity_subscription_for_metrics_dataset_for_users(self) -> None:
-        org_id = self.organization.id
-        use_case_id = UseCaseID.SESSIONS
-
-        aggregate = "percentage(users_crashed, users) AS _crash_rate_alert_aggregate"
-        entity_subscription = get_entity_subscription(
-            query_type=SnubaQuery.Type.CRASH_RATE,
-            dataset=Dataset.Metrics,
-            aggregate=aggregate,
-            time_window=3600,
-            extra_fields={"org_id": self.organization.id},
-        )
-        assert isinstance(entity_subscription, MetricsSetsEntitySubscription)
-        assert entity_subscription.aggregate == aggregate
-        assert entity_subscription.get_entity_extra_params() == {
-            "organization": self.organization.id,
-            "granularity": 10,
-        }
-        assert entity_subscription.dataset == Dataset.Metrics
-        session_status = resolve_tag_key(use_case_id, org_id, "session.status")
-        session_status_crashed = resolve_tag_value(use_case_id, org_id, "crashed")
-        snql_query = entity_subscription.build_query_builder(
-            "", [self.project.id], None, {"organization_id": self.organization.id}
-        ).get_snql_query()
-        key = lambda func: func.alias
-        assert sorted(snql_query.query.select, key=key) == sorted(
-            [
-                Function("uniq", parameters=[Column("value")], alias="count"),
-                Function(
-                    "uniqIf",
-                    parameters=[
-                        Column(name="value"),
-                        Function(
-                            function="equals",
-                            parameters=[
-                                Column(session_status),
-                                session_status_crashed,
-                            ],
-                        ),
-                    ],
-                    alias="crashed",
-                ),
-            ],
-            key=key,
-        )
-        assert snql_query.query.where == [
-            Condition(Column("project_id"), Op.IN, [self.project.id]),
-            Condition(Column("org_id"), Op.EQ, self.organization.id),
-            Condition(
-                Column("metric_id"),
-                Op.EQ,
-                resolve(
-                    UseCaseID.SESSIONS,
-                    self.organization.id,
-                    entity_subscription.metric_key.value,
-                ),
-            ),
-        ]
-
     def test_get_entity_subscription_for_metrics_dataset_for_users_with_metrics_layer(self) -> None:
         with Feature("organizations:use-metrics-layer-in-alerts"):
             org_id = self.organization.id

+ 0 - 65
tests/sentry/snuba/test_tasks.py

@@ -986,71 +986,6 @@ class BuildSnqlQueryTest(TestCase):
             expected_conditions,
         )
 
-    def test_simple_sessions(self):
-        expected_conditions = [
-            Condition(Column(name="project_id"), Op.IN, [self.project.id]),
-            Condition(Column(name="org_id"), Op.EQ, self.organization.id),
-        ]
-
-        self.run_test(
-            SnubaQuery.Type.CRASH_RATE,
-            Dataset.Sessions,
-            "percentage(sessions_crashed, sessions) as _crash_rate_alert_aggregate",
-            "",
-            expected_conditions,
-            entity_extra_fields={"org_id": self.organization.id},
-        )
-
-    def test_simple_users(self):
-        expected_conditions = [
-            Condition(Column(name="project_id"), Op.IN, [self.project.id]),
-            Condition(Column(name="org_id"), Op.EQ, self.organization.id),
-        ]
-        self.run_test(
-            SnubaQuery.Type.CRASH_RATE,
-            Dataset.Sessions,
-            "percentage(users_crashed, users) as _crash_rate_alert_aggregate",
-            "",
-            expected_conditions,
-            entity_extra_fields={"org_id": self.organization.id},
-        )
-
-    def test_query_and_environment_sessions(self):
-        env = self.create_environment(self.project, name="development")
-        expected_conditions = [
-            Condition(Column(name="release"), Op.IN, ["ahmed@12.2"]),
-            Condition(Column(name="project_id"), Op.IN, [self.project.id]),
-            Condition(Column(name="environment"), Op.EQ, "development"),
-            Condition(Column(name="org_id"), Op.EQ, self.organization.id),
-        ]
-        self.run_test(
-            SnubaQuery.Type.CRASH_RATE,
-            Dataset.Sessions,
-            "percentage(sessions_crashed, sessions) as _crash_rate_alert_aggregate",
-            "release:ahmed@12.2",
-            expected_conditions,
-            entity_extra_fields={"org_id": self.organization.id},
-            environment=env,
-        )
-
-    def test_query_and_environment_users(self):
-        env = self.create_environment(self.project, name="development")
-        expected_conditions = [
-            Condition(Column(name="release"), Op.IN, ["ahmed@12.2"]),
-            Condition(Column(name="project_id"), Op.IN, [self.project.id]),
-            Condition(Column(name="environment"), Op.EQ, "development"),
-            Condition(Column(name="org_id"), Op.EQ, self.organization.id),
-        ]
-        self.run_test(
-            SnubaQuery.Type.CRASH_RATE,
-            Dataset.Sessions,
-            "percentage(users_crashed, users) as _crash_rate_alert_aggregate",
-            "release:ahmed@12.2",
-            expected_conditions,
-            entity_extra_fields={"org_id": self.organization.id},
-            environment=env,
-        )
-
     def test_simple_sessions_for_metrics(self):
         with Feature("organizations:use-metrics-layer-in-alerts"):
             org_id = self.organization.id