Skip to content

Commit d41082e

Browse files
authored
feat(snql): Enable snql on facets performance endpoints (#30557)
* feat(snql): Enable snql on facets performance endpoints - Histogram enabled in the histogram specific PR - Had to remove an orderby, but it wasn't actually doing anything since we dropped orderbys when there are only aggregates in the selected columns * fix: Need to allow additional conditions on discover.query - This is cause there's a hardcoded condition on tags_key, and we don't want to allow that to be used at will by customers in our search syntax, otherwise this could just be appended to our query string
1 parent 7d646a8 commit d41082e

File tree

3 files changed

+34
-4
lines changed

3 files changed

+34
-4
lines changed

src/sentry/api/endpoints/organization_events_facets_performance.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,18 @@ def get(self, request, organization):
7373
if tag_key in TAG_ALIASES:
7474
tag_key = TAG_ALIASES.get(tag_key)
7575

76+
use_snql = features.has(
77+
"organizations:performance-use-snql", organization, actor=request.user
78+
)
79+
7680
def data_fn(offset, limit):
7781
with sentry_sdk.start_span(op="discover.endpoint", description="discover_query"):
7882
referrer = "api.organization-events-facets-performance.top-tags"
7983
tag_data = query_tag_data(
8084
filter_query=filter_query,
8185
aggregate_column=aggregate_column,
8286
referrer=referrer,
87+
use_snql=use_snql,
8388
params=params,
8489
)
8590

@@ -156,6 +161,10 @@ def get(self, request, organization):
156161
if tag_key in TAG_ALIASES:
157162
tag_key = TAG_ALIASES.get(tag_key)
158163

164+
use_snql = features.has(
165+
"organizations:performance-use-snql", organization, actor=request.user
166+
)
167+
159168
def data_fn(offset, limit, raw_limit):
160169
with sentry_sdk.start_span(op="discover.endpoint", description="discover_query"):
161170
referrer = "api.organization-events-facets-performance-histogram"
@@ -168,6 +177,7 @@ def data_fn(offset, limit, raw_limit):
168177
orderby=self.get_orderby(request),
169178
offset=offset,
170179
referrer=referrer,
180+
use_snql=use_snql,
171181
)
172182

173183
if not top_tags:
@@ -182,9 +192,7 @@ def data_fn(offset, limit, raw_limit):
182192
filter_query=filter_query,
183193
aggregate_column=aggregate_column,
184194
referrer=referrer,
185-
use_snql=features.has(
186-
"organizations:performance-use-snql", organization, actor=request.user
187-
),
195+
use_snql=use_snql,
188196
params=params,
189197
limit=raw_limit,
190198
num_buckets_per_key=num_buckets_per_key,
@@ -243,6 +251,7 @@ def get_result(self, limit, cursor=None):
243251
def query_tag_data(
244252
params: Mapping[str, str],
245253
referrer: str,
254+
use_snql: bool,
246255
filter_query: Optional[str] = None,
247256
aggregate_column: Optional[str] = None,
248257
) -> Optional[Dict]:
@@ -276,8 +285,8 @@ def query_tag_data(
276285
],
277286
query=filter_query,
278287
params=params,
279-
orderby=["-count", "tags_value"],
280288
referrer=f"{referrer}.all_transactions",
289+
use_snql=use_snql,
281290
limit=1,
282291
)
283292

@@ -300,6 +309,7 @@ def query_top_tags(
300309
tag_key: str,
301310
limit: int,
302311
referrer: str,
312+
use_snql: bool,
303313
orderby: Optional[List[str]],
304314
offset: Optional[int] = None,
305315
aggregate_column: Optional[str] = None,
@@ -348,8 +358,13 @@ def query_top_tags(
348358
[translated_aggregate_column, "IS NOT NULL", None],
349359
["tags_key", "IN", [tag_key]],
350360
],
361+
extra_snql_condition=[
362+
Condition(Column(translated_aggregate_column), Op.IS_NOT_NULL),
363+
Condition(Column("tags_key"), Op.EQ, tag_key),
364+
],
351365
functions_acl=["array_join"],
352366
referrer=f"{referrer}.top_tags",
367+
use_snql=use_snql,
353368
limit=limit,
354369
offset=offset,
355370
)

src/sentry/snuba/discover.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ def query(
212212
auto_aggregations=False,
213213
use_aggregate_conditions=False,
214214
conditions=None,
215+
extra_snql_condition=None,
215216
functions_acl=None,
216217
use_snql=False,
217218
):
@@ -239,6 +240,7 @@ def query(
239240
use_aggregate_conditions (bool) Set to true if aggregates conditions should be used at all.
240241
conditions (Sequence[any]) List of conditions that are passed directly to snuba without
241242
any additional processing.
243+
extra_snql_condition (Sequence[Condition]) Replacement for conditions while migrating to snql
242244
use_snql (bool) Whether to directly build the query in snql, instead of using the older
243245
json construction
244246
"""
@@ -263,6 +265,8 @@ def query(
263265
limit=limit,
264266
offset=offset,
265267
)
268+
if extra_snql_condition is not None:
269+
builder.add_conditions(extra_snql_condition)
266270
snql_query = builder.get_snql_query()
267271

268272
result = raw_snql_query(snql_query, referrer)

tests/snuba/api/endpoints/test_organization_events_facets_performance.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,14 @@ def test_aggregate_zero(self):
263263
assert data[0]["comparison"] == 0
264264
assert data[0]["tags_key"] == "color"
265265
assert data[0]["tags_value"] == "purple"
266+
267+
268+
class OrganizationEventsFacetsPerformanceEndpointTestWithSnql(
269+
OrganizationEventsFacetsPerformanceEndpointTest
270+
):
271+
feature_list = (
272+
"organizations:discover-basic",
273+
"organizations:global-views",
274+
"organizations:performance-view",
275+
"organizations:performance-use-snql",
276+
)

0 commit comments

Comments
 (0)