diff --git a/pyproject.toml b/pyproject.toml index 2d7ed63e83..d2efb29b63 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,7 @@ dependencies = [ "sentry-conventions>=0.15.0", "sentry-kafka-schemas>=2.1.36", "sentry-options>=1.1.1", - "sentry-protos>=0.34.0", + "sentry-protos>=0.40.0", "sentry-redis-tools>=0.5.1", "sentry-relay>=0.9.25", "sentry-sdk>=2.35.0", diff --git a/snuba/web/rpc/common/common.py b/snuba/web/rpc/common/common.py index 69a3df324e..554018c5ea 100644 --- a/snuba/web/rpc/common/common.py +++ b/snuba/web/rpc/common/common.py @@ -90,6 +90,54 @@ def attribute_key_to_expression(attr_key: AttributeKey) -> Expression: raise BadSnubaRPCRequestException(str(e)) from e +_SEMVER_COMPONENT_COUNT = 4 # major.minor.patch.build + + +def semver_sort_key(expr: Expression, alias: str | None = None) -> Expression: + """Return a Tuple(Array(UInt32), UInt8, String) sort key for semantic-version ORDER BY. + + This is the sort applied for the ``SORT_NATURAL`` OrderBy option: callers opt + in per request (there is no hardcoded attribute list), and the key is applied + to whatever column they order by. + + Strips a leading 'package@' prefix, isolates the release part (before '-'), + maps each dot-component to UInt32, pads to 4 elements so "1.2" == "1.2.0", + and adds a stability flag (0=prerelease, 1=stable) so prerelease versions sort + before their corresponding stable release. Works on Altinity 25.3/25.8. + + The third tuple element is the raw (non-null) string. It is a tiebreaker: + distinct strings that map to the same numeric key + stability flag (e.g. "1.2" + and "1.2.0", or two prereleases of the same version) would otherwise compare + equal, which makes ordering non-deterministic and — more importantly — lets + the flextime pagination's strict `less` boundary filter skip rows tied with + the cursor. Appending the raw string gives a deterministic total order over + distinct release strings and keeps the page-boundary comparison exact. Both + ORDER BY and pagination call this function, so they stay consistent. + """ + x = Argument(None, "x") + # sentry.release is coalesced from multiple attribute columns and therefore + # returns Nullable(String). ClickHouse forbids Nullable(Array(…)), so strip + # the nullable wrapper before applying any string → array functions. + non_null = f.ifNull(expr, literal("")) + version_no_prefix = f.arrayElement(f.splitByChar(literal("@"), non_null), literal(-1)) + release_part = f.arrayElement(f.splitByChar(literal("-"), version_no_prefix), literal(1)) + numeric_key = f.arrayResize( + f.arrayMap( + Lambda(None, ("x",), f.toUInt32OrZero(x)), + f.splitByChar(literal("."), release_part), + ), + literal(_SEMVER_COMPONENT_COUNT), + ) + # A release is "stable" only when the version is a plain dotted-numeric + # string ("1.2.3", "1.2"). Anything else is a prerelease and sorts before + # the matching stable release: SemVer prereleases ("1.2.3-beta.1") as well as + # PEP 440 dot-style dev/pre builds ("24.7.0.dev0+", the common form on + # sentry.release) — the latter has no '-', so a bare '-' check would wrongly + # tag it stable and sort the dev build after its GA release. + is_stable = f.match(version_no_prefix, literal(r"^[0-9]+(\.[0-9]+)*$")) + return FunctionCall(alias, "tuple", (numeric_key, is_stable, non_null)) + + def _trace_item_filter_key_expression( attr_to_key_expression_callable: Callable[[AttributeKey], Expression], key: AttributeKey, diff --git a/snuba/web/rpc/common/pagination.py b/snuba/web/rpc/common/pagination.py index 5abcd23f9c..32f1650a0d 100644 --- a/snuba/web/rpc/common/pagination.py +++ b/snuba/web/rpc/common/pagination.py @@ -18,7 +18,10 @@ from snuba.query.dsl import Functions as f from snuba.query.dsl import column, literal from snuba.query.expressions import Expression -from snuba.web.rpc.common.common import attribute_key_to_expression +from snuba.web.rpc.common.common import ( + attribute_key_to_expression, + semver_sort_key, +) from snuba.web.rpc.storage_routing.routing_strategies.storage_routing import TimeWindow @@ -27,6 +30,9 @@ class FlexibleTimeWindowPageWithFilters: _TIME_WINDOW_START_KEY = f"{_TIME_WINDOW_PREFIX}.start_timestamp" _TIME_WINDOW_END_KEY = f"{_TIME_WINDOW_PREFIX}.end_timestamp" _FILTER_PREFIX = "sentry__filter" + # Marks a page-boundary column whose ORDER BY used SORT_NATURAL (semver), so + # get_filters applies the same semver key on both sides of the comparison. + _NATURAL_FILTER_PREFIX = "sentry__natural_filter" def __init__(self, page_token: PageToken): self._page_token = page_token @@ -70,38 +76,57 @@ def get_filters(self) -> Expression | None: column_names: list[str] = [] column_values: list[Expression] = [] + # Parallel to column_names: True when that column's ORDER BY used + # SORT_NATURAL, so the boundary comparison must use the semver key too. + column_is_natural: list[bool] = [] for filter in self.page_token.filter_offset.and_filter.filters: - if filter.HasField( - "comparison_filter" - ) and filter.comparison_filter.key.name.startswith(self._FILTER_PREFIX): - if filter.comparison_filter.key.name == f"{self._FILTER_PREFIX}.timestamp": - column_names.append("timestamp") - if filter.comparison_filter.value.HasField("val_str"): - column_values.append(f.toDateTime(filter.comparison_filter.value.val_str)) - elif filter.comparison_filter.value.HasField("val_double"): - column_values.append(literal(filter.comparison_filter.value.val_double)) - elif filter.comparison_filter.value.HasField("val_int"): - column_values.append(literal(filter.comparison_filter.value.val_int)) - - else: - # strip the _FILTER_PREFIX from the attribute key and the dot - column_names.append( - filter.comparison_filter.key.name[len(self._FILTER_PREFIX) + 1 :] - ) - column_values.append( - literal( - getattr( - filter.comparison_filter.value, - str(filter.comparison_filter.value.WhichOneof("value")), - ) + if not filter.HasField("comparison_filter"): + continue + key_name = filter.comparison_filter.key.name + is_natural = key_name.startswith(f"{self._NATURAL_FILTER_PREFIX}.") + is_regular = key_name.startswith(f"{self._FILTER_PREFIX}.") + if not (is_natural or is_regular): + continue + + if key_name == f"{self._FILTER_PREFIX}.timestamp": + column_names.append("timestamp") + column_is_natural.append(False) + if filter.comparison_filter.value.HasField("val_str"): + column_values.append(f.toDateTime(filter.comparison_filter.value.val_str)) + elif filter.comparison_filter.value.HasField("val_double"): + column_values.append(literal(filter.comparison_filter.value.val_double)) + elif filter.comparison_filter.value.HasField("val_int"): + column_values.append(literal(filter.comparison_filter.value.val_int)) + else: + # strip the matching prefix (and the dot) to recover the alias + prefix = self._NATURAL_FILTER_PREFIX if is_natural else self._FILTER_PREFIX + column_names.append(key_name[len(prefix) + 1 :]) + column_is_natural.append(is_natural) + column_values.append( + literal( + getattr( + filter.comparison_filter.value, + str(filter.comparison_filter.value.WhichOneof("value")), ) ) + ) # Assumes everything in the ORDER BY is ordered by DESC if column_names: - res = f.less( - f.tuple(*(column(c_name) for c_name in column_names)), f.tuple(*column_values) - ) + col_exprs = [] + val_exprs = [] + for c_name, c_value, is_natural in zip( + column_names, column_values, column_is_natural, strict=True + ): + # For SORT_NATURAL columns, apply the same semver key on both sides + # so the page-boundary comparison uses the same ordering as ORDER BY. + if is_natural: + col_exprs.append(semver_sort_key(column(c_name))) + val_exprs.append(semver_sort_key(c_value)) + else: + col_exprs.append(column(c_name)) + val_exprs.append(c_value) + res = f.less(f.tuple(*col_exprs), f.tuple(*val_exprs)) return res return None @@ -177,22 +202,35 @@ def create( # find the attribute in the in_msg.columns attribute that has the same label as the `column` attribute in the order_by_clause # call `attribute_key_to_expression` on it and us its alias as the name of the AttributeKey in the ComparisonFilter attribute_expression = None + selected_key = None for selected_column in in_msg.columns: if selected_column.label == order_by_clause.column.label: attribute_expression = attribute_key_to_expression( selected_column.key ) + selected_key = selected_column.key break if attribute_expression is None: raise ValueError( f"No attribute expression found for column: {order_by_clause.column.label}" ) + # Mark the column as SORT_NATURAL (semver) when the request + # ordered it that way, so get_filters wraps both sides in the + # semver key and the page boundary matches the ORDER BY. Mirror + # the resolver's guard: only string columns get the semver key. + is_natural = ( + order_by_clause.sort == TraceItemTableRequest.OrderBy.SORT_NATURAL + and selected_key is not None + and selected_key.type == AttributeKey.TYPE_STRING + ) + prefix = cls._NATURAL_FILTER_PREFIX if is_natural else cls._FILTER_PREFIX + filters.append( TraceItemFilter( comparison_filter=ComparisonFilter( key=AttributeKey( - name=f"{cls._FILTER_PREFIX}.{attribute_expression.alias}", + name=f"{prefix}.{attribute_expression.alias}", ), op=ComparisonFilter.OP_LESS_THAN, value=last_result_value, diff --git a/snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py b/snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py index dc737bd35d..a3037576b8 100644 --- a/snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py +++ b/snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py @@ -1,4 +1,7 @@ +import re import uuid +from collections.abc import Mapping +from typing import Any from google.protobuf.json_format import MessageToDict from sentry_protos.snuba.v1.endpoint_trace_item_attributes_pb2 import ( @@ -33,6 +36,7 @@ next_monday, prev_monday, project_id_and_org_conditions, + semver_sort_key, treeify_or_and_conditions, ) from snuba.web.rpc.common.debug_info import extract_response_meta @@ -79,6 +83,44 @@ def _order_by_name_descending(request: TraceItemAttributeNamesRequest) -> bool: ) +def _order_by_natural(request: TraceItemAttributeNamesRequest) -> bool: + """Whether the caller requested SORT_NATURAL (semver) ordering of names.""" + return request.order_by.sort == TraceItemAttributeNamesRequest.OrderBy.SORT_NATURAL + + +_SEMVER_NUMERIC_RE = re.compile(r"^[0-9]+(\.[0-9]+)*$") + + +def _semver_sort_key_py(name: str) -> tuple[tuple[int, int, int, int], int, str]: + """Python mirror of common.semver_sort_key, used to re-sort names in Python so + the merged (ClickHouse + synthetic) result matches the ClickHouse ORDER BY.""" + non_null = name or "" + version_no_prefix = non_null.split("@")[-1] + release_part = version_no_prefix.split("-")[0] + # Mirror ClickHouse toUInt32OrZero: only ASCII decimal parses, anything else + # (including Unicode digits like "²", where str.isdigit() is True but int() + # raises) maps to 0. + components = [int(c) if (c.isascii() and c.isdigit()) else 0 for c in release_part.split(".")] + components = (components + [0, 0, 0, 0])[:4] + is_stable = 1 if _SEMVER_NUMERIC_RE.match(version_no_prefix) else 0 + return ( + (components[0], components[1], components[2], components[3]), + is_stable, + non_null, + ) + + +def _name_order_by_expression(natural: bool) -> Expression: + """ClickHouse ORDER BY expression for the attribute name. + + Default orders by the raw (type, name) tuple; SORT_NATURAL orders by the + semver key of the name part so versions sort numerically. + """ + if natural: + return semver_sort_key(f.tupleElement(column("attr_key"), 2)) + return column("attr_key") + + class AttributeKeyCollector(ProtoVisitor): def __init__(self) -> None: self.keys: set[str] = set() @@ -344,6 +386,7 @@ def get_co_occurring_attributes( alias="attr_key", ) + natural = _order_by_natural(request) if _order_by_count(request): # Opt-in frequency ordering: group by key and count how many rows # (co-occurring attribute sets) contain each key. @@ -359,8 +402,9 @@ def get_co_occurring_attributes( ), expression=column("count"), ), - # stable tiebreak for keys with the same frequency - OrderBy(direction=OrderByDirection.ASC, expression=column("attr_key")), + # stable tiebreak for keys with the same frequency (semver key when + # SORT_NATURAL was requested) + OrderBy(direction=OrderByDirection.ASC, expression=_name_order_by_expression(natural)), ] else: # Default (order_by unset or COLUMN_NAME): distinct keys ordered by name. @@ -374,7 +418,7 @@ def get_co_occurring_attributes( order_by = [ OrderBy( direction=OrderByDirection.DESC if name_descending else OrderByDirection.ASC, - expression=column("attr_key"), + expression=_name_order_by_expression(natural), ), ] @@ -428,6 +472,17 @@ def t(row: Row) -> TraceItemAttributeNamesResponse.Attribute: attribute.count = int(count) return attribute + # Name-ordering key that mirrors the ClickHouse ORDER BY: the raw (type, name) + # tuple by default, or the semver key of the name under SORT_NATURAL. + natural = _order_by_natural(request) + + def _name_key(row: Mapping[str, Any]) -> Any: + attr_key = row.get("attr_key", ("TYPE_STRING", "")) + attr_type, attr_name = attr_key[0], attr_key[1] + if natural: + return _semver_sort_key_py(attr_name) + return (attr_type, attr_name) + data = query_res.result.get("data", []) if request.type in (AttributeKey.TYPE_UNSPECIFIED, AttributeKey.TYPE_STRING): non_stored = [ @@ -435,13 +490,13 @@ def t(row: Row) -> TraceItemAttributeNamesResponse.Attribute: for key_name in NON_STORED_ATTRIBUTE_KEYS if request.value_substring_match in key_name ] - non_stored.sort(key=lambda row: tuple(row["attr_key"])) + non_stored.sort(key=_name_key) if _order_by_count(request): # Order the real (counted) rows to match ClickHouse: count in the # requested direction, then name ASC (two stable passes). The synthetic # non-stored attributes have no real count, so pin them first regardless # of sort direction rather than relying on a sentinel value. - data.sort(key=lambda row: tuple(row.get("attr_key", ("TYPE_STRING", "")))) + data.sort(key=_name_key) data.sort(key=lambda row: row.get("count", 0), reverse=request.order_by.descending) data = non_stored + data else: @@ -450,7 +505,7 @@ def t(row: Row) -> TraceItemAttributeNamesResponse.Attribute: # ORDER BY (a COLUMN_NAME descending request must stay descending here too). data.extend(non_stored) data.sort( - key=lambda row: tuple(row.get("attr_key", ("TYPE_STRING", ""))), + key=_name_key, reverse=_order_by_name_descending(request), ) diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py index edc018a1cb..cec19afed4 100644 --- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py +++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py @@ -64,6 +64,7 @@ attribute_key_to_expression, base_conditions_and, get_field_existence_expression, + semver_sort_key, timestamp_in_range_condition, trace_item_filters_to_expression, treeify_or_and_conditions, @@ -285,6 +286,10 @@ def _groupby_order_by_expression(attr_key: AttributeKey) -> Expression: while keeping the cast valid as a function of the grouped column. TYPE_ARRAY keys are rejected upstream (see _validate_select_and_groupby / _validate_order_by), so they never reach here. + + The SORT_NATURAL (semver) ordering is applied by `_convert_order_by`, not here, so + GROUP BY keeps the raw expression while ORDER BY can wrap it in the semver key. + ClickHouse accepts ORDER BY on a function of a GROUP BY key, so the two can differ. """ if attr_key.name == "sentry.timestamp": return snuba_column("timestamp") @@ -340,10 +345,19 @@ def _convert_order_by( # covers `sentry.timestamp` ordering anywhere else.) GROUP BY uses the same # expression so an aggregation query that orders by `sentry.timestamp` stays # valid. + expression = _groupby_order_by_expression(x.column.key) + # SORT_NATURAL requests semver-aware ordering (client-driven, no hardcoded + # attribute list). It only applies to string columns; numeric/timestamp + # columns already sort numerically, so leave them untouched. + if ( + x.sort == TraceItemTableRequest.OrderBy.SORT_NATURAL + and x.column.key.type == AttributeKey.TYPE_STRING + ): + expression = semver_sort_key(expression) res.append( OrderBy( direction=direction, - expression=_groupby_order_by_expression(x.column.key), + expression=expression, ) ) elif x.column.HasField("conditional_aggregation"): diff --git a/snuba/web/rpc/v1/trace_item_attribute_values.py b/snuba/web/rpc/v1/trace_item_attribute_values.py index f569c44353..683897a21a 100644 --- a/snuba/web/rpc/v1/trace_item_attribute_values.py +++ b/snuba/web/rpc/v1/trace_item_attribute_values.py @@ -30,6 +30,7 @@ add_existence_check_to_subscriptable_references, attribute_key_to_expression, base_conditions_and, + semver_sort_key, treeify_or_and_conditions, ) from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException @@ -136,6 +137,17 @@ def _build_query( ) treeify_or_and_conditions(inner_query) add_existence_check_to_subscriptable_references(inner_query) + # The value column normally orders lexicographically. When the caller opts + # into SORT_NATURAL, order it by the semver key so versions sort numerically + # ("1.2.9" before "1.2.10") with prereleases before their stable release. + # count() stays the primary ordering so the most common values still come + # first; the sort key only changes the tiebreak among equally frequent values. + # An unset/SORT_DEFAULT sort keeps the historical lexicographic order. + if request.order_by.sort == TraceItemAttributeValuesRequest.OrderBy.SORT_NATURAL: + value_order_expression: Expression = semver_sort_key(column("attr_value")) + else: + value_order_expression = column("attr_value") + res = CompositeQuery( from_clause=inner_query, selected_columns=[ @@ -154,7 +166,7 @@ def _build_query( ], order_by=[ OrderBy(direction=OrderByDirection.DESC, expression=column("count()")), - OrderBy(direction=OrderByDirection.ASC, expression=column("attr_value")), + OrderBy(direction=OrderByDirection.ASC, expression=value_order_expression), ], groupby=[column("attr_value")], limit=request.limit, diff --git a/tests/web/rpc/test_common.py b/tests/web/rpc/test_common.py index 3de48d1c8c..b6d7bd6cd5 100644 --- a/tests/web/rpc/test_common.py +++ b/tests/web/rpc/test_common.py @@ -56,6 +56,7 @@ dedupe_and_conditions, next_monday, prev_monday, + semver_sort_key, trace_item_filters_to_expression, treeify_or_and_conditions, use_array_map_columns, @@ -1309,6 +1310,39 @@ def test_not_like_wildcard_matches_only_absent(self) -> None: assert self._execute(ComparisonFilter.OP_NOT_LIKE, value="%") == ["green"] +class TestSemverSortKey: + def test_expression_structure(self) -> None: + from snuba.query.dsl import column as snuba_column + + expr = semver_sort_key(snuba_column("release")) + assert isinstance(expr, FunctionCall) + assert expr.function_name == "tuple" + assert len(expr.parameters) == 3 + numeric_key_expr, is_stable_expr, raw_str_expr = expr.parameters + assert isinstance(numeric_key_expr, FunctionCall) + assert numeric_key_expr.function_name == "arrayResize" + # Stability flag: match(version, '^[0-9]+(\\.[0-9]+)*$') — 1 for a plain + # dotted-numeric (stable) version, 0 for any prerelease form. + assert isinstance(is_stable_expr, FunctionCall) + assert is_stable_expr.function_name == "match" + # Third element is the raw (non-null) string tiebreaker: ifNull(expr, ''). + assert isinstance(raw_str_expr, FunctionCall) + assert raw_str_expr.function_name == "ifNull" + + def test_alias_is_forwarded(self) -> None: + from snuba.query.dsl import column as snuba_column + + expr = semver_sort_key(snuba_column("release"), alias="semver_key") + assert isinstance(expr, FunctionCall) + assert expr.alias == "semver_key" + + def test_no_alias_by_default(self) -> None: + from snuba.query.dsl import column as snuba_column + + expr = semver_sort_key(snuba_column("release")) + assert expr.alias is None + + class TestAnyAttributeFilterOption: """The `enable_any_attribute_filter` sentry-option gates whether any_attribute_filter is translated into a predicate or treated as diff --git a/tests/web/rpc/v1/test_endpoint_trace_item_attribute_names.py b/tests/web/rpc/v1/test_endpoint_trace_item_attribute_names.py index d29345f6a2..af321e5446 100644 --- a/tests/web/rpc/v1/test_endpoint_trace_item_attribute_names.py +++ b/tests/web/rpc/v1/test_endpoint_trace_item_attribute_names.py @@ -99,6 +99,39 @@ def test_basic(self) -> None: ) assert res.attributes == expected + def test_natural_sort(self) -> None: + # Version-like attribute names: lexicographically "1.2.10" < "1.2.2" < + # "1.2.9"; with SORT_NATURAL (semver) they order "1.2.2" < "1.2.9" < "1.2.10". + items_storage = get_writable_storage(StorageKey("eap_items")) + write_raw_unprocessed_events( + items_storage, + [ + gen_item_message( + start_timestamp=BASE_TIME, + attributes={name: AnyValue(string_value="x")}, + ) + for name in ("1.2.9", "1.2.10", "1.2.2") + ], + ) + req = TraceItemAttributeNamesRequest( + meta=RequestMeta( + project_ids=[1, 2, 3], + organization_id=1, + cogs_category="something", + referrer="something", + start_timestamp=Timestamp(seconds=int((BASE_TIME - timedelta(days=1)).timestamp())), + end_timestamp=Timestamp(seconds=int((BASE_TIME + timedelta(days=1)).timestamp())), + ), + limit=100, + type=AttributeKey.Type.TYPE_STRING, + value_substring_match="1.2", + order_by=TraceItemAttributeNamesRequest.OrderBy( + sort=TraceItemAttributeNamesRequest.OrderBy.SORT_NATURAL, + ), + ) + res = EndpointTraceItemAttributeNames().execute(req) + assert [a.name for a in res.attributes] == ["1.2.2", "1.2.9", "1.2.10"] + def test_simple_float_backward_compat(self) -> None: req = TraceItemAttributeNamesRequest( meta=RequestMeta( diff --git a/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.py b/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.py index 3103f3d8e9..70e21f59cf 100644 --- a/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.py +++ b/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.py @@ -4506,3 +4506,132 @@ def test_order_by_bug() -> None: ) with pytest.raises(BadSnubaRPCRequestException, match=error_message): _validate_order_by(message) + + +@pytest.mark.clickhouse_db +@pytest.mark.redis_db +class TestSemverSorting: + """ORDER BY with the SORT_NATURAL option applies the semver key so versions + sort numerically (1.2.9 before 1.2.10) with pre-releases before their + corresponding stable release. Without SORT_NATURAL the ordering stays + lexicographic (there is no hardcoded per-attribute behavior). + """ + + _RELEASES = [ + "1.2.9", + "1.2.10", + "1.2.3", + "1.2.3-beta.1", + "1.2.2", + "my-pkg@2.0.0", + "1.2", + "1.2.0", + # PEP 440 dot-style dev build (Sentry's own sentry.release format) and + # its GA release, to verify the dev build sorts before GA. + "24.7.0.dev0+abc123", + "24.7.0", + ] + + @pytest.fixture(autouse=True) + def _setup(self, clickhouse_db: None, redis_db: None) -> None: + for i, release in enumerate(self._RELEASES): + write_eap_item( + start_timestamp=BASE_TIME + timedelta(minutes=i), + raw_attributes={"sentry.release": release, "semver_test_marker": "1"}, + ) + + def _query_releases(self, descending: bool = False, natural: bool = True) -> list[str]: + sort = ( + TraceItemTableRequest.OrderBy.SORT_NATURAL + if natural + else TraceItemTableRequest.OrderBy.SORT_UNSPECIFIED + ) + message = TraceItemTableRequest( + meta=RequestMeta( + project_ids=[1], + organization_id=1, + cogs_category="something", + referrer="something", + start_timestamp=START_TIMESTAMP, + end_timestamp=END_TIMESTAMP, + trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, + ), + filter=TraceItemFilter( + exists_filter=ExistsFilter( + key=AttributeKey(type=AttributeKey.TYPE_STRING, name="semver_test_marker") + ) + ), + columns=[ + Column(key=AttributeKey(type=AttributeKey.TYPE_STRING, name="sentry.release")) + ], + order_by=[ + TraceItemTableRequest.OrderBy( + column=Column( + key=AttributeKey(type=AttributeKey.TYPE_STRING, name="sentry.release") + ), + descending=descending, + sort=sort, + ) + ], + limit=len(self._RELEASES) + 10, + ) + response = EndpointTraceItemTable().execute(message) + return [v.val_str for v in response.column_values[0].results] + + def test_numeric_ordering(self) -> None: + releases = self._query_releases() + assert releases.index("1.2.9") < releases.index("1.2.10"), ( + "1.2.9 must sort before 1.2.10 (numeric, not lexicographic)" + ) + + def test_default_sort_is_lexicographic(self) -> None: + # Without SORT_NATURAL there is no semver behavior (no hardcoded + # attributes), so plain lexicographic order applies: "1.2.10" < "1.2.9". + releases = self._query_releases(natural=False) + assert releases.index("1.2.10") < releases.index("1.2.9"), ( + "without SORT_NATURAL, ordering is lexicographic" + ) + + def test_prerelease_before_stable(self) -> None: + releases = self._query_releases() + assert releases.index("1.2.3-beta.1") < releases.index("1.2.3"), ( + "prerelease 1.2.3-beta.1 must sort before stable 1.2.3" + ) + + def test_dot_dev_prerelease_before_stable(self) -> None: + releases = self._query_releases() + assert releases.index("24.7.0.dev0+abc123") < releases.index("24.7.0"), ( + "PEP 440 dot-style dev build must sort before its GA release" + ) + + def test_prerelease_of_newer_after_older_stable(self) -> None: + releases = self._query_releases() + assert releases.index("1.2.3-beta.1") > releases.index("1.2.2"), ( + "prerelease 1.2.3-beta.1 must sort after older stable 1.2.2" + ) + + def test_package_prefix_stripped(self) -> None: + releases = self._query_releases() + assert releases.index("my-pkg@2.0.0") > releases.index("1.2.10"), ( + "my-pkg@2.0.0 should sort as version 2.0.0 (after 1.x)" + ) + + def test_length_normalisation(self) -> None: + releases = self._query_releases() + idx_12 = releases.index("1.2") + idx_120 = releases.index("1.2.0") + # Both normalise to [1,2,0,0], so they are adjacent. The raw-string + # tiebreaker then breaks the tie deterministically: "1.2" < "1.2.0". + assert idx_120 - idx_12 == 1, ( + "1.2 and 1.2.0 normalise equally and must be adjacent, with '1.2' " + "first via the raw-string tiebreaker" + ) + + def test_desc_is_reverse_of_asc(self) -> None: + # The raw-string tiebreaker in semver_sort_key gives distinct release + # strings a deterministic total order (e.g. "1.2" < "1.2.0"), so DESC is + # the exact reverse of ASC even for versions that share the same numeric + # key. + asc = self._query_releases(descending=False) + desc = self._query_releases(descending=True) + assert asc == list(reversed(desc)) diff --git a/tests/web/rpc/v1/test_trace_item_attribute_values_v1.py b/tests/web/rpc/v1/test_trace_item_attribute_values_v1.py index 3467e698bd..6296d556be 100644 --- a/tests/web/rpc/v1/test_trace_item_attribute_values_v1.py +++ b/tests/web/rpc/v1/test_trace_item_attribute_values_v1.py @@ -148,6 +148,88 @@ def test_simple_case(self, setup_teardown: Any) -> None: assert response.values == ["derpderp", "blah", "durp", "herp", "herpderp"] assert response.counts == [2, 1, 1, 1, 1] + def _write_version_values(self) -> None: + # Distinct version-like values (one occurrence each). Lexicographically + # "1.2.10" < "1.2.2" < "1.2.9"; naturally "1.2.2" < "1.2.9" < "1.2.10". + # Written per-test (not in the shared fixture) so the extra items don't + # perturb the count-based assertions in other tests. + items_storage = get_writable_storage(StorageKey("eap_items")) + write_raw_unprocessed_events( + items_storage, + [ + gen_item_message( + start_timestamp=BASE_TIME, + attributes={"natural_ver": AnyValue(string_value=v)}, + ) + for v in ("1.2.9", "1.2.10", "1.2.2") + ], + ) + + def test_natural_sort(self, setup_teardown: Any) -> None: + # SORT_NATURAL applies the semver key, so version components sort + # numerically (1.2.2 < 1.2.9 < 1.2.10) regardless of the attribute. + self._write_version_values() + message = TraceItemAttributeValuesRequest( + meta=COMMON_META, + limit=10, + key=AttributeKey(name="natural_ver", type=AttributeKey.TYPE_STRING), + order_by=TraceItemAttributeValuesRequest.OrderBy( + column=TraceItemAttributeValuesRequest.OrderBy.COLUMN_VALUE, + sort=TraceItemAttributeValuesRequest.OrderBy.SORT_NATURAL, + ), + ) + response = AttributeValuesRequest().execute(message) + assert response.values == ["1.2.2", "1.2.9", "1.2.10"] + + def test_default_sort_is_lexicographic(self, setup_teardown: Any) -> None: + # Unset sort keeps the historical lexicographic ordering, where "1.2.10" + # sorts before "1.2.2" and "1.2.9". + self._write_version_values() + message = TraceItemAttributeValuesRequest( + meta=COMMON_META, + limit=10, + key=AttributeKey(name="natural_ver", type=AttributeKey.TYPE_STRING), + ) + response = AttributeValuesRequest().execute(message) + assert response.values == ["1.2.10", "1.2.2", "1.2.9"] + + def test_release_sort_is_semver_aware(self, setup_teardown: Any) -> None: + # SORT_NATURAL is the semver sort, so prerelease sorts before its stable + # release and the "pkg@" prefix is stripped. + items_storage = get_writable_storage(StorageKey("eap_items")) + releases = ["1.2.3-beta.1", "1.2.3", "1.2.9", "1.2.10", "my-pkg@2.0.0"] + write_raw_unprocessed_events( + items_storage, + [ + gen_item_message( + start_timestamp=BASE_TIME, + attributes={"sentry.release": AnyValue(string_value=r)}, + ) + for r in releases + ], + ) + message = TraceItemAttributeValuesRequest( + meta=COMMON_META, + limit=10, + key=AttributeKey(name="sentry.release", type=AttributeKey.TYPE_STRING), + order_by=TraceItemAttributeValuesRequest.OrderBy( + column=TraceItemAttributeValuesRequest.OrderBy.COLUMN_VALUE, + sort=TraceItemAttributeValuesRequest.OrderBy.SORT_NATURAL, + ), + ) + response = AttributeValuesRequest().execute(message) + # gen_item_message stamps a default sentry.release on every fixture item, + # so other release values may appear; assert only the relative order of + # the releases we wrote, which must follow semver ordering. + ordered = [v for v in response.values if v in set(releases)] + assert ordered == [ + "1.2.3-beta.1", + "1.2.3", + "1.2.9", + "1.2.10", + "my-pkg@2.0.0", + ] + def test_with_value_substring_match(self, setup_teardown: Any) -> None: message = TraceItemAttributeValuesRequest( meta=COMMON_META, diff --git a/uv.lock b/uv.lock index 8df52ba2ee..aa0711b7c8 100644 --- a/uv.lock +++ b/uv.lock @@ -1023,7 +1023,7 @@ wheels = [ [[package]] name = "sentry-protos" -version = "0.34.0" +version = "0.40.0" source = { registry = "https://pypi.devinfra.sentry.io/simple" } dependencies = [ { name = "grpc-stubs", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, @@ -1031,7 +1031,7 @@ dependencies = [ { name = "protobuf", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, ] wheels = [ - { url = "https://pypi.devinfra.sentry.io/wheels/sentry_protos-0.34.0-py3-none-any.whl", hash = "sha256:917fc77bc17662680fbbb93ed056de5a44201982c367a1179842e355ae5ca244" }, + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_protos-0.40.0-py3-none-any.whl", hash = "sha256:7705c5b409339ff670e6d9c5cbc9b19b03d9d8525c35bc09d47a162cba156fc9" }, ] [[package]] @@ -1234,7 +1234,7 @@ requires-dist = [ { name = "sentry-conventions", specifier = ">=0.15.0" }, { name = "sentry-kafka-schemas", specifier = ">=2.1.36" }, { name = "sentry-options", specifier = ">=1.1.1" }, - { name = "sentry-protos", specifier = ">=0.34.0" }, + { name = "sentry-protos", specifier = ">=0.40.0" }, { name = "sentry-redis-tools", specifier = ">=0.5.1" }, { name = "sentry-relay", specifier = ">=0.9.25" }, { name = "sentry-sdk", specifier = ">=2.35.0" },