Keep childSpanIds in start-time order and add nodesBySpanId map

Based on feedback:
1. Keep childSpanIds sorted by start time (as they come from the tree)
2. Add nodesBySpanId: Map<string, TreeNode> to Trace type for direct tree navigation
3. Update getTraceSpanIdsAsTree to return {root, nodesBySpanId}
4. Update critical path to find last-finishing child from start-time sorted children
5. Update statistics self-time calculation to use childSpanIds instead of rebuilding parent-child map

This eliminates the parentChildOfMap rebuilding and allows components to leverage
the tree structure more efficiently.

Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>

Author: Copilot

packages/jaeger-ui/src/components/TracePage/CriticalPath/utils/findLastFinishingChildSpan.tsx

@@ -13,20 +13,32 @@ const findLastFinishingChildSpan = (
   currentSpan: Span,
   returningChildStartTime?: number
 ): Span | undefined => {
-  let lastFinishingChildSpanId: string | undefined;
-  if (returningChildStartTime) {
-    lastFinishingChildSpanId = currentSpan.childSpanIds.find(
-      each =>
-        // Look up the span using the map
-        spanMap.has(each) &&
-        spanMap.get(each)!.startTime + spanMap.get(each)!.duration < returningChildStartTime
-    );
-  } else {
-    // If `returningChildStartTime` is not provided, select the first child span.
-    // As they are sorted based on endTime
-    lastFinishingChildSpanId = currentSpan.childSpanIds[0];
+  let lastFinishingChildSpan: Span | undefined;
+  let latestEndTime = -1;
+
+  // Iterate through children (now sorted by start time) to find the one that finishes last
+  for (const childId of currentSpan.childSpanIds) {
+    const childSpan = spanMap.get(childId);
+    if (!childSpan) continue;
+
+    const childEndTime = childSpan.startTime + childSpan.duration;
+
+    if (returningChildStartTime) {
+      // Find child that finishes just before returningChildStartTime
+      if (childEndTime < returningChildStartTime && childEndTime > latestEndTime) {
+        latestEndTime = childEndTime;
+        lastFinishingChildSpan = childSpan;
+      }
+    } else {
+      // Find child that finishes last overall
+      if (childEndTime > latestEndTime) {
+        latestEndTime = childEndTime;
+        lastFinishingChildSpan = childSpan;
+      }
+    }
   }
-  return lastFinishingChildSpanId ? spanMap.get(lastFinishingChildSpanId) : undefined;
+
+  return lastFinishingChildSpan;
 };
 
 export default findLastFinishingChildSpan;

packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx

@@ -1,42 +1,23 @@
 // Copyright (c) 2020 The Jaeger Authors.
 // SPDX-License-Identifier: Apache-2.0
 
-import memoizeOne from 'memoize-one';
 import { Trace, Span } from '../../../types/trace';
 import { ITableSpan } from './types';
 import colorGenerator from '../../../utils/color-generator';
 
 const serviceName = 'Service Name';
 const operationName = 'Operation Name';
 
-function parentChildOfMap(allSpans: Span[]): Record<string, Span[]> {
-  const parentChildOfMap: Record<string, Span[]> = {};
-  allSpans.forEach(s => {
-    if (s.references) {
-      // Filter for CHILD_OF we don't want to calculate FOLLOWS_FROM (prod-cons)
-      const parentIDs = s.references.filter(r => r.refType === 'CHILD_OF').map(r => r.spanID);
-      parentIDs.forEach((pID: string) => {
-        parentChildOfMap[pID] = parentChildOfMap[pID] || [];
-        parentChildOfMap[pID].push(s);
-      });
-    }
-  });
-  return parentChildOfMap;
-}
-
-const memoizedParentChildOfMap = memoizeOne(parentChildOfMap);
-
-function getChildOfSpans(parentID: string, allSpans: Span[]): Span[] {
-  return memoizedParentChildOfMap(allSpans)[parentID] || [];
-}
-
-function computeSelfTime(parentSpan: Span, allSpans: Span[]): number {
+function computeSelfTime(parentSpan: Span, allSpans: Span[], spanMap: Map<string, Span>): number {
   if (!parentSpan.hasChildren) return parentSpan.duration;
 
   let parentSpanSelfTime = parentSpan.duration;
   let previousChildEndTime = parentSpan.startTime;
 
-  const children = getChildOfSpans(parentSpan.spanID, allSpans).sort((a, b) => a.startTime - b.startTime);
+  // Use childSpanIds from the span (already sorted by startTime from the tree)
+  const children = parentSpan.childSpanIds
+    .map(id => spanMap.get(id))
+    .filter((child): child is Span => child !== undefined);
 
   const parentSpanEndTime = parentSpan.startTime + parentSpan.duration;
 
@@ -79,7 +60,13 @@ function computeSelfTime(parentSpan: Span, allSpans: Span[]): number {
   return parentSpanSelfTime;
 }
 
-function computeColumnValues(trace: Trace, span: Span, allSpans: Span[], resultValue: StatsPerTag) {
+function computeColumnValues(
+  trace: Trace,
+  span: Span,
+  allSpans: Span[],
+  spanMap: Map<string, Span>,
+  resultValue: StatsPerTag
+) {
   const resultValueChange = resultValue;
   resultValueChange.count += 1;
   resultValueChange.total += span.duration;
@@ -90,7 +77,7 @@ function computeColumnValues(trace: Trace, span: Span, allSpans: Span[], resultV
     resultValueChange.max = span.duration;
   }
 
-  const tempSelf = computeSelfTime(span, allSpans);
+  const tempSelf = computeSelfTime(span, allSpans, spanMap);
   if (resultValueChange.selfMin > tempSelf) {
     resultValueChange.selfMin = tempSelf;
   }
@@ -178,6 +165,8 @@ function getTagValueFromSpan(tagKey: string, span: Span) {
  */
 function valueFirstDropdown(selectedTagKey: string, trace: Trace) {
   const allSpans = trace.spans;
+  // Use the pre-built spanMap or create one if not available (e.g., in tests)
+  const spanMap = trace.spanMap || new Map(allSpans.map(s => [s.spanID, s]));
 
   // used to build the table
   const allTableValues = [];
@@ -199,6 +188,7 @@ function valueFirstDropdown(selectedTagKey: string, trace: Trace) {
       trace,
       allSpans[i],
       allSpans,
+      spanMap,
       statsPerTagValue[tagValue] ?? getDefaultStatsValue(trace)
     );
 
@@ -251,7 +241,7 @@ function valueFirstDropdown(selectedTagKey: string, trace: Trace) {
     // Others is calculated
     let resultValue = getDefaultStatsValue(trace);
     for (let i = 0; i < spanWithNoSelectedTag.length; i++) {
-      resultValue = computeColumnValues(trace, spanWithNoSelectedTag[i], allSpans, resultValue);
+      resultValue = computeColumnValues(trace, spanWithNoSelectedTag[i], allSpans, spanMap, resultValue);
     }
     if (resultValue.count !== 0) {
       // Others is build
@@ -290,6 +280,7 @@ function valueFirstDropdown(selectedTagKey: string, trace: Trace) {
 function buildDetail(
   tempArray: Span[],
   allSpans: Span[],
+  spanMap: Map<string, Span>,
   selectedTagKeySecond: string,
   parentName: string,
   trace: Trace
@@ -310,6 +301,7 @@ function buildDetail(
       trace,
       tempArray[i],
       allSpans,
+      spanMap,
       statsPerTagValue[tagValue] ?? getDefaultStatsValue(trace)
     );
 
@@ -355,7 +347,12 @@ function buildDetail(
 /**
  * Used to generate detail rest.
  */
-function generateDetailRest(allColumnValues: ITableSpan[], selectedTagKeySecond: string, trace: Trace) {
+function generateDetailRest(
+  allColumnValues: ITableSpan[],
+  selectedTagKeySecond: string,
+  trace: Trace,
+  spanMap: Map<string, Span>
+) {
   const allSpans = trace.spans;
   const newTable = [];
   for (let i = 0; i < allColumnValues.length; i++) {
@@ -386,7 +383,7 @@ function generateDetailRest(allColumnValues: ITableSpan[], selectedTagKeySecond:
             }
           }
           if (rest) {
-            resultValue = computeColumnValues(trace, allSpans[j], allSpans, resultValue);
+            resultValue = computeColumnValues(trace, allSpans[j], allSpans, spanMap, resultValue);
           }
         }
       }
@@ -432,6 +429,8 @@ function valueSecondDropdown(
 ) {
   const allSpans = trace.spans;
   const allTableValues = [];
+  // Use the pre-built spanMap or create one if not available (e.g., in tests)
+  const spanMap = trace.spanMap || new Map(allSpans.map(s => [s.spanID, s]));
 
   const isSecondDropdownTag = selectedTagKeySecond !== serviceName && selectedTagKeySecond !== operationName;
 
@@ -468,6 +467,7 @@ function valueSecondDropdown(
     const newColumnValues = buildDetail(
       spansWithSecondTag,
       allSpans,
+      spanMap,
       selectedTagKeySecond,
       actualTableValues[i].name,
       trace
@@ -484,7 +484,7 @@ function valueSecondDropdown(
 
   // if second dropdown is a tag a rest must be created
   if (isSecondDropdownTag) {
-    return generateDetailRest(allTableValues, selectedTagKeySecond, trace);
+    return generateDetailRest(allTableValues, selectedTagKeySecond, trace, spanMap);
     // if no tag is selected the values can be returned
   }
   return allTableValues;

packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx

@@ -22,7 +22,7 @@ function transformTracesToPaths(
       // Use the pre-built spanMap and tree from the trace object if available,
       // otherwise build them on demand (e.g., in tests)
       const spanMap = data.spanMap || null;
-      const tree = data.tree || getTraceSpanIdsAsTree(data, spanMap);
+      const tree = data.tree || getTraceSpanIdsAsTree(data, spanMap).root;
       const { traceID } = data;
       // Ensure we have a spanMap for looking up spans
       const effectiveSpanMap = spanMap || new Map(data.spans.map(span => [span.spanID, span]));

packages/jaeger-ui/src/model/transform-trace-data.tsx

@@ -105,7 +105,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
   }
   // tree is necessary to sort the spans, so children follow parents, and
   // siblings are sorted by start time
-  const tree = getTraceSpanIdsAsTree(data, spanMap);
+  const { root: tree, nodesBySpanId } = getTraceSpanIdsAsTree(data, spanMap);
   const spans: Span[] = [];
   const svcCounts: Record<string, number> = {};
 
@@ -122,16 +122,8 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     span.relativeStartTime = span.startTime - traceStartTime;
     span.depth = depth - 1;
     span.hasChildren = node.children.length > 0;
-    // Get the childSpanIds sorted based on endTime without changing tree structure
-    // TODO move this enrichment into Critical Path computation
-    span.childSpanIds = node.children
-      .slice()
-      .sort((a, b) => {
-        const spanA = spanMap.get(a.value)!;
-        const spanB = spanMap.get(b.value)!;
-        return spanB.startTime + spanB.duration - (spanA.startTime + spanA.duration);
-      })
-      .map(each => each.value);
+    // Get the childSpanIds sorted by startTime (as they are in the tree)
+    span.childSpanIds = node.children.map(each => each.value);
     span.warnings = span.warnings || [];
     span.tags = span.tags || [];
     span.references = span.references || [];
@@ -182,6 +174,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     // Optimized data structures - created once during trace transformation
     spanMap,
     tree,
+    nodesBySpanId,
     // Can't use spread operator for intersection types
     // repl: https://goo.gl/4Z23MJ
     // issue: https://github.com/facebook/flow/issues/1511

packages/jaeger-ui/src/selectors/trace.test.js

@@ -11,7 +11,7 @@ describe('getTraceSpanIdsAsTree()', () => {
   const getTraceSpansAsMap = trace =>
     trace.spans.reduce((map, span) => map.set(span.spanID, span), new Map());
   it('builds the tree properly', () => {
-    const tree = traceSelectors.getTraceSpanIdsAsTree(generatedTrace);
+    const { root: tree, nodesBySpanId } = traceSelectors.getTraceSpanIdsAsTree(generatedTrace);
     const spanMap = getTraceSpansAsMap(generatedTrace);
 
     tree.walk((value, node) => {
@@ -22,6 +22,12 @@ describe('getTraceSpanIdsAsTree()', () => {
         expect(parentId).toBe(expectedParentValue);
       });
     });
+
+    // Verify nodesBySpanId contains all spans
+    expect(nodesBySpanId.size).toBe(generatedTrace.spans.length);
+    generatedTrace.spans.forEach(span => {
+      expect(nodesBySpanId.has(span.spanID)).toBe(true);
+    });
   });
 
   it('#115 - handles FOLLOW_FROM refs', () => {

packages/jaeger-ui/src/selectors/trace.tsx

@@ -18,8 +18,7 @@ export const TREE_ROOT_ID = '__root__';
  *
  * @param  {Trace}             trace The trace to build the tree of spanIDs.
  * @param  {Map<string, Span>} spanMap map from span IDs to Spans
- * @return {TreeNode}          A tree of spanIDs derived from the relationships
- *                             between spans in the trace.
+ * @return {{ root: TreeNode, nodesBySpanId: Map }}  The tree root and a map of spanID to TreeNode
  */
 export function getTraceSpanIdsAsTree(
   trace: { spans: SpanData[] },
@@ -60,5 +59,5 @@ export function getTraceSpanIdsAsTree(
     }
   });
   root.children.sort(comparator);
-  return root;
+  return { root, nodesBySpanId: nodesById };
 }

packages/jaeger-ui/src/types/trace.tsx

@@ -83,6 +83,7 @@ export type Trace = TraceData & {
   // Optional to support test scenarios where traces may be mocked
   spanMap?: Map<string, Span>;
   tree?: TreeNode<string>;
+  nodesBySpanId?: Map<string, TreeNode<string>>;
 
   // OTEL facade - lazy-initialized and memoized
   _otelFacade?: IOtelTrace;