diff --git a/src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py b/src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py index 19ce04218773b3..a190905ba9253d 100644 --- a/src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py +++ b/src/sentry/api/endpoints/organization_trace_item_attributes_ranked.py @@ -20,13 +20,15 @@ from sentry.api.bases import NoProjects, OrganizationEventsEndpointBase from sentry.api.endpoints.organization_trace_item_attributes import adjust_start_end_window 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.exceptions import InvalidSearchQuery from sentry.models.organization import Organization from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.spans.attributes import SPANS_STATS_EXCLUDED_ATTRIBUTES from sentry.search.eap.spans.definitions import SPAN_DEFINITIONS from sentry.search.eap.types import SearchResolverConfig, SupportedTraceItemType -from sentry.search.eap.utils import can_expose_attribute, translate_internal_to_public_alias +from sentry.search.eap.utils import can_expose_attribute_to_api, translate_internal_to_public_alias from sentry.search.events import fields from sentry.seer.endpoints.compare import compare_distributions from sentry.seer.signed_seer_api import SeerViewerContext @@ -58,6 +60,7 @@ def get(self, request: Request, organization: Organization) -> Response: above = request.GET.get("above") == "1" query_1 = request.GET.get("query_1", "") # Suspect query query_2 = request.GET.get("query_2", "") # Query for all the spans with the base query + include_internal = is_active_superuser(request) or is_active_staff(request) if query_1 == query_2: return Response({"rankedAttributes": []}) @@ -68,6 +71,7 @@ def get(self, request: Request, organization: Organization) -> Response: above=above, query_1=query_1, query_2=query_2, + include_internal=include_internal, ) cohort_2_distribution = distributions_result["cohort_2_distribution"] @@ -131,7 +135,19 @@ def get(self, request: Request, organization: Organization) -> Response: if public_alias is None: public_alias = attr - if not public_alias.startswith("tags[") and ( + if ( + can_expose_attribute_to_api( + attr, + SupportedTraceItemType.SPANS, + include_internal=include_internal, + ) + and can_expose_attribute_to_api( + public_alias, + SupportedTraceItemType.SPANS, + include_internal=include_internal, + ) + and not public_alias.startswith("tags[") + ) and ( not public_alias.startswith("sentry.") or public_alias == "sentry.normalized_description" ): @@ -175,6 +191,7 @@ def query_attribute_distributions( above: bool, query_1: str, query_2: str, + include_internal: bool = False, ) -> AttributeDistributionsResponse: """ Query for and compute span attribute distributions for outlier (query_1) and baseline (query_2) cohorts. @@ -190,7 +207,9 @@ def query_attribute_distributions( AttributeDistributionsResponse with distribution data and total counts for both cohorts """ resolver_config = SearchResolverConfig( - extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + api_attribute_visibility_item_type=SupportedTraceItemType.SPANS.value, + api_attribute_visibility_include_internal=include_internal, ) resolver = SearchResolver( @@ -261,6 +280,16 @@ def query_attribute_distributions( cohort_1, _, _ = resolver.resolve_query(query_1) cohort_2, _, _ = resolver.resolve_query(query_2) + if resolver.has_hidden_api_attributes(): + return AttributeDistributionsResponse( + cohort_2_distribution=[], + cohort_2_distribution_map={}, + total_cohort_2=0, + cohort_1_distribution=[], + cohort_1_distribution_map={}, + total_cohort_1=0, + cohort_1_function_value=function_value, + ) # Fetch attribute names for parallelization adjusted_start_date, adjusted_end_date = adjust_start_end_window( @@ -291,6 +320,12 @@ def query_attribute_distributions( for i, attr_proto in enumerate(attrs_response.attributes): if attr_proto.name in SPANS_STATS_EXCLUDED_ATTRIBUTES: continue + if not can_expose_attribute_to_api( + attr_proto.name, + SupportedTraceItemType.SPANS, + include_internal=include_internal, + ): + continue chunked_attributes[i % PARALLELIZATION_FACTOR].append( AttributeKey(name=attr_proto.name, type=AttributeKey.TYPE_STRING) @@ -367,7 +402,11 @@ def run_table_query_with_error_handling(query_string): processed_cohort_2_buckets = set() for attribute in cohort_2_data: - if not can_expose_attribute(attribute.attribute_name, SupportedTraceItemType.SPANS): + if not can_expose_attribute_to_api( + attribute.attribute_name, + SupportedTraceItemType.SPANS, + include_internal=include_internal, + ): continue for bucket in attribute.buckets: @@ -376,7 +415,11 @@ def run_table_query_with_error_handling(query_string): ) for attribute in cohort_1_data: - if not can_expose_attribute(attribute.attribute_name, SupportedTraceItemType.SPANS): + if not can_expose_attribute_to_api( + attribute.attribute_name, + SupportedTraceItemType.SPANS, + include_internal=include_internal, + ): continue for bucket in attribute.buckets: cohort_1_distribution.append((attribute.attribute_name, bucket.label, bucket.value)) diff --git a/src/sentry/api/endpoints/organization_trace_item_stats.py b/src/sentry/api/endpoints/organization_trace_item_stats.py index 4aac077e097808..b0bd798a1f65fe 100644 --- a/src/sentry/api/endpoints/organization_trace_item_stats.py +++ b/src/sentry/api/endpoints/organization_trace_item_stats.py @@ -17,6 +17,8 @@ from sentry.api.event_search import translate_escape_sequences from sentry.api.serializers.base 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.search.eap.columns import ColumnDefinitions from sentry.search.eap.constants import SUPPORTED_STATS_TYPES @@ -31,7 +33,8 @@ SPANS_STATS_EXCLUDED_ATTRIBUTES_PUBLIC_ALIAS, ) from sentry.search.eap.spans.definitions import SPAN_DEFINITIONS -from sentry.search.eap.types import SearchResolverConfig +from sentry.search.eap.types import SearchResolverConfig, SupportedTraceItemType +from sentry.search.eap.utils import can_expose_attribute_to_api from sentry.snuba import rpc_dataset_common from sentry.snuba.occurrences_rpc import Occurrences from sentry.snuba.referrer import Referrer @@ -143,9 +146,15 @@ def get(self, request: Request, organization: Organization) -> Response: return Response(serializer.errors, status=400) serialized = serializer.validated_data - stats_config = get_trace_item_stats_config(serialized.get("itemType", "spans")) + item_type = serialized.get("itemType", "spans") + stats_config = get_trace_item_stats_config(item_type) + api_item_type = SupportedTraceItemType(item_type) + include_internal = is_active_superuser(request) or is_active_staff(request) - resolver_config = SearchResolverConfig() + resolver_config = SearchResolverConfig( + api_attribute_visibility_item_type=api_item_type.value, + api_attribute_visibility_include_internal=include_internal, + ) resolver = SearchResolver( params=snuba_params, config=resolver_config, definitions=stats_config.definitions ) @@ -159,7 +168,7 @@ def get_table_results(): with handle_query_errors(): return stats_config.rpc_class.run_table_query( params=snuba_params, - config=SearchResolverConfig(), + config=resolver_config, offset=0, limit=serialized.get("traceItemsLimit", 1000), sampling_mode=snuba_params.sampling_mode, @@ -180,6 +189,7 @@ def run_stats_query_with_item_ids(item_id_filter): search_resolver=resolver, max_buckets=1, skip_translate_internal_to_public_alias=True, + include_internal=include_internal, ) def run_stats_query_with_error_handling(attributes): @@ -192,6 +202,7 @@ def run_stats_query_with_error_handling(attributes): config=resolver_config, search_resolver=resolver, attributes=attributes, + include_internal=include_internal, ) def data_fn(offset: int, limit: int): @@ -219,6 +230,20 @@ def data_fn(offset: int, limit: int): if public_alias in stats_config.excluded_attributes: continue + if not ( + can_expose_attribute_to_api( + internal_name, + api_item_type, + include_internal=include_internal, + ) + and can_expose_attribute_to_api( + public_alias, + api_item_type, + include_internal=include_internal, + ) + ): + continue + if value_substring_match: if value_substring_match in public_alias: sanitized_keys.append(internal_name) diff --git a/src/sentry/api/endpoints/project_trace_item_details.py b/src/sentry/api/endpoints/project_trace_item_details.py index dc62ea7520b76b..9c6d0ddb408d7a 100644 --- a/src/sentry/api/endpoints/project_trace_item_details.py +++ b/src/sentry/api/endpoints/project_trace_item_details.py @@ -29,7 +29,7 @@ TraceItemAttribute, ) from sentry.search.eap.utils import ( - can_expose_attribute, + can_expose_attribute_to_api, get_deprecated_source_internal_names, is_sentry_convention_replacement_attribute, translate_internal_to_public_alias, @@ -118,7 +118,7 @@ def convert_rpc_attribute_to_json( for attribute in attributes: internal_name = attribute["name"] - if not can_expose_attribute( + if not can_expose_attribute_to_api( internal_name, trace_item_type, include_internal=include_internal ): continue @@ -209,6 +209,7 @@ def convert_rpc_attribute_to_json( def serialize_meta( attributes: list[dict], trace_item_type: SupportedTraceItemType, + include_internal: bool = False, ) -> dict: internal_name = "" attribute = {} @@ -243,6 +244,12 @@ def extract_key(key: str) -> str | None: result = json.loads(attribute["value"]["valStr"]) # Map the internal field key name back to its public name if field_key in attribute_map: + if not can_expose_attribute_to_api( + field_key, + trace_item_type, + include_internal=include_internal, + ): + continue item_type: Literal["string", "number", "boolean"] if ( "valInt" in attribute_map[field_key] @@ -270,7 +277,11 @@ def extract_key(key: str) -> str | None: return meta_result -def serialize_links(attributes: list[dict]) -> list[dict] | None: +def serialize_links( + attributes: list[dict], + trace_item_type: SupportedTraceItemType, + include_internal: bool = False, +) -> list[dict] | None: """Links are temporarily stored in `sentry.links` so lets parse that back out and return separately""" link_attribute = None for attribute in attributes: @@ -285,7 +296,14 @@ def serialize_links(attributes: list[dict]) -> list[dict] | None: value = link_attribute.get("value", {}).get("valStr", None) if value is not None: links = json.loads(value) - return [serialize_link(link) for link in links] + return [ + serialize_link( + link, + trace_item_type=trace_item_type, + include_internal=include_internal, + ) + for link in links + ] else: return None except Exception as e: @@ -293,7 +311,11 @@ def serialize_links(attributes: list[dict]) -> list[dict] | None: return None -def serialize_link(link: dict) -> dict: +def serialize_link( + link: dict, + trace_item_type: SupportedTraceItemType, + include_internal: bool = False, +) -> dict: clean_link = { "itemId": link["span_id"], "traceId": link["trace_id"], @@ -307,6 +329,11 @@ def serialize_link(link: dict) -> dict: {"name": k, "value": v, "type": infer_type(v)} for k, v in attributes.items() if infer_type(v) is not None + and can_expose_attribute_to_api( + k, + trace_item_type, + include_internal=include_internal, + ) ] return clean_link @@ -428,8 +455,12 @@ def get(request: Request, project: Project, item_id: str) -> Response: include_internal=include_internal, include_arrays=include_arrays, ), - "meta": serialize_meta(resp["attributes"], item_type), - "links": serialize_links(resp["attributes"]), + "meta": serialize_meta( + resp["attributes"], item_type, include_internal=include_internal + ), + "links": serialize_links( + resp["attributes"], item_type, include_internal=include_internal + ), } return Response(resp_dict) diff --git a/src/sentry/data_export/utils.py b/src/sentry/data_export/utils.py index 3852f9ed1b983d..52e087f512228a 100644 --- a/src/sentry/data_export/utils.py +++ b/src/sentry/data_export/utils.py @@ -13,7 +13,7 @@ from sentry.search.eap.constants import PROTOBUF_TYPE_TO_SEARCH_TYPE from sentry.search.eap.rpc_utils import anyvalue_to_python from sentry.search.eap.types import SupportedTraceItemType -from sentry.search.eap.utils import can_expose_attribute, translate_internal_to_public_alias +from sentry.search.eap.utils import can_expose_attribute_to_api, translate_internal_to_public_alias from sentry.search.events.constants import TIMEOUT_ERROR_MESSAGE from sentry.snuba import discover from sentry.utils import metrics, snuba @@ -146,7 +146,7 @@ def trace_item_to_row( _merge_trace_export_cell(row, "id", item.item_id.hex() if item.item_id else None) for internal_key, av in item.attributes.items(): - if not can_expose_attribute(internal_key, item_type, include_internal=False): + if not can_expose_attribute_to_api(internal_key, item_type, include_internal=False): continue which = av.WhichOneof("value") value = anyvalue_to_python(av) diff --git a/src/sentry/snuba/occurrences_rpc.py b/src/sentry/snuba/occurrences_rpc.py index 658cfe995f27ad..e29b685eb99b7c 100644 --- a/src/sentry/snuba/occurrences_rpc.py +++ b/src/sentry/snuba/occurrences_rpc.py @@ -281,9 +281,14 @@ def run_stats_query( max_buckets: int = 75, skip_translate_internal_to_public_alias: bool = False, occurrence_category: OccurrenceCategory | None = None, + include_internal: bool = False, ) -> list[dict[str, Any]]: search_resolver = search_resolver or cls.get_resolver(params, config) stats_filter, _, _ = search_resolver.resolve_query(query_string) + if search_resolver.has_hidden_api_attributes(): + # Hidden attributes here came from the query filter. Dropping them would + # broaden the query and return results for different search semantics. + return [] stats_filter = and_trace_item_filters( stats_filter, cls._build_category_filter(occurrence_category) @@ -315,7 +320,10 @@ def run_stats_query( response = snuba_rpc.trace_item_stats_rpc(stats_request) stats = [] - from sentry.search.eap.utils import can_expose_attribute, translate_internal_to_public_alias + from sentry.search.eap.utils import ( + can_expose_attribute_to_api, + translate_internal_to_public_alias, + ) for result in response.results: if "attributeDistributions" in stats_types and result.HasField( @@ -323,8 +331,10 @@ def run_stats_query( ): attrs: dict[str, list[dict[str, Any]]] = defaultdict(list) for attribute in result.attribute_distributions.attributes: - if not can_expose_attribute( - attribute.attribute_name, SupportedTraceItemType.OCCURRENCES + if not can_expose_attribute_to_api( + attribute.attribute_name, + SupportedTraceItemType.OCCURRENCES, + include_internal=include_internal, ): continue diff --git a/src/sentry/snuba/rpc_dataset_common.py b/src/sentry/snuba/rpc_dataset_common.py index 37bcdc4dade70d..e7bd099272ed7c 100644 --- a/src/sentry/snuba/rpc_dataset_common.py +++ b/src/sentry/snuba/rpc_dataset_common.py @@ -390,6 +390,8 @@ def _run_table_query( ) -> EAPResponse: """Run the query""" table_request = cls.get_table_rpc_request(query) + if query.resolver.has_hidden_api_attributes(): + return {"data": [], "meta": {"fields": {}}, "confidence": []} rpc_request = table_request.rpc_request try: rpc_response = snuba_rpc.table_rpc([rpc_request])[0] diff --git a/src/sentry/snuba/spans_rpc.py b/src/sentry/snuba/spans_rpc.py index 3cfb58e20d4158..6f1cd5e888a2d9 100644 --- a/src/sentry/snuba/spans_rpc.py +++ b/src/sentry/snuba/spans_rpc.py @@ -25,7 +25,7 @@ SearchResolverConfig, SupportedTraceItemType, ) -from sentry.search.eap.utils import can_expose_attribute, translate_internal_to_public_alias +from sentry.search.eap.utils import can_expose_attribute_to_api, translate_internal_to_public_alias from sentry.search.events.types import SAMPLING_MODES, SnubaParams from sentry.snuba import rpc_dataset_common from sentry.utils import json, snuba_rpc @@ -222,9 +222,12 @@ def run_stats_query( attributes: list[AttributeKey] | None = None, max_buckets: int = 75, skip_translate_internal_to_public_alias: bool = False, + include_internal: bool = False, ) -> list[dict[str, Any]]: search_resolver = search_resolver or cls.get_resolver(params, config) stats_filter, _, _ = search_resolver.resolve_query(query_string) + if search_resolver.has_hidden_api_attributes(): + return [] meta = search_resolver.resolve_meta( referrer=referrer, sampling_mode=params.sampling_mode, @@ -257,8 +260,10 @@ def run_stats_query( ): attrs = defaultdict(list) for attribute in result.attribute_distributions.attributes: - if not can_expose_attribute( - attribute.attribute_name, SupportedTraceItemType.SPANS + if not can_expose_attribute_to_api( + attribute.attribute_name, + SupportedTraceItemType.SPANS, + include_internal=include_internal, ): continue diff --git a/tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py b/tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py index a94db4c70311bd..b55c31687d3dec 100644 --- a/tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py +++ b/tests/snuba/api/endpoints/test_organization_trace_item_attributes_ranked.py @@ -66,6 +66,11 @@ def test_no_project(self) -> None: assert response.status_code == 200, response.data assert response.data == {"rankedAttributes": []} + def test_internal_convention_attribute_query_returns_empty_for_non_staff(self) -> None: + response = self.do_request(query={"query_1": "sentry.dsc.trace_id:abc123"}) + assert response.status_code == 200, response.data + assert response.data == {"rankedAttributes": []} + @patch("sentry.api.endpoints.organization_trace_item_attributes_ranked.compare_distributions") def test_distribution_values(self, mock_compare_distributions) -> None: mock_compare_distributions.return_value = { diff --git a/tests/snuba/api/endpoints/test_organization_trace_item_stats.py b/tests/snuba/api/endpoints/test_organization_trace_item_stats.py index 30ebc361fd44e1..38d51a1e9ea240 100644 --- a/tests/snuba/api/endpoints/test_organization_trace_item_stats.py +++ b/tests/snuba/api/endpoints/test_organization_trace_item_stats.py @@ -182,6 +182,38 @@ def test_substring_match_returns_known_public_aliases(self) -> None: assert "custom_attr" not in attribute_distribution assert "other_attr" not in attribute_distribution + def test_internal_convention_attributes_are_staff_only(self) -> None: + self.store_span( + self.create_span( + {"sentry_tags": {"dsc.trace_id": "abc123"}}, + start_ts=self.ten_mins_ago, + ), + ) + + response = self.do_request( + query={ + "statsType": ["attributeDistributions"], + "substringMatch": "dsc", + } + ) + assert response.status_code == 200, response.data + assert response.data == {"data": []} + + 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={ + "statsType": ["attributeDistributions"], + "substringMatch": "dsc", + } + ) + assert response.status_code == 200, response.data + assert len(response.data["data"]) == 1 + attribute_distribution = response.data["data"][0]["attribute_distributions"]["data"] + assert attribute_distribution["sentry.dsc.trace_id"] == [{"label": "abc123", "value": 1.0}] + def test_query_filters_spans_before_stats(self) -> None: tags = [ ({"browser": "chrome", "device": "desktop"}, 100), diff --git a/tests/snuba/api/endpoints/test_project_trace_item_details.py b/tests/snuba/api/endpoints/test_project_trace_item_details.py index 7694cabfbad340..e6148dc97ceb49 100644 --- a/tests/snuba/api/endpoints/test_project_trace_item_details.py +++ b/tests/snuba/api/endpoints/test_project_trace_item_details.py @@ -309,6 +309,80 @@ def test_simple_using_spans_item_type(self) -> None: == self.one_min_ago.replace(microsecond=0, tzinfo=None).isoformat() + "Z" ) + def test_internal_convention_span_attributes_are_staff_only(self) -> None: + span = self.create_span( + { + "tags": { + "sentry._meta.fields.attributes.sentry.dsc.trace_id": '{"reason": "internal"}' + }, + "sentry_tags": {"dsc.trace_id": "abc123"}, + }, + start_ts=self.one_min_ago, + ) + span["trace_id"] = self.trace_uuid + item_id = span["span_id"] + self.store_span(span) + + response = self.do_request("spans", item_id) + assert response.status_code == 200, response.content + attribute_names = {attr["name"] for attr in response.data["attributes"]} + assert "dsc.trace_id" not in attribute_names + assert "sentry.dsc.trace_id" not in response.data["meta"] + + 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("spans", item_id) + assert response.status_code == 200, response.content + attributes = {attr["name"]: attr for attr in response.data["attributes"]} + assert attributes["dsc.trace_id"] == { + "name": "dsc.trace_id", + "type": "str", + "value": "abc123", + } + assert response.data["meta"]["sentry.dsc.trace_id"] == {"reason": "internal"} + + def test_stripped_internal_convention_span_attributes_are_staff_only(self) -> None: + span = self.create_span( + { + "description": "test span", + "tags": { + "dsc.trace_id": "abc123", + "_internal.normalized_description": "normalized", + }, + }, + start_ts=self.one_min_ago, + ) + span["trace_id"] = self.trace_uuid + item_id = span["span_id"] + + self.store_span(span) + + response = self.do_request("spans", item_id) + assert response.status_code == 200, response.content + attribute_names = {attr["name"] for attr in response.data["attributes"]} + assert "dsc.trace_id" not in attribute_names + assert "_internal.normalized_description" 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("spans", item_id) + assert response.status_code == 200, response.content + attributes = {attr["name"]: attr for attr in response.data["attributes"]} + assert attributes["dsc.trace_id"] == { + "name": "dsc.trace_id", + "type": "str", + "value": "abc123", + } + assert attributes["_internal.normalized_description"] == { + "name": "_internal.normalized_description", + "type": "str", + "value": "normalized", + } + def test_simple_using_spans_item_type_with_sentry_conventions(self) -> None: span_1 = self.create_span( {"description": "foo", "sentry_tags": {"status": "success"}}, @@ -496,7 +570,7 @@ def test_sentry_links(self) -> None: { "description": "foo", "sentry_tags": { - "links": '[{"trace_id":"d099bf9ad5a143cf8f83a98081d0ed3b","span_id":"8873a98879faf06d","sampled":true,"attributes":{"sentry.link.type":"parent","sentry.dropped_attributes_count":1}}]', + "links": '[{"trace_id":"d099bf9ad5a143cf8f83a98081d0ed3b","span_id":"8873a98879faf06d","sampled":true,"attributes":{"sentry.link.type":"parent","sentry.dropped_attributes_count":1,"dsc.trace_id":"abc123","_internal.normalized_description":"normalized"}}]', }, }, start_ts=self.one_min_ago,