Skip to content

Commit 24ab0c2

Browse files
authored
ref(querybuilder): Remove ParamsType from tasks (#74570)
- This removes the usage of ParamsType from tasks in favour of SnubaParams - Added stats_period as an option to SnubaParams as a way to define start and end - Added a filter_params property on SnubaParams to make this switchover easy so I'm impacting less code (eg. don't have to change the params references in `format_top_events_timeseries_results` even though `params={}` now)
1 parent f7390a6 commit 24ab0c2

File tree

8 files changed

+107
-55
lines changed

8 files changed

+107
-55
lines changed

src/sentry/search/events/builder/discover.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
ParamsType,
3333
QueryBuilderConfig,
3434
SelectType,
35+
SnubaParams,
3536
WhereType,
3637
)
3738
from sentry.snuba.dataset import Dataset
@@ -159,6 +160,7 @@ def __init__(
159160
dataset: Dataset,
160161
params: ParamsType,
161162
interval: int,
163+
snuba_params: SnubaParams | None = None,
162164
query: str | None = None,
163165
selected_columns: list[str] | None = None,
164166
equations: list[str] | None = None,
@@ -171,6 +173,7 @@ def __init__(
171173
super().__init__(
172174
dataset,
173175
params,
176+
snuba_params=snuba_params,
174177
query=query,
175178
selected_columns=selected_columns,
176179
equations=equations,
@@ -275,6 +278,7 @@ def __init__(
275278
params: ParamsType,
276279
interval: int,
277280
top_events: list[dict[str, Any]],
281+
snuba_params: SnubaParams | None = None,
278282
other: bool = False,
279283
query: str | None = None,
280284
selected_columns: list[str] | None = None,
@@ -290,6 +294,7 @@ def __init__(
290294
super().__init__(
291295
dataset,
292296
params,
297+
snuba_params=snuba_params,
293298
interval=interval,
294299
query=query,
295300
selected_columns=list(set(selected_columns + timeseries_functions)),

src/sentry/search/events/builder/profile_functions.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ def __init__(
8686
params: ParamsType,
8787
interval: int,
8888
top_events: list[dict[str, Any]],
89+
snuba_params: SnubaParams | None = None,
8990
other: bool = False,
9091
query: str | None = None,
9192
selected_columns: list[str] | None = None,
@@ -100,6 +101,7 @@ def __init__(
100101
super().__init__(
101102
dataset,
102103
params,
104+
snuba_params=snuba_params,
103105
interval=interval,
104106
query=query,
105107
selected_columns=list(set(selected_columns + timeseries_functions)),

src/sentry/search/events/types.py

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
from collections import namedtuple
44
from collections.abc import Mapping, Sequence
55
from copy import deepcopy
6-
from dataclasses import dataclass
6+
from dataclasses import dataclass, field
77
from datetime import datetime, timezone
88
from typing import Any, NotRequired, Optional, TypedDict, Union
99

10+
from django.utils import timezone as django_timezone
1011
from snuba_sdk.aliased_expression import AliasedExpression
1112
from snuba_sdk.column import Column
1213
from snuba_sdk.conditions import BooleanCondition, Condition
@@ -74,24 +75,36 @@ class EventsResponse(TypedDict):
7475

7576
@dataclass
7677
class SnubaParams:
77-
start: datetime | None
78-
end: datetime | None
78+
start: datetime | None = None
79+
end: datetime | None = None
80+
stats_period: str | None = None
7981
# The None value in this sequence is because the filter params could include that
80-
environments: Sequence[Environment | None]
81-
projects: Sequence[Project]
82-
user: RpcUser | None
83-
teams: Sequence[Team]
84-
organization: Organization | None
82+
environments: Sequence[Environment | None] = field(default_factory=list)
83+
projects: Sequence[Project] = field(default_factory=list)
84+
user: RpcUser | None = None
85+
teams: Sequence[Team] = field(default_factory=list)
86+
organization: Organization | None = None
8587

8688
def __post_init__(self) -> None:
8789
if self.start:
8890
self.start = self.start.replace(tzinfo=timezone.utc)
8991
if self.end:
9092
self.end = self.end.replace(tzinfo=timezone.utc)
93+
if self.start is None and self.end is None:
94+
self.parse_stats_period()
95+
if self.organization is None and len(self.projects) > 0:
96+
self.organization = self.projects[0].organization
9197

9298
# Only used in the trend query builder
9399
self.aliases: dict[str, Alias] | None = {}
94100

101+
def parse_stats_period(self) -> None:
102+
if self.stats_period is not None:
103+
self.end = django_timezone.now()
104+
from sentry.api.utils import get_datetime_from_stats_period
105+
106+
self.start = get_datetime_from_stats_period(self.stats_period, self.end)
107+
95108
@property
96109
def environment_names(self) -> Sequence[str]:
97110
return (
@@ -122,6 +135,33 @@ def interval(self) -> float | None:
122135
return (self.end - self.start).total_seconds()
123136
return None
124137

138+
@property
139+
def organization_id(self) -> int | None:
140+
if self.organization is not None:
141+
return self.organization.id
142+
return None
143+
144+
@property
145+
def filter_params(self) -> ParamsType:
146+
# Compatibility function so we can switch over to this dataclass more easily
147+
filter_params: ParamsType = {
148+
"project_id": list(self.project_ids),
149+
"projects": list(self.projects),
150+
"project_objects": list(self.projects),
151+
"environment": list(self.environment_names),
152+
"team_id": list(self.team_ids),
153+
"environment_objects": [env for env in self.environments if env is not None],
154+
}
155+
if self.organization_id:
156+
filter_params["organization_id"] = self.organization_id
157+
if self.start:
158+
filter_params["start"] = self.start
159+
if self.end:
160+
filter_params["end"] = self.end
161+
if self.stats_period:
162+
filter_params["statsPeriod"] = self.stats_period
163+
return filter_params
164+
125165
def copy(self) -> SnubaParams:
126166
return deepcopy(self)
127167

src/sentry/snuba/functions.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ def top_events_timeseries(
133133
rollup,
134134
limit,
135135
organization,
136+
snuba_params=None,
136137
equations=None,
137138
referrer=None,
138139
top_events=None,
@@ -152,6 +153,7 @@ def top_events_timeseries(
152153
selected_columns,
153154
query=user_query,
154155
params=params,
156+
snuba_params=snuba_params,
155157
equations=equations,
156158
orderby=orderby,
157159
limit=limit,
@@ -163,6 +165,7 @@ def top_events_timeseries(
163165
top_functions_builder = ProfileTopFunctionsTimeseriesQueryBuilder(
164166
dataset=Dataset.Functions,
165167
params=params,
168+
snuba_params=snuba_params,
166169
interval=rollup,
167170
top_events=top_events["data"],
168171
other=False,
@@ -186,6 +189,7 @@ def top_events_timeseries(
186189
top_functions_builder,
187190
params,
188191
rollup,
192+
snuba_params=snuba_params,
189193
top_events=top_events,
190194
allow_empty=allow_empty,
191195
zerofill_results=zerofill_results,
@@ -198,13 +202,17 @@ def format_top_events_timeseries_results(
198202
query_builder,
199203
params,
200204
rollup,
205+
snuba_params=None,
201206
top_events=None,
202207
allow_empty=True,
203208
zerofill_results=True,
204209
result_key_order=None,
205210
):
206211
if top_events is None:
207212
assert top_events, "Need to provide top events" # TODO: support this use case
213+
if snuba_params is not None and len(params) == 0:
214+
# Compatibility so its easier to convert to SnubaParams
215+
params = snuba_params.filter_params
208216

209217
if not allow_empty and not len(result.get("data", [])):
210218
return SnubaTSResult(

src/sentry/tasks/check_am2_compatibility.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from sentry.models.organization import Organization
1919
from sentry.models.project import Project
2020
from sentry.search.events.builder.metrics import MetricsQueryBuilder
21-
from sentry.search.events.types import ParamsType, QueryBuilderConfig
21+
from sentry.search.events.types import QueryBuilderConfig, SnubaParams
2222
from sentry.silo.base import SiloMode
2323
from sentry.snuba.dataset import Dataset
2424
from sentry.snuba.discover import query as discover_query
@@ -468,21 +468,22 @@ def get_outdated_sdks(cls, found_sdks_per_project):
468468
return outdated_sdks_per_project
469469

470470
@classmethod
471-
def get_sdks_version_used(cls, organization_id, project_objects):
471+
def get_sdks_version_used(cls, organization, project_objects):
472472
# We use the count() operation in order to group by project, sdk.name and sdk.version.
473473
selected_columns = ["count()", "project", "sdk.name", "sdk.version"]
474-
params: ParamsType = {
475-
"organization_id": organization_id,
476-
"project_objects": project_objects,
477-
"start": datetime.now(tz=timezone.utc) - timedelta(days=QUERY_TIME_RANGE_IN_DAYS),
478-
"end": datetime.now(tz=timezone.utc),
479-
}
474+
params = SnubaParams(
475+
start=datetime.now(tz=timezone.utc) - timedelta(days=QUERY_TIME_RANGE_IN_DAYS),
476+
end=datetime.now(tz=timezone.utc),
477+
organization=organization,
478+
projects=project_objects,
479+
)
480480

481481
try:
482482
results = discover_query(
483483
selected_columns=selected_columns,
484484
query="event.type:transaction",
485-
params=params,
485+
params={},
486+
snuba_params=params,
486487
referrer="api.organization-events",
487488
)
488489

@@ -637,7 +638,7 @@ def run_compatibility_check(cls, org_id):
637638
# We mark whether a metric is not supported.
638639
unsupported_alerts.append((alert_id, aggregate, query))
639640

640-
outdated_sdks_per_project = cls.get_sdks_version_used(organization.id, all_projects)
641+
outdated_sdks_per_project = cls.get_sdks_version_used(organization, all_projects)
641642
if outdated_sdks_per_project is None:
642643
with sentry_sdk.isolation_scope() as scope:
643644
scope.set_tag("org_id", organization.id)

src/sentry/tasks/on_demand_metrics.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from django.utils import timezone
99

1010
from sentry import options
11-
from sentry.api.utils import get_date_range_from_params
1211
from sentry.models.dashboard_widget import (
1312
DashboardWidgetQuery,
1413
DashboardWidgetQueryOnDemand,
@@ -24,7 +23,7 @@
2423
)
2524
from sentry.search.events import fields
2625
from sentry.search.events.builder.discover import DiscoverQueryBuilder
27-
from sentry.search.events.types import EventsResponse, ParamsType, QueryBuilderConfig
26+
from sentry.search.events.types import EventsResponse, QueryBuilderConfig, SnubaParams
2827
from sentry.snuba.dataset import Dataset
2928
from sentry.snuba.metrics.extraction import OnDemandMetricSpecVersioning
3029
from sentry.snuba.referrer import Referrer
@@ -467,21 +466,19 @@ def _query_cardinality(
467466
# Restrict period down to an allowlist so we're not slamming snuba with giant queries
468467
if period not in [TASK_QUERY_PERIOD, DASHBOARD_QUERY_PERIOD]:
469468
raise Exception("Cardinality can only be queried with 1h or 30m")
470-
params: ParamsType = {
471-
"statsPeriod": period,
472-
"organization_id": organization.id,
473-
"project_objects": list(Project.objects.filter(organization=organization)),
474-
}
475-
start, end = get_date_range_from_params(params)
476-
params["start"] = start
477-
params["end"] = end
469+
params = SnubaParams(
470+
stats_period=period,
471+
organization=organization,
472+
projects=list(Project.objects.filter(organization=organization)),
473+
)
478474

479475
columns_to_check = [column for column in query_columns if not fields.is_function(column)]
480476
unique_columns = [f"count_unique({column})" for column in columns_to_check]
481477

482478
query_builder = DiscoverQueryBuilder(
483479
dataset=Dataset.Discover,
484-
params=params,
480+
params={},
481+
snuba_params=params,
485482
selected_columns=unique_columns,
486483
config=QueryBuilderConfig(
487484
transform_alias_to_input_format=True,

src/sentry/tasks/statistical_detectors.py

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
from sentry.models.project import Project
3535
from sentry.models.statistical_detectors import RegressionType
3636
from sentry.profiles.utils import get_from_profiling_service
37-
from sentry.search.events.types import ParamsType
37+
from sentry.search.events.types import SnubaParams
3838
from sentry.seer.breakpoints import BreakpointData
3939
from sentry.sentry_metrics import indexer
4040
from sentry.sentry_metrics.use_case_id_registry import UseCaseID
@@ -503,12 +503,11 @@ def emit_function_regression_issue(
503503
project_ids = [int(regression["project"]) for regression in regressions]
504504
projects = [projects_by_id[project_id] for project_id in project_ids]
505505

506-
params: ParamsType = {
507-
"start": start,
508-
"end": start + timedelta(minutes=1),
509-
"project_id": project_ids,
510-
"project_objects": projects,
511-
}
506+
params = SnubaParams(
507+
start=start,
508+
end=start + timedelta(minutes=1),
509+
projects=projects,
510+
)
512511

513512
conditions = [
514513
And(
@@ -523,7 +522,8 @@ def emit_function_regression_issue(
523522
result = functions.query(
524523
selected_columns=["project.id", "fingerprint", "examples()"],
525524
query="is_application:1",
526-
params=params,
525+
params={},
526+
snuba_params=params,
527527
orderby=["project.id"],
528528
limit=len(regressions),
529529
referrer=Referrer.API_PROFILING_FUNCTIONS_STATISTICAL_DETECTOR_EXAMPLE.value,
@@ -878,12 +878,11 @@ def query_functions(projects: list[Project], start: datetime) -> list[DetectorPa
878878
# we just need to query for the 1 minute of data.
879879
start = start - timedelta(hours=1)
880880
start = start.replace(minute=0, second=0, microsecond=0)
881-
params: ParamsType = {
882-
"start": start,
883-
"end": start + timedelta(minutes=1),
884-
"project_id": [project.id for project in projects],
885-
"project_objects": projects,
886-
}
881+
params = SnubaParams(
882+
start=start,
883+
end=start + timedelta(minutes=1),
884+
projects=projects,
885+
)
887886

888887
# TODOs: handle any errors
889888
query_results = functions.query(
@@ -895,7 +894,8 @@ def query_functions(projects: list[Project], start: datetime) -> list[DetectorPa
895894
"p95()",
896895
],
897896
query="is_application:1",
898-
params=params,
897+
params={},
898+
snuba_params=params,
899899
orderby=["project.id", "-count()"],
900900
limitby=("project.id", FUNCTIONS_PER_PROJECT),
901901
limit=FUNCTIONS_PER_PROJECT * len(projects),
@@ -924,16 +924,14 @@ def query_functions_timeseries(
924924
agg_function: str,
925925
) -> Generator[tuple[int, int | str, SnubaTSResult], None, None]:
926926
projects = [project for project, _ in functions_list]
927-
project_ids = [project.id for project in projects]
928927

929928
# take the last 14 days as our window
930929
end = start.replace(minute=0, second=0, microsecond=0) + timedelta(hours=1)
931-
params: ParamsType = {
932-
"start": end - timedelta(days=14),
933-
"end": end,
934-
"project_id": project_ids,
935-
"project_objects": projects,
936-
}
930+
params = SnubaParams(
931+
start=end - timedelta(days=14),
932+
end=end,
933+
projects=projects,
934+
)
937935
interval = 3600 # 1 hour
938936

939937
chunk: list[dict[str, Any]] = [
@@ -948,7 +946,8 @@ def query_functions_timeseries(
948946
timeseries_columns=[agg_function],
949947
selected_columns=["project.id", "fingerprint"],
950948
user_query="is_application:1",
951-
params=params,
949+
params={},
950+
snuba_params=params,
952951
orderby=None, # unused because top events is specified
953952
rollup=interval,
954953
limit=len(chunk),

0 commit comments

Comments
 (0)