Convert utils/TreeNode.js and selectors/trace.js to Typescript (#1777)

## Which problem is this PR solving? - missing typing ## Description of the changes - convert two files to TS and fix compile/lint errors in the callers --------- Signed-off-by: Yuri Shkuro <[email protected]>
jaegertracing · Sep 12, 2023 · 17eb291 · 17eb291
1 parent e4a8912
commit 17eb291
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 43 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -22,7 +22,7 @@ module.exports = {
   settings: {
     'import/resolver': {
       node: {
-        extensions: ['.js', '.jsx', 'json', '.tsx'],
+        extensions: ['.js', '.jsx', 'json', '.ts', '.tsx'],
       },
     },
   },

diff --git a/packages/jaeger-ui/src/model/transform-trace-data.tsx b/packages/jaeger-ui/src/model/transform-trace-data.tsx
@@ -14,7 +14,7 @@
 
 import _isEqual from 'lodash/isEqual';
 
-import { getTraceSpanIdsAsTree } from '../selectors/trace';
+import { getTraceSpanIdsAsTree, TREE_ROOT_ID } from '../selectors/trace';
 import { getConfigValue } from '../utils/config/get-config';
 import { getTraceName } from './trace-viewer';
 import { KeyValuePair, Span, SpanData, Trace, TraceData } from '../types/trace';
@@ -79,26 +79,25 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
 
   let traceEndTime = 0;
   let traceStartTime = Number.MAX_SAFE_INTEGER;
-  const spanIdCounts = new Map();
+  const spanIdCounts = new Map<string, number>();
   const spanMap = new Map<string, Span>();
   // filter out spans with empty start times
   // eslint-disable-next-line no-param-reassign
   data.spans = data.spans.filter(span => Boolean(span.startTime));
 
-  const max = data.spans.length;
-  for (let i = 0; i < max; i++) {
+  const numSpans = data.spans.length;
+  for (let i = 0; i < numSpans; i++) {
     const span: Span = data.spans[i] as Span;
     const { startTime, duration, processID } = span;
-    //
-    let spanID = span.spanID;
-    // check for start / end time for the trace
+    // update trace's start / end time
     if (startTime < traceStartTime) {
       traceStartTime = startTime;
     }
     if (startTime + duration > traceEndTime) {
       traceEndTime = startTime + duration;
     }
     // make sure span IDs are unique
+    let spanID = span.spanID;
     const idCount = spanIdCounts.get(spanID);
     if (idCount != null) {
       // eslint-disable-next-line no-console
@@ -118,12 +117,12 @@ 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);
+  const tree = getTraceSpanIdsAsTree(data, spanMap);
   const spans: Span[] = [];
   const svcCounts: Record<string, number> = {};
 
-  tree.walk((spanID: string, node: TreeNode, depth = 0) => {
-    if (spanID === '__root__') {
+  tree.walk((spanID: string, node: TreeNode<string>, depth = 0) => {
+    if (spanID === TREE_ROOT_ID) {
       return;
     }
     const span = spanMap.get(spanID) as Span;
@@ -136,6 +135,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     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) => {
@@ -176,7 +176,9 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     spans,
     traceID,
     traceName,
-    // can't use spread operator for intersection types
+    // TODO why not store `tree` here for easier access to tree structure?
+    // ...
+    // Can't use spread operator for intersection types
     // repl: https://goo.gl/4Z23MJ
     // issue: https://github.com/facebook/flow/issues/1511
     processes: data.processes,

diff --git a/packages/jaeger-ui/src/selectors/trace.js → packages/jaeger-ui/src/selectors/trace.tsx b/packages/jaeger-ui/src/selectors/trace.js → packages/jaeger-ui/src/selectors/trace.tsx
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import TreeNode from '../utils/TreeNode';
+import { Span, SpanData } from '../types/trace';
 
 export const TREE_ROOT_ID = '__root__';
 
@@ -26,16 +27,23 @@ export const TREE_ROOT_ID = '__root__';
  *
  * The children are sorted by `span.startTime` after the tree is built.
  *
- * @param  {Trace} trace The trace to build the tree of spanIDs.
- * @return {TreeNode}    A tree of spanIDs derived from the relationships
- *                       between spans in the trace.
+ * @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.
  */
-export function getTraceSpanIdsAsTree(trace) {
-  const nodesById = new Map(trace.spans.map(span => [span.spanID, new TreeNode(span.spanID)]));
-  const spansById = new Map(trace.spans.map(span => [span.spanID, span]));
-  const root = new TreeNode(TREE_ROOT_ID);
+export function getTraceSpanIdsAsTree(
+  trace: { spans: SpanData[] },
+  spanMap: Map<string, Span> | null = null
+) {
+  const nodesById = new Map(trace.spans.map(span => [span.spanID, new TreeNode<string>(span.spanID)]));
+  const spansById = spanMap ?? new Map(trace.spans.map(span => [span.spanID, span]));
+  const root = new TreeNode<string>(TREE_ROOT_ID);
   trace.spans.forEach(span => {
     const node = nodesById.get(span.spanID);
+    if (!node) {
+      return;
+    }
     if (Array.isArray(span.references) && span.references.length) {
       const { refType, spanID: parentID } = span.references[0];
       if (refType === 'CHILD_OF' || refType === 'FOLLOWS_FROM') {
@@ -48,14 +56,17 @@ export function getTraceSpanIdsAsTree(trace) {
       root.children.push(node);
     }
   });
-  const comparator = (nodeA, nodeB) => {
+  const comparator = (nodeA: TreeNode<string>, nodeB: TreeNode<string>): number => {
     const a = spansById.get(nodeA.value);
     const b = spansById.get(nodeB.value);
+    if (!a || !b) {
+      return 0;
+    }
     return +(a.startTime > b.startTime) || +(a.startTime === b.startTime) - 1;
   };
   trace.spans.forEach(span => {
     const node = nodesById.get(span.spanID);
-    if (node.children.length > 1) {
+    if (node && node.children.length > 1) {
       node.children.sort(comparator);
     }
   });

diff --git a/packages/jaeger-ui/src/utils/TreeNode.js → packages/jaeger-ui/src/utils/TreeNode.tsx b/packages/jaeger-ui/src/utils/TreeNode.js → packages/jaeger-ui/src/utils/TreeNode.tsx
@@ -12,25 +12,29 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-export default class TreeNode {
-  static iterFunction(fn, depth = 0) {
-    return node => fn(node.value, node, depth);
+export default class TreeNode<TValue> {
+  value: TValue;
+  children: Array<TreeNode<TValue>>;
+
+  static iterFunction<TValue>(fn: Function, depth = 0) {
+    return (node: TreeNode<TValue>) => fn(node.value, node, depth);
   }
 
-  static searchFunction(search) {
+  static searchFunction<TValue>(search: any) {
     if (typeof search === 'function') {
       return search;
     }
 
-    return (value, node) => (search instanceof TreeNode ? node === search : value === search);
+    return (value: TValue, node: TreeNode<TValue>) =>
+      search instanceof TreeNode ? node === search : value === search;
   }
 
-  constructor(value, children = []) {
+  constructor(value: TValue, children: Array<TreeNode<TValue>> = []) {
     this.value = value;
     this.children = children;
   }
 
-  get depth() {
+  get depth(): number {
     return this.children.reduce((depth, child) => Math.max(child.depth + 1, depth), 1);
   }
 
@@ -40,12 +44,12 @@ export default class TreeNode {
     return i;
   }
 
-  addChild(child) {
+  addChild(child: TreeNode<TValue> | TValue) {
     this.children.push(child instanceof TreeNode ? child : new TreeNode(child));
     return this;
   }
 
-  find(search) {
+  find(search: Function): TreeNode<TValue> | null {
     const searchFn = TreeNode.iterFunction(TreeNode.searchFunction(search));
     if (searchFn(this)) {
       return this;
@@ -59,10 +63,13 @@ export default class TreeNode {
     return null;
   }
 
-  getPath(search) {
+  getPath(search: Function) {
     const searchFn = TreeNode.iterFunction(TreeNode.searchFunction(search));
 
-    const findPath = (currentNode, currentPath) => {
+    const findPath = (
+      currentNode: TreeNode<TValue>,
+      currentPath: Array<TreeNode<TValue>>
+    ): Array<TreeNode<TValue>> | null => {
       // skip if we already found the result
       const attempt = currentPath.concat([currentNode]);
       // base case: return the array when there is a match
@@ -82,14 +89,20 @@ export default class TreeNode {
     return findPath(this, []);
   }
 
-  walk(fn, depth = 0) {
-    const nodeStack = [];
-    let actualDepth = depth;
+  walk(fn: Function, startDepth = 0) {
+    type StackEntry = {
+      node: TreeNode<TValue>;
+      depth: number;
+    };
+    const nodeStack: Array<StackEntry> = [];
+    let actualDepth = startDepth;
     nodeStack.push({ node: this, depth: actualDepth });
     while (nodeStack.length) {
-      const { node, depth: nodeDepth } = nodeStack.pop();
-      fn(node.value, node, nodeDepth);
-      actualDepth = nodeDepth + 1;
+      const entry: StackEntry = nodeStack[nodeStack.length - 1];
+      nodeStack.pop();
+      const { node, depth } = entry;
+      fn(node.value, node, depth);
+      actualDepth = depth + 1;
       let i = node.children.length - 1;
       while (i >= 0) {
         nodeStack.push({ node: node.children[i], depth: actualDepth });
@@ -98,10 +111,14 @@ export default class TreeNode {
     }
   }
 
-  paths(fn) {
-    const stack = [];
+  paths(fn: Function) {
+    type StackEntry = {
+      node: TreeNode<TValue>;
+      childIndex: number;
+    };
+    const stack: Array<StackEntry> = [];
     stack.push({ node: this, childIndex: 0 });
-    const paths = [];
+    const paths: Array<TValue> = [];
     while (stack.length) {
       const { node, childIndex } = stack[stack.length - 1];
       if (node.children.length >= childIndex + 1) {

diff --git a/packages/jaeger-ui/tsconfig.lint.json b/packages/jaeger-ui/tsconfig.lint.json
@@ -50,10 +50,8 @@
     "src/reducers/services.js",
     "src/reducers/trace.js",
     "src/selectors/dependencies.js",
-    "src/selectors/trace.js",
     "src/utils/configure-store.js",
     "src/utils/sort.js",
-    "src/utils/TreeNode.js",
     "package.json"
   ]
 }