Browse Source

feat(agg-spans): Support http.method filter (#58443)

Allow passing an http.method filter to the aggregate span waterfall.
Also, return empty span descriptions if it is not a parameterized
span description to avoid confusion and restrict getting descriptions
from "sentry_tags"
Shruthi 1 year ago
parent
commit
eb3047703c

+ 13 - 7
src/sentry/api/endpoints/organization_spans_aggregation.py

@@ -104,7 +104,6 @@ class BaseAggregateSpans:
             the md5 hash of the value A-C-D and for span E would be md5 hash of the value A-C1-E.
         """
         key = span_tree["key"]
-        description = span_tree["description"]
         start_timestamp = span_tree["start_timestamp_ms"]
         if root_prefix is None:
             prefix = key
@@ -151,7 +150,7 @@ class BaseAggregateSpans:
                 "parent_node_fingerprint": parent_node_fingerprint,
                 "group": span_tree["group"],
                 "op": span_tree["op"],
-                "description": description if span_tree["group"] == NULL_GROUP else description,
+                "description": "" if span_tree["group"] == NULL_GROUP else span_tree["description"],
                 "start_timestamp": start_timestamp,
                 "start_ms": start_timestamp,  # TODO: Remove after updating frontend, duplicated for backward compatibility
                 "avg(exclusive_time)": span_tree["exclusive_time"],
@@ -263,9 +262,7 @@ class AggregateNodestoreSpans(BaseAggregateSpans):
                         "group": span.get("sentry_tags", {}).get("group")
                         or span.get("data", {}).get("span.group", NULL_GROUP),
                         "group_raw": span["hash"],
-                        "description": span.get("sentry_tags", {}).get("description")
-                        or span.get("data", {}).get("span.description")
-                        or span.get("description", ""),
+                        "description": span.get("sentry_tags", {}).get("description", ""),
                         "op": span.get("op", ""),
                         "start_timestamp_ms": span["start_timestamp"]
                         * 1000,  # timestamp is unix timestamp, convert to ms
@@ -315,6 +312,7 @@ class OrganizationSpansAggregationEndpoint(OrganizationEventsEndpointBase):
             return Response(status=404)
 
         transaction = request.query_params.get("transaction", None)
+        http_method = request.query_params.get("http.method", None)
         if transaction is None:
             return Response(
                 status=status.HTTP_400_BAD_REQUEST, data={"details": "Transaction not provided"}
@@ -326,12 +324,16 @@ class OrganizationSpansAggregationEndpoint(OrganizationEventsEndpointBase):
                 status=status.HTTP_400_BAD_REQUEST, data={"details": "Backend not supported"}
             )
 
+        query = f"transaction:{transaction}"
+        if http_method is not None:
+            query += f" transaction.method:{http_method}"
+
         if backend == "indexedSpans":
             builder = SpansIndexedQueryBuilder(
                 dataset=Dataset.SpansIndexed,
                 params=params,
                 selected_columns=["transaction_id", "count()"],
-                query=f"transaction:{transaction}",
+                query=query,
                 limit=100,
             )
 
@@ -370,9 +372,13 @@ class OrganizationSpansAggregationEndpoint(OrganizationEventsEndpointBase):
 
             return Response(data=aggregated_tree)
 
+        conditions = [["transaction", "=", transaction]]
+        if http_method is not None:
+            conditions.append(["http.method", "=", http_method])
+
         events = eventstore.backend.get_events(
             filter=eventstore.Filter(
-                conditions=[["transaction", "=", transaction]],
+                conditions=conditions,
                 start=params["start"],
                 end=params["end"],
                 project_ids=params["project_id"],

+ 72 - 1
tests/snuba/api/endpoints/test_organization_spans_aggregation.py

@@ -4,6 +4,7 @@ from unittest import mock
 from uuid import uuid4
 
 from django.urls import reverse
+from snuba_sdk import Column, Condition, Function, Op
 
 from sentry.api.endpoints.organization_spans_aggregation import NULL_GROUP
 from sentry.testutils.cases import APITestCase, SnubaTestCase
@@ -250,6 +251,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "B",
                         "span.description": "connect",
                     },
+                    "sentry_tags": {
+                        "description": "connect",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -265,6 +269,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "C",
                         "span.description": "connect",
                     },
+                    "sentry_tags": {
+                        "description": "connect",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -280,6 +287,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "D",
                         "span.description": "resolve_orderby",
                     },
+                    "sentry_tags": {
+                        "description": "resolve_orderby",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -329,6 +339,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "B",
                         "span.description": "connect",
                     },
+                    "sentry_tags": {
+                        "description": "connect",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -344,6 +357,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "C",
                         "span.description": "connect",
                     },
+                    "sentry_tags": {
+                        "description": "connect",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -359,6 +375,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "D",
                         "span.description": "resolve_orderby",
                     },
+                    "sentry_tags": {
+                        "description": "resolve_orderby",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -374,6 +393,9 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
                         "span.group": "D",
                         "span.description": "resolve_orderby",
                     },
+                    "sentry_tags": {
+                        "description": "resolve_orderby",
+                    },
                 },
                 {
                     "same_process_as_parent": True,
@@ -488,5 +510,54 @@ class OrganizationSpansAggregationTest(APITestCase, SnubaTestCase):
             data = response.data
             root_fingerprint = hashlib.md5(b"e238e6c2e2466b07-C-discover.snql").hexdigest()[:16]
             assert root_fingerprint in data
-            assert data[root_fingerprint]["description"] == "resolve_columns"
+            assert data[root_fingerprint]["description"] == ""
             assert data[root_fingerprint]["count()"] == 2
+
+    @mock.patch("sentry.api.endpoints.organization_spans_aggregation.raw_snql_query")
+    def test_http_method_filter(self, mock_query):
+        with self.feature(self.FEATURES):
+            response = self.client.get(
+                self.url,
+                data={"transaction": "api/0/foo", "backend": "nodestore", "http.method": "GET"},
+                format="json",
+            )
+
+        assert response.data
+        data = response.data
+        root_fingerprint = hashlib.md5(b"e238e6c2e2466b07").hexdigest()[:16]
+        assert root_fingerprint in data
+        assert data[root_fingerprint]["count()"] == 2
+
+        with self.feature(self.FEATURES):
+            response = self.client.get(
+                self.url,
+                data={"transaction": "api/0/foo", "backend": "nodestore", "http.method": "POST"},
+                format="json",
+            )
+
+        assert response.data == {}
+
+        with self.feature(self.FEATURES):
+            self.client.get(
+                self.url,
+                data={"transaction": "api/0/foo", "backend": "indexedSpans", "http.method": "GET"},
+                format="json",
+            )
+
+            assert (
+                Condition(
+                    lhs=Function(
+                        function="ifNull",
+                        parameters=[
+                            Column(
+                                name="tags[transaction.method]",
+                            ),
+                            "",
+                        ],
+                        alias=None,
+                    ),
+                    op=Op.EQ,
+                    rhs="GET",
+                )
+                in mock_query.mock_calls[0].args[0].query.where
+            )