Skip to content

Commit 7e460cf

Browse files
authored
fix(arithmetic): Handle quoted search args in arithmetic (#112962)
- Because the search args in arithmetic wasn't aware of these backticks it couldn't handle the case where they would have `"` in them. This updates the grammar so this is handled and adds corresponding test cases
1 parent 424cd05 commit 7e460cf

File tree

3 files changed

+42
-1
lines changed

3 files changed

+42
-1
lines changed

src/sentry/discover/arithmetic.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,19 @@ def flatten(remaining):
122122
123123
function_value = function_name open_paren spaces function_args? spaces closed_paren
124124
function_args = aggregate_param (spaces comma spaces aggregate_param)*
125-
aggregate_param = quoted_aggregate_param / raw_aggregate_param
125+
aggregate_param = backtick_search_param / quoted_aggregate_param / raw_aggregate_param
126126
raw_aggregate_param = ~r"[^()\t\n, \"]+"
127127
quoted_aggregate_param = '"' ('\\"' / ~r'[^\t\n\"]')* '"'
128+
backtick_search_param = backtick search_value backtick
128129
# Different from a field value, since a function arg may not be a valid field
129130
function_name = ~r"[a-zA-Z_0-9]+"
130131
numeric_value = ~r"[+-]?[0-9]+\.?[0-9]*"
131132
field_value = ~r"[a-zA-Z_\.]+"
133+
# Search value is any text except a backtick which will exit back to normal parsing
134+
search_value = ~r"[^`]*"
132135
133136
comma = ","
137+
backtick = "`"
134138
open_paren = "("
135139
closed_paren = ")"
136140
spaces = " "*

tests/sentry/discover/test_arithmetic.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ def test_field_values(a, op, b) -> None:
263263
"opportunity_score(measurements.score.fcp)",
264264
),
265265
(100, "*", "opportunity_score(measurements.score.cls)"),
266+
("count_if(`test:foo`)", "+", "ttfd_contribution_rate()"),
267+
('count_if(`test:"blah blah"`)', "+", "ttfd_contribution_rate()"),
268+
('count_if(`test:"blah blah"`,test, test)', "+", "sum_if(`test:\"blah'blah'blah\"`)"),
266269
],
267270
)
268271
def test_function_values(lhs, op, rhs) -> None:

tests/snuba/api/endpoints/test_organization_events_trace_metrics.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,40 @@ def test_count_if_with_arbitrary_search_term(self):
781781
assert len(data) == 1
782782
assert data[0]["count_if(`release:abcdef`,value,request_duration,distribution,none)"] == 1
783783

784+
def test_count_if_with_quotes(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={"agent_name": "Agent Run"},
793+
),
794+
]
795+
self.store_eap_items(trace_metrics)
796+
797+
response = self.do_request(
798+
{
799+
"field": [
800+
'equation|count_if(`agent_name:"Agent Run"`,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(`agent_name:"Agent Run"`,value,request_duration,distribution,none)'
814+
]
815+
== 1
816+
)
817+
784818
def test_count_if_in_equation(self):
785819
trace_metrics = [
786820
self.create_trace_metric("request_duration", 200.0, "distribution", metric_unit="none"),

0 commit comments

Comments
 (0)