Skip to content

Commit e750cbe

Browse files
authored
chore(attr breakdowns): Clean up calling the RRF function (#106118)
- Stop calling the RRF scorer - Requires followup with the frontend and then a backend PR to make sure RRR changes are still being used properly without breaking anything else in the cleanup
1 parent 7dc04e3 commit e750cbe

File tree

2 files changed

+8
-37
lines changed

2 files changed

+8
-37
lines changed

src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
from sentry.search.eap.utils import can_expose_attribute, translate_internal_to_public_alias
3131
from sentry.search.events import fields
3232
from sentry.seer.endpoints.compare import compare_distributions
33-
from sentry.seer.workflows.compare import keyed_rrf_score
3433
from sentry.snuba.referrer import Referrer
3534
from sentry.snuba.spans_rpc import Spans
3635
from sentry.utils import snuba_rpc
@@ -306,13 +305,6 @@ def run_table_query_with_error_handling(query_string):
306305
)
307306
total_baseline = total_spans - total_outliers
308307

309-
scored_attrs_rrf = keyed_rrf_score(
310-
baseline=cohort_2_distribution,
311-
outliers=cohort_1_distribution,
312-
total_outliers=total_outliers,
313-
total_baseline=total_baseline,
314-
)
315-
316308
logger.info(
317309
"compare_distributions params: baseline=%s, outliers=%s, total_outliers=%s, total_baseline=%s, config=%s, meta=%s",
318310
cohort_2_distribution,
@@ -338,10 +330,7 @@ def run_table_query_with_error_handling(query_string):
338330
)
339331
logger.info("scored_attrs_rrr: %s", scored_attrs_rrr)
340332

341-
# Create RRR order mapping from compare_distributions results
342-
# scored_attrs_rrr returns a dict with 'results' key containing list of [attribute_name, score] pairs
343333
rrr_results = scored_attrs_rrr.get("results", [])
344-
rrr_order_map = {attr_name: i for i, (attr_name, _) in enumerate(rrr_results)}
345334

346335
ranked_distribution: dict[str, Any] = {
347336
"rankedAttributes": [],
@@ -354,9 +343,7 @@ def run_table_query_with_error_handling(query_string):
354343
"cohort2Total": total_baseline,
355344
}
356345

357-
for i, scored_attr_tuple in enumerate(scored_attrs_rrf):
358-
attr = scored_attr_tuple[0]
359-
346+
for i, (attr, _) in enumerate(rrr_results):
360347
public_alias, _, _ = translate_internal_to_public_alias(
361348
attr, "string", SupportedTraceItemType.SPANS
362349
)
@@ -371,10 +358,8 @@ def run_table_query_with_error_handling(query_string):
371358
"attributeName": public_alias,
372359
"cohort1": cohort_1_distribution_map.get(attr),
373360
"cohort2": cohort_2_distribution_map.get(attr),
374-
"order": { # TODO: aayush-se remove this once we have selected a single ranking method
375-
"rrf": i,
376-
"rrr": rrr_order_map.get(attr),
377-
},
361+
# TODO(aayush-se): Remove order field once frontend stops using it
362+
"order": {"rrr": i},
378363
}
379364
ranked_distribution["rankedAttributes"].append(distribution)
380365

tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -160,29 +160,23 @@ def test_function_with_multiple_arguments(self, mock_compare_distributions) -> N
160160
assert "rankedAttributes" in response.data
161161

162162
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.compare_distributions")
163-
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.keyed_rrf_score")
164163
def test_baseline_distribution_includes_baseline_only_buckets(
165-
self, mock_keyed_rrf_score, mock_compare_distributions
164+
self, mock_compare_distributions
166165
) -> None:
167166
"""Test that buckets existing only in baseline (not in suspect) are included in scoring.
168167
169168
This specifically tests the fix for the bug where attribute values that exist
170169
in all spans but NOT in the suspect cohort were missing from the baseline
171170
distribution passed to scoring algorithms.
172171
"""
173-
# Capture what's passed to the scoring functions
172+
# Capture what's passed to compare_distributions
174173
captured_baseline = None
175174

176-
def capture_baseline(*args, **kwargs):
175+
def capture_compare(*args, **kwargs):
177176
nonlocal captured_baseline
178177
captured_baseline = kwargs.get("baseline")
179-
# Return results matching the actual internal attribute names
180-
return [("sentry.browser", 1.0), ("sentry.device", 0.8)]
181-
182-
def capture_compare(*args, **kwargs):
183178
return {"results": [("sentry.browser", 0.9), ("sentry.device", 0.7)]}
184179

185-
mock_keyed_rrf_score.side_effect = capture_baseline
186180
mock_compare_distributions.side_effect = capture_compare
187181

188182
tags = [
@@ -292,12 +286,11 @@ def test_filters_out_internal_and_private_attributes(self, mock_compare_distribu
292286
), f"Attribute '{attr_name}' should be filtered (meta attribute)"
293287

294288
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.compare_distributions")
295-
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.keyed_rrf_score")
296289
@patch(
297290
"sentry.api.endpoints.organization_trace_item_attributes_ranked.translate_internal_to_public_alias"
298291
)
299292
def test_includes_user_defined_attributes_when_translate_returns_none(
300-
self, mock_translate, mock_keyed_rrf_score, mock_compare_distributions
293+
self, mock_translate, mock_compare_distributions
301294
) -> None:
302295
"""Test that user-defined attributes are included when translate_internal_to_public_alias returns None.
303296
@@ -316,14 +309,7 @@ def mock_translate_func(attr, *_):
316309

317310
mock_translate.side_effect = mock_translate_func
318311

319-
# Mock primary scoring (keyed_rrf_score) to include our test attributes
320-
mock_keyed_rrf_score.return_value = [
321-
("custom_user_attr", 0.9),
322-
("tags[filtered_tag]", 0.8),
323-
("regular_attr", 0.7),
324-
]
325-
326-
# Mock secondary scoring for RRR ordering
312+
# Mock compare_distributions to return our test attributes (now the primary scoring)
327313
mock_compare_distributions.return_value = {
328314
"results": [
329315
("custom_user_attr", 0.9),

0 commit comments

Comments
 (0)