Просмотр исходного кода

fix(trace-explorer): Quantize breakdown start times for better grouping (#70374)

Using the min breakdown duration actually just removes potentially
useful data from the breakdown. Instead, let's round them based on the
some proportion so that they can be merged with neighbouring breakdowns
where possible.
Tony Xiao 10 месяцев назад
Родитель
Сommit
a266d81bd6

+ 60 - 12
src/sentry/api/endpoints/organization_traces.py

@@ -1,4 +1,5 @@
 import dataclasses
+import itertools
 import math
 from collections import defaultdict
 from collections.abc import Callable, Mapping, MutableMapping
@@ -68,6 +69,7 @@ class OrganizationTracesSerializer(serializers.Serializer):
     )
     suggestedQuery = serializers.CharField(required=False)
     minBreakdownDuration = serializers.IntegerField(default=0, min_value=0)
+    minBreakdownPercentage = serializers.FloatField(default=0.0, min_value=0.0, max_value=1.0)
     maxSpansPerTrace = serializers.IntegerField(default=1, min_value=1, max_value=100)
 
 
@@ -108,6 +110,7 @@ class OrganizationTracesEndpoint(OrganizationEventsV2EndpointBase):
             max_spans_per_trace=serialized["maxSpansPerTrace"],
             breakdown_categories=serialized.get("breakdownCategory", []),
             min_breakdown_duration=serialized["minBreakdownDuration"],
+            min_breakdown_percentage=serialized["minBreakdownPercentage"],
             get_all_projects=lambda: self.get_projects(
                 request,
                 organization,
@@ -148,6 +151,7 @@ class TraceSamplesExecutor:
         max_spans_per_trace: int,
         breakdown_categories: list[str],
         min_breakdown_duration: int,
+        min_breakdown_percentage: float,
         get_all_projects: Callable[[], list[Project]],
     ):
         self.params = params
@@ -162,6 +166,7 @@ class TraceSamplesExecutor:
         self.max_spans_per_trace = max_spans_per_trace
         self.breakdown_categories = breakdown_categories
         self.min_breakdown_duration = min_breakdown_duration
+        self.min_breakdown_percentage = min_breakdown_percentage
         self.get_all_projects = get_all_projects
         self._all_projects: list[Project] | None = None
 
@@ -608,13 +613,22 @@ class TraceSamplesExecutor:
     ) -> list[TraceResult]:
         # mapping of trace id to a tuple of start/finish times
         traces_range = {
-            row["trace"]: (row["first_seen()"], row["last_seen()"])
+            row["trace"]: {
+                "start": row["first_seen()"],
+                "end": row["last_seen()"],
+                "min": int(
+                    self.min_breakdown_percentage * (row["last_seen()"] - row["first_seen()"])
+                ),
+            }
             for row in traces_metas_results["data"]
         }
 
         spans = [
-            *traces_breakdown_projects_results["data"],
-            *traces_breakdown_categories_results["data"],
+            span
+            for span in itertools.chain(
+                traces_breakdown_projects_results["data"],
+                traces_breakdown_categories_results["data"],
+            )
         ]
         spans.sort(key=lambda span: (span["precise.start_ts"], span["precise.finish_ts"]))
 
@@ -940,6 +954,30 @@ class TraceSamplesExecutor:
         return suggested_spans_query
 
 
+def quantize_range(span_start, span_end, trace_range):
+    trace_start = trace_range["start"]
+    trace_end = trace_range["end"]
+    min_duration = trace_range["min"]
+
+    span_duration = span_end - span_start
+
+    if min_duration > 0:
+        rounded_start = (
+            round((span_start - trace_start) / min_duration) * min_duration + trace_start
+        )
+    else:
+        rounded_start = span_start
+
+    # To avoid creating gaps at the end of the trace,
+    # do not adjust the end if it's at the trace end.
+    if span_end >= trace_end:
+        rounded_end = span_end
+    else:
+        rounded_end = rounded_start + span_duration
+
+    return rounded_start, rounded_end
+
+
 def process_breakdowns(data, traces_range):
     breakdowns: Mapping[str, list[TraceInterval]] = defaultdict(list)
     stacks: Mapping[str, list[TraceInterval]] = defaultdict(list)
@@ -954,9 +992,10 @@ def process_breakdowns(data, traces_range):
     def breakdown_push(trace, interval):
         # Clip the intervals os that it is within range of the trace
         if trace_range := traces_range.get(trace):
-            left, right = trace_range
-            interval["start"] = clip(interval["start"], left, right)
-            interval["end"] = clip(interval["end"], left, right)
+            start = trace_range["start"]
+            end = trace_range["end"]
+            interval["start"] = clip(interval["start"], start, end)
+            interval["end"] = clip(interval["end"], start, end)
 
         breakdown = breakdowns[trace]
 
@@ -1018,12 +1057,21 @@ def process_breakdowns(data, traces_range):
     for row in data:
         trace = row["trace"]
 
+        precise_start = int(row["precise.start_ts"] * 1000)
+        precise_end = int(row["precise.finish_ts"] * 1000)
+
+        span_start, span_end = quantize_range(
+            precise_start,
+            precise_end,
+            traces_range[trace],
+        )
+
         cur: TraceInterval = {
             "kind": "project",
             "project": row["project"],
             "opCategory": row.get("span.category"),
-            "start": int(row["precise.start_ts"] * 1000),
-            "end": int(row["precise.finish_ts"] * 1000),
+            "start": span_start,
+            "end": span_end,
         }
 
         # Clear the stack of any intervals that end before the current interval
@@ -1037,16 +1085,16 @@ def process_breakdowns(data, traces_range):
         # that time has already be attributed to the most recent interval.
         stack_clear(trace, until=cur["end"])
 
-    for trace, (trace_start, trace_end) in traces_range.items():
+    for trace, trace_range in traces_range.items():
         # Check to see if there is still a gap before the trace ends and fill it
         # with an other interval.
 
         other: TraceInterval = {
+            "kind": "other",
             "project": None,
             "opCategory": None,
-            "start": trace_start,
-            "end": trace_end,
-            "kind": "other",
+            "start": trace_range["start"],
+            "end": trace_range["end"],
         }
 
         # Clear the remaining intervals on the stack to find the latest end time

+ 75 - 14
tests/sentry/api/endpoints/test_organization_traces.py

@@ -766,7 +766,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.1,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -798,7 +798,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -844,7 +844,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.1,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -890,7 +890,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 75)},
+            {"a" * 32: (0, 75, 0)},
             {
                 "a"
                 * 32: [
@@ -936,7 +936,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -968,7 +968,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.1,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -1000,7 +1000,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 75)},
+            {"a" * 32: (0, 75, 0)},
             {
                 "a"
                 * 32: [
@@ -1053,7 +1053,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.06,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -1106,7 +1106,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -1159,7 +1159,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.075,
                 },
             ],
-            {"a" * 32: (0, 75)},
+            {"a" * 32: (0, 75, 0)},
             {
                 "a"
                 * 32: [
@@ -1212,7 +1212,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.06,
                 },
             ],
-            {"a" * 32: (0, 60)},
+            {"a" * 32: (0, 60, 0)},
             {
                 "a"
                 * 32: [
@@ -1265,7 +1265,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.04,
                 },
             ],
-            {"a" * 32: (0, 50)},
+            {"a" * 32: (0, 50, 0)},
             {
                 "a"
                 * 32: [
@@ -1297,7 +1297,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.1,
                 },
             ],
-            {"a" * 32: (0, 50)},
+            {"a" * 32: (0, 50, 0)},
             {
                 "a"
                 * 32: [
@@ -1322,7 +1322,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     "precise.finish_ts": 0.05,
                 },
             ],
-            {"a" * 32: (0, 100)},
+            {"a" * 32: (0, 100, 0)},
             {
                 "a"
                 * 32: [
@@ -1344,8 +1344,69 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
             },
             id="adds other interval at end",
         ),
+        pytest.param(
+            [
+                {
+                    "trace": "a" * 32,
+                    "project": "foo",
+                    "transaction": "foo1",
+                    "precise.start_ts": 0,
+                    "precise.finish_ts": 0.012,
+                },
+                {
+                    "trace": "a" * 32,
+                    "project": "foo",
+                    "transaction": "foo1",
+                    "precise.start_ts": 0.013,
+                    "precise.finish_ts": 0.024,
+                },
+                {
+                    "trace": "a" * 32,
+                    "project": "foo",
+                    "transaction": "foo1",
+                    "precise.start_ts": 0.032,
+                    "precise.finish_ts": 0.040,
+                },
+            ],
+            {"a" * 32: (0, 40, 10)},
+            {
+                "a"
+                * 32: [
+                    {
+                        "project": "foo",
+                        "opCategory": None,
+                        "start": 0,
+                        "end": 21,
+                        "kind": "project",
+                    },
+                    {
+                        "project": None,
+                        "opCategory": None,
+                        "start": 21,
+                        "end": 30,
+                        "kind": "missing",
+                    },
+                    {
+                        "project": "foo",
+                        "opCategory": None,
+                        "start": 30,
+                        "end": 40,
+                        "kind": "project",
+                    },
+                ],
+            },
+            id="merge quantized spans",
+        ),
     ],
 )
 def test_process_breakdowns(data, traces_range, expected):
+    traces_range = {
+        trace: {
+            "start": trace_start,
+            "end": trace_end,
+            "min": trace_min,
+        }
+        for trace, (trace_start, trace_end, trace_min) in traces_range.items()
+    }
     result = process_breakdowns(data, traces_range)
     assert result == expected