feat(trace-explorer): Update the breakdown generation (#69294)

Previously, the breakdown generation produced a bunch of back to back
non overlapping intervals. This meant that a continuous time spent in a
single project can be broken down into chunks by some time spent in
other projects making it hard to associate spans to this breakdown. This
change connects these broken intervals by allowing the breakdowns to
contain overlapping intervals.

src/sentry/api/endpoints/organization_traces.py

@@ -30,7 +30,7 @@ class TraceInterval(TypedDict):
     project: str | None
     start: int
     end: int
-    kind: Literal["project", "missing", "unknown"]
+    kind: Literal["project", "missing", "other"]
 
 
 class TraceResult(TypedDict):
@@ -308,67 +308,57 @@ def process_breakdowns(data, traces_range):
     stacks: Mapping[str, list[TraceInterval]] = defaultdict(list)
 
     def breakdown_push(trace, interval):
-        trace_range = traces_range.get(trace)
-        if trace_range:
+        # 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)
 
         breakdown = breakdowns[trace]
 
-        if breakdown:
+        # Find the last interval. If there is an interval on the stack, it
+        # should take priority over intervals in the breakdown because intervals
+        # on the stack are always active, where intervals on the breakdown are
+        # the most recently started, and it's possible older intervals end after
+        # the newer intervals
+        last_interval = stack_peek(trace)
+        if last_interval is None and breakdown:
             last_interval = breakdown[-1]
 
-            # An interval that overlaps with existing part of the breakdown was
-            # pushed, truncate it to remove the overlapping area.
-            if last_interval["end"] > interval["start"]:
-                interval["start"] = last_interval["end"]
-
-        # empty interval, skip it
-        if interval["start"] == interval["end"]:
-            return
-
-        if breakdown:
-            last_interval = breakdown[-1]
-
-            # A gap in the breakdown was found, fill it with an interval
-            if last_interval["end"] < interval["start"]:
-                last = stack_peek(trace)
-                if last:
-                    # Something is still on the stack, so attribute this gap to
-                    # that project
-                    breakdown.append(
-                        {
-                            "project": last["project"],
-                            "start": last_interval["end"],
-                            "end": interval["start"],
-                            "kind": "project",
-                        }
-                    )
-                else:
-                    # Nothing is found on the stack, so label it as missing
-                    breakdown.append(
-                        {
-                            "project": None,
-                            "start": last_interval["end"],
-                            "end": interval["start"],
-                            "kind": "missing",
-                        }
-                    )
+        if last_interval and last_interval["end"] < interval["start"]:
+            # A gap in the breakdown was found, fill it with a missing interval
+            breakdown.append(
+                {
+                    "project": None,
+                    "start": last_interval["end"],
+                    "end": interval["start"],
+                    "kind": "missing",
+                }
+            )
 
         breakdown.append(interval)
 
     def stack_push(trace, interval):
-        last = stack_peek(trace)
+        last_interval = stack_peek(trace)
         if (
-            last is not None
-            and last["project"] == interval["project"]
-            and last["end"] >= interval["start"]
+            last_interval
+            and last_interval["project"] == interval["project"]
+            and last_interval["end"] >= interval["start"]
         ):
-            # The new interval can be merged with last interval
-            last["end"] = max(interval["end"], last["end"])
-        else:
-            stacks[trace].append(interval)
+            # update the end of this interval and it will
+            # be updated in the breakdown as well
+            last_interval["end"] = max(interval["end"], last_interval["end"])
+            return
+
+        # Make sure to push the breakdown before the stack. This is because
+        # pushing the breakdown can fill in missing intervals but that needs
+        # to know what the current state of the stack is. If we push the
+        # interval onto the stack first, it would not generate the missing
+        # intervals correctly.
+        breakdown_push(trace, interval)
+
+        stack = stacks[trace]
+        stack.append(interval)
 
     def stack_peek(trace):
         if not stacks[trace]:
@@ -376,18 +366,13 @@ def process_breakdowns(data, traces_range):
         return stacks[trace][-1]
 
     def stack_pop(trace):
-        interval = stacks[trace].pop()
+        return stacks[trace].pop()
 
-        return interval
-
-    def stack_clear(trace, until=None, insert=True):
+    def stack_clear(trace, until=None):
         while stacks[trace]:
-            if until is not None and stack_peek(trace)["end"] > until:
+            if until is not None and stack_peek(trace)["end"] >= until:
                 break
-
-            interval = stack_pop(trace)
-            if insert:
-                breakdown_push(trace, interval)
+            stack_pop(trace)
 
     for row in data:
         trace = row["trace"]
@@ -399,68 +384,36 @@ def process_breakdowns(data, traces_range):
             "kind": "project",
         }
 
-        # Nothing on the stack yet, so directly push onto the stack and wait for
-        # next item to come so an interval can be determined
-        if not stack_peek(trace):
-            stack_push(trace, cur)
-            continue
-
         # Clear the stack of any intervals that end before the current interval
         # starts while pushing them to the breakdowns.
-        stack_clear(trace, until=cur["start"], insert=True)
+        stack_clear(trace, until=cur["start"])
 
-        # At this point, any interval remaining on the stack MUST overlap with
-        # the current interval we're working with because we've cleared the
-        # stack of all intervals that have ended.
-
-        # This is the last interval that is active during the current interval
-        prev = stack_peek(trace)
-
-        if prev is not None and prev["project"] == cur["project"]:
-            # Same project as the previous interval, so push it to the stack and
-            # let them merge.
-            stack_push(trace, cur)
-            continue
-
-        if prev is not None:
-            # This implies that there is some overlap between this transaction
-            # and the previous transaction. So we need push the interval up to
-            # the current item to the breakdown.
-
-            breakdown_push(
-                trace,
-                {
-                    "project": prev["project"],
-                    "start": prev["start"],
-                    "end": cur["start"],
-                    "kind": "project",
-                },
-            )
+        stack_push(trace, cur)
 
         # Clear the stack of any intervals that end before the current interval
         # ends. Here we do not need to push them to the breakdowns because
         # that time has already be attributed to the most recent interval.
-        stack_clear(trace, until=cur["end"], insert=False)
+        stack_clear(trace, until=cur["end"])
 
-        stack_push(trace, cur)
+    for trace, (trace_start, trace_end) in traces_range.items():
+        # Check to see if there is still a gap before the trace ends and fill it
+        # with an other interval.
 
-    for trace in stacks:
-        stack_clear(trace, insert=True)
+        other = {
+            "project": None,
+            "start": trace_start,
+            "end": trace_end,
+            "kind": "other",
+        }
 
-        # Check to see if there is still a gap before the trace ends and fill it
-        # with an unknown interval.
-        breakdown = breakdowns[trace]
-        trace_range = traces_range.get(trace)
-        if breakdown and trace_range:
-            left, right = trace_range
-            if breakdown[-1]["end"] < right:
-                breakdown.append(
-                    {
-                        "project": None,
-                        "start": breakdown[-1]["end"],
-                        "end": right,
-                        "kind": "unknown",
-                    }
-                )
+        # Clear the remaining intervals on the stack to find the latest end time
+        # of the intervals. This will be used to decide if there are any portion
+        # of the trace that was not covered by one of the intervals.
+        while stacks[trace]:
+            interval = stack_pop(trace)
+            other["start"] = max(other["start"], interval["end"])
+
+        if other["start"] < other["end"]:
+            breakdown_push(trace, other)
 
     return breakdowns

tests/sentry/api/endpoints/test_organization_traces.py

@@ -243,7 +243,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         {
                             "project": project_1.slug,
                             "start": int(timestamps[0].timestamp() * 1000),
-                            "end": int(timestamps[1].timestamp() * 1000),
+                            "end": int(timestamps[0].timestamp() * 1000) + 60_100,
                             "kind": "project",
                         },
                         {
@@ -252,12 +252,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                             "end": int(timestamps[3].timestamp() * 1000) + 30_003,
                             "kind": "project",
                         },
-                        {
-                            "project": project_1.slug,
-                            "start": int(timestamps[3].timestamp() * 1000) + 30_003,
-                            "end": int(timestamps[0].timestamp() * 1000) + 60_100,
-                            "kind": "project",
-                        },
                     ],
                     "spans": [
                         {"id": span_ids[3], "parent_span": span_ids[0], "span.duration": 30_003.0},
@@ -280,7 +274,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         {
                             "project": project_1.slug,
                             "start": int(timestamps[4].timestamp() * 1000),
-                            "end": int(timestamps[5].timestamp() * 1000),
+                            "end": int(timestamps[4].timestamp() * 1000) + 90_123,
                             "kind": "project",
                         },
                         {
@@ -289,12 +283,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                             "end": int(timestamps[6].timestamp() * 1000) + 20_006,
                             "kind": "project",
                         },
-                        {
-                            "project": project_1.slug,
-                            "start": int(timestamps[6].timestamp() * 1000) + 20_006,
-                            "end": int(timestamps[4].timestamp() * 1000) + 90_123,
-                            "kind": "project",
-                        },
                     ],
                     "spans": [
                         {"id": span_ids[6], "parent_span": span_ids[4], "span.duration": 20_006.0},
@@ -358,7 +346,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 25,
+                        "end": 100,
                         "kind": "project",
                     },
                     {
@@ -367,12 +355,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 75,
                         "kind": "project",
                     },
-                    {
-                        "project": "foo",
-                        "start": 75,
-                        "end": 100,
-                        "kind": "project",
-                    },
                 ],
             },
             id="two transactions different project nested",
@@ -408,13 +390,13 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 25,
+                        "end": 50,
                         "kind": "project",
                     },
                     {
                         "project": "bar",
                         "start": 25,
-                        "end": 50,
+                        "end": 75,
                         "kind": "project",
                     },
                     {
@@ -606,13 +588,13 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 20,
+                        "end": 100,
                         "kind": "project",
                     },
                     {
                         "project": "bar",
                         "start": 20,
-                        "end": 40,
+                        "end": 80,
                         "kind": "project",
                     },
                     {
@@ -621,18 +603,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 60,
                         "kind": "project",
                     },
-                    {
-                        "project": "bar",
-                        "start": 60,
-                        "end": 80,
-                        "kind": "project",
-                    },
-                    {
-                        "project": "foo",
-                        "start": 80,
-                        "end": 100,
-                        "kind": "project",
-                    },
                 ],
             },
             id="three transactions different project nested",
@@ -668,7 +638,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 25,
+                        "end": 100,
                         "kind": "project",
                     },
                     {
@@ -683,12 +653,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 75,
                         "kind": "project",
                     },
-                    {
-                        "project": "foo",
-                        "start": 75,
-                        "end": 100,
-                        "kind": "project",
-                    },
                 ],
             },
             id="three transactions different project 2 overlap the first",
@@ -724,7 +688,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 20,
+                        "end": 50,
                         "kind": "project",
                     },
                     {
@@ -733,12 +697,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 30,
                         "kind": "project",
                     },
-                    {
-                        "project": "foo",
-                        "start": 30,
-                        "end": 50,
-                        "kind": "project",
-                    },
                     {
                         "project": "baz",
                         "start": 50,
@@ -780,7 +738,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 20,
+                        "end": 50,
                         "kind": "project",
                     },
                     {
@@ -789,12 +747,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 30,
                         "kind": "project",
                     },
-                    {
-                        "project": "foo",
-                        "start": 30,
-                        "end": 40,
-                        "kind": "project",
-                    },
                     {
                         "project": "baz",
                         "start": 40,
@@ -836,7 +788,7 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                     {
                         "project": "foo",
                         "start": 0,
-                        "end": 10,
+                        "end": 50,
                         "kind": "project",
                     },
                     {
@@ -845,12 +797,6 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "end": 20,
                         "kind": "project",
                     },
-                    {
-                        "project": "foo",
-                        "start": 20,
-                        "end": 50,
-                        "kind": "project",
-                    },
                 ],
             },
             id="three transactions same project with another project between",
@@ -903,11 +849,11 @@ class OrganizationTracesEndpointTest(BaseSpansTestCase, APITestCase):
                         "project": None,
                         "start": 50,
                         "end": 100,
-                        "kind": "unknown",
+                        "kind": "other",
                     },
                 ],
             },
-            id="adds unknown interval at end",
+            id="adds other interval at end",
         ),
     ],
 )