Skip to content

Commit 40c7acc

Browse files
authored
ref(eap-spans): add granularity to SnubaParams (#88081)
1. Add `granularity_secs` to `SnubaParams` and replace `rollup` parameter in several functions. 2. Add `timeseries_granularity_secs` to `SnubaParams` which is used to prevent checking `granularity_secs is not None` everywhere 3. Add `is_timeseries_request` to `SnubaParams` which is used within the `epm` function to either divide by the overall query interval or the granularity Note: There is still a **ton** of usage for `rollup`, so we still have `get_rollup` in `src/sentry/api/bases/organization_events.py`. But in an effort to save time I tried to make the changes exclusively to the rpc related files.
1 parent e8b0503 commit 40c7acc

File tree

11 files changed

+51
-58
lines changed

11 files changed

+51
-58
lines changed

src/sentry/api/bases/organization_events.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,7 @@ def handle_readable_device(
427427
def get_rollup(
428428
self, request: Request, snuba_params: SnubaParams, top_events: int, use_rpc: bool
429429
) -> int:
430+
"""TODO: we should eventually rely on `SnubaParams.granularity_secs` instead"""
430431
try:
431432
rollup = get_rollup_from_request(
432433
request,
@@ -497,6 +498,7 @@ def get_event_stats_data(
497498
return {"data": []}
498499

499500
rollup = self.get_rollup(request, snuba_params, top_events, use_rpc)
501+
snuba_params.granularity_secs = rollup
500502
self.validate_comparison_delta(comparison_delta, snuba_params, organization)
501503

502504
query_columns = get_query_columns(columns, rollup)

src/sentry/api/endpoints/organization_events_stats.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,6 @@ def _get_event_stats(
301301
orderby=self.get_orderby(request),
302302
limit=top_events,
303303
referrer=referrer,
304-
granularity_secs=rollup,
305304
config=SearchResolverConfig(
306305
auto_fields=False,
307306
use_aggregate_conditions=True,
@@ -337,7 +336,6 @@ def _get_event_stats(
337336
params=snuba_params,
338337
query_string=query,
339338
y_axes=query_columns,
340-
granularity_secs=rollup,
341339
referrer=referrer,
342340
config=SearchResolverConfig(
343341
auto_fields=False,

src/sentry/api/endpoints/organization_events_timeseries.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ def get(self, request: Request, organization: Organization) -> Response:
176176

177177
self.validate_comparison_delta(comparison_delta, snuba_params, organization)
178178
rollup = self.get_rollup(request, snuba_params, top_events, use_rpc)
179+
snuba_params.granularity_secs = rollup
179180
axes = request.GET.getlist("yAxis", ["count()"])
180181

181182
with handle_query_errors():
@@ -241,7 +242,6 @@ def get_event_stats(
241242
orderby=self.get_orderby(request),
242243
limit=top_events,
243244
referrer=referrer,
244-
granularity_secs=rollup,
245245
config=SearchResolverConfig(
246246
auto_fields=False,
247247
use_aggregate_conditions=True,
@@ -275,7 +275,6 @@ def get_event_stats(
275275
params=snuba_params,
276276
query_string=query,
277277
y_axes=query_columns,
278-
granularity_secs=rollup,
279278
referrer=referrer,
280279
config=SearchResolverConfig(
281280
auto_fields=False,

src/sentry/discover/compare_timeseries.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class TSResultForComparison(TypedDict):
3333
def make_rpc_request(
3434
query: str,
3535
aggregate: str,
36-
time_window: int,
3736
snuba_params: SnubaParams,
3837
) -> TSResultForComparison:
3938
query = apply_dataset_query_conditions(SnubaQuery.Type.PERFORMANCE, query, None)
@@ -46,7 +45,6 @@ def make_rpc_request(
4645
query_string=query_parts["query"],
4746
y_axes=query_parts["selected_columns"],
4847
referrer=Referrer.JOB_COMPARE_TIMESERIES.value,
49-
granularity_secs=time_window,
5048
config=SearchResolverConfig(),
5149
)
5250

@@ -181,12 +179,12 @@ def compare_timeseries_for_alert_rule(alert_rule: AlertRule):
181179
organization=organization,
182180
start=now - timedelta(days=1),
183181
end=now,
182+
granularity_secs=snuba_query.time_window,
184183
)
185184

186185
rpc_result = make_rpc_request(
187186
snuba_query.query,
188187
snuba_query.aggregate,
189-
time_window=snuba_query.time_window,
190188
snuba_params=snuba_params,
191189
)
192190

src/sentry/search/eap/columns.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,9 @@
2626
ResolvedArguments: TypeAlias = list[ResolvedArgument]
2727

2828

29-
class QuerySettings(TypedDict):
30-
snuba_params: SnubaParams
31-
granularity_secs: int | None
32-
33-
3429
class ResolverSettings(TypedDict):
3530
extrapolation_mode: ExtrapolationMode.ValueType
36-
query_settings: QuerySettings
31+
snuba_params: SnubaParams
3732

3833

3934
@dataclass(frozen=True, kw_only=True)
@@ -250,7 +245,7 @@ def resolve(
250245
alias: str,
251246
search_type: constants.SearchType,
252247
resolved_arguments: ResolvedArguments,
253-
query_settings: QuerySettings,
248+
snuba_params: SnubaParams,
254249
) -> ResolvedFormula | ResolvedAggregate | ResolvedConditionalAggregate:
255250
raise NotImplementedError()
256251

@@ -269,7 +264,7 @@ def resolve(
269264
alias: str,
270265
search_type: constants.SearchType,
271266
resolved_arguments: ResolvedArguments,
272-
query_settings: QuerySettings,
267+
snuba_params: SnubaParams,
273268
) -> ResolvedAggregate:
274269
if len(resolved_arguments) > 1:
275270
raise InvalidSearchQuery(
@@ -315,7 +310,7 @@ def resolve(
315310
alias: str,
316311
search_type: constants.SearchType,
317312
resolved_arguments: ResolvedArguments,
318-
query_settings: QuerySettings,
313+
snuba_params: SnubaParams,
319314
) -> ResolvedConditionalAggregate:
320315
key, filter = self.aggregate_resolver(resolved_arguments)
321316
return ResolvedConditionalAggregate(
@@ -345,15 +340,15 @@ def resolve(
345340
alias: str,
346341
search_type: constants.SearchType,
347342
resolved_arguments: list[AttributeKey | Any],
348-
query_settings: QuerySettings,
343+
snuba_params: SnubaParams,
349344
) -> ResolvedFormula:
350345
resolver_settings = ResolverSettings(
351346
extrapolation_mode=(
352347
ExtrapolationMode.EXTRAPOLATION_MODE_SAMPLE_WEIGHTED
353348
if self.extrapolation
354349
else ExtrapolationMode.EXTRAPOLATION_MODE_NONE
355350
),
356-
query_settings=query_settings,
351+
snuba_params=snuba_params,
357352
)
358353

359354
return ResolvedFormula(

src/sentry/search/eap/resolver.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
ColumnDefinitions,
4141
ConditionalAggregateDefinition,
4242
FormulaDefinition,
43-
QuerySettings,
4443
ResolvedAggregate,
4544
ResolvedAttribute,
4645
ResolvedConditionalAggregate,
@@ -797,13 +796,10 @@ def resolve_function(self, column: str, match: Match | None = None) -> tuple[
797796
)
798797

799798
resolved_function = function_definition.resolve(
800-
alias,
801-
search_type,
802-
resolved_arguments,
803-
QuerySettings(
804-
snuba_params=self.params,
805-
granularity_secs=self.granularity_secs,
806-
),
799+
alias=alias,
800+
search_type=search_type,
801+
resolved_arguments=resolved_arguments,
802+
snuba_params=self.params,
807803
)
808804

809805
resolved_context = None

src/sentry/search/eap/spans/formulas.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,11 +392,13 @@ def time_spent_percentage(
392392

393393
def epm(_: ResolvedArguments, settings: ResolverSettings) -> Column.BinaryFormula:
394394
extrapolation_mode = settings["extrapolation_mode"]
395-
interval = settings["query_settings"]["snuba_params"].interval
396-
granularity_secs = settings["query_settings"]["granularity_secs"]
395+
is_timeseries_request = settings["snuba_params"].is_timeseries_request
397396

398-
# having a granularity_secs implies that the request is for a time series, and each datapoint should be divided by it
399-
divisor = granularity_secs if granularity_secs else interval
397+
divisor = (
398+
settings["snuba_params"].timeseries_granularity_secs
399+
if is_timeseries_request
400+
else settings["snuba_params"].interval
401+
)
400402

401403
return Column.BinaryFormula(
402404
left=Column(

src/sentry/search/events/types.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ class SnubaParams:
8585
start: datetime | None = None
8686
end: datetime | None = None
8787
stats_period: str | None = None
88+
# granularity is used with timeseries requests to specifiy bucket size
89+
granularity_secs: int | None = None
8890
# The None value in this sequence is because the filter params could include that
8991
environments: Sequence[Environment | None] = field(default_factory=list)
9092
projects: Sequence[Project] = field(default_factory=list)
@@ -140,6 +142,16 @@ def rpc_end_date(self) -> Timestamp:
140142
timestamp.FromDatetime(self.end_date)
141143
return timestamp
142144

145+
@property
146+
def timeseries_granularity_secs(self) -> int:
147+
if self.granularity_secs is None:
148+
raise InvalidSearchQuery("granularity is required")
149+
return self.granularity_secs
150+
151+
@property
152+
def is_timeseries_request(self) -> bool:
153+
return self.granularity_secs is not None
154+
143155
@property
144156
def date_range(self) -> timedelta:
145157
return self.end_date - self.start_date

src/sentry/seer/anomaly_detection/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ def fetch_historical_data(
269269
end=end,
270270
stats_period=None,
271271
environments=environments,
272+
granularity_secs=granularity,
272273
)
273274

274275
if dataset == metrics_performance:
@@ -278,7 +279,6 @@ def fetch_historical_data(
278279
params=snuba_params,
279280
query_string=snuba_query.query,
280281
y_axes=query_columns,
281-
granularity_secs=granularity,
282282
referrer=(
283283
Referrer.ANOMALY_DETECTION_HISTORICAL_DATA_QUERY.value
284284
if is_store_data_request

src/sentry/snuba/entity_subscription.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ def build_rpc_request(
271271
organization=Organization.objects.get_from_cache(id=self.org_id),
272272
start=now - timedelta(days=1),
273273
end=now,
274+
granularity_secs=self.time_window,
274275
)
275276

276277
rpc_request, _, _ = get_timeseries_query(
@@ -280,7 +281,6 @@ def build_rpc_request(
280281
groupby=[],
281282
referrer=referrer,
282283
config=SearchResolverConfig(),
283-
granularity_secs=self.time_window,
284284
)
285285

286286
return rpc_request

0 commit comments

Comments
 (0)