feat(explore): Omit internal attributes from responses (#97414)

The span buffer adds some internal attributes for tracking purposes,
they are prefixed with `__sentry_internal`. We're also adding a new
official convention for prefixed attributes, `sentry._internal`. Right
now, we hide these in the UI in the trace waterfall, but not anywhere
else. Instead, use a more robust approach where we filter those
attributes out _on the backend_, but they are still visible to
superusers.

Author: gggritso

src/sentry/api/endpoints/organization_trace_item_attributes.py

@@ -25,6 +25,8 @@
 from sentry.api.paginator import ChainPaginator, GenericOffsetPaginator
 from sentry.api.serializers import serialize
 from sentry.api.utils import handle_query_errors
+from sentry.auth.staff import is_active_staff
+from sentry.auth.superuser import is_active_superuser
 from sentry.models.organization import Organization
 from sentry.models.release import Release
 from sentry.models.releaseenvironment import ReleaseEnvironment
@@ -237,6 +239,8 @@ def get(self, request: Request, organization: Organization) -> Response:
             else AttributeKey.Type.TYPE_STRING
         )
 
+        include_internal = is_active_superuser(request) or is_active_staff(request)
+
         def data_fn(offset: int, limit: int):
             rpc_request = TraceItemAttributeNamesRequest(
                 meta=meta,
@@ -253,7 +257,9 @@ def data_fn(offset: int, limit: int):
             if use_sentry_conventions:
                 attribute_keys = {}
                 for attribute in rpc_response.attributes:
-                    if attribute.name and can_expose_attribute(attribute.name, trace_item_type):
+                    if attribute.name and can_expose_attribute(
+                        attribute.name, trace_item_type, include_internal=include_internal
+                    ):
                         attr_key = as_attribute_key(
                             attribute.name, serialized["attribute_type"], trace_item_type
                         )
@@ -280,7 +286,10 @@ def data_fn(offset: int, limit: int):
                             attribute.name, serialized["attribute_type"], trace_item_type
                         )
                         for attribute in rpc_response.attributes
-                        if attribute.name and can_expose_attribute(attribute.name, trace_item_type)
+                        if attribute.name
+                        and can_expose_attribute(
+                            attribute.name, trace_item_type, include_internal=include_internal
+                        )
                     ],
                 )
             )

src/sentry/api/endpoints/project_trace_item_details.py

@@ -17,11 +17,13 @@
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.project import ProjectEndpoint
 from sentry.api.exceptions import BadRequest
+from sentry.auth.staff import is_active_staff
+from sentry.auth.superuser import is_active_superuser
 from sentry.models.project import Project
 from sentry.search.eap import constants
 from sentry.search.eap.types import SupportedTraceItemType, TraceItemAttribute
 from sentry.search.eap.utils import (
-    PRIVATE_ATTRIBUTES,
+    can_expose_attribute,
     is_sentry_convention_replacement_attribute,
     translate_internal_to_public_alias,
     translate_to_sentry_conventions,
@@ -34,15 +36,18 @@ def convert_rpc_attribute_to_json(
     attributes: list[dict],
     trace_item_type: SupportedTraceItemType,
     use_sentry_conventions: bool = False,
+    include_internal: bool = False,
 ) -> list[TraceItemAttribute]:
     result: list[TraceItemAttribute] = []
     seen_sentry_conventions: set[str] = set()
     for attribute in attributes:
         internal_name = attribute["name"]
-        if internal_name in PRIVATE_ATTRIBUTES.get(trace_item_type, []):
-            continue
-        if internal_name.startswith(constants.META_PREFIX):
+
+        if not can_expose_attribute(
+            internal_name, trace_item_type, include_internal=include_internal
+        ):
             continue
+
         source = attribute["value"]
         if len(source) == 0:
             raise BadRequest(f"unknown field in protobuf: {internal_name}")
@@ -127,6 +132,9 @@ def extract_key(key: str) -> str | None:
         if field_key is None:
             continue
 
+        # TODO: This should probably also omit internal attributes. It's not
+        # clear why it doesn't, but this behavior seems important for logs.
+
         try:
             result = json.loads(attribute["value"]["valStr"])
             # Map the internal field key name back to its public name
@@ -295,11 +303,16 @@ def get(request: Request, project: Project, item_id: str) -> Response:
             actor=request.user,
         )
 
+        include_internal = is_active_superuser(request) or is_active_staff(request)
+
         resp_dict = {
             "itemId": serialize_item_id(resp["itemId"], item_type),
             "timestamp": resp["timestamp"],
             "attributes": convert_rpc_attribute_to_json(
-                resp["attributes"], item_type, use_sentry_conventions
+                resp["attributes"],
+                item_type,
+                use_sentry_conventions,
+                include_internal=include_internal,
             ),
             "meta": serialize_meta(resp["attributes"], item_type),
             "links": serialize_links(resp["attributes"]),

src/sentry/search/eap/constants.py

@@ -178,3 +178,5 @@
 META_PREFIX = "sentry._meta"
 META_FIELD_PREFIX = f"{META_PREFIX}.fields"
 META_ATTRIBUTE_PREFIX = f"{META_FIELD_PREFIX}.attributes"
+
+SENTRY_INTERNAL_PREFIXES = ["__sentry_internal", "sentry._internal."]

src/sentry/search/eap/utils.py

@@ -15,7 +15,7 @@
 
 from sentry.exceptions import InvalidSearchQuery
 from sentry.search.eap.columns import ResolvedAttribute
-from sentry.search.eap.constants import SAMPLING_MODE_MAP
+from sentry.search.eap.constants import SAMPLING_MODE_MAP, SENTRY_INTERNAL_PREFIXES
 from sentry.search.eap.ourlogs.attributes import (
     LOGS_INTERNAL_TO_PUBLIC_ALIAS_MAPPINGS,
     LOGS_INTERNAL_TO_SECONDARY_ALIASES_MAPPING,
@@ -189,11 +189,22 @@ def get_secondary_aliases(
     return mapping.get(internal_alias)
 
 
-def can_expose_attribute(attribute: str, item_type: SupportedTraceItemType) -> bool:
-    return attribute not in PRIVATE_ATTRIBUTES.get(item_type, {}) and not any(
+def can_expose_attribute(
+    attribute: str, item_type: SupportedTraceItemType, include_internal: bool = False
+) -> bool:
+    # Always omit private attributes
+    if attribute in PRIVATE_ATTRIBUTES.get(item_type, {}) or any(
         attribute.lower().startswith(prefix.lower())
         for prefix in PRIVATE_ATTRIBUTE_PREFIXES.get(item_type, {})
-    )
+    ):
+        return False
+
+    # Omit internal attributes, unless explicitly requested. Usually, only
+    # Sentry staff should see these.
+    if any(attribute.lower().startswith(prefix.lower()) for prefix in SENTRY_INTERNAL_PREFIXES):
+        return include_internal
+
+    return True
 
 
 def handle_downsample_meta(meta: DownsampledStorageMeta) -> bool:

src/sentry/seer/endpoints/seer_rpc.py

@@ -320,7 +320,10 @@ def get_attribute_names(*, org_id: int, project_ids: list[int], stats_period: st
                 SupportedTraceItemType.SPANS,
             )["name"]
             for attr in fields_resp.attributes
-            if attr.name and can_expose_attribute(attr.name, SupportedTraceItemType.SPANS)
+            if attr.name
+            and can_expose_attribute(
+                attr.name, SupportedTraceItemType.SPANS, include_internal=False
+            )
         ]
 
         fields[type_str].extend(parsed_fields)

tests/snuba/api/endpoints/test_organization_trace_item_attributes.py

@@ -206,7 +206,7 @@ def test_attribute_collision(self) -> None:
 
 
 class OrganizationTraceItemAttributesEndpointSpansTest(
-    OrganizationTraceItemAttributesEndpointTestBase, BaseSpansTestCase
+    OrganizationTraceItemAttributesEndpointTestBase, BaseSpansTestCase, SpanTestCase
 ):
     feature_flags = {"organizations:visibility-explore-view": True}
     item_type = SupportedTraceItemType.SPANS
@@ -521,6 +521,43 @@ def test_attribute_collision(self) -> None:
             {"key": "tags[span.op,string]", "name": "tags[span.op,string]"},
         ]
 
+    def test_sentry_internal_attributes(self) -> None:
+        self.store_spans(
+            [
+                self.create_span(
+                    {
+                        "tags": {
+                            "normal_attr": "normal_value",
+                            "__sentry_internal_span_buffer_outcome": "different",
+                            "__sentry_internal_test": "internal_value",
+                        }
+                    },
+                    start_ts=before_now(days=0, minutes=10),
+                ),
+            ],
+            is_eap=True,
+        )
+
+        response = self.do_request(query={"substringMatch": ""})
+        assert response.status_code == 200
+
+        attribute_names = {attr["name"] for attr in response.data}
+        assert "normal_attr" in attribute_names
+        assert "__sentry_internal_span_buffer_outcome" not in attribute_names
+        assert "__sentry_internal_test" not in attribute_names
+
+        staff_user = self.create_user(is_staff=True)
+        self.create_member(user=staff_user, organization=self.organization)
+        self.login_as(user=staff_user, staff=True)
+
+        response = self.do_request(query={"substringMatch": ""})
+        assert response.status_code == 200
+
+        attribute_names = {attr["name"] for attr in response.data}
+        assert "normal_attr" in attribute_names
+        assert "__sentry_internal_span_buffer_outcome" in attribute_names
+        assert "__sentry_internal_test" in attribute_names
+
 
 class OrganizationTraceItemAttributeValuesEndpointBaseTest(APITestCase, SnubaTestCase):
     feature_flags: dict[str, bool]

tests/snuba/api/endpoints/test_project_trace_item_details.py

@@ -467,3 +467,40 @@ def test_sentry_links(self) -> None:
                 ],
             }
         ]
+
+    def test_sentry_internal_attributes(self) -> None:
+        span_1 = self.create_span(
+            {
+                "description": "test span",
+                "tags": {
+                    "normal_attr": "normal_value",
+                    "__sentry_internal_span_buffer_outcome": "different",
+                    "__sentry_internal_test": "internal_value",
+                },
+            },
+            start_ts=self.one_min_ago,
+        )
+        span_1["trace_id"] = self.trace_uuid
+        item_id = span_1["span_id"]
+
+        self.store_spans([span_1], is_eap=True)
+
+        trace_details_response = self.do_request("spans", item_id)
+        assert trace_details_response.status_code == 200
+
+        attribute_names = [attr["name"] for attr in trace_details_response.data["attributes"]]
+        assert "normal_attr" in attribute_names
+        assert "__sentry_internal_span_buffer_outcome" not in attribute_names
+        assert "__sentry_internal_test" not in attribute_names
+
+        staff_user = self.create_user(is_staff=True)
+        self.create_member(user=staff_user, organization=self.organization)
+        self.login_as(user=staff_user, staff=True)
+
+        trace_details_response = self.do_request("spans", item_id)
+        assert trace_details_response.status_code == 200
+
+        attribute_names = [attr["name"] for attr in trace_details_response.data["attributes"]]
+        assert "normal_attr" in attribute_names
+        assert "__sentry_internal_span_buffer_outcome" in attribute_names
+        assert "__sentry_internal_test" in attribute_names