feat(sessions): Relax time range constraints in metrics impl (#34043)

The sessions API currently has stricter time range constraints than the
metrics API. Remove these constraints in the metrics implementation of
the sessions API.

In the future, we might want to introduce new, centralized restrictions
on metrics queries.

src/sentry/api/endpoints/organization_sessions.py

@@ -6,9 +6,10 @@ from rest_framework.exceptions import ParseError
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import features, release_health
+from sentry import release_health
 from sentry.api.bases import NoProjects, OrganizationEventsEndpointBase
-from sentry.snuba.sessions_v2 import AllowedResolution, InvalidField, InvalidParams, QueryDefinition
+from sentry.api.utils import get_date_range_from_params
+from sentry.snuba.sessions_v2 import InvalidField, InvalidParams, QueryDefinition
 
 
 # NOTE: this currently extends `OrganizationEventsEndpointBase` for `handle_query_errors` only, which should ideally be decoupled from the base class.
@@ -35,21 +36,13 @@ class OrganizationSessionsEndpoint(OrganizationEventsEndpointBase):
         if not release_health.is_metrics_based() and request.GET.get("interval") == "10s":
             query_params["interval"] = "1m"
 
-        if release_health.is_metrics_based():
-            allowed_resolution = AllowedResolution.ten_seconds
-        elif features.has(
-            "organizations:minute-resolution-sessions", organization, actor=request.user
-        ):
-            allowed_resolution = AllowedResolution.one_minute
-        else:
-            allowed_resolution = AllowedResolution.one_hour
+        start, _ = get_date_range_from_params(query_params)
+        query_config = release_health.sessions_query_config(organization, start)
 
         return QueryDefinition(
             query_params,
             params,
-            allowed_resolution=allowed_resolution,
-            # FIXME: This won't work with duplex backend
-            allow_session_status_query=release_health.is_metrics_based(),
+            query_config=query_config,
         )
 
     @contextmanager

src/sentry/release_health/base.py

@@ -1,8 +1,11 @@
 from __future__ import annotations
 
+from dataclasses import dataclass
 from datetime import datetime
+from enum import Enum
 from typing import (
     TYPE_CHECKING,
+    Any,
     Literal,
     Mapping,
     Optional,
@@ -14,6 +17,7 @@ from typing import (
     Union,
 )
 
+# from sentry.models.organization import Organization
 from sentry.utils.services import Service
 
 if TYPE_CHECKING:
@@ -47,6 +51,21 @@ GroupByFieldName = Literal[
 FilterFieldName = Literal["project", "release", "environment"]
 
 
+class AllowedResolution(Enum):
+    one_hour = (3600, "one hour")
+    one_minute = (60, "one minute")
+    ten_seconds = (10, "ten seconds")
+
+
+@dataclass(frozen=True)
+class SessionsQueryConfig:
+    """Backend-dependent config for sessions_v2 query"""
+
+    allowed_resolution: AllowedResolution
+    allow_session_status_query: bool
+    restrict_date_range: bool
+
+
 class SessionsQuery(TypedDict):
     org_id: OrganizationId
     project_ids: Sequence[ProjectId]
@@ -213,6 +232,7 @@ class ReleaseHealthBackend(Service):
         "check_has_health_data",
         "get_release_sessions_time_bounds",
         "check_releases_have_health_data",
+        "sessions_query_config",
         "run_sessions_query",
         "get_release_health_data_overview",
         "get_crash_free_breakdown",
@@ -292,6 +312,10 @@ class ReleaseHealthBackend(Service):
 
         raise NotImplementedError()
 
+    def sessions_query_config(self, organization: Any, start: datetime) -> SessionsQueryConfig:
+        """Return the backend-dependent config for sessions_v2.QueryDefinition"""
+        raise NotImplementedError()
+
     def run_sessions_query(
         self,
         org_id: int,

src/sentry/release_health/duplex.py

@@ -48,6 +48,7 @@ from sentry.release_health.base import (
     ReleaseName,
     ReleasesAdoption,
     ReleaseSessionsTimeBounds,
+    SessionsQueryConfig,
     SessionsQueryResult,
     StatsPeriod,
 )
@@ -936,6 +937,21 @@ class DuplexReleaseHealthBackend(ReleaseHealthBackend):
             org_id,
         )
 
+    def sessions_query_config(
+        self, organization: Organization, start: datetime
+    ) -> SessionsQueryConfig:
+        # Same should compare condition as run_sessions_query:
+        should_compare = _coerce_utc(start) > self.metrics_start
+        if should_compare and features.has(
+            "organizations:release-health-return-metrics", organization
+        ):
+            # Note: This is not watertight. If metrics_result in _dispatch_call_inner
+            # is None because of a crash, we return sessions data, but with the metrics-based
+            # config.
+            return self.metrics.sessions_query_config(organization, start)
+        else:
+            return self.sessions.sessions_query_config(organization, start)
+
     def run_sessions_query(
         self,
         org_id: int,

src/sentry/release_health/metrics.py

@@ -25,6 +25,7 @@ from snuba_sdk.expressions import Expression, Granularity, Limit, Offset
 from snuba_sdk.query import SelectableExpression
 
 from sentry.models import Environment
+from sentry.models.organization import Organization
 from sentry.models.project import Project
 from sentry.release_health.base import (
     CrashFreeBreakdown,
@@ -46,6 +47,7 @@ from sentry.release_health.base import (
     ReleasesAdoption,
     ReleaseSessionsTimeBounds,
     SessionCounts,
+    SessionsQueryConfig,
     SessionsQueryResult,
     StatsPeriod,
     UserCounts,
@@ -63,7 +65,7 @@ from sentry.sentry_metrics.utils import (
 from sentry.snuba.dataset import Dataset, EntityKey
 from sentry.snuba.metrics.naming_layer.mri import SessionMRI
 from sentry.snuba.sessions import _make_stats, get_rollup_starts_and_buckets, parse_snuba_datetime
-from sentry.snuba.sessions_v2 import QueryDefinition
+from sentry.snuba.sessions_v2 import AllowedResolution, QueryDefinition
 from sentry.utils.dates import to_datetime, to_timestamp
 from sentry.utils.snuba import QueryOutsideRetentionError, raw_snql_query
 
@@ -405,6 +407,15 @@ class MetricsReleaseHealthBackend(ReleaseHealthBackend):
 
         return rv
 
+    def sessions_query_config(
+        self, organization: Organization, start: datetime
+    ) -> SessionsQueryConfig:
+        return SessionsQueryConfig(
+            allowed_resolution=AllowedResolution.ten_seconds,
+            allow_session_status_query=True,
+            restrict_date_range=False,
+        )
+
     def run_sessions_query(
         self,
         org_id: int,

src/sentry/release_health/sessions.py

@@ -4,6 +4,8 @@ from typing import Mapping, Optional, Sequence, Set, Tuple, Union
 
 import sentry_sdk
 
+from sentry import features
+from sentry.models.organization import Organization
 from sentry.release_health.base import (
     CrashFreeBreakdown,
     CurrentAndPreviousCrashFreeRates,
@@ -21,6 +23,7 @@ from sentry.release_health.base import (
     ReleaseName,
     ReleasesAdoption,
     ReleaseSessionsTimeBounds,
+    SessionsQueryConfig,
     SessionsQueryResult,
     StatsPeriod,
 )
@@ -40,7 +43,12 @@ from sentry.snuba.sessions import (
     _get_release_sessions_time_bounds,
     get_current_and_previous_crash_free_rates,
 )
-from sentry.snuba.sessions_v2 import QueryDefinition, _run_sessions_query, massage_sessions_result
+from sentry.snuba.sessions_v2 import (
+    AllowedResolution,
+    QueryDefinition,
+    _run_sessions_query,
+    massage_sessions_result,
+)
 
 
 class SessionsReleaseHealthBackend(ReleaseHealthBackend):
@@ -76,6 +84,22 @@ class SessionsReleaseHealthBackend(ReleaseHealthBackend):
             project_releases=project_releases, environments=environments, now=now
         )
 
+    def sessions_query_config(
+        self, organization: Organization, start: datetime
+    ) -> SessionsQueryConfig:
+        if features.has(
+            "organizations:minute-resolution-sessions",
+            organization,
+        ):
+            allowed_resolution = AllowedResolution.one_minute
+        else:
+            allowed_resolution = AllowedResolution.one_hour
+        return SessionsQueryConfig(
+            allowed_resolution=allowed_resolution,
+            allow_session_status_query=False,
+            restrict_date_range=True,
+        )
+
     def run_sessions_query(
         self,
         org_id: int,

src/sentry/snuba/sessions_v2.py

@@ -2,12 +2,12 @@ import itertools
 import logging
 import math
 from datetime import datetime, timedelta
-from enum import Enum
 from typing import Any, Dict, List, Optional, Tuple
 
 import pytz
 
 from sentry.api.utils import get_date_range_from_params
+from sentry.release_health.base import AllowedResolution, SessionsQueryConfig
 from sentry.search.events.filter import get_filter
 from sentry.utils.dates import parse_stats_period, to_datetime, to_timestamp
 from sentry.utils.snuba import Dataset, raw_query, resolve_condition
@@ -241,12 +241,6 @@ class InvalidField(Exception):
     pass
 
 
-class AllowedResolution(Enum):
-    one_hour = (3600, "one hour")
-    one_minute = (60, "one minute")
-    ten_seconds = (10, "ten seconds")
-
-
 class QueryDefinition:
     """
     This is the definition of the query the user wants to execute.
@@ -254,13 +248,7 @@ class QueryDefinition:
     `fields` and `groupby` definitions as [`ColumnDefinition`] objects.
     """
 
-    def __init__(
-        self,
-        query,
-        params,
-        allowed_resolution: AllowedResolution,
-        allow_session_status_query: bool = False,
-    ):
+    def __init__(self, query, params, query_config: SessionsQueryConfig):
         self.query = query.get("query", "")
         self.raw_fields = raw_fields = query.getlist("field", [])
         self.raw_groupby = raw_groupby = query.getlist("groupBy", [])
@@ -281,7 +269,11 @@ class QueryDefinition:
                 raise InvalidField(f'Invalid groupBy: "{key}"')
             self.groupby.append(GROUPBY_MAP[key])
 
-        start, end, rollup = get_constrained_date_range(query, allowed_resolution)
+        start, end, rollup = get_constrained_date_range(
+            query,
+            allowed_resolution=query_config.allowed_resolution,
+            restrict_date_range=query_config.restrict_date_range,
+        )
         self.rollup = rollup
         self.start = start
         self.end = end
@@ -310,7 +302,7 @@ class QueryDefinition:
 
         # this makes sure that literals in complex queries are properly quoted,
         # and unknown fields are raised as errors
-        if allow_session_status_query:
+        if query_config.allow_session_status_query:
             # NOTE: "''" is added because we use the event search parser, which
             # resolves "session.status" to ifNull(..., "''")
             column_resolver = lambda col: resolve_column(col, ["session.status", "''"])
@@ -358,6 +350,7 @@ def get_constrained_date_range(
     params,
     allowed_resolution: AllowedResolution = AllowedResolution.one_hour,
     max_points=MAX_POINTS,
+    restrict_date_range=True,
 ) -> Tuple[datetime, datetime, int]:
     interval = parse_stats_period(params.get("interval", "1h"))
     interval = int(3600 if interval is None else interval.total_seconds())
@@ -402,7 +395,7 @@ def get_constrained_date_range(
         seconds=int(rounding_interval * math.ceil(date_range.total_seconds() / rounding_interval))
     )
 
-    if using_minute_resolution:
+    if using_minute_resolution and restrict_date_range:
         if date_range.total_seconds() > 6 * ONE_HOUR:
             raise InvalidParams(
                 "The time-range when using one-minute resolution intervals is restricted to 6 hours."

tests/sentry/release_health/test_duplex.py

@@ -6,6 +6,7 @@ from django.utils.datastructures import MultiValueDict
 from freezegun import freeze_time
 
 from sentry.release_health import duplex
+from sentry.release_health.base import AllowedResolution, SessionsQueryConfig
 from sentry.release_health.duplex import ComparatorType as Ct
 from sentry.release_health.duplex import (
     DuplexReleaseHealthBackend,
@@ -13,7 +14,8 @@ from sentry.release_health.duplex import (
     ListSet,
     get_sessionsv2_schema,
 )
-from sentry.snuba.sessions_v2 import AllowedResolution, QueryDefinition
+from sentry.snuba.sessions_v2 import QueryDefinition
+from sentry.testutils.helpers.features import Feature
 
 
 @pytest.mark.parametrize(
@@ -526,8 +528,30 @@ def test_get_sessionsv2_schema():
             }
         ),
         params={},
-        allowed_resolution=AllowedResolution.one_hour,
+        query_config=SessionsQueryConfig(AllowedResolution.one_hour, False, True),
     )
     schema = get_sessionsv2_schema(datetime.now(timezone.utc), query)
     assert schema["sum(session)"] == FixedList(22 * [Ct.Counter] + 2 * [Ct.Ignore])
     assert schema["avg(session.duration)"] == FixedList(22 * [Ct.Quantile] + 2 * [Ct.Ignore])
+
+
+def test_sessionsv2_config():
+    with Feature("organizations:release-health-return-metrics"):
+        backend = DuplexReleaseHealthBackend(datetime(2022, 4, 28, 16, 0, tzinfo=timezone.utc))
+        organization = None
+
+        # sessions backend:
+        assert backend.sessions_query_config(
+            organization, datetime(2022, 4, 28, 15, 59, tzinfo=timezone.utc)
+        ) == SessionsQueryConfig(
+            AllowedResolution.one_minute, allow_session_status_query=False, restrict_date_range=True
+        )
+
+        # metrics backend:
+        assert backend.sessions_query_config(
+            organization, datetime(2022, 4, 28, 16, 1, tzinfo=timezone.utc)
+        ) == SessionsQueryConfig(
+            AllowedResolution.ten_seconds,
+            allow_session_status_query=True,
+            restrict_date_range=False,
+        )

tests/snuba/api/endpoints/test_organization_sessions.py

@@ -1216,3 +1216,15 @@ class OrganizationSessionsEndpointMetricsTest(
         )
         assert response.status_code == 400, response.content
         assert response.data == {"detail": "Cannot order by sum(session) with the current filters"}
+
+    @freeze_time(MOCK_DATETIME)
+    def test_unrestricted_date_range(self):
+        response = self.do_request(
+            {
+                "project": [-1],
+                "statsPeriod": "7h",
+                "interval": "5m",
+                "field": ["sum(session)"],
+            }
+        )
+        assert response.status_code == 200

tests/snuba/sessions/test_sessions_v2.py

@@ -6,6 +6,8 @@ import pytz
 from django.http import QueryDict
 from freezegun import freeze_time
 
+from sentry.release_health.base import SessionsQueryConfig
+
 # from sentry.testutils import TestCase
 from sentry.snuba.sessions_v2 import (
     AllowedResolution,
@@ -18,10 +20,12 @@ from sentry.snuba.sessions_v2 import (
 
 
 def _make_query(qs, allow_minute_resolution=True):
-    allowed_resolution = (
-        AllowedResolution.one_minute if allow_minute_resolution else AllowedResolution.one_hour
+    query_config = SessionsQueryConfig(
+        (AllowedResolution.one_minute if allow_minute_resolution else AllowedResolution.one_hour),
+        allow_session_status_query=False,
+        restrict_date_range=True,
     )
-    return QueryDefinition(QueryDict(qs), {}, allowed_resolution)
+    return QueryDefinition(QueryDict(qs), {}, query_config)
 
 
 def result_sorted(result):