feat(attribute-values): Add counts to response and orderby

pyproject.toml

@@ -31,7 +31,7 @@ dependencies = [
     "sentry-arroyo>=2.38.7",
     "sentry-conventions>=0.3.0",
     "sentry-kafka-schemas>=2.1.24",
-    "sentry-protos>=0.8.14",
+    "sentry-protos>=0.12.0",
     "sentry-redis-tools>=0.5.1",
     "sentry-relay>=0.9.25",
     "sentry-sdk>=2.35.0",

snuba/web/rpc/v1/trace_item_attribute_values.py

@@ -25,7 +25,7 @@
 from snuba.query.data_source.simple import Entity, LogicalDataSource
 from snuba.query.dsl import Functions as f
 from snuba.query.dsl import column
-from snuba.query.expressions import Expression
+from snuba.query.expressions import Expression, FunctionCall
 from snuba.query.logical import Query
 from snuba.query.query_settings import HTTPQuerySettings
 from snuba.request import Request as SnubaRequest
@@ -128,10 +128,20 @@ def _build_query(
                 name="attr_value",
                 expression=f.distinct(column(attr_value.alias, alias="attr_value")),
             ),
+            SelectedExpression(
+                name="count()",
+                expression=FunctionCall(
+                    alias="count()",
+                    function_name="count",
+                    parameters=(),
+                ),
+            ),
         ],
         order_by=[
+            OrderBy(direction=OrderByDirection.DESC, expression=column("count()")),
             OrderBy(direction=OrderByDirection.ASC, expression=column("attr_value")),
         ],
+        groupby=[column("attr_value")],
         limit=request.limit,
         offset=(request.page_token.offset if request.page_token.HasField("offset") else 0),
     )
@@ -184,6 +194,7 @@ def _execute(self, in_msg: TraceItemAttributeValuesRequest) -> TraceItemAttribut
         if in_msg.key.name == "sentry.item_id" and in_msg.value_substring_match:
             return TraceItemAttributeValuesResponse(
                 values=[in_msg.value_substring_match],
+                counts=[1],
                 page_token=None,
             )
         in_msg.limit = in_msg.limit or 1000
@@ -193,14 +204,19 @@ def _execute(self, in_msg: TraceItemAttributeValuesRequest) -> TraceItemAttribut
             request=snuba_request,
             timer=self._timer,
         )
-        values = [r["attr_value"] for r in res.result.get("data", [])]
+        values, counts = [], []
+        for row in res.result.get("data", []):
+            values.append(row["attr_value"])
+            counts.append(row.get("count()", 0))
         if len(values) == 0:
             return TraceItemAttributeValuesResponse(
                 values=values,
+                counts=counts,
                 page_token=None,
             )
         return TraceItemAttributeValuesResponse(
             values=values,
+            counts=counts,
             page_token=(
                 PageToken(offset=in_msg.page_token.offset + len(values))
                 if in_msg.page_token.HasField("offset") or len(values) == 0

tests/web/rpc/v1/test_trace_item_attribute_values_v1.py

@@ -93,6 +93,13 @@ def setup_teardown(eap: None, redis_db: None) -> Generator[List[bytes], None, No
                 "tag2": AnyValue(string_value="derp"),
             },
         ),
+        gen_item_message(
+            start_timestamp=start_timestamp,
+            attributes={
+                "tag1": AnyValue(string_value="derpderp"),
+                "tag2": AnyValue(string_value="derp"),
+            },
+        ),
         gen_item_message(
             start_timestamp=start_timestamp,
             attributes={"tag2": AnyValue(string_value="hehe")},
@@ -135,7 +142,8 @@ def test_simple_case(self, setup_teardown: Any) -> None:
             key=AttributeKey(name="tag1", type=AttributeKey.TYPE_STRING),
         )
         response = AttributeValuesRequest().execute(message)
-        assert response.values == ["blah", "derpderp", "durp", "herp", "herpderp"]
+        assert response.values == ["derpderp", "blah", "durp", "herp", "herpderp"]
+        assert response.counts == [2, 1, 1, 1, 1]
 
     def test_with_value_substring_match(self, setup_teardown: Any) -> None:
         message = TraceItemAttributeValuesRequest(
@@ -146,6 +154,7 @@ def test_with_value_substring_match(self, setup_teardown: Any) -> None:
         )
         response = AttributeValuesRequest().execute(message)
         assert response.values == ["derpderp", "herp", "herpderp"]
+        assert response.counts == [2, 1, 1]
 
     def test_empty_results(self) -> None:
         req = TraceItemAttributeValuesRequest(
@@ -162,6 +171,7 @@ def test_empty_results(self) -> None:
         )
         res = AttributeValuesRequest().execute(req)
         assert res.values == []
+        assert res.counts == []
 
     def test_item_id_substring_match(self, setup_teardown: List[bytes]) -> None:
         first_msg_bytes = setup_teardown[0]
@@ -182,6 +192,7 @@ def test_item_id_substring_match(self, setup_teardown: List[bytes]) -> None:
         )
         res = AttributeValuesRequest().execute(req)
         assert res.values == [item_id]
+        assert res.counts == [1]
 
     def test_deprecated_alias_attribute(self) -> None:
         """db.system.name request returns values stored only under deprecated key db.system."""
@@ -218,3 +229,4 @@ def test_deprecated_alias_attribute(self) -> None:
         )
         response = AttributeValuesRequest().execute(message)
         assert sorted(response.values) == ["postgresql", "redis"]
+        assert response.counts == [1, 1]