From 89096c1275228661ee0ee4d8065e7df586ead168 Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Wed, 8 Jan 2025 10:10:17 -0500 Subject: [PATCH] update total weights helper function to use browser name filter --- src/sentry/search/events/datasets/metrics.py | 19 ++++++++++- .../endpoints/test_organization_events_mep.py | 34 +++++++++++++------ 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/sentry/search/events/datasets/metrics.py b/src/sentry/search/events/datasets/metrics.py index 849d0f14c9cd51..16623dc6f1c870 100644 --- a/src/sentry/search/events/datasets/metrics.py +++ b/src/sentry/search/events/datasets/metrics.py @@ -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 @@ -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() diff --git a/tests/snuba/api/endpoints/test_organization_events_mep.py b/tests/snuba/api/endpoints/test_organization_events_mep.py index 2cdc155a7c19c7..91e56ec1a6be14 100644 --- a/tests/snuba/api/endpoints/test_organization_events_mep.py +++ b/tests/snuba/api/endpoints/test_organization_events_mep.py @@ -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, ) @@ -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,