Skip to content

Commit fed6269

Browse files
Zylphrexjerryzhou196
authored andcommitted
ref(eap): Remove AnyResolved type (#104130)
This type was added as a clutch at the time but doesn't seem necessary anymore.
1 parent 621760b commit fed6269

File tree

3 files changed

+62
-51
lines changed

3 files changed

+62
-51
lines changed

src/sentry/search/eap/columns.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class ResolvedColumn:
5454
# Indicates this attribute is a secondary alias for the attribute.
5555
# It exists for compatibility or convenience reasons and should NOT be preferred.
5656
secondary_alias: bool = False
57+
is_aggregate: bool
5758

5859
def process_column(self, value: Any) -> Any:
5960
"""Given the value from results, return a processed value if a processor is defined otherwise return it"""
@@ -81,6 +82,18 @@ def proto_type(self) -> AttributeKey.Type.ValueType:
8182
else:
8283
return constants.TYPE_MAP[self.search_type]
8384

85+
@property
86+
def proto_definition(
87+
self,
88+
) -> (
89+
LiteralValue
90+
| AttributeKey
91+
| AttributeAggregation
92+
| AttributeConditionalAggregation
93+
| Column.BinaryFormula
94+
):
95+
raise NotImplementedError
96+
8497

8598
@dataclass(frozen=True, kw_only=True)
8699
class ResolvedLiteral(ResolvedColumn):
@@ -575,17 +588,6 @@ def project_term_resolver(
575588
return int(raw_value)
576589

577590

578-
# Any of the resolved attributes, mostly to clean typing up so there's not this giant list all over the code
579-
AnyResolved = (
580-
ResolvedAttribute
581-
| ResolvedAggregate
582-
| ResolvedConditionalAggregate
583-
| ResolvedFormula
584-
| ResolvedEquation
585-
| ResolvedLiteral
586-
)
587-
588-
589591
@dataclass(frozen=True)
590592
class ColumnDefinitions:
591593
aggregates: dict[str, AggregateDefinition]

src/sentry/search/eap/resolver.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@
4242
from sentry.search.eap import constants
4343
from sentry.search.eap.columns import (
4444
AggregateDefinition,
45-
AnyResolved,
4645
AttributeArgumentDefinition,
4746
ColumnDefinitions,
4847
ConditionalAggregateDefinition,
4948
FormulaDefinition,
5049
ResolvedAggregate,
5150
ResolvedAttribute,
51+
ResolvedColumn,
5252
ResolvedConditionalAggregate,
5353
ResolvedEquation,
5454
ResolvedFormula,
@@ -1081,7 +1081,7 @@ def resolve_function(
10811081
return self._resolved_function_cache[alias]
10821082

10831083
def resolve_equations(self, equations: list[str]) -> tuple[
1084-
list[AnyResolved],
1084+
list[ResolvedColumn],
10851085
list[VirtualColumnDefinition],
10861086
]:
10871087
formulas = []
@@ -1093,7 +1093,7 @@ def resolve_equations(self, equations: list[str]) -> tuple[
10931093
return formulas, contexts
10941094

10951095
def resolve_equation(self, equation: str) -> tuple[
1096-
AnyResolved,
1096+
ResolvedColumn,
10971097
list[VirtualColumnDefinition],
10981098
]:
10991099
"""Resolve an equation creating a ResolvedEquation object, we don't just return a Column.BinaryFormula since

src/sentry/snuba/rpc_dataset_common.py

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
import sentry_sdk
99
from google.protobuf.json_format import MessageToJson
10+
from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import (
11+
AttributeConditionalAggregation,
12+
)
1013
from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig
1114
from sentry_protos.snuba.v1.endpoint_time_series_pb2 import (
1215
Expression,
@@ -19,14 +22,20 @@
1922
TraceItemTableRequest,
2023
TraceItemTableResponse,
2124
)
25+
from sentry_protos.snuba.v1.formula_pb2 import Literal
2226
from sentry_protos.snuba.v1.request_common_pb2 import (
2327
PageToken,
2428
RequestMeta,
2529
ResponseMeta,
2630
TraceItemFilterWithType,
2731
TraceItemType,
2832
)
29-
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue, Function
33+
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import (
34+
AttributeAggregation,
35+
AttributeKey,
36+
AttributeValue,
37+
Function,
38+
)
3039
from sentry_protos.snuba.v1.trace_item_filter_pb2 import (
3140
AndFilter,
3241
ComparisonFilter,
@@ -37,16 +46,7 @@
3746
from sentry.api.event_search import SearchFilter, SearchKey, SearchValue
3847
from sentry.discover import arithmetic
3948
from sentry.exceptions import InvalidSearchQuery
40-
from sentry.search.eap.columns import (
41-
AnyResolved,
42-
ColumnDefinitions,
43-
ResolvedAggregate,
44-
ResolvedAttribute,
45-
ResolvedConditionalAggregate,
46-
ResolvedEquation,
47-
ResolvedFormula,
48-
ResolvedLiteral,
49-
)
49+
from sentry.search.eap.columns import ColumnDefinitions, ResolvedAttribute, ResolvedColumn
5050
from sentry.search.eap.constants import DOUBLE, MAX_ROLLUP_POINTS, VALID_GRANULARITIES
5151
from sentry.search.eap.resolver import SearchResolver
5252
from sentry.search.eap.rpc_utils import and_trace_item_filters
@@ -97,7 +97,7 @@ class TableRequest:
9797
"""Container for rpc requests"""
9898

9999
rpc_request: TraceItemTableRequest
100-
columns: list[AnyResolved]
100+
columns: list[ResolvedColumn]
101101

102102

103103
def check_timeseries_has_data(timeseries: SnubaData, y_axes: list[str]):
@@ -140,39 +140,48 @@ def get_resolver(
140140
@classmethod
141141
def categorize_column(
142142
cls,
143-
column: AnyResolved,
143+
column: ResolvedColumn,
144144
) -> Column:
145-
# Can't do bare literals, so they're actually formulas with +0
146-
if isinstance(column, (ResolvedFormula, ResolvedEquation, ResolvedLiteral)):
147-
return Column(formula=column.proto_definition, label=column.public_alias)
148-
elif isinstance(column, ResolvedAggregate):
149-
return Column(aggregation=column.proto_definition, label=column.public_alias)
150-
elif isinstance(column, ResolvedConditionalAggregate):
151-
return Column(
152-
conditional_aggregation=column.proto_definition, label=column.public_alias
153-
)
154-
else:
155-
return Column(key=column.proto_definition, label=column.public_alias)
145+
proto_definition = column.proto_definition
146+
147+
if isinstance(proto_definition, AttributeKey):
148+
return Column(key=proto_definition, label=column.public_alias)
149+
150+
if isinstance(proto_definition, AttributeAggregation):
151+
return Column(aggregation=proto_definition, label=column.public_alias)
152+
153+
if isinstance(proto_definition, AttributeConditionalAggregation):
154+
return Column(conditional_aggregation=proto_definition, label=column.public_alias)
155+
156+
if isinstance(proto_definition, Column.BinaryFormula):
157+
return Column(formula=proto_definition, label=column.public_alias)
158+
159+
if isinstance(proto_definition, Literal):
160+
return Column(literal=proto_definition, label=column.public_alias)
161+
162+
raise TypeError(f"Unsupported proto definition type: {type(proto_definition)}")
156163

157164
@classmethod
158165
def categorize_aggregate(
159166
cls,
160-
column: AnyResolved,
167+
column: ResolvedColumn,
161168
) -> Expression:
162-
if isinstance(column, (ResolvedFormula, ResolvedEquation)):
169+
proto_definition = column.proto_definition
170+
171+
if isinstance(proto_definition, AttributeAggregation):
172+
return Expression(aggregation=proto_definition, label=column.public_alias)
173+
174+
if isinstance(proto_definition, AttributeConditionalAggregation):
175+
return Expression(conditional_aggregation=proto_definition, label=column.public_alias)
176+
177+
if isinstance(proto_definition, Column.BinaryFormula):
163178
# TODO: Remove when https://github.com/getsentry/eap-planning/issues/206 is merged, since we can use formulas in both APIs at that point
164179
return Expression(
165-
formula=transform_binary_formula_to_expression(column.proto_definition),
180+
formula=transform_binary_formula_to_expression(proto_definition),
166181
label=column.public_alias,
167182
)
168-
elif isinstance(column, ResolvedAggregate):
169-
return Expression(aggregation=column.proto_definition, label=column.public_alias)
170-
elif isinstance(column, ResolvedConditionalAggregate):
171-
return Expression(
172-
conditional_aggregation=column.proto_definition, label=column.public_alias
173-
)
174-
else:
175-
raise Exception(f"Unknown column type {type(column)}")
183+
184+
raise TypeError(f"Unsupported proto definition type: {type(proto_definition)}")
176185

177186
@classmethod
178187
def get_cross_trace_queries(cls, query: TableQuery) -> list[TraceItemFilterWithType]:
@@ -251,7 +260,7 @@ def get_table_rpc_request(cls, query: TableQuery) -> TableRequest:
251260
# incomplete traces.
252261
meta.downsampled_storage_config.mode = DownsampledStorageConfig.MODE_HIGHEST_ACCURACY
253262

254-
all_columns: list[AnyResolved] = []
263+
all_columns: list[ResolvedColumn] = []
255264
equations, equation_contexts = resolver.resolve_equations(
256265
query.equations if query.equations else []
257266
)
@@ -594,7 +603,7 @@ def get_timeseries_query(
594603
extra_conditions: TraceItemFilter | None = None,
595604
) -> tuple[
596605
TimeSeriesRequest,
597-
list[AnyResolved],
606+
list[ResolvedColumn],
598607
list[ResolvedAttribute],
599608
]:
600609
selected_equations, selected_axes = arithmetic.categorize_columns(y_axes)

0 commit comments

Comments
 (0)