Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eap-api): Snuba receives and returns doubles #6782

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ python-rapidjson==1.8
redis==4.5.4
sentry-arroyo==2.19.4
sentry-kafka-schemas==0.1.129
sentry-protos==0.1.49
sentry-protos==0.1.51
sentry-redis-tools==0.3.0
sentry-relay==0.9.4
sentry-sdk==2.18.0
Expand Down
30 changes: 23 additions & 7 deletions snuba/web/rpc/common/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ def transform(exp: Expression) -> Expression:
"sentry.segment_id": AttributeKey.Type.TYPE_STRING, # this is converted by a processor on the storage
"sentry.segment_name": AttributeKey.Type.TYPE_STRING,
"sentry.is_segment": AttributeKey.Type.TYPE_BOOLEAN,
"sentry.duration_ms": AttributeKey.Type.TYPE_FLOAT,
"sentry.exclusive_time_ms": AttributeKey.Type.TYPE_FLOAT,
"sentry.duration_ms": AttributeKey.Type.TYPE_DOUBLE,
"sentry.exclusive_time_ms": AttributeKey.Type.TYPE_DOUBLE,
"sentry.retention_days": AttributeKey.Type.TYPE_INT,
"sentry.name": AttributeKey.Type.TYPE_STRING,
"sentry.sampling_weight": AttributeKey.Type.TYPE_FLOAT,
"sentry.sampling_factor": AttributeKey.Type.TYPE_FLOAT,
"sentry.sampling_weight": AttributeKey.Type.TYPE_DOUBLE,
"sentry.sampling_factor": AttributeKey.Type.TYPE_DOUBLE,
"sentry.timestamp": AttributeKey.Type.TYPE_UNSPECIFIED,
"sentry.start_timestamp": AttributeKey.Type.TYPE_UNSPECIFIED,
"sentry.end_timestamp": AttributeKey.Type.TYPE_UNSPECIFIED,
Expand Down Expand Up @@ -124,7 +124,10 @@ def _build_label_mapping_key(attr_key: AttributeKey) -> str:
)
if attr_key.type == AttributeKey.Type.TYPE_INT:
return f.CAST(column(attr_key.name[len("sentry.") :]), "Int64", alias=alias)
if attr_key.type == AttributeKey.Type.TYPE_FLOAT:
if (
attr_key.type == AttributeKey.Type.TYPE_FLOAT
or attr_key.type == AttributeKey.Type.TYPE_DOUBLE
):
return f.CAST(
column(attr_key.name[len("sentry.") :]), "Float64", alias=alias
)
Expand All @@ -133,7 +136,11 @@ def _build_label_mapping_key(attr_key: AttributeKey) -> str:
)

if attr_key.name in NORMALIZED_COLUMNS:
if NORMALIZED_COLUMNS[attr_key.name] == attr_key.type:
# the second if statement allows Sentry to send TYPE_FLOAT to Snuba when Snuba still has to be backward compatible with TYPE_FLOATS
if NORMALIZED_COLUMNS[attr_key.name] == attr_key.type or (
attr_key.type == AttributeKey.Type.TYPE_FLOAT
and NORMALIZED_COLUMNS[attr_key.name] == AttributeKey.Type.TYPE_DOUBLE
):
return column(attr_key.name[len("sentry.") :], alias=attr_key.name)
raise BadSnubaRPCRequestException(
f"Attribute {attr_key.name} must be requested as {NORMALIZED_COLUMNS[attr_key.name]}, got {attr_key.type}"
Expand All @@ -144,7 +151,10 @@ def _build_label_mapping_key(attr_key: AttributeKey) -> str:
return SubscriptableReference(
alias=alias, column=column("attr_str"), key=literal(attr_key.name)
)
if attr_key.type == AttributeKey.Type.TYPE_FLOAT:
if (
attr_key.type == AttributeKey.Type.TYPE_FLOAT
or attr_key.type == AttributeKey.Type.TYPE_DOUBLE
):
return SubscriptableReference(
alias=alias, column=column("attr_num"), key=literal(attr_key.name)
)
Expand Down Expand Up @@ -284,6 +294,8 @@ def trace_item_filters_to_expression(item_filter: TraceItemFilter) -> Expression
v_expression = literal(v.val_str)
case "val_float":
v_expression = literal(v.val_float)
case "val_double":
v_expression = literal(v.val_double)
case "val_int":
v_expression = literal(v.val_int)
case "val_null":
Expand All @@ -300,6 +312,10 @@ def trace_item_filters_to_expression(item_filter: TraceItemFilter) -> Expression
v_expression = literals_array(
None, list(map(lambda x: literal(x), v.val_float_array.values))
)
case "val_double_array":
v_expression = literals_array(
None, list(map(lambda x: literal(x), v.val_double_array.values))
)
case default:
raise NotImplementedError(
f"translation of AttributeValue type {default} is not implemented"
Expand Down
54 changes: 49 additions & 5 deletions snuba/web/rpc/v1/endpoint_get_traces.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@
_DEFAULT_ROW_LIMIT = 10_000
_BUFFER_WINDOW = 2 * 3600 # 2 hours


def _convert_key_to_support_doubles_and_floats_for_backward_compat(
key: TraceAttribute.Key.ValueType,
) -> TraceAttribute.Key.ValueType:
return TraceAttribute.Key.ValueType(-1 * key)


_ATTRIBUTES: dict[
TraceAttribute.Key.ValueType,
tuple[str, AttributeKey.Type.ValueType],
Expand All @@ -69,6 +76,16 @@
AttributeKey.Type.TYPE_STRING,
),
}
# for every AttributeKey of TYPE_FLOAT a user may add during the backward compat period, this adds the TYPE_DOUBLE equivalent
_attributes_backward_compat = dict()
for k in _ATTRIBUTES:
v = _ATTRIBUTES[k]
if v[1] == AttributeKey.Type.TYPE_FLOAT:
_attributes_backward_compat[
_convert_key_to_support_doubles_and_floats_for_backward_compat(k)
] = (v[0], AttributeKey.Type.TYPE_DOUBLE)
_ATTRIBUTES.update(_attributes_backward_compat)

_TYPES_TO_CLICKHOUSE: dict[
AttributeKey.Type.ValueType,
tuple[str, Callable[[Any], AttributeValue]],
Expand All @@ -85,6 +102,10 @@
"Float64",
lambda x: AttributeValue(val_float=float(x)),
),
AttributeKey.Type.TYPE_DOUBLE: (
"Float64",
lambda x: AttributeValue(val_double=float(x)),
),
}


Expand All @@ -102,11 +123,19 @@ def _attribute_to_expression(
alias=_ATTRIBUTES[trace_attribute.key][0],
)
if trace_attribute.key == TraceAttribute.Key.KEY_START_TIMESTAMP:
attribute = _ATTRIBUTES[trace_attribute.key]
attribute = (
_ATTRIBUTES[
_convert_key_to_support_doubles_and_floats_for_backward_compat(
trace_attribute.key
)
]
if trace_attribute.type == AttributeKey.Type.TYPE_DOUBLE
else _ATTRIBUTES[trace_attribute.key]
)
return f.cast(
f.min(column("start_timestamp")),
_TYPES_TO_CLICKHOUSE[attribute[1]][0],
alias=_ATTRIBUTES[trace_attribute.key][0],
alias=attribute[0],
)
if trace_attribute.key == TraceAttribute.Key.KEY_ROOT_SPAN_NAME:
# TODO: Change to return the root span name instead of the trace's first span's name.
Expand All @@ -116,7 +145,15 @@ def _attribute_to_expression(
alias=_ATTRIBUTES[trace_attribute.key][0],
)
if trace_attribute.key in _ATTRIBUTES:
attribute = _ATTRIBUTES[trace_attribute.key]
attribute = (
_ATTRIBUTES[
_convert_key_to_support_doubles_and_floats_for_backward_compat(
trace_attribute.key
)
]
if trace_attribute.type == AttributeKey.Type.TYPE_DOUBLE
else _ATTRIBUTES[trace_attribute.key]
)
return f.cast(
column(attribute[0]),
_TYPES_TO_CLICKHOUSE[attribute[1]][0],
Expand Down Expand Up @@ -165,8 +202,15 @@ def _convert_results(
TraceAttribute,
] = defaultdict(TraceAttribute)
for attribute in request.attributes:
value = row[_ATTRIBUTES[attribute.key][0]]
type = _ATTRIBUTES[attribute.key][1]
backward_compat_attribute_key = (
_convert_key_to_support_doubles_and_floats_for_backward_compat(
attribute.key
)
if attribute.type == AttributeKey.Type.TYPE_DOUBLE
else attribute.key
)
value = row[_ATTRIBUTES[backward_compat_attribute_key][0]]
type = _ATTRIBUTES[backward_compat_attribute_key][1]
values[attribute.key] = TraceAttribute(
key=attribute.key,
value=_TYPES_TO_CLICKHOUSE[type][1](value),
Expand Down
1 change: 1 addition & 0 deletions snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def convert_to_snuba_request(req: TraceItemAttributeNamesRequest) -> SnubaReques
)
elif req.type in (
AttributeKey.Type.TYPE_FLOAT,
AttributeKey.Type.TYPE_DOUBLE,
AttributeKey.Type.TYPE_INT,
AttributeKey.Type.TYPE_BOOLEAN,
):
Expand Down
12 changes: 10 additions & 2 deletions snuba/web/rpc/v1/resolvers/R_eap_spans/common/aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

CUSTOM_COLUMN_PREFIX = "__snuba_custom_column__"

_FLOATING_POINT_PRECISION = 8


@dataclass(frozen=True)
class ExtrapolationContext(ABC):
Expand Down Expand Up @@ -612,7 +614,11 @@ def aggregation_to_expression(aggregation: AttributeAggregation) -> Expression:
alias = aggregation.label if aggregation.label else None
alias_dict = {"alias": alias} if alias else {}
function_map: dict[Function.ValueType, CurriedFunctionCall | FunctionCall] = {
Function.FUNCTION_SUM: f.sum(f.multiply(field, sign_column), **alias_dict),
Function.FUNCTION_SUM: f.round(
f.sum(f.multiply(field, sign_column)),
_FLOATING_POINT_PRECISION,
**alias_dict,
),
Function.FUNCTION_AVERAGE: f.divide(
f.sum(f.multiply(field, sign_column)),
f.sumIf(sign_column, get_field_existence_expression(aggregation)),
Expand All @@ -628,7 +634,9 @@ def aggregation_to_expression(aggregation: AttributeAggregation) -> Expression:
Function.FUNCTION_P90: cf.quantile(0.9)(field, **alias_dict),
Function.FUNCTION_P95: cf.quantile(0.95)(field, **alias_dict),
Function.FUNCTION_P99: cf.quantile(0.99)(field, **alias_dict),
Function.FUNCTION_AVG: f.avg(field, **alias_dict),
Function.FUNCTION_AVG: f.round(
f.avg(field), _FLOATING_POINT_PRECISION, **alias_dict
),
Function.FUNCTION_MAX: f.max(field, **alias_dict),
Function.FUNCTION_MIN: f.min(field, **alias_dict),
Function.FUNCTION_UNIQ: f.uniq(field, **alias_dict),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ def _build_query(request: TraceItemTableRequest) -> Query:
raise BadSnubaRPCRequestException(
"Column is neither an aggregate or an attribute"
)

res = Query(
from_clause=entity,
selected_columns=selected_columns,
Expand Down Expand Up @@ -250,8 +249,10 @@ def _convert_results(
converters[column.label] = lambda x: AttributeValue(val_int=int(x))
elif column.key.type == AttributeKey.TYPE_FLOAT:
converters[column.label] = lambda x: AttributeValue(val_float=float(x))
elif column.key.type == AttributeKey.TYPE_DOUBLE:
converters[column.label] = lambda x: AttributeValue(val_double=float(x))
elif column.HasField("aggregation"):
converters[column.label] = lambda x: AttributeValue(val_float=float(x))
converters[column.label] = lambda x: AttributeValue(val_double=float(x))
else:
raise BadSnubaRPCRequestException(
"column is neither an attribute or aggregation"
Expand Down
14 changes: 14 additions & 0 deletions tests/web/rpc/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ def test_timestamp_columns(self) -> None:
) == f.CAST(
column(col[len("sentry.") :]), "Float64", alias=col + "_TYPE_FLOAT"
)
assert attribute_key_to_expression(
AttributeKey(
type=AttributeKey.TYPE_DOUBLE,
name=col,
),
) == f.CAST(
column(col[len("sentry.") :]), "Float64", alias=col + "_TYPE_DOUBLE"
)

def test_normalized_col(self) -> None:
for col in [
Expand Down Expand Up @@ -71,6 +79,12 @@ def test_attributes(self) -> None:
alias="derp_TYPE_FLOAT", column=column("attr_num"), key=literal("derp")
)

assert attribute_key_to_expression(
AttributeKey(type=AttributeKey.TYPE_DOUBLE, name="derp"),
) == SubscriptableReference(
alias="derp_TYPE_DOUBLE", column=column("attr_num"), key=literal("derp")
)

assert attribute_key_to_expression(
AttributeKey(type=AttributeKey.TYPE_INT, name="derp"),
) == f.CAST(
Expand Down
Loading
Loading