Skip to content

Commit d2880df

Browse files
authored
feat(tracemetrics): Lift metric name and type to top level query params (#102512)
It doesn't make a lot of sense to query multiple metrics together and in fact, the query would be very inefficient. Instead, opt to lift the metric name and type to a top level query param for easier handling.
1 parent 9b0e4b2 commit d2880df

File tree

11 files changed

+416
-47
lines changed

11 files changed

+416
-47
lines changed

src/sentry/api/endpoints/organization_events.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@
2929
from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetTypes
3030
from sentry.models.organization import Organization
3131
from sentry.ratelimits.config import RateLimitConfig
32+
from sentry.search.eap.trace_metrics.config import (
33+
TraceMetricsSearchResolverConfig,
34+
get_trace_metric_info,
35+
)
3236
from sentry.search.eap.types import AdditionalQueries, FieldsACL, SearchResolverConfig
3337
from sentry.snuba import (
3438
discover,
@@ -527,7 +531,11 @@ def get_rpc_config():
527531
)
528532
elif scoped_dataset == TraceMetrics:
529533
# tracemetrics uses aggregate conditions
530-
return SearchResolverConfig(
534+
metric_name, metric_type = get_trace_metric_info(request)
535+
536+
return TraceMetricsSearchResolverConfig(
537+
metric_name=metric_name,
538+
metric_type=metric_type,
531539
use_aggregate_conditions=use_aggregate_conditions,
532540
auto_fields=True,
533541
disable_aggregate_extrapolation=disable_aggregate_extrapolation,

src/sentry/api/endpoints/organization_events_stats.py

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
from sentry.constants import MAX_TOP_EVENTS
2020
from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetTypes
2121
from sentry.models.organization import Organization
22+
from sentry.search.eap.trace_metrics.config import (
23+
TraceMetricsSearchResolverConfig,
24+
get_trace_metric_info,
25+
)
2226
from sentry.search.eap.types import SearchResolverConfig
2327
from sentry.search.events.types import SnubaParams
2428
from sentry.snuba import (
@@ -233,6 +237,34 @@ def _get_event_stats(
233237
if should_upsample:
234238
final_columns = transform_query_columns_for_error_upsampling(query_columns)
235239

240+
def get_rpc_config():
241+
if scoped_dataset not in RPC_DATASETS:
242+
raise NotImplementedError
243+
244+
if scoped_dataset == TraceMetrics:
245+
# tracemetrics uses aggregate conditions
246+
metric_name, metric_type = get_trace_metric_info(request)
247+
248+
return TraceMetricsSearchResolverConfig(
249+
metric_name=metric_name,
250+
metric_type=metric_type,
251+
auto_fields=False,
252+
use_aggregate_conditions=True,
253+
disable_aggregate_extrapolation=request.GET.get(
254+
"disableAggregateExtrapolation", "0"
255+
)
256+
== "1",
257+
)
258+
259+
return SearchResolverConfig(
260+
auto_fields=False,
261+
use_aggregate_conditions=True,
262+
disable_aggregate_extrapolation=request.GET.get(
263+
"disableAggregateExtrapolation", "0"
264+
)
265+
== "1",
266+
)
267+
236268
if top_events > 0:
237269
raw_groupby = self.get_field_list(organization, request)
238270
if "timestamp" in raw_groupby:
@@ -247,14 +279,7 @@ def _get_event_stats(
247279
limit=top_events,
248280
include_other=include_other,
249281
referrer=referrer,
250-
config=SearchResolverConfig(
251-
auto_fields=False,
252-
use_aggregate_conditions=True,
253-
disable_aggregate_extrapolation=request.GET.get(
254-
"disableAggregateExtrapolation", "0"
255-
)
256-
== "1",
257-
),
282+
config=get_rpc_config(),
258283
sampling_mode=snuba_params.sampling_mode,
259284
equations=self.get_equation_list(organization, request),
260285
)
@@ -285,14 +310,7 @@ def _get_event_stats(
285310
query_string=query,
286311
y_axes=final_columns,
287312
referrer=referrer,
288-
config=SearchResolverConfig(
289-
auto_fields=False,
290-
use_aggregate_conditions=True,
291-
disable_aggregate_extrapolation=request.GET.get(
292-
"disableAggregateExtrapolation", "0"
293-
)
294-
== "1",
295-
),
313+
config=get_rpc_config(),
296314
sampling_mode=snuba_params.sampling_mode,
297315
comparison_delta=comparison_delta,
298316
)

src/sentry/search/eap/columns.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from sentry.api.event_search import SearchFilter
2323
from sentry.exceptions import InvalidSearchQuery
2424
from sentry.search.eap import constants
25-
from sentry.search.eap.types import EAPResponse
25+
from sentry.search.eap.types import EAPResponse, SearchResolverConfig
2626
from sentry.search.events.types import SnubaParams
2727

2828
ResolvedArgument: TypeAlias = AttributeKey | str | int | float
@@ -33,6 +33,7 @@ class ResolverSettings(TypedDict):
3333
extrapolation_mode: ExtrapolationMode.ValueType
3434
snuba_params: SnubaParams
3535
query_result_cache: dict[str, EAPResponse]
36+
search_config: SearchResolverConfig
3637

3738

3839
@dataclass(frozen=True, kw_only=True)
@@ -298,7 +299,7 @@ def resolve(
298299
resolved_arguments: ResolvedArguments,
299300
snuba_params: SnubaParams,
300301
query_result_cache: dict[str, EAPResponse],
301-
extrapolation_override: bool = False,
302+
search_config: SearchResolverConfig,
302303
) -> ResolvedFormula | ResolvedAggregate | ResolvedConditionalAggregate:
303304
raise NotImplementedError()
304305

@@ -319,7 +320,7 @@ def resolve(
319320
resolved_arguments: ResolvedArguments,
320321
snuba_params: SnubaParams,
321322
query_result_cache: dict[str, EAPResponse],
322-
extrapolation_override: bool = False,
323+
search_config: SearchResolverConfig,
323324
) -> ResolvedAggregate:
324325
if len(resolved_arguments) > 1:
325326
raise InvalidSearchQuery(
@@ -341,7 +342,9 @@ def resolve(
341342
search_type=search_type,
342343
internal_type=self.internal_type,
343344
processor=self.processor,
344-
extrapolation=self.extrapolation if not extrapolation_override else False,
345+
extrapolation=(
346+
self.extrapolation if not search_config.disable_aggregate_extrapolation else False
347+
),
345348
argument=resolved_attribute,
346349
)
347350

@@ -367,7 +370,7 @@ def resolve(
367370
resolved_arguments: ResolvedArguments,
368371
snuba_params: SnubaParams,
369372
query_result_cache: dict[str, EAPResponse],
370-
extrapolation_override: bool = False,
373+
search_config: SearchResolverConfig,
371374
) -> ResolvedConditionalAggregate:
372375
key, aggregate_filter = self.aggregate_resolver(resolved_arguments)
373376
return ResolvedConditionalAggregate(
@@ -378,7 +381,9 @@ def resolve(
378381
filter=aggregate_filter,
379382
key=key,
380383
processor=self.processor,
381-
extrapolation=self.extrapolation if not extrapolation_override else False,
384+
extrapolation=(
385+
self.extrapolation if not search_config.disable_aggregate_extrapolation else False
386+
),
382387
)
383388

384389

@@ -401,16 +406,17 @@ def resolve(
401406
resolved_arguments: list[AttributeKey | Any],
402407
snuba_params: SnubaParams,
403408
query_result_cache: dict[str, EAPResponse],
404-
extrapolation_override: bool = False,
409+
search_config: SearchResolverConfig,
405410
) -> ResolvedFormula:
406411
resolver_settings = ResolverSettings(
407412
extrapolation_mode=(
408413
ExtrapolationMode.EXTRAPOLATION_MODE_SAMPLE_WEIGHTED
409-
if self.extrapolation and not extrapolation_override
414+
if self.extrapolation and not search_config.disable_aggregate_extrapolation
410415
else ExtrapolationMode.EXTRAPOLATION_MODE_NONE
411416
),
412417
snuba_params=snuba_params,
413418
query_result_cache=query_result_cache,
419+
search_config=search_config,
414420
)
415421

416422
return ResolvedFormula(

src/sentry/search/eap/resolver.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,7 @@ def resolve_function(
10641064
resolved_arguments=resolved_arguments,
10651065
snuba_params=self.params,
10661066
query_result_cache=self._query_result_cache,
1067-
extrapolation_override=self.config.disable_aggregate_extrapolation,
1067+
search_config=self.config,
10681068
)
10691069

10701070
resolved_context = None
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
from dataclasses import dataclass
2+
from typing import Literal, cast
3+
4+
from rest_framework.request import Request
5+
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue
6+
from sentry_protos.snuba.v1.trace_item_filter_pb2 import (
7+
AndFilter,
8+
ComparisonFilter,
9+
TraceItemFilter,
10+
)
11+
12+
from sentry.search.eap.resolver import SearchResolver
13+
from sentry.search.eap.types import SearchResolverConfig
14+
15+
MetricType = Literal["counter", "gauge", "distribution"]
16+
17+
18+
@dataclass(frozen=True, kw_only=True)
19+
class TraceMetricsSearchResolverConfig(SearchResolverConfig):
20+
metric_name: str
21+
metric_type: MetricType
22+
23+
def extra_conditions(self, search_resolver: SearchResolver) -> TraceItemFilter | None:
24+
if not self.metric_name or not self.metric_type:
25+
return None
26+
27+
metric_name, _ = search_resolver.resolve_column("metric.name")
28+
if not isinstance(metric_name.proto_definition, AttributeKey):
29+
raise ValueError("Unable to resolve metric.name")
30+
31+
metric_type, _ = search_resolver.resolve_column("metric.type")
32+
if not isinstance(metric_type.proto_definition, AttributeKey):
33+
raise ValueError("Unable to resolve metric.type")
34+
35+
metric_name_filter = TraceItemFilter(
36+
comparison_filter=ComparisonFilter(
37+
key=metric_name.proto_definition,
38+
op=ComparisonFilter.OP_EQUALS,
39+
value=AttributeValue(val_str=self.metric_name),
40+
)
41+
)
42+
metric_type_filter = TraceItemFilter(
43+
comparison_filter=ComparisonFilter(
44+
key=metric_type.proto_definition,
45+
op=ComparisonFilter.OP_EQUALS,
46+
value=AttributeValue(val_str=self.metric_type),
47+
)
48+
)
49+
50+
return TraceItemFilter(
51+
and_filter=AndFilter(filters=[metric_name_filter, metric_type_filter])
52+
)
53+
54+
55+
ALLOWED_METRIC_TYPES: list[MetricType] = ["counter", "gauge", "distribution"]
56+
57+
58+
def get_trace_metric_info(
59+
request: Request,
60+
) -> tuple[str, MetricType]:
61+
metric_name = request.GET.get("metricName")
62+
metric_type = request.GET.get("metricType")
63+
64+
# TODO: remove this when these args are not optional anymore
65+
if not metric_name:
66+
metric_name = ""
67+
if not metric_type:
68+
metric_type = ""
69+
70+
# errors = {}
71+
72+
# if not metric_name:
73+
# errors["metricName"] = ErrorDetail("This field is required.", code="required")
74+
75+
# if not metric_type:
76+
# errors["metricType"] = ErrorDetail("This field is required.", code="required")
77+
# elif metric_type not in ALLOWED_METRIC_TYPES:
78+
# errors["metricType"] = ErrorDetail(
79+
# string=f'"{metric_type}" is not a valid choice.', code="invalid_choice"
80+
# )
81+
82+
# if errors:
83+
# raise ValidationError(errors)
84+
85+
# assert metric_name
86+
87+
return metric_name, cast(MetricType, metric_type)

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
Function,
99
)
1010

11+
from sentry.exceptions import InvalidSearchQuery
1112
from sentry.search.eap.columns import (
1213
FormulaDefinition,
1314
ResolvedArguments,
1415
ResolverSettings,
1516
ValueArgumentDefinition,
1617
)
18+
from sentry.search.eap.trace_metrics.config import TraceMetricsSearchResolverConfig
1719

1820

1921
def _rate_internal(
@@ -22,6 +24,10 @@ def _rate_internal(
2224
"""
2325
Calculate rate per X for trace metrics using the value attribute.
2426
"""
27+
search_config = settings["search_config"]
28+
if not isinstance(search_config, TraceMetricsSearchResolverConfig):
29+
raise InvalidSearchQuery("unexpected search config")
30+
2531
extrapolation_mode = settings["extrapolation_mode"]
2632
is_timeseries_request = settings["snuba_params"].is_timeseries_request
2733

@@ -31,6 +37,8 @@ def _rate_internal(
3137
else settings["snuba_params"].interval
3238
)
3339

40+
metric_type = search_config.metric_type if search_config.metric_type else metric_type
41+
3442
if metric_type == "counter":
3543
aggregate_func = Function.FUNCTION_SUM
3644
else:
@@ -55,7 +63,6 @@ def per_second(args: ResolvedArguments, settings: ResolverSettings) -> Column.Bi
5563
"""
5664
Calculate rate per second for trace metrics using the value attribute.
5765
"""
58-
5966
metric_type = cast(str, args[1]) if len(args) > 1 else "counter"
6067
return _rate_internal(1, metric_type, settings)
6168

src/sentry/search/eap/types.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
from dataclasses import dataclass, field
22
from enum import Enum
3-
from typing import Literal, NotRequired, TypedDict
3+
from typing import TYPE_CHECKING, Literal, NotRequired, TypedDict
44

55
from sentry_protos.snuba.v1.request_common_pb2 import PageToken
66
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import Reliability
7+
from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter
78

89
from sentry.search.events.types import EventsResponse
910

11+
if TYPE_CHECKING:
12+
from sentry.search.eap.resolver import SearchResolver
13+
1014

1115
@dataclass(frozen=True)
1216
class FieldsACL:
1317
functions: set[str] = field(default_factory=set)
1418
attributes: set[str] = field(default_factory=set)
1519

1620

17-
@dataclass(frozen=True)
21+
@dataclass(frozen=True, kw_only=True)
1822
class SearchResolverConfig:
1923
# Automatically add id, etc. if there are no aggregates
2024
auto_fields: bool = False
@@ -30,6 +34,9 @@ class SearchResolverConfig:
3034
# Whether to set the timestamp granularities to stable buckets
3135
stable_timestamp_quantization: bool = True
3236

37+
def extra_conditions(self, search_resolver: "SearchResolver") -> TraceItemFilter | None:
38+
return None
39+
3340

3441
CONFIDENCES: dict[Reliability.ValueType, Literal["low", "high"]] = {
3542
Reliability.RELIABILITY_LOW: "low",

0 commit comments

Comments
 (0)