Browse Source

feat(trace): Default to all projects for traces (#68119)

- This adds an include_all_projects to get_snuba_params which works the
same way as if the user had passed `project=-1`
- This is cause we always want to load all the projects for a trace
since a trace can span any number of projects in a given org
William Mak 11 months ago
parent
commit
8f0b6bfcaf

+ 15 - 0
src/sentry/api/endpoints/organization_events_trace.py

@@ -8,6 +8,7 @@ from typing import Any, Deque, Optional, TypedDict, TypeVar, cast
 import sentry_sdk
 from django.http import Http404, HttpRequest, HttpResponse
 from rest_framework.exceptions import ParseError
+from rest_framework.request import Request
 from rest_framework.response import Response
 from sentry_relay.consts import SPAN_STATUS_CODE_TO_NAME
 from snuba_sdk import Column, Condition, Function, Op
@@ -743,6 +744,20 @@ class OrganizationEventsTraceEndpointBase(OrganizationEventsV2EndpointBase):
         "GET": ApiPublishStatus.PRIVATE,
     }
 
+    def get_projects(self, request: Request, organization, project_ids=None, project_slugs=None):
+        """The trace endpoint always wants to get all projects regardless of what's passed into the API
+
+        This is because a trace can span any number of projects in an organization. But we still want to
+        use the get_projects function to check for any permissions. So we'll just pass project_ids=-1 everytime
+        which is what would be sent if we wanted all projects"""
+        return super().get_projects(
+            request,
+            organization,
+            project_ids={-1},
+            project_slugs=None,
+            include_all_accessible=True,
+        )
+
     def has_feature(self, organization: Organization, request: HttpRequest) -> bool:
         return bool(
             features.has("organizations:performance-view", organization, actor=request.user)

+ 19 - 7
tests/snuba/api/endpoints/test_organization_events_trace.py

@@ -1571,7 +1571,7 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         self.load_trace()
         with self.feature(self.FEATURES):
             response = self.client_get(
-                data={"project": -1},
+                data={},
             )
         assert response.status_code == 200, response.content
         trace_transaction = response.data["transactions"][0]
@@ -1584,7 +1584,7 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         self.load_trace()
         with self.feature(self.FEATURES):
             response = self.client_get(
-                data={"project": -1, "limit": 200},
+                data={"limit": 200},
             )
         assert response.status_code == 200, response.content
         trace_transaction = response.data["transactions"][0]
@@ -1611,7 +1611,7 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         """Can't use detailed with useSpans, so this should actually just 400"""
         with self.feature(self.FEATURES):
             response = self.client_get(
-                data={"project": -1, "detailed": 1},
+                data={"detailed": 1},
             )
 
         assert response.status_code == 400, response.content
@@ -1629,7 +1629,7 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         self.load_trace()
         with self.feature(self.FEATURES):
             response = self.client_get(
-                data={"project": -1},
+                data={},
             )
 
         assert sorted(
@@ -1649,7 +1649,6 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         with self.feature(self.FEATURES):
             response = self.client_get(
                 data={
-                    "project": -1,
                     "timestamp": self.root_event.timestamp,
                     # Limit of one means the only result is the target event
                     "limit": 1,
@@ -1666,7 +1665,6 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         with self.feature(self.FEATURES):
             response = self.client_get(
                 data={
-                    "project": -1,
                     "timestamp": self.root_event.timestamp,
                     "statsPeriod": "90d",
                 },
@@ -1682,7 +1680,7 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         self.load_trace()
         with self.feature(self.FEATURES):
             response = self.client_get(
-                data={"project": -1},
+                data={},
             )
         assert response.status_code == 200, response.content
         trace_transaction = response.data["transactions"][0]
@@ -1693,6 +1691,20 @@ class OrganizationEventsTraceEndpointTestUsingSpans(OrganizationEventsTraceEndpo
         assert root["measurements"]["fid"]["value"] == 3.5
         assert root["measurements"]["fid"]["type"] == "duration"
 
+    def test_project_param(self):
+        self.load_trace()
+        with self.feature(self.FEATURES):
+            # If project is included we should still return the entire trace
+            response = self.client_get(
+                data={"project": self.project.id},
+            )
+        assert response.status_code == 200, response.content
+        trace_transaction = response.data["transactions"][0]
+        self.assert_trace_data(trace_transaction)
+        # We shouldn't have detailed fields here
+        assert "transaction.status" not in trace_transaction
+        assert "tags" not in trace_transaction
+
 
 class OrganizationEventsTraceMetaEndpointTest(OrganizationEventsTraceEndpointBase):
     url_name = "sentry-api-0-organization-events-trace-meta"