Skip to content

Commit

Permalink
update total weights helper function to use browser name filter
Browse files Browse the repository at this point in the history
  • Loading branch information
edwardgou-sentry committed Jan 8, 2025
1 parent b3f39c2 commit 89096c1
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
19 changes: 18 additions & 1 deletion src/sentry/search/events/datasets/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from snuba_sdk import Column, Condition, Function, Op, OrderBy

from sentry import features
from sentry.api.event_search import SearchFilter
from sentry.api.event_search import ParenExpression, SearchFilter, parse_search_query
from sentry.exceptions import IncompatibleMetricsQuery, InvalidSearchQuery
from sentry.search.events import constants, fields
from sentry.search.events.builder import metrics
Expand Down Expand Up @@ -1796,11 +1796,28 @@ def _resolve_total_score_weights_function(self, column: str, alias: str | None)
if column in self.total_score_weights and self.total_score_weights[column] is not None:
return Function("toFloat64", [self.total_score_weights[column]], alias)

# Pull out browser.name filters from the query
parsed_terms = parse_search_query(self.builder.query)
query = " ".join(
term.to_query_string()
for term in parsed_terms
if (isinstance(term, SearchFilter) and term.key.name == "browser.name")
or (
isinstance(term, ParenExpression)
and all( # type: ignore[unreachable]
(isinstance(child_term, SearchFilter) and child_term.key.name == "browser.name")
or child_term == "OR"
for child_term in term.children
)
)
)

total_query = metrics.MetricsQueryBuilder(
dataset=self.builder.dataset,
params={},
snuba_params=self.builder.params,
selected_columns=[f"sum({column})"],
query=query,
)

total_query.columns += self.builder.resolve_groupby()
Expand Down
34 changes: 23 additions & 11 deletions tests/snuba/api/endpoints/test_organization_events_mep.py
Original file line number Diff line number Diff line change
Expand Up @@ -3025,61 +3025,73 @@ def test_opportunity_score_with_fixed_weights_and_missing_vitals(self):
self.store_transaction_metric(
0.5,
metric="measurements.score.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
1.0,
metric="measurements.score.weight.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.2,
metric="measurements.score.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
1.0,
metric="measurements.score.weight.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.2,
metric="measurements.score.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.5,
metric="measurements.score.weight.inp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.1,
metric="measurements.score.lcp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Firefox"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.3,
metric="measurements.score.weight.lcp",
tags={"transaction": "foo_transaction"},
tags={"transaction": "foo_transaction", "browser.name": "Firefox"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.2,
metric="measurements.score.inp",
tags={"transaction": "bar_transaction"},
tags={"transaction": "bar_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.5,
metric="measurements.score.weight.inp",
tags={"transaction": "bar_transaction"},
tags={"transaction": "bar_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.5,
metric="measurements.score.total",
tags={"transaction": "foo_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)
self.store_transaction_metric(
0.5,
metric="measurements.score.total",
tags={"transaction": "bar_transaction", "browser.name": "Safari"},
timestamp=self.min_ago,
)

Expand All @@ -3090,7 +3102,7 @@ def test_opportunity_score_with_fixed_weights_and_missing_vitals(self):
"transaction",
"total_opportunity_score()",
],
"query": "event.type:transaction",
"query": 'event.type:transaction transaction.op:[pageload,""] (browser.name:Safari OR browser.name:Firefox) avg(measurements.score.total):>0',
"orderby": "transaction",
"dataset": "metrics",
"per_page": 50,
Expand Down

0 comments on commit 89096c1

Please sign in to comment.