fix(attribute-values): Properly support boolean attributes (#8000)

## Summary

The `TraceItemAttributeValues` RPC enumerates the distinct values an
attribute
takes (used to populate filter-value autocomplete). It previously
assumed every
attribute was a string, so querying a **boolean** attribute broke. This
makes
the endpoint properly support booleans.

## Changes

- **Existence check on the right column.** Booleans live in
`attributes_bool`,
not `attributes_string`. A small `_ATTRIBUTE_TYPE_TO_COLUMN` map drives
the
  `has(...)` check by attribute type; unsupported types raise a clear
`BadSnubaRPCRequestException`. Adding more types later is a one-line map
entry.
- **Substring match guarded to strings.** `value_substring_match`
compiles to a
SQL `LIKE`, which only makes sense for strings — it now raises on
non-string
  keys instead of producing a broken query.
- **Lowercase boolean values.** The response proto's `values` field is
`repeated string`, so booleans must be serialized as strings. They're
returned
as lowercase `"true"`/`"false"` — matching how booleans are stringified
across
the EAP filter API — so the returned values round-trip as filter inputs.
Serializing via `bool(value)` also guards against ClickHouse returning
`0`/`1`.

## Tests

- `test_boolean_case` — enumerating a boolean attribute returns
  `["true", "false"]` with the expected counts.
- `test_substring_match_on_boolean_rejected` — substring match on a
boolean is
  rejected.
- `test_unsupported_attribute_type_rejected` — non-string/bool types are
  rejected.

---------

Co-authored-by: Pierre Massat <pierre.massat@sentry.io>

Author: wmak

snuba/web/rpc/v1/trace_item_attribute_values.py

@@ -48,12 +48,29 @@ def _map_key_names_for_existence_check(request_key: AttributeKey) -> list[str]:
     return names
 
 
+# Attribute value types this endpoint can enumerate, mapped to the ClickHouse
+# attribute map they live in. The response proto only carries strings, so every
+# supported type must also have a well-defined string form (see _execute).
+_ATTRIBUTE_TYPE_TO_COLUMN: dict["AttributeKey.Type.ValueType", str] = {
+    AttributeKey.TYPE_STRING: "attributes_string",
+    AttributeKey.TYPE_BOOLEAN: "attributes_bool",
+}
+
+
 def _build_conditions(request: TraceItemAttributeValuesRequest) -> Expression:
     attribute_key = attribute_key_to_expression(request.key)
 
+    try:
+        attributes_column = _ATTRIBUTE_TYPE_TO_COLUMN[request.key.type]
+    except KeyError:
+        raise BadSnubaRPCRequestException("Only string and boolean attributes can be used")
+
+    # Use mapContains (not has) for key existence: it's the correct ClickHouse
+    # function for Map columns and is handled by HashBucketFunctionTransformer
+    # for the bucketed string/float maps as well as the un-bucketed bool map.
     key_existence = combine_or_conditions(
         [
-            f.has(column("attributes_string"), name)
+            f.mapContains(column(attributes_column), name)
             for name in _map_key_names_for_existence_check(request.key)
         ]
     )
@@ -63,6 +80,10 @@ def _build_conditions(request: TraceItemAttributeValuesRequest) -> Expression:
         conditions.append(f.equals(column("item_type"), request.meta.trace_item_type))
 
     if request.value_substring_match:
+        if request.key.type != AttributeKey.TYPE_STRING:
+            raise BadSnubaRPCRequestException(
+                "substring matches can only be used on string attributes"
+            )
         conditions.append(
             f.like(
                 attribute_key,
@@ -200,9 +221,14 @@ def _execute(self, in_msg: TraceItemAttributeValuesRequest) -> TraceItemAttribut
             request=snuba_request,
             timer=self._timer,
         )
+        # The response proto only carries strings. Boolean values are serialized
+        # to the lowercase "true"/"false" form used across the EAP filter API so
+        # that returned values round-trip as filter inputs.
+        is_boolean = in_msg.key.type == AttributeKey.TYPE_BOOLEAN
         values, counts = [], []
         for row in res.result.get("data", []):
-            values.append(row["attr_value"])
+            value = row["attr_value"]
+            values.append(str(bool(value)).lower() if is_boolean else value)
             counts.append(row.get("count()", 0))
         if len(values) == 0:
             return TraceItemAttributeValuesResponse(

tests/web/rpc/v1/test_trace_item_attribute_values_v1.py

@@ -14,6 +14,7 @@
 
 from snuba.datasets.storages.factory import get_storage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException
 from snuba.web.rpc.v1.trace_item_attribute_values import AttributeValuesRequest
 from tests.base import BaseApiTest
 from tests.helpers import write_raw_unprocessed_events
@@ -63,13 +64,15 @@ def setup_teardown(eap: None, redis_db: None) -> Generator[List[bytes], None, No
             attributes={
                 "tag1": AnyValue(string_value="herp"),
                 "tag2": AnyValue(string_value="herp"),
+                "custom_flag": AnyValue(bool_value=True),
             },
         ),
         gen_item_message(
             start_timestamp=start_timestamp,
             attributes={
                 "tag1": AnyValue(string_value="herpderp"),
                 "tag2": AnyValue(string_value="herp"),
+                "custom_flag": AnyValue(bool_value=False),
             },
         ),
         gen_item_message(
@@ -108,6 +111,7 @@ def setup_teardown(eap: None, redis_db: None) -> Generator[List[bytes], None, No
             start_timestamp=start_timestamp,
             attributes={
                 "tag1": AnyValue(string_value="some_last_value"),
+                "sentry.is_segment": AnyValue(bool_value=False),
             },
         ),
         gen_item_message(
@@ -251,3 +255,46 @@ def test_pagination(self, setup_teardown: Any) -> None:
             response = AttributeValuesRequest().execute(message)
             assert response.values == [expected]
             assert response.counts == [1]
+
+    def test_boolean_case(self, setup_teardown: Any) -> None:
+        message = TraceItemAttributeValuesRequest(
+            meta=COMMON_META,
+            limit=5,
+            key=AttributeKey(name="sentry.is_segment", type=AttributeKey.TYPE_BOOLEAN),
+        )
+        response = AttributeValuesRequest().execute(message)
+        assert response.values == ["true", "false"]
+        assert response.counts == [8, 1]
+
+    def test_boolean_existence_check(self, setup_teardown: Any) -> None:
+        # `custom_flag` is set on exactly two items (one True, one False); the
+        # other items do not have the key. The existence check must exclude the
+        # items missing the key, otherwise they'd be miscounted as "false".
+        message = TraceItemAttributeValuesRequest(
+            meta=COMMON_META,
+            limit=5,
+            key=AttributeKey(name="custom_flag", type=AttributeKey.TYPE_BOOLEAN),
+        )
+        response = AttributeValuesRequest().execute(message)
+        # Equal counts, so ties break on attr_value ASC ("false" before "true").
+        assert response.values == ["false", "true"]
+        assert response.counts == [1, 1]
+
+    def test_substring_match_on_boolean_rejected(self, setup_teardown: Any) -> None:
+        message = TraceItemAttributeValuesRequest(
+            meta=COMMON_META,
+            limit=5,
+            key=AttributeKey(name="sentry.is_segment", type=AttributeKey.TYPE_BOOLEAN),
+            value_substring_match="tru",
+        )
+        with pytest.raises(BadSnubaRPCRequestException):
+            AttributeValuesRequest().execute(message)
+
+    def test_unsupported_attribute_type_rejected(self, setup_teardown: Any) -> None:
+        message = TraceItemAttributeValuesRequest(
+            meta=COMMON_META,
+            limit=5,
+            key=AttributeKey(name="sentry.duration_ms", type=AttributeKey.TYPE_DOUBLE),
+        )
+        with pytest.raises(BadSnubaRPCRequestException):
+            AttributeValuesRequest().execute(message)