Skip to content

Commit 96b2a13

Browse files
authored
feat(get_traces): use cross item query path for single item query (#7656)
## Summary Unifies single-item and cross-item query execution paths by using SQL subqueries instead of fetching trace IDs in separate queries. This simplifies the codebase and improves performance by letting ClickHouse optimize the subquery internally. ## Changes - **Removed** `get_trace_ids_for_cross_item_query()` function that fetched trace IDs in a separate query - **Enhanced** `get_trace_ids_sql_for_cross_item_query()` to return both SQL string and query result for metadata extraction - **Refactored** `EndpointGetTraces` to use subquery approach via `_execute_with_subquery_optimization()` - **Updated** time series and trace item table resolvers to inject SQL subqueries using `DangerousRawSQL` - **Added** feature flag `use_cross_item_path_for_single_item_queries` to enable unified path for all queries (not just cross-item) - **Added** test class that runs all existing tests with feature flag enabled to verify parity ## Benefits - **Simplified codebase**: Net -101 lines (221 deletions, 120 additions) - **Unified execution path**: Single code path for both query types - **Better performance**: ClickHouse can optimize the subquery internally instead of two round-trips 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent 0a77754 commit 96b2a13

File tree

6 files changed

+119
-221
lines changed

6 files changed

+119
-221
lines changed

snuba/web/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
from __future__ import annotations
22

33
from dataclasses import dataclass
4-
from typing import Any, Mapping, TypedDict, cast
4+
from typing import Any, Dict, Mapping, TypedDict, cast
55

66
from snuba.reader import Column, Result, Row, transform_rows
77
from snuba.utils.serializable_exception import JsonSerializable, SerializableException
88

99

1010
class QueryExtraData(TypedDict):
11-
stats: Mapping[str, Any]
11+
stats: Dict[str, Any]
1212
sql: str
1313
experiments: Mapping[str, Any]
1414

snuba/web/rpc/v1/endpoint_get_traces.py

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException
5555
from snuba.web.rpc.v1.resolvers.common.cross_item_queries import (
5656
convert_trace_filters_to_trace_item_filter_with_type,
57-
get_trace_ids_for_cross_item_query,
5857
get_trace_ids_sql_for_cross_item_query,
5958
)
6059

@@ -473,11 +472,12 @@ def _execute_with_subquery_optimization(self, in_msg: GetTracesRequest) -> GetTr
473472
Execute cross-item query using subquery optimization.
474473
Gets SQL from trace IDs query and uses it as a subquery in metadata query.
475474
"""
476-
# Get SQL for trace IDs query (dry run)
477-
trace_ids_sql = get_trace_ids_sql_for_cross_item_query(
475+
# Get SQL for trace IDs query (dry run) and its query result
476+
trace_ids_sql, trace_ids_query_result = get_trace_ids_sql_for_cross_item_query(
478477
in_msg,
479478
in_msg.meta,
480479
convert_trace_filters_to_trace_item_filter_with_type(list(in_msg.filters)),
480+
self.routing_decision.tier,
481481
self._timer,
482482
)
483483

@@ -486,13 +486,12 @@ def _execute_with_subquery_optimization(self, in_msg: GetTracesRequest) -> GetTr
486486
request=in_msg,
487487
trace_ids_sql=trace_ids_sql,
488488
)
489-
490-
# Build response
489+
# Build response - include both query results for proper metadata extraction
491490
response_meta = extract_response_meta(
492491
in_msg.meta.request_id,
493492
in_msg.meta.debug,
494-
[metadata_query_result],
495-
[self._timer],
493+
[trace_ids_query_result, metadata_query_result],
494+
[self._timer, self._timer],
496495
)
497496

498497
return GetTracesResponse(
@@ -503,25 +502,18 @@ def _execute_with_subquery_optimization(self, in_msg: GetTracesRequest) -> GetTr
503502

504503
def _execute(self, in_msg: GetTracesRequest) -> GetTracesResponse:
505504
_validate_order_by(in_msg)
506-
# Feature flag: Use subquery optimization for cross-item queries
507-
if self._is_cross_event_query(in_msg.filters) and state.get_config(
508-
"enable_cross_item_subquery_optimization", True
509-
):
510-
return self._execute_with_subquery_optimization(in_msg)
505+
506+
# Feature flag: Use cross-item query path for all queries (single-item and cross-item)
507+
use_cross_item_path = self._is_cross_event_query(in_msg.filters) or state.get_config(
508+
"use_cross_item_path_for_single_item_queries", False
509+
)
511510

512511
# Original code path (unchanged)
513512
query_results: list[Any] = []
514513

515514
# Get a dict of trace IDs and timestamps.
516-
if self._is_cross_event_query(in_msg.filters):
517-
trace_ids, trace_ids_query_results = get_trace_ids_for_cross_item_query(
518-
in_msg,
519-
in_msg.meta,
520-
convert_trace_filters_to_trace_item_filter_with_type(list(in_msg.filters)),
521-
self._timer,
522-
return_query_results=True,
523-
)
524-
query_results.extend(trace_ids_query_results)
515+
if use_cross_item_path:
516+
return self._execute_with_subquery_optimization(in_msg)
525517
else:
526518
trace_ids, trace_ids_query_result = self._get_trace_ids_for_single_item_query(
527519
request=in_msg

snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@
2727
from snuba.datasets.entities.entity_key import EntityKey
2828
from snuba.datasets.entities.factory import get_entity
2929
from snuba.datasets.pluggable_dataset import PluggableDataset
30+
from snuba.downsampled_storage_tiers import Tier
3031
from snuba.query import OrderBy, OrderByDirection, SelectedExpression
3132
from snuba.query.data_source.simple import Entity
3233
from snuba.query.dsl import Functions as f
33-
from snuba.query.dsl import column, in_cond, literal, literals_array
34-
from snuba.query.expressions import Expression
34+
from snuba.query.dsl import column, in_cond, literal
35+
from snuba.query.expressions import DangerousRawSQL, Expression
3536
from snuba.query.logical import Query
3637
from snuba.query.query_settings import HTTPQuerySettings
3738
from snuba.request import Request as SnubaRequest
@@ -62,7 +63,7 @@
6263
get_count_column,
6364
)
6465
from snuba.web.rpc.v1.resolvers.common.cross_item_queries import (
65-
get_trace_ids_for_cross_item_query,
66+
get_trace_ids_sql_for_cross_item_query,
6667
)
6768
from snuba.web.rpc.v1.resolvers.common.formula_reliability import (
6869
FormulaReliabilityCalculator,
@@ -317,7 +318,9 @@ def _proto_expression_to_ast_expression(
317318
raise ValueError(f"Unknown expression type: {default}")
318319

319320

320-
def build_query(request: TimeSeriesRequest, timer: Optional[Timer] = None) -> Query:
321+
def build_query(
322+
request: TimeSeriesRequest, sampling_tier: Optional[Tier] = None, timer: Optional[Timer] = None
323+
) -> Query:
321324
entity = Entity(
322325
key=EntityKey("eap_items"),
323326
schema=get_entity(EntityKey("eap_items")).get_data_model(),
@@ -347,14 +350,14 @@ def build_query(request: TimeSeriesRequest, timer: Optional[Timer] = None) -> Qu
347350

348351
# Handle cross item queries by first getting trace IDs
349352
additional_conditions = []
350-
if request.trace_filters and timer is not None:
351-
trace_ids = get_trace_ids_for_cross_item_query(
352-
request, request.meta, list(request.trace_filters), timer
353+
if request.trace_filters and timer is not None and sampling_tier is not None:
354+
trace_ids_sql, _ = get_trace_ids_sql_for_cross_item_query(
355+
request, request.meta, list(request.trace_filters), sampling_tier, timer
353356
)
354357
additional_conditions.append(
355358
in_cond(
356359
column("trace_id"),
357-
literals_array(None, [literal(trace_id) for trace_id in trace_ids]),
360+
DangerousRawSQL(None, f"({trace_ids_sql})"),
358361
)
359362
)
360363

@@ -416,7 +419,10 @@ def build_query(request: TimeSeriesRequest, timer: Optional[Timer] = None) -> Qu
416419

417420

418421
def _build_snuba_request(
419-
request: TimeSeriesRequest, query_settings: HTTPQuerySettings, timer: Optional[Timer] = None
422+
request: TimeSeriesRequest,
423+
query_settings: HTTPQuerySettings,
424+
sampling_tier: Optional[Tier] = None,
425+
timer: Optional[Timer] = None,
420426
) -> SnubaRequest:
421427
if request.meta.trace_item_type == TraceItemType.TRACE_ITEM_TYPE_LOG:
422428
team = "ourlogs"
@@ -430,7 +436,7 @@ def _build_snuba_request(
430436
return SnubaRequest(
431437
id=uuid.UUID(request.meta.request_id),
432438
original_body=MessageToDict(request),
433-
query=build_query(request, timer),
439+
query=build_query(request, sampling_tier, timer),
434440
query_settings=query_settings,
435441
attribution_info=AttributionInfo(
436442
referrer=request.meta.referrer,
@@ -467,7 +473,9 @@ def resolve(
467473
except Exception as e:
468474
sentry_sdk.capture_message(f"Error merging clickhouse settings: {e}")
469475

470-
snuba_request = _build_snuba_request(in_msg, query_settings, self._timer)
476+
snuba_request = _build_snuba_request(
477+
in_msg, query_settings, routing_decision.tier, self._timer
478+
)
471479
res = run_query(
472480
dataset=PluggableDataset(name="eap", all_entities=[]),
473481
request=snuba_request,

snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
from snuba.datasets.entities.entity_key import EntityKey
3030
from snuba.datasets.entities.factory import get_entity
3131
from snuba.datasets.pluggable_dataset import PluggableDataset
32+
from snuba.downsampled_storage_tiers import Tier
3233
from snuba.protos.common import NORMALIZED_COLUMNS_EAP_ITEMS
3334
from snuba.query import OrderBy, OrderByDirection, SelectedExpression
3435
from snuba.query.data_source.simple import Entity
3536
from snuba.query.dsl import Functions as f
3637
from snuba.query.dsl import and_cond, in_cond, literal, literals_array, or_cond
3738
from snuba.query.dsl import column as snuba_column
38-
from snuba.query.expressions import Expression, SubscriptableReference
39+
from snuba.query.expressions import DangerousRawSQL, Expression, SubscriptableReference
3940
from snuba.query.logical import Query
4041
from snuba.query.query_settings import HTTPQuerySettings
4142
from snuba.request import Request as SnubaRequest
@@ -67,7 +68,7 @@
6768
get_count_column,
6869
)
6970
from snuba.web.rpc.v1.resolvers.common.cross_item_queries import (
70-
get_trace_ids_for_cross_item_query,
71+
get_trace_ids_sql_for_cross_item_query,
7172
)
7273
from snuba.web.rpc.v1.resolvers.common.trace_item_table import convert_results
7374

@@ -404,6 +405,7 @@ def _get_offset_from_page_token(page_token: PageToken | None) -> int:
404405
def build_query(
405406
request: TraceItemTableRequest,
406407
time_window: TimeWindow | None = None,
408+
sampling_tier: Optional[Tier] = None,
407409
timer: Optional[Timer] = None,
408410
) -> Query:
409411
entity = Entity(
@@ -429,15 +431,12 @@ def build_query(
429431

430432
# Handle cross item queries by first getting trace IDs
431433
additional_conditions: List[Expression] = []
432-
if request.trace_filters and timer is not None:
433-
trace_ids = get_trace_ids_for_cross_item_query(
434-
request, request.meta, list(request.trace_filters), timer
434+
if request.trace_filters and timer is not None and sampling_tier is not None:
435+
trace_ids_sql, _ = get_trace_ids_sql_for_cross_item_query(
436+
request, request.meta, list(request.trace_filters), sampling_tier, timer
435437
)
436438
additional_conditions.append(
437-
in_cond(
438-
snuba_column("trace_id"),
439-
literals_array(None, [literal(trace_id) for trace_id in trace_ids]),
440-
)
439+
in_cond(snuba_column("trace_id"), DangerousRawSQL(None, f"({trace_ids_sql})"))
441440
)
442441
if time_window is not None:
443442
additional_conditions.append(
@@ -528,6 +527,7 @@ def _build_snuba_request(
528527
request: TraceItemTableRequest,
529528
query_settings: HTTPQuerySettings,
530529
time_window: TimeWindow | None = None,
530+
sampling_tier: Optional[Tier] = None,
531531
timer: Optional[Timer] = None,
532532
) -> SnubaRequest:
533533
if request.meta.trace_item_type == TraceItemType.TRACE_ITEM_TYPE_LOG:
@@ -542,7 +542,7 @@ def _build_snuba_request(
542542
return SnubaRequest(
543543
id=uuid.UUID(request.meta.request_id),
544544
original_body=MessageToDict(request),
545-
query=build_query(request, time_window, timer),
545+
query=build_query(request, time_window, sampling_tier, timer),
546546
query_settings=query_settings,
547547
attribution_info=AttributionInfo(
548548
referrer=request.meta.referrer,
@@ -582,7 +582,7 @@ def resolve(
582582
start_timestamp=in_msg.meta.start_timestamp, end_timestamp=in_msg.meta.end_timestamp
583583
)
584584
snuba_request = _build_snuba_request(
585-
in_msg, query_settings, routing_decision.time_window, self._timer
585+
in_msg, query_settings, routing_decision.time_window, routing_decision.tier, self._timer
586586
)
587587
res = run_query(
588588
dataset=PluggableDataset(name="eap", all_entities=[]),

0 commit comments

Comments
 (0)