Skip to content

Commit 1cbd57a

Browse files
aayush-sejasonyuezhang
authored andcommitted
filter to only the attributes available in the groupby
1 parent 2fcf982 commit 1cbd57a

File tree

2 files changed

+44
-4
lines changed

2 files changed

+44
-4
lines changed

src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55

66
from rest_framework.request import Request
77
from rest_framework.response import Response
8+
from sentry_protos.snuba.v1.endpoint_trace_item_attributes_pb2 import TraceItemAttributeNamesRequest
89
from sentry_protos.snuba.v1.endpoint_trace_item_stats_pb2 import (
910
AttributeDistributionsRequest,
1011
StatsType,
1112
TraceItemStatsRequest,
1213
)
14+
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey
1315

1416
from sentry import features
1517
from sentry.api.api_owners import ApiOwner
@@ -27,7 +29,7 @@
2729
from sentry.seer.workflows.compare import keyed_rrf_score
2830
from sentry.snuba.referrer import Referrer
2931
from sentry.snuba.spans_rpc import Spans
30-
from sentry.utils.snuba_rpc import trace_item_stats_rpc
32+
from sentry.utils.snuba_rpc import attribute_names_rpc, trace_item_stats_rpc
3133

3234
logger = logging.getLogger(__name__)
3335

@@ -120,6 +122,20 @@ def get(self, request: Request, organization: Organization) -> Response:
120122
if query_1 == query_2:
121123
return Response({"rankedAttributes": []})
122124

125+
# First, fetch attribute names to use as an allowlist
126+
attribute_names_request = TraceItemAttributeNamesRequest(
127+
meta=meta,
128+
type=AttributeKey.Type.TYPE_STRING,
129+
limit=1000,
130+
)
131+
attribute_names_response = attribute_names_rpc(attribute_names_request)
132+
133+
allowed_attribute_names = {
134+
attr.name
135+
for attr in attribute_names_response.attributes
136+
if attr.name and can_expose_attribute(attr.name, SupportedTraceItemType.SPANS)
137+
}
138+
123139
cohort_1, _, _ = resolver.resolve_query(query_1)
124140
cohort_1_request = TraceItemStatsRequest(
125141
filter=cohort_1,
@@ -199,7 +215,10 @@ def get(self, request: Request, organization: Organization) -> Response:
199215
processed_cohort_2_buckets = set()
200216

201217
for attribute in cohort_2_data.results[0].attribute_distributions.attributes:
202-
if not can_expose_attribute(attribute.attribute_name, SupportedTraceItemType.SPANS):
218+
# Filter attributes to only those in the allowlist
219+
if attribute.attribute_name not in allowed_attribute_names or not can_expose_attribute(
220+
attribute.attribute_name, SupportedTraceItemType.SPANS
221+
):
203222
continue
204223

205224
for bucket in attribute.buckets:
@@ -208,7 +227,10 @@ def get(self, request: Request, organization: Organization) -> Response:
208227
)
209228

210229
for attribute in cohort_1_data.results[0].attribute_distributions.attributes:
211-
if not can_expose_attribute(attribute.attribute_name, SupportedTraceItemType.SPANS):
230+
# Filter attributes to only those in the allowlist
231+
if attribute.attribute_name not in allowed_attribute_names or not can_expose_attribute(
232+
attribute.attribute_name, SupportedTraceItemType.SPANS
233+
):
212234
continue
213235
for bucket in attribute.buckets:
214236
cohort_1_distribution.append((attribute.attribute_name, bucket.label, bucket.value))

tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,9 @@ def capture_compare(*args, **kwargs):
246246

247247
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.compare_distributions")
248248
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.trace_item_stats_rpc")
249+
@patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.attribute_names_rpc")
249250
def test_filters_out_internal_and_private_attributes(
250-
self, mock_trace_item_stats_rpc, mock_compare_distributions
251+
self, mock_attribute_names_rpc, mock_trace_item_stats_rpc, mock_compare_distributions
251252
) -> None:
252253
"""Test that internal and private attributes are filtered from the response.
253254
@@ -336,6 +337,23 @@ def create_mock_response():
336337
# Both cohorts return the same attributes
337338
mock_trace_item_stats_rpc.return_value = create_mock_response()
338339

340+
# Mock the attribute names RPC to return all the attribute names (including internal ones)
341+
mock_attr_names_response = MagicMock()
342+
mock_attributes = []
343+
for attr_name in [
344+
"sentry.device",
345+
"browser",
346+
"sentry.links",
347+
"sentry._meta.fields.attributes.browser",
348+
"sentry._internal.some_metric",
349+
"__sentry_internal.debug_info",
350+
]:
351+
attr = MagicMock()
352+
attr.name = attr_name
353+
mock_attributes.append(attr)
354+
mock_attr_names_response.attributes = mock_attributes
355+
mock_attribute_names_rpc.return_value = mock_attr_names_response
356+
339357
mock_compare_distributions.return_value = {
340358
"results": [
341359
("sentry.device", 0.9),

0 commit comments

Comments
 (0)