Skip to content

Commit b7c8860

Browse files
authored
fix(TraceItemsDetails): dedupe int,bool and float attrs (#7260)
Addresses [EAP-121](https://linear.app/getsentry/issue/EAP-121/traceitemdetails-endpoint-should-merge-attributes-without-leaving) For `bool` and `int` attributes, we also store a `float` version in the database to simplify aggregations. When returning data to the user we should only return the original type, and ignore the float duplicate. Waiting on getsentry/sentry#94553 to be merged before I can merge this PR
1 parent 29096ac commit b7c8860

File tree

2 files changed

+52
-5
lines changed

2 files changed

+52
-5
lines changed

snuba/web/rpc/v1/endpoint_trace_item_details.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,17 +174,28 @@ def _convert_results(
174174

175175
for k, v in row["attributes_string"].items():
176176
attrs.append(TraceItemDetailsAttribute(name=k, value=AttributeValue(val_str=v)))
177-
for k, v in row["attributes_float"].items():
178-
attrs.append(
179-
TraceItemDetailsAttribute(name=k, value=AttributeValue(val_double=v))
180-
)
177+
178+
int_attr_names = set()
181179
for k, v in row["attributes_int"].items():
180+
int_attr_names.add(k)
182181
attrs.append(TraceItemDetailsAttribute(name=k, value=AttributeValue(val_int=v)))
182+
183+
bool_attr_names = set()
183184
for k, v in row["attributes_bool"].items():
185+
bool_attr_names.add(k)
184186
attrs.append(
185187
TraceItemDetailsAttribute(name=k, value=AttributeValue(val_bool=v))
186188
)
187189

190+
for k, v in row["attributes_float"].items():
191+
if k in int_attr_names or k in bool_attr_names:
192+
# for bool & int attributes, we also store a float version in the database
193+
# to simplfy aggregations. when returning data to the user we should only return
194+
# the original type, and ignore the float duplicate
195+
continue
196+
attrs.append(
197+
TraceItemDetailsAttribute(name=k, value=AttributeValue(val_double=v))
198+
)
188199
return item_id, timestamp, attrs
189200

190201

tests/web/rpc/v1/test_endpoint_trace_item_details.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818

1919
from snuba.datasets.storages.factory import get_storage
2020
from snuba.datasets.storages.storage_key import StorageKey
21-
from snuba.web.rpc.v1.endpoint_trace_item_details import EndpointTraceItemDetails
21+
from snuba.web.rpc.v1.endpoint_trace_item_details import (
22+
EndpointTraceItemDetails,
23+
_convert_results,
24+
)
2225
from snuba.web.rpc.v1.endpoint_trace_item_table import EndpointTraceItemTable
2326
from tests.base import BaseApiTest
2427
from tests.helpers import write_raw_unprocessed_events
@@ -325,3 +328,36 @@ def test_endpoint_on_spans(self, setup_spans_in_db: Any) -> None:
325328
"str_tag",
326329
}:
327330
assert k in attributes_returned, k
331+
332+
333+
def test_convert_results_dedupes() -> None:
334+
"""
335+
Makes sure that _convert_results dedupes int/bool and float
336+
attributes. We store float versions of int and bool attributes
337+
for computational reasons but we don't want to return the
338+
duplicate float attrs to the user.
339+
"""
340+
data = [
341+
{
342+
"timestamp": 1750964400,
343+
"hex_item_id": "e70ef5b1b5bc4611840eff9964b7a767",
344+
"trace_id": "cb190d6e7d5743d5bc1494c650592cd2",
345+
"organization_id": 1,
346+
"project_id": 1,
347+
"item_type": 1,
348+
"attributes_string": {
349+
"relay_protocol_version": "3",
350+
"sentry.segment_id": "30c64b1f21b54799",
351+
},
352+
"attributes_int": {"sentry.duration_ms": 152},
353+
"attributes_float": {"sentry.is_segment": 1.0, "num_of_spans": 50.0},
354+
"attributes_bool": {
355+
"my.true.bool.field": True,
356+
"sentry.is_segment": True,
357+
"my.false.bool.field": False,
358+
},
359+
}
360+
]
361+
_, _, attrs = _convert_results(data)
362+
is_segment_attrs = list(filter(lambda x: x.name == "sentry.is_segment", attrs))
363+
assert len(is_segment_attrs) == 1

0 commit comments

Comments
 (0)