ref: Use Map() for trace.spans for performance (#690)

BYK · web-flow · commit a6dee826c983 · 2025-02-04T21:48:07.000Z
diff --git a/.changeset/sharp-paws-press.md b/.changeset/sharp-paws-press.md
@@ -0,0 +1,8 @@
+---
+'@spotlightjs/spotlight': patch
+'@spotlightjs/electron': patch
+'@spotlightjs/overlay': patch
+'@spotlightjs/astro': patch
+---
+
+Make things snappy by using Map() for trace span look ups
diff --git a/packages/overlay/src/integrations/sentry/components/explore/traces/TraceDetails/components/TraceTreeview.tsx b/packages/overlay/src/integrations/sentry/components/explore/traces/TraceDetails/components/TraceTreeview.tsx
@@ -30,7 +30,7 @@ export default function TraceTreeview({ traceId }: TraceTreeViewProps) {
           <span>&mdash;</span>
           <span>
             <strong className="text-primary-200 font-bold">{getFormattedSpanDuration(trace)}</strong> recorded in{' '}
-            <strong className="text-primary-200 font-bold">{trace.spans.length.toLocaleString()} spans</strong>
+            <strong className="text-primary-200 font-bold">{trace.spans.size.toLocaleString()} spans</strong>
           </span>
         </div>
       </div>
diff --git a/packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx b/packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx
@@ -65,7 +65,7 @@ export default function TraceList() {
                     <div>{getFormattedSpanDuration(trace)}</div>
                     <div>&mdash;</div>
                     <div>
-                      {trace.spans.length.toLocaleString()} spans, {trace.transactions.length.toLocaleString()} txns
+                      {trace.spans.size.toLocaleString()} spans, {trace.transactions.length.toLocaleString()} txns
                     </div>
                   </div>
                 </div>
diff --git a/packages/overlay/src/integrations/sentry/components/insights/Resources.tsx b/packages/overlay/src/integrations/sentry/components/insights/Resources.tsx
@@ -27,7 +27,7 @@ const calculateResourceInfo = ({ resource, spanData }: { resource: string; spanD
   const avgDuration = totalTimeInMs / specificResources.length;
   const avgEncodedSize =
     specificResources.reduce((acc: number, span: Span) => {
-      const contentLength = span.data && span.data['http.response_content_length'];
+      const contentLength = span.data?.['http.response_content_length'];
       if (typeof contentLength === 'number') {
         return acc + contentLength;
       }
diff --git a/packages/overlay/src/integrations/sentry/components/insights/index.tsx b/packages/overlay/src/integrations/sentry/components/insights/index.tsx
@@ -2,7 +2,7 @@ import { useState } from 'react';
 import { Navigate, Outlet, Route, Routes } from 'react-router-dom';
 import Tabs from '~/components/Tabs';
 import { useSpotlightContext } from '~/lib/useSpotlightContext';
-import { useSentrySpans } from '../../data/useSentrySpans';
+import { useSentrySpanCounts } from '../../data/useSentrySpans';
 import { createTab } from '../../utils/tabs';
 import HiddenItemsButton from '../HiddenItemsButton';
 import Queries from './Queries';
@@ -13,11 +13,11 @@ import WebVitalsDetail from './webVitals/WebVitalsDetail';
 
 export default function InsightsTabDetails() {
   const context = useSpotlightContext();
-  const { allSpans, localSpans } = useSentrySpans();
+  const { allSpans, localSpans } = useSentrySpanCounts();
 
   const [showAll, setShowAll] = useState(!context.experiments['sentry:focus-local-events']);
 
-  const hiddenItemCount = allSpans.length - localSpans.length;
+  const hiddenItemCount = allSpans - localSpans;
 
   const tabs = [
     createTab('queries', 'Queries'),
diff --git a/packages/overlay/src/integrations/sentry/data/sentryDataCache.ts b/packages/overlay/src/integrations/sentry/data/sentryDataCache.ts
@@ -41,7 +41,9 @@ class SentryDataCache {
       event_id?: string;
     })[] = [],
   ) {
-    initial.forEach(e => this.pushEvent(e));
+    for (const evt of initial) {
+      this.pushEvent(evt);
+    }
   }
 
   setSidecarUrl(url: string) {
@@ -153,9 +155,9 @@ class SentryDataCache {
 
         // recompute tree as we might have txn out of order
         // XXX: we're trusting timestamps, which are not trustworthy
-        const allSpans: Span[] = [];
-        trace.transactions.forEach(txn => {
-          allSpans.push({
+        const spanMap: Map<string, Span> = new Map();
+        for (const txn of trace.transactions) {
+          spanMap.set(txn.contexts.trace.span_id, {
             ...txn.contexts.trace,
             tags: txn?.tags,
             start_timestamp: txn.start_timestamp,
@@ -165,16 +167,16 @@ class SentryDataCache {
           });
 
           if (txn.spans) {
-            allSpans.push(
-              ...txn.spans.map(s => ({
-                ...s,
-                timestamp: toTimestamp(s.timestamp),
-                start_timestamp: toTimestamp(s.start_timestamp),
-              })),
-            );
+            for (const span of txn.spans) {
+              spanMap.set(span.span_id, {
+                ...span,
+                timestamp: toTimestamp(span.timestamp),
+                start_timestamp: toTimestamp(span.start_timestamp),
+              });
+            }
           }
-        });
-        trace.spans = allSpans;
+        }
+        trace.spans = spanMap;
         trace.spanTree = groupSpans(trace.spans);
       } else {
         trace.errors += 1;
@@ -232,7 +234,7 @@ class SentryDataCache {
   getSpanById(traceId: string, spanId: string) {
     const trace = this.tracesById[traceId];
     if (!trace) return undefined;
-    return trace.spans.find(s => s.span_id === spanId);
+    return trace.spans.get(spanId);
   }
 
   resetData() {
diff --git a/packages/overlay/src/integrations/sentry/data/useSentrySpans.ts b/packages/overlay/src/integrations/sentry/data/useSentrySpans.ts
@@ -1,16 +1,41 @@
 import { useContext } from 'react';
-import { Span, Trace } from '../types';
+import type { Span, Trace } from '../types';
 import sentryDataCache from './sentryDataCache';
 import { SentryEventsContext } from './sentryEventsContext';
 import { useSentryHelpers } from './useSentryHelpers';
 
-export const useSentrySpans = () => {
+export function useSentryTraces() {
   useContext(SentryEventsContext);
   const helpers = useSentryHelpers();
   const allTraces = sentryDataCache.getTraces();
   const localTraces = allTraces.filter(t => helpers.isLocalToSession(t.trace_id) !== false);
 
-  const allSpans: Span[] = allTraces.reduce((acc: Span[], trace: Trace) => [...acc, ...trace.spans], []);
-  const localSpans: Span[] = localTraces.reduce((acc: Span[], trace: Trace) => [...acc, ...trace.spans], []);
+  return { allTraces, localTraces };
+}
+
+function spanReducer(acc: Span[], trace: Trace) {
+  for (const span of trace.spans.values()) {
+    acc.push(span);
+  }
+  return acc;
+}
+
+function spanCountReducer(sum: number, trace: Trace) {
+  return sum + trace.spans.size;
+}
+
+export const useSentrySpans = () => {
+  const { allTraces, localTraces } = useSentryTraces();
+  const allSpans: Span[] = allTraces.reduce(spanReducer, []);
+  const localSpans: Span[] = localTraces.reduce(spanReducer, []);
   return { allSpans, localSpans };
 };
+
+export const useSentrySpanCounts = () => {
+  const { allTraces, localTraces } = useSentryTraces();
+
+  return {
+    allSpans: allTraces.reduce(spanCountReducer, 0),
+    localSpans: localTraces.reduce(spanCountReducer, 0),
+  };
+};
diff --git a/packages/overlay/src/integrations/sentry/sentry-integration.ts b/packages/overlay/src/integrations/sentry/sentry-integration.ts
@@ -48,10 +48,13 @@ export const spotlightIntegration = () => {
  * Flags if the event is a transaction created from an interaction with the spotlight UI.
  */
 function isSpotlightInteraction(event: Event): boolean {
-  return (
-    (event.type === 'transaction' &&
-      event.contexts?.trace?.op === 'ui.action.click' &&
-      event.spans?.some(s => s.description?.includes('#sentry-spotlight'))) ||
-    false
-  );
+  if (event.type === 'transaction' && event.contexts?.trace?.op === 'ui.action.click' && event.spans) {
+    for (const span of event.spans.values()) {
+      if (span.description?.includes('#sentry-spotlight')) {
+        return true;
+      }
+    }
+  }
+
+  return false;
 }
diff --git a/packages/overlay/src/integrations/sentry/types.ts b/packages/overlay/src/integrations/sentry/types.ts
@@ -135,7 +135,7 @@ export type Trace = TraceContext & {
   status: string;
   rootTransaction: SentryTransactionEvent | null;
   rootTransactionName: string;
-  spans: Span[];
+  spans: Map<string, Span>;
   spanTree: Span[];
 };
 
diff --git a/packages/overlay/src/integrations/sentry/utils/traces.spec.ts b/packages/overlay/src/integrations/sentry/utils/traces.spec.ts
@@ -18,7 +18,7 @@ function mockSpan({ duration, ...span }: Partial<Span> & { duration?: number } =
 
 describe('groupSpans', () => {
   test('empty span list', () => {
-    expect(groupSpans([])).toEqual([]);
+    expect(groupSpans(new Map())).toEqual([]);
   });
 
   test('simple parent and child relationship', () => {
@@ -31,7 +31,13 @@ describe('groupSpans', () => {
       parent_span_id: parent1.span_id,
       trace_id: parent1.trace_id,
     });
-    const result = groupSpans([parent1, span1, span2]);
+    const result = groupSpans(
+      new Map<string, Span>([
+        [parent1.span_id, parent1],
+        [span1.span_id, span1],
+        [span2.span_id, span2],
+      ]),
+    );
     console.debug(result);
     expect(result.length).toEqual(1);
     expect(result[0].span_id).toEqual(parent1.span_id);
@@ -63,7 +69,16 @@ describe('groupSpans', () => {
       parent_span_id: parent2.span_id,
       trace_id: parent2.trace_id,
     });
-    const result = groupSpans([parent1, span1, span2, parent2, span3, span4]);
+    const result = groupSpans(
+      new Map<string, Span>([
+        [parent1.span_id, parent1],
+        [span1.span_id, span1],
+        [span2.span_id, span2],
+        [parent2.span_id, parent2],
+        [span3.span_id, span3],
+        [span4.span_id, span4],
+      ]),
+    );
     console.debug(result);
     expect(result.length).toEqual(2);
     expect(result[0].span_id).toEqual(parent1.span_id);
@@ -85,7 +100,12 @@ describe('groupSpans', () => {
       parent_span_id,
       trace_id: span1.trace_id,
     });
-    const result = groupSpans([span1, span2]);
+    const result = groupSpans(
+      new Map<string, Span>([
+        [span1.span_id, span1],
+        [span2.span_id, span2],
+      ]),
+    );
     console.debug(result);
     expect(result.length).toEqual(1);
     expect(result[0].op).toEqual('orphan');
@@ -106,7 +126,12 @@ describe('groupSpans', () => {
       parent_span_id: generateUuidv4(),
       trace_id: span1.trace_id,
     });
-    const result = groupSpans([span1, span2]);
+    const result = groupSpans(
+      new Map<string, Span>([
+        [span1.span_id, span1],
+        [span2.span_id, span2],
+      ]),
+    );
     console.debug(result);
     expect(result.length).toEqual(2);
     expect(result[0].span_id).toEqual(span1.parent_span_id);
diff --git a/packages/overlay/src/integrations/sentry/utils/traces.ts b/packages/overlay/src/integrations/sentry/utils/traces.ts
@@ -2,17 +2,17 @@ import { log } from '../../../lib/logger';
 import { Span } from '../types';
 
 // mutates spans in place and adds children, as well as returns the top level tree
-export function groupSpans(spans: Span[]) {
+export function groupSpans(spans: Map<string, Span>) {
   // ordered
   const tree: Span[] = [];
-  // hash with pointers
-  const idLookup = new Map<string, Span>();
 
   // need to sort root(s) first
-  const sortedSpans = [...spans].sort((a, b) => (a.parent_span_id ? 1 : 0) - (b.parent_span_id ? 1 : 0));
+  const sortedSpans = Array.from(spans.values()).sort(
+    (a, b) => (a.parent_span_id ? 1 : 0) - (b.parent_span_id ? 1 : 0),
+  );
 
   sortedSpans.forEach(span => {
-    let parent = getParentOfSpan(span, idLookup, sortedSpans);
+    let parent = getParentOfSpan(span, spans, sortedSpans);
     span.children ||= [];
     if (parent) {
       if (!parent.children) {
@@ -37,7 +37,7 @@ export function groupSpans(spans: Span[]) {
         timestamp: span.timestamp,
         status: 'unknown',
       };
-      idLookup.set(parent.span_id, parent);
+      spans.set(parent.span_id, parent);
       // sortedSpans.splice(spanIdx, 0, parent);
       if (parentParent) {
         parentParent.children ||= [];
@@ -48,7 +48,7 @@ export function groupSpans(spans: Span[]) {
     } else {
       tree.push(span);
     }
-    idLookup.set(span.span_id, span);
+    spans.set(span.span_id, span);
   });
 
   return tree;

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ const calculateResourceInfo = ({ resource, spanData }: { resource: string; spanD`
`27`	`27`	`const avgDuration = totalTimeInMs / specificResources.length;`
`28`	`28`	`const avgEncodedSize =`
`29`	`29`	`specificResources.reduce((acc: number, span: Span) => {`
`30`		`- const contentLength = span.data && span.data['http.response_content_length'];`
	`30`	`+ const contentLength = span.data?.['http.response_content_length'];`
`31`	`31`	`if (typeof contentLength === 'number') {`
`32`	`32`	`return acc + contentLength;`
`33`	`33`	`}`