From bb3cbc3f432ed74a49dc7a515091d27f01e2f7c6 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Fri, 27 Sep 2024 16:45:45 +0000 Subject: [PATCH] Revert "feat(eap): Use RPC to fetch tag values (#77510)" This reverts commit a8e4e05fb164b3234ca12bd02784d5fc8b6cdc23. Co-authored-by: Zylphrex <10239353+Zylphrex@users.noreply.github.com> --- .../endpoints/organization_spans_fields.py | 67 +-- src/sentry/snuba/referrer.py | 2 - .../test_organization_spans_fields.py | 388 ++++-------------- 3 files changed, 72 insertions(+), 385 deletions(-) diff --git a/src/sentry/api/endpoints/organization_spans_fields.py b/src/sentry/api/endpoints/organization_spans_fields.py index 5bb9b0f27e6846..d3697277dbae73 100644 --- a/src/sentry/api/endpoints/organization_spans_fields.py +++ b/src/sentry/api/endpoints/organization_spans_fields.py @@ -7,8 +7,6 @@ from rest_framework.request import Request from rest_framework.response import Response from sentry_protos.snuba.v1alpha.endpoint_tags_list_pb2 import ( - AttributeValuesRequest, - AttributeValuesResponse, TraceItemAttributesRequest, TraceItemAttributesResponse, ) @@ -22,7 +20,6 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.bases import NoProjects, OrganizationEventsV2EndpointBase -from sentry.api.event_search import translate_escape_sequences from sentry.api.paginator import ChainPaginator from sentry.api.serializers import serialize from sentry.api.utils import handle_query_errors @@ -198,68 +195,6 @@ def get(self, request: Request, organization, key: str) -> Response: max_span_tag_values = options.get("performance.spans-tags-values.max") - serializer = OrganizationSpansFieldsEndpointSerializer(data=request.GET) - if not serializer.is_valid(): - return Response(serializer.errors, status=400) - serialized = serializer.validated_data - - if serialized["dataset"] == "spans" and features.has( - "organizations:visibility-explore-dataset", organization, actor=request.user - ): - start_timestamp = Timestamp() - start_timestamp.FromDatetime( - snuba_params.start_date.replace(hour=0, minute=0, second=0, microsecond=0) - ) - - end_timestamp = Timestamp() - end_timestamp.FromDatetime( - snuba_params.end_date.replace(hour=0, minute=0, second=0, microsecond=0) - + timedelta(days=1) - ) - - query = translate_escape_sequences(request.GET.get("query", "")) - rpc_request = AttributeValuesRequest( - meta=RequestMeta( - organization_id=organization.id, - cogs_category="performance", - referrer=Referrer.API_SPANS_TAG_VALUES_RPC.value, - project_ids=snuba_params.project_ids, - start_timestamp=start_timestamp, - end_timestamp=end_timestamp, - trace_item_name=TraceItemName.TRACE_ITEM_NAME_EAP_SPANS, - ), - name=key, - value_substring_match=query, - limit=max_span_tag_values, - offset=0, - ) - rpc_response = snuba.rpc(rpc_request, AttributeValuesResponse) - - paginator = ChainPaginator( - [ - [ - TagValue( - key=key, - value=tag_value, - times_seen=None, - first_seen=None, - last_seen=None, - ) - for tag_value in rpc_response.values - if tag_value - ] - ], - max_limit=max_span_tag_values, - ) - - return self.paginate( - request=request, - paginator=paginator, - on_results=lambda results: serialize(results, request.user), - default_per_page=max_span_tag_values, - max_per_page=max_span_tag_values, - ) - executor = SpanFieldValuesAutocompletionExecutor( snuba_params=snuba_params, key=key, @@ -404,7 +339,7 @@ def get_autocomplete_query_base(self) -> BaseQueryBuilder: def get_autocomplete_results(self, query: BaseQueryBuilder) -> list[TagValue]: with handle_query_errors(): - results = query.process_results(query.run_query(Referrer.API_SPANS_TAG_VALUES.value)) + results = query.process_results(query.run_query(Referrer.API_SPANS_TAG_KEYS.value)) return [ TagValue( diff --git a/src/sentry/snuba/referrer.py b/src/sentry/snuba/referrer.py index fc41130707c75a..fb4ec76c6796cb 100644 --- a/src/sentry/snuba/referrer.py +++ b/src/sentry/snuba/referrer.py @@ -474,8 +474,6 @@ class Referrer(Enum): API_TRACE_EXPLORER_TRACE_SPANS_LIST = "api.trace-explorer.trace-spans-list" API_SPANS_TAG_KEYS = "api.spans.tags-keys" API_SPANS_TAG_KEYS_RPC = "api.spans.tags-keys.rpc" - API_SPANS_TAG_VALUES = "api.spans.tags-values" - API_SPANS_TAG_VALUES_RPC = "api.spans.tags-values.rpc" API_SPANS_TRACE_VIEW = "api.spans.trace-view" # Performance Mobile UI Module diff --git a/tests/sentry/api/endpoints/test_organization_spans_fields.py b/tests/sentry/api/endpoints/test_organization_spans_fields.py index b17b6d71eefac8..19cbccde568fc2 100644 --- a/tests/sentry/api/endpoints/test_organization_spans_fields.py +++ b/tests/sentry/api/endpoints/test_organization_spans_fields.py @@ -1,7 +1,5 @@ -from unittest import mock from uuid import uuid4 -import pytest from django.urls import reverse from sentry.testutils.cases import APITestCase, BaseSpansTestCase @@ -123,7 +121,6 @@ def test_tags_list(self): class OrganizationSpansTagKeyValuesEndpointTest(BaseSpansTestCase, APITestCase): - is_eap = False view = "sentry-api-0-organization-spans-fields-values" def setUp(self): @@ -132,7 +129,7 @@ def setUp(self): def do_request(self, key: str, query=None, features=None, **kwargs): if features is None: - features = ["organizations:performance-trace-explorer"] + features = ["organizations:performance-trace-explorer", "organizations:global-views"] with self.feature(features): return self.client.get( reverse( @@ -161,264 +158,44 @@ def test_tags_keys(self): uuid4().hex, uuid4().hex, span_id=uuid4().hex[:15], - organization_id=self.organization.id, parent_span_id=None, timestamp=timestamp, transaction="foo", duration=100, exclusive_time=100, tags={"tag": tag}, - is_eap=self.is_eap, ) response = self.do_request("tag") assert response.status_code == 200, response.data assert response.data == [ { - "count": mock.ANY, + "count": 1, "key": "tag", "value": "bar", "name": "bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), }, { - "count": mock.ANY, + "count": 1, "key": "tag", "value": "baz", "name": "baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), }, { - "count": mock.ANY, + "count": 1, "key": "tag", "value": "foo", "name": "foo", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] - - def test_transaction_keys_autocomplete(self): - timestamp = before_now(days=0, minutes=10).replace(microsecond=0) - for transaction in ["foo", "*bar", "*baz"]: - self.store_segment( - self.project.id, - uuid4().hex, - uuid4().hex, - span_id=uuid4().hex[:15], - organization_id=self.organization.id, - parent_span_id=None, - timestamp=timestamp, - transaction=transaction, - duration=100, - exclusive_time=100, - is_eap=self.is_eap, - ) - - key = "transaction" - - response = self.do_request(key) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "foo", - "name": "foo", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] - - def test_transaction_keys_autocomplete_substring(self): - timestamp = before_now(days=0, minutes=10).replace(microsecond=0) - for transaction in ["foo", "*bar", "*baz"]: - self.store_segment( - self.project.id, - uuid4().hex, - uuid4().hex, - span_id=uuid4().hex[:15], - organization_id=self.organization.id, - parent_span_id=None, - timestamp=timestamp, - transaction=transaction, - duration=100, - exclusive_time=100, - is_eap=self.is_eap, - ) - - key = "transaction" - - response = self.do_request(key, query={"query": "b"}) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] - - def test_transaction_keys_autocomplete_substring_with_asterisk(self): - timestamp = before_now(days=0, minutes=10).replace(microsecond=0) - for transaction in ["foo", "*bar", "*baz"]: - self.store_segment( - self.project.id, - uuid4().hex, - uuid4().hex, - span_id=uuid4().hex[:15], - organization_id=self.organization.id, - parent_span_id=None, - timestamp=timestamp, - transaction=transaction, - duration=100, - exclusive_time=100, - is_eap=self.is_eap, - ) - - key = "transaction" - - response = self.do_request(key, query={"query": r"\*b"}) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] - - def test_tags_keys_autocomplete(self): - timestamp = before_now(days=0, minutes=10).replace(microsecond=0) - for tag in ["foo", "*bar", "*baz"]: - self.store_segment( - self.project.id, - uuid4().hex, - uuid4().hex, - span_id=uuid4().hex[:15], - organization_id=self.organization.id, - parent_span_id=None, - timestamp=timestamp, - transaction="transaction", - duration=100, - exclusive_time=100, - tags={"tag": tag}, - is_eap=self.is_eap, - ) - - key = "tag" - - response = self.do_request(key) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "foo", - "name": "foo", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] - - def test_tags_keys_autocomplete_substring(self): - timestamp = before_now(days=0, minutes=10).replace(microsecond=0) - for tag in ["foo", "*bar", "*baz"]: - self.store_segment( - self.project.id, - uuid4().hex, - uuid4().hex, - span_id=uuid4().hex[:15], - organization_id=self.organization.id, - parent_span_id=None, - timestamp=timestamp, - transaction="transaction", - duration=100, - exclusive_time=100, - tags={"tag": tag}, - is_eap=self.is_eap, - ) - - key = "tag" - - response = self.do_request(key, query={"query": "b"}) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), }, ] - def test_tags_keys_autocomplete_substring_with_asterisks(self): + def test_tags_keys_autocomplete_default(self): timestamp = before_now(days=0, minutes=10).replace(microsecond=0) for tag in ["foo", "*bar", "*baz"]: self.store_segment( @@ -426,38 +203,70 @@ def test_tags_keys_autocomplete_substring_with_asterisks(self): uuid4().hex, uuid4().hex, span_id=uuid4().hex[:15], - organization_id=self.organization.id, parent_span_id=None, timestamp=timestamp, - transaction="transaction", + transaction=tag, duration=100, exclusive_time=100, tags={"tag": tag}, - is_eap=self.is_eap, ) - key = "tag" + for key in ["tag", "transaction"]: + response = self.do_request(key) + assert response.status_code == 200, response.data + assert response.data == [ + { + "count": 1, + "key": key, + "value": "*bar", + "name": "*bar", + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + }, + { + "count": 1, + "key": key, + "value": "*baz", + "name": "*baz", + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + }, + { + "count": 1, + "key": key, + "value": "foo", + "name": "foo", + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + }, + ] - response = self.do_request(key, query={"query": r"\*b"}) - assert response.status_code == 200, response.data - assert response.data == [ - { - "count": mock.ANY, - "key": key, - "value": "*bar", - "name": "*bar", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - { - "count": mock.ANY, - "key": key, - "value": "*baz", - "name": "*baz", - "firstSeen": mock.ANY, - "lastSeen": mock.ANY, - }, - ] + for key, query in [ + ("tag", "b"), + ("transaction", "b"), + ("tag", r"\*b"), + ("transaction", r"\*b"), + ]: + response = self.do_request(key, query={"query": query}) + assert response.status_code == 200, response.data + assert response.data == [ + { + "count": 1, + "key": key, + "value": "*bar", + "name": "*bar", + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + }, + { + "count": 1, + "key": key, + "value": "*baz", + "name": "*baz", + "firstSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), + }, + ] def test_tags_keys_autocomplete_noop(self): timestamp = before_now(days=0, minutes=10).replace(microsecond=0) @@ -467,14 +276,12 @@ def test_tags_keys_autocomplete_noop(self): uuid4().hex, uuid4().hex, span_id=uuid4().hex[:15], - organization_id=self.organization.id, parent_span_id=None, timestamp=timestamp, transaction=tag, duration=100, exclusive_time=100, tags={"tag": tag}, - is_eap=self.is_eap, ) for key in [ @@ -506,13 +313,8 @@ def test_tags_keys_autocomplete_project(self): self.create_project(id=base_id + 299, name="bar") self.create_project(id=base_id + 399, name="baz") - features = [ - "organizations:performance-trace-explorer", - "organizations:global-views", - ] - for key in ["project", "project.name"]: - response = self.do_request(key, features=features) + response = self.do_request(key) assert response.status_code == 200, response.data assert sorted(response.data, key=lambda v: v["value"]) == [ { @@ -541,7 +343,7 @@ def test_tags_keys_autocomplete_project(self): }, ] - response = self.do_request(key, query={"query": "ba"}, features=features) + response = self.do_request(key, query={"query": "ba"}) assert response.status_code == 200, response.data assert sorted(response.data, key=lambda v: v["value"]) == [ { @@ -564,7 +366,7 @@ def test_tags_keys_autocomplete_project(self): key = "project.id" - response = self.do_request(key, features=features) + response = self.do_request(key) assert response.status_code == 200, response.data assert sorted(response.data, key=lambda v: v["value"]) == [ { @@ -593,7 +395,7 @@ def test_tags_keys_autocomplete_project(self): }, ] - response = self.do_request(key, query={"query": "99"}, features=features) + response = self.do_request(key, query={"query": "99"}) assert response.status_code == 200, response.data assert sorted(response.data, key=lambda v: v["value"]) == [ { @@ -622,12 +424,10 @@ def test_tags_keys_autocomplete_span_status(self): uuid4().hex, uuid4().hex, span_id=uuid4().hex[:15], - organization_id=self.organization.id, parent_span_id=None, timestamp=timestamp, transaction="foo", status=status, - is_eap=self.is_eap, ) response = self.do_request("span.status") @@ -679,49 +479,3 @@ def test_tags_keys_autocomplete_span_status(self): "lastSeen": timestamp.strftime("%Y-%m-%dT%H:%M:%S+00:00"), }, ] - - -class OrganizationEAPSpansTagKeyValuesEndpointTest(OrganizationSpansTagKeyValuesEndpointTest): - is_eap = True - - def do_request(self, key: str, query=None, features=None, **kwargs): - if features is None: - features = ["organizations:performance-trace-explorer"] - - features.append("organizations:visibility-explore-dataset") - - if query is None: - query = {} - query["dataset"] = "spans" - query["type"] = "string" - - with self.feature(features): - return self.client.get( - reverse( - self.view, - kwargs={"organization_id_or_slug": self.organization.slug, "key": key}, - ), - query, - format="json", - **kwargs, - ) - - @pytest.mark.skip("autcomplete project doesnt work yet") - def test_tags_keys_autocomplete_project(self): - super().test_tags_keys_autocomplete_project() - - @pytest.mark.skip("autcomplete span.status doesnt work yet") - def test_tags_keys_autocomplete_span_status(self): - super().test_tags_keys_autocomplete_project() - - @pytest.mark.skip("autcomplete transaction doesnt work yet") - def test_transaction_keys_autocomplete(self): - super().test_transaction_keys_autocomplete() - - @pytest.mark.skip("autcomplete transaction doesnt work yet") - def test_transaction_keys_autocomplete_substring(self): - super().test_transaction_keys_autocomplete_substring() - - @pytest.mark.skip("autcomplete transaction doesnt work yet") - def test_transaction_keys_autocomplete_substring_with_asterisk(self): - super().test_transaction_keys_autocomplete_substring_with_asterisk()