Skip to content

Commit 24a0acd

Browse files
authored
feat(metrics): Add equation support to tracemetrics (#112609)
- This adds equation support to the tracemetrics dataset - I'm really not sure what the extra_conditions are doing here, they add a top level condition on the metric details, but we're already adding them per-aggregate anyways? I'm just going to leave this alone (but add equation support) for now and move on
1 parent b691b5d commit 24a0acd

File tree

3 files changed

+65
-17
lines changed

3 files changed

+65
-17
lines changed

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from rest_framework.request import Request
55
from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter
66

7+
from sentry.discover.arithmetic import parse_arithmetic
78
from sentry.exceptions import InvalidSearchQuery
89
from sentry.search.eap.columns import ResolvedTraceMetricAggregate, ResolvedTraceMetricFormula
910
from sentry.search.eap.resolver import SearchResolver
@@ -41,32 +42,39 @@ def _extra_conditions_from_columns(
4142
selected_columns: list[str] | None,
4243
equations: list[str] | None,
4344
) -> TraceItemFilter | None:
45+
"""While we also add the metric conditions inside -if combinators for each aggregate, adding it to the top level
46+
where should help clickhouse prune more rows"""
4447
aggregate_all_metrics = False
4548
selected_metrics: set[TraceMetric] = set()
49+
columns: set[str] = set()
4650

4751
if selected_columns:
4852
stripped_columns = [column.strip() for column in selected_columns]
4953
for column in stripped_columns:
5054
match = fields.is_function(column)
51-
if not match:
52-
continue
55+
if match:
56+
columns.add(column)
5357

54-
resolved_function, _ = search_resolver.resolve_function(column)
58+
if equations:
59+
for equation in equations:
60+
_, _, terms = parse_arithmetic(equation)
61+
for term in terms:
62+
columns.add(term)
5563

56-
if not isinstance(
57-
resolved_function, ResolvedTraceMetricAggregate
58-
) and not isinstance(resolved_function, ResolvedTraceMetricFormula):
59-
continue
64+
for column in columns:
65+
resolved_function, _ = search_resolver.resolve_function(column)
6066

61-
if resolved_function.trace_metric is None:
62-
# found an aggregation across all metrics, not just 1
63-
aggregate_all_metrics = True
64-
continue
67+
if not isinstance(resolved_function, ResolvedTraceMetricAggregate) and not isinstance(
68+
resolved_function, ResolvedTraceMetricFormula
69+
):
70+
continue
6571

66-
selected_metrics.add(resolved_function.trace_metric)
72+
if resolved_function.trace_metric is None:
73+
# found an aggregation across all metrics, not just 1
74+
aggregate_all_metrics = True
75+
continue
6776

68-
if equations:
69-
raise InvalidSearchQuery("Cannot support equations on trace metrics yet")
77+
selected_metrics.add(resolved_function.trace_metric)
7078

7179
# no selected metrics, no filter needed
7280
if not selected_metrics:

src/sentry/snuba/trace_metrics.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ def run_table_query(
4242
additional_queries: AdditionalQueries | None = None,
4343
) -> EAPResponse:
4444
"""timestamp_precise is always displayed in the UI in lieu of timestamp but since the TraceItem table isn't a DateTime64
45-
so we need to always order by it regardless of what is actually passed to the orderby."""
45+
so we need to always order by it regardless of what is actually passed to the orderby.
46+
"""
4647
if (
4748
orderby is not None
4849
and len(orderby) == 1
@@ -59,6 +60,7 @@ def run_table_query(
5960
rpc_dataset_common.TableQuery(
6061
query_string=query_string,
6162
selected_columns=selected_columns,
63+
equations=equations,
6264
orderby=orderby,
6365
offset=offset,
6466
limit=limit,

tests/snuba/api/endpoints/test_organization_events_trace_metrics.py

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
from sentry.conf.types.sentry_config import SentryMode
88
from sentry.utils.snuba_rpc import table_rpc
9-
from tests.snuba.api.endpoints.test_organization_events import OrganizationEventsEndpointTestBase
9+
from tests.snuba.api.endpoints.test_organization_events import (
10+
OrganizationEventsEndpointTestBase,
11+
)
1012

1113

1214
class OrganizationEventsTraceMetricsEndpointTest(OrganizationEventsEndpointTestBase):
@@ -283,7 +285,9 @@ def test_per_second_formula_with_gauge_metric_type(self) -> None:
283285
"per_second(value, cpu_usage, gauge, -)": pytest.approx(2 / 600, abs=0.001)
284286
}
285287

286-
def test_per_second_formula_with_gauge_metric_type_without_top_level_metric_type(self) -> None:
288+
def test_per_second_formula_with_gauge_metric_type_without_top_level_metric_type(
289+
self,
290+
) -> None:
287291
gauge_metrics = [
288292
self.create_trace_metric("cpu_usage", 75.0, "gauge"),
289293
self.create_trace_metric("cpu_usage", 80.0, "gauge"),
@@ -777,6 +781,40 @@ def test_count_if_with_arbitrary_search_term(self):
777781
assert len(data) == 1
778782
assert data[0]["count_if(`release:abcdef`,value,request_duration,distribution,none)"] == 1
779783

784+
def test_count_if_in_equation(self):
785+
trace_metrics = [
786+
self.create_trace_metric("request_duration", 200.0, "distribution", metric_unit="none"),
787+
self.create_trace_metric(
788+
"request_duration",
789+
200.0,
790+
"distribution",
791+
metric_unit="none",
792+
attributes={"release": "abcdef"},
793+
),
794+
]
795+
self.store_eap_items(trace_metrics)
796+
797+
response = self.do_request(
798+
{
799+
"field": [
800+
"equation|count_if(`release:abcdef`,value,request_duration,distribution,none) + count_if(`!release:abcdef`,value,request_duration,distribution,none)",
801+
],
802+
"project": self.project.id,
803+
"dataset": self.dataset,
804+
"statsPeriod": "10m",
805+
}
806+
)
807+
assert response.status_code == 200, response.content
808+
data = response.data["data"]
809+
810+
assert len(data) == 1
811+
assert (
812+
data[0][
813+
"equation|count_if(`release:abcdef`,value,request_duration,distribution,none) + count_if(`!release:abcdef`,value,request_duration,distribution,none)"
814+
]
815+
== 2
816+
)
817+
780818
def test_p50_if_with_arbitrary_search_term(self):
781819
trace_metrics = [
782820
self.create_trace_metric(

0 commit comments

Comments
 (0)