Browse Source

ref(trace) make autogrouping opt in and opt out (#78840)

This changes allows for the trace tree autogrouping and missing
instrumentations to be configurable.

Todo: 
- ~ensure links to missing instrumentation work even if users dont have
missing instrumentation enabled~
- ~ensure links to autogroups and their children work if users dont have
autogrouping enabled~
- ~add select toggle~
Jonas 4 months ago
parent
commit
249a5dbe73

+ 18 - 24
static/app/components/events/interfaces/performance/eventTraceView.tsx

@@ -34,6 +34,7 @@ const DEFAULT_ISSUE_DETAILS_TRACE_VIEW_PREFERENCES: TracePreferencesState = {
     },
     layoutOptions: [],
   },
+  missing_instrumentation: true,
   autogroup: {
     parent: true,
     sibling: true,
@@ -54,18 +55,17 @@ function EventTraceViewInner({event, organization}: EventTraceViewInnerProps) {
   const traceId = event.contexts.trace!.trace_id!;
   const location = useLocation();
 
-  const traceResults = useTrace({
+  const trace = useTrace({
     traceSlug: traceId ? traceId : undefined,
     limit: 10000,
   });
-  const metaResults = useTraceMeta([{traceSlug: traceId, timestamp: undefined}]);
-  const tree = useTraceTree({traceResults, metaResults, replayRecord: null});
+  const meta = useTraceMeta([{traceSlug: traceId, timestamp: undefined}]);
+  const tree = useTraceTree({trace, meta, replay: null});
 
-  const hasNoTransactions = metaResults.data?.transactions === 0;
-  const shouldLoadTraceRoot =
-    !traceResults.isPending && traceResults.data && !hasNoTransactions;
+  const hasNoTransactions = meta.data?.transactions === 0;
+  const shouldLoadTraceRoot = !trace.isPending && trace.data && !hasNoTransactions;
 
-  const rootEvent = useTraceRootEvent(shouldLoadTraceRoot ? traceResults.data! : null);
+  const rootEvent = useTraceRootEvent(shouldLoadTraceRoot ? trace.data! : null);
 
   const preferences = useMemo(
     () =>
@@ -90,12 +90,12 @@ function EventTraceViewInner({event, organization}: EventTraceViewInnerProps) {
     });
   }, [location.query.statsPeriod, traceId]);
 
-  if (
-    traceResults.isPending ||
-    rootEvent.isPending ||
-    !rootEvent.data ||
-    hasNoTransactions
-  ) {
+  const scrollToNode = useMemo(() => {
+    const firstTransactionEventId = trace.data?.transactions[0]?.event_id;
+    return {eventId: firstTransactionEventId};
+  }, [trace.data]);
+
+  if (trace.isPending || rootEvent.isPending || !rootEvent.data || hasNoTransactions) {
     return null;
   }
 
@@ -107,22 +107,16 @@ function EventTraceViewInner({event, organization}: EventTraceViewInnerProps) {
       >
         <TraceViewWaterfallWrapper>
           <TraceViewWaterfall
-            traceSlug={undefined}
             tree={tree}
+            trace={trace}
+            replay={null}
             rootEvent={rootEvent}
+            traceSlug={undefined}
             organization={organization}
             traceEventView={traceEventView}
-            metaResults={metaResults}
+            meta={meta}
             source="issues"
-            replayRecord={null}
-            scrollToNode={
-              traceResults.data?.transactions[0]?.event_id
-                ? {
-                    // Scroll/highlight the current transaction
-                    path: [`txn-${traceResults.data.transactions[0].event_id}`],
-                  }
-                : undefined
-            }
+            scrollToNode={scrollToNode}
             isEmbedded
           />
         </TraceViewWaterfallWrapper>

+ 228 - 109
static/app/views/performance/newTraceDetails/index.tsx

@@ -14,6 +14,7 @@ import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
 import * as qs from 'query-string';
 
+import {addSuccessMessage} from 'sentry/actionCreators/indicator';
 import {Button} from 'sentry/components/button';
 import useFeedbackWidget from 'sentry/components/feedback/widget/useFeedbackWidget';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
@@ -21,7 +22,7 @@ import NoProjectMessage from 'sentry/components/noProjectMessage';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {ALL_ACCESS_PROJECTS} from 'sentry/constants/pageFilters';
-import {t} from 'sentry/locale';
+import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {EventTransaction} from 'sentry/types/event';
 import type {Organization} from 'sentry/types/organization';
@@ -29,6 +30,7 @@ import type {Project} from 'sentry/types/project';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {browserHistory} from 'sentry/utils/browserHistory';
 import EventView from 'sentry/utils/discover/eventView';
+import type {TraceSplitResults} from 'sentry/utils/performance/quickTrace/types';
 import {
   cancelAnimationTimeout,
   requestAnimationTimeout,
@@ -81,8 +83,14 @@ import {
 } from './traceState/traceStateProvider';
 import {Trace} from './trace';
 import {traceAnalytics} from './traceAnalytics';
-import {isTraceNode} from './traceGuards';
+import {
+  isAutogroupedNode,
+  isParentAutogroupedNode,
+  isSiblingAutogroupedNode,
+  isTraceNode,
+} from './traceGuards';
 import {TraceMetadataHeader} from './traceMetadataHeader';
+import {TracePreferencesDropdown} from './tracePreferencesDropdown';
 import {TraceShortcuts} from './traceShortcutsModal';
 import type {TraceReducer, TraceReducerState} from './traceState';
 import {
@@ -90,8 +98,8 @@ import {
   traceNodeAnalyticsName,
 } from './traceTreeAnalytics';
 import TraceTypeWarnings from './traceTypeWarnings';
+import {useTraceOnLoad} from './useTraceOnLoad';
 import {useTraceQueryParamStateSync} from './useTraceQueryParamStateSync';
-import {useTraceScrollToEventOnLoad} from './useTraceScrollToEventOnLoad';
 import {useTraceScrollToPath} from './useTraceScrollToPath';
 
 function logTraceMetadata(
@@ -181,16 +189,17 @@ export function TraceView() {
     []
   );
 
-  const metaResults = useTraceMeta([{traceSlug, timestamp: queryParams.timestamp}]);
-  const traceResults = useTrace({traceSlug, timestamp: queryParams.timestamp});
-  const tree = useTraceTree({traceSlug, traceResults, metaResults, replayRecord: null});
-  const rootEvent = useTraceRootEvent(traceResults.data ?? null);
+  const meta = useTraceMeta([{traceSlug, timestamp: queryParams.timestamp}]);
+  const trace = useTrace({traceSlug, timestamp: queryParams.timestamp});
+  const rootEvent = useTraceRootEvent(trace.data ?? null);
+  const tree = useTraceTree({traceSlug, trace, meta, replay: null});
+
+  const title = useMemo(() => {
+    return `${t('Trace Details')} - ${traceSlug}`;
+  }, [traceSlug]);
 
   return (
-    <SentryDocumentTitle
-      title={`${t('Trace Details')} - ${traceSlug}`}
-      orgSlug={organization.slug}
-    >
+    <SentryDocumentTitle title={title} orgSlug={organization.slug}>
       <TraceStateProvider
         initialPreferences={preferences}
         preferencesStorageKey="trace-view-preferences"
@@ -205,13 +214,14 @@ export function TraceView() {
             />
             <TraceInnerLayout>
               <TraceViewWaterfall
-                traceSlug={traceSlug}
                 tree={tree}
-                organization={organization}
+                trace={trace}
+                meta={meta}
+                replay={null}
                 rootEvent={rootEvent}
+                traceSlug={traceSlug}
                 traceEventView={traceEventView}
-                metaResults={metaResults}
-                replayRecord={null}
+                organization={organization}
                 source="performance"
                 isEmbedded={false}
               />
@@ -235,11 +245,12 @@ const VITALS_TAB: TraceReducerState['tabs']['tabs'][0] = {
 
 type TraceViewWaterfallProps = {
   isEmbedded: boolean;
-  metaResults: TraceMetaQueryResults;
+  meta: TraceMetaQueryResults;
   organization: Organization;
-  replayRecord: ReplayRecord | null;
+  replay: ReplayRecord | null;
   rootEvent: UseApiQueryResult<EventTransaction, RequestError>;
   source: string;
+  trace: UseApiQueryResult<TraceSplitResults<TraceTree.Transaction>, RequestError>;
   traceEventView: EventView;
   traceSlug: string | undefined;
   tree: TraceTree;
@@ -253,16 +264,20 @@ type TraceViewWaterfallProps = {
 
 export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
   const api = useApi();
+  const filters = usePageFilters();
   const {projects} = useProjects();
   const organization = useOrganization();
-  const [forceRender, rerender] = useReducer(x => (x + 1) % Number.MAX_SAFE_INTEGER, 0);
+
   const traceState = useTraceState();
   const traceDispatch = useTraceStateDispatch();
   const traceStateEmitter = useTraceStateEmitter();
-  const filters = usePageFilters();
-  const traceScheduler = useMemo(() => new TraceScheduler(), []);
+
+  const [forceRender, rerender] = useReducer(x => (x + 1) % Number.MAX_SAFE_INTEGER, 0);
+
   const traceView = useMemo(() => new TraceViewModel(), []);
+  const traceScheduler = useMemo(() => new TraceScheduler(), []);
 
+  const scrollQueueRef = useTraceScrollToPath(props.scrollToNode);
   const forceRerender = useCallback(() => {
     flushSync(rerender);
   }, []);
@@ -282,7 +297,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
   );
 
   useEffect(() => {
-    if (!props.replayTraces?.length || props.tree.type !== 'trace') {
+    if (!props.replayTraces?.length || props.tree?.type !== 'trace') {
       return undefined;
     }
 
@@ -293,7 +308,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
       organization: props.organization,
       urlParams: qs.parse(location.search),
       rerender: forceRerender,
-      metaResults: props.metaResults,
+      meta: props.meta,
     });
 
     return () => cleanup();
@@ -304,6 +319,11 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
   const traceStateRef = useRef<TraceReducerState>(traceState);
   traceStateRef.current = traceState;
 
+  const traceStatePreferencesRef = useRef<
+    Pick<TraceReducerState['preferences'], 'autogroup' | 'missing_instrumentation'>
+  >(traceState.preferences);
+  traceStatePreferencesRef.current = traceState.preferences;
+
   // Initialize the view manager right after the state reducer
   const viewManager = useMemo(() => {
     return new VirtualizedViewManager(
@@ -440,7 +460,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
             resultsLookup: lookup,
             resultIteratorIndex: undefined,
             resultIndex: undefined,
-            previousNode: activeNodeSearchResult,
+            previousNode: null,
             node: activeNode,
           });
           return;
@@ -449,6 +469,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
         const resultIndex: number | undefined = matches?.[0]?.index;
         const resultIteratorIndex: number | undefined = matches?.[0] ? 0 : undefined;
         const node: TraceTreeNode<TraceTree.NodeValue> | null = matches?.[0]?.value;
+
         traceDispatch({
           type: 'set results',
           results: matches,
@@ -605,6 +626,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
       return TraceTree.ExpandToPath(props.tree, TraceTree.PathToNode(node), {
         api,
         organization: props.organization,
+        preferences: traceStatePreferencesRef.current,
       }).then(() => {
         const maybeNode = TraceTree.Find(props.tree.root, n => n === node);
 
@@ -612,22 +634,9 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
           return null;
         }
 
-        let index = props.tree.list.indexOf(maybeNode);
-
-        if (node && index === -1) {
-          let parent_node = node.parent;
-          while (parent_node) {
-            // Transactions break autogrouping chains, so we can stop here
-            props.tree.expand(parent_node, true);
-            // This is very wasteful as it performs O(n^2) search each time we expand a node...
-            // In most cases though, we should be operating on a tree with sub 10k elements and hopefully
-            // a low autogrouped node count.
-            index = node ? props.tree.list.findIndex(n => n === node) : -1;
-            if (index !== -1) {
-              break;
-            }
-            parent_node = parent_node.parent;
-          }
+        const index = TraceTree.EnforceVisibility(props.tree, maybeNode);
+        if (index === -1) {
+          return null;
         }
 
         scrollRowIntoView(maybeNode, index, 'center if outside', true);
@@ -679,62 +688,101 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
   // Callback that is invoked when the trace loads and reaches its initialied state,
   // that is when the trace tree data and any data that the trace depends on is loaded,
   // but the trace is not yet rendered in the view.
-  const onTraceLoad = useCallback(
-    (
-      _trace: TraceTree,
-      nodeToScrollTo: TraceTreeNode<TraceTree.NodeValue> | null,
-      indexOfNodeToScrollTo: number | null
-    ) => {
-      const query = qs.parse(location.search);
+  const onTraceLoad = useCallback(() => {
+    // The tree has the data fetched, but does not yet respect the user preferences.
+    // We will autogroup and inject missing instrumentation if the preferences are set.
+    // and then we will perform a search to find the node the user is interested in.
+    const query = qs.parse(location.search);
+    if (query.fov && typeof query.fov === 'string') {
+      viewManager.maybeInitializeTraceViewFromQS(query.fov);
+    }
+    if (traceStateRef.current.preferences.missing_instrumentation) {
+      TraceTree.DetectMissingInstrumentation(props.tree.root);
+    }
+    if (traceStateRef.current.preferences.autogroup.sibling) {
+      TraceTree.AutogroupSiblingSpanNodes(props.tree.root);
+    }
+    if (traceStateRef.current.preferences.autogroup.parent) {
+      TraceTree.AutogroupDirectChildrenSpanNodes(props.tree.root);
+    }
 
-      if (query.fov && typeof query.fov === 'string') {
-        viewManager.maybeInitializeTraceViewFromQS(query.fov);
+    // Construct the visual representation of the tree
+    props.tree.build();
+
+    const eventId = scrollQueueRef.current?.eventId;
+    const [type, path] = scrollQueueRef.current?.path?.[0]?.split('-') ?? [];
+    scrollQueueRef.current = null;
+
+    let node =
+      (path === 'root' && props.tree.root.children[0]) ||
+      (path && TraceTree.FindByID(props.tree.root, path)) ||
+      (eventId && TraceTree.FindByID(props.tree.root, eventId)) ||
+      null;
+
+    // If the node points to a span, but we found an autogrouped node, then
+    // perform another search inside the autogrouped node to find the more detailed
+    // location of the span. This is necessary because the id of the autogrouped node
+    // is in some cases inferred from the spans it contains and searching by the span id
+    // just gives us the first match which may not be the one the user is looking for.
+    if (node) {
+      if (isAutogroupedNode(node) && type !== 'ag') {
+        if (isParentAutogroupedNode(node)) {
+          node = TraceTree.FindByID(node.head, eventId ?? path) ?? node;
+        } else if (isSiblingAutogroupedNode(node)) {
+          node = node.children.find(n => TraceTree.FindByID(n, eventId ?? path)) ?? node;
+        }
       }
+    }
 
-      if (nodeToScrollTo !== null && indexOfNodeToScrollTo !== null) {
-        // At load time, we want to scroll the row into view, but we need to wait for the view
-        // to initialize before we can do that. We listen for the 'initialize virtualized list' and scroll
-        // to the row in the view if it is not in view yet. If its in the view, then scroll to it immediately.
-        traceScheduler.once('initialize virtualized list', () => {
-          function onTargetRowMeasure() {
-            if (!nodeToScrollTo || !viewManager.row_measurer.cache.has(nodeToScrollTo)) {
-              return;
-            }
-            viewManager.row_measurer.off('row measure end', onTargetRowMeasure);
-            if (viewManager.isOutsideOfView(nodeToScrollTo)) {
-              viewManager.scrollRowIntoViewHorizontally(
-                nodeToScrollTo!,
-                0,
-                48,
-                'measured'
-              );
-            }
-          }
-          viewManager.scrollToRow(indexOfNodeToScrollTo, 'center');
-          viewManager.row_measurer.on('row measure end', onTargetRowMeasure);
-          previouslyScrolledToNodeRef.current = nodeToScrollTo;
+    const index = node ? TraceTree.EnforceVisibility(props.tree, node) : -1;
 
-          setRowAsFocused(
-            nodeToScrollTo,
-            null,
-            traceStateRef.current.search.resultsLookup,
-            indexOfNodeToScrollTo
-          );
-          traceDispatch({
-            type: 'set roving index',
-            node: nodeToScrollTo,
-            index: indexOfNodeToScrollTo,
-            action_source: 'load',
-          });
-        });
-      }
+    if (traceStateRef.current.search.query) {
+      onTraceSearch(traceStateRef.current.search.query, node, 'persist');
+    }
+
+    if (index === -1 || !node) {
+      Sentry.withScope(scope => {
+        scope.setFingerprint(['trace-view-scroll-to-node-error']);
+        scope.captureMessage('Failed to scroll to node in trace tree');
+      });
+
+      return;
+    }
 
-      if (traceStateRef.current.search.query) {
-        onTraceSearch(traceStateRef.current.search.query, nodeToScrollTo, 'persist');
+    // At load time, we want to scroll the row into view, but we need to wait for the view
+    // to initialize before we can do that. We listen for the 'initialize virtualized list' and scroll
+    // to the row in the view if it is not in view yet. If its in the view, then scroll to it immediately.
+    traceScheduler.once('initialize virtualized list', () => {
+      function onTargetRowMeasure() {
+        if (!node || !viewManager.row_measurer.cache.has(node)) {
+          return;
+        }
+        viewManager.row_measurer.off('row measure end', onTargetRowMeasure);
+        if (viewManager.isOutsideOfView(node)) {
+          viewManager.scrollRowIntoViewHorizontally(node!, 0, 48, 'measured');
+        }
       }
-    },
-    [setRowAsFocused, traceDispatch, onTraceSearch, viewManager, traceScheduler]
-  );
+      viewManager.scrollToRow(index, 'center');
+      viewManager.row_measurer.on('row measure end', onTargetRowMeasure);
+      previouslyScrolledToNodeRef.current = node;
+
+      setRowAsFocused(node, null, traceStateRef.current.search.resultsLookup, index);
+      traceDispatch({
+        type: 'set roving index',
+        node: node,
+        index: index,
+        action_source: 'load',
+      });
+    });
+  }, [
+    setRowAsFocused,
+    traceDispatch,
+    onTraceSearch,
+    viewManager,
+    traceScheduler,
+    props.tree,
+    scrollQueueRef,
+  ]);
 
   // Setup the middleware for the trace reducer
   useLayoutEffect(() => {
@@ -855,27 +903,89 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
     };
   }, [viewManager, traceScheduler, props.tree]);
 
+  const onLoadScrollStatus = useTraceOnLoad({
+    onTraceLoad,
+    pathToNodeOrEventId: scrollQueueRef.current,
+    tree: props.tree,
+  });
+
   // Sync part of the state with the URL
   const traceQueryStateSync = useMemo(() => {
     return {search: traceState.search.query};
   }, [traceState.search.query]);
   useTraceQueryParamStateSync(traceQueryStateSync);
 
-  const scrollQueueRef = useTraceScrollToPath(props.scrollToNode);
+  const onAutogroupChange = useCallback(() => {
+    const value = !traceState.preferences.autogroup.parent;
+
+    if (!value) {
+      let removeCount = 0;
+      removeCount += TraceTree.RemoveSiblingAutogroupNodes(props.tree.root);
+      removeCount += TraceTree.RemoveDirectChildrenAutogroupNodes(props.tree.root);
+
+      addSuccessMessage(
+        removeCount > 0
+          ? tct('Autogrouping disabled, removed [count] autogroup spans', {
+              count: removeCount,
+            })
+          : t('Autogrouping disabled')
+      );
+    } else {
+      let autogroupCount = 0;
+      autogroupCount += TraceTree.AutogroupSiblingSpanNodes(props.tree.root);
+      autogroupCount += TraceTree.AutogroupDirectChildrenSpanNodes(props.tree.root);
+      addSuccessMessage(
+        autogroupCount > 0
+          ? tct('Autogrouping enabled, detected [count] autogrouping spans', {
+              count: autogroupCount,
+            })
+          : t('Autogrouping enabled')
+      );
+    }
 
-  useTraceScrollToEventOnLoad({
-    rerender,
-    onTraceLoad,
-    scrollQueueRef,
-    scheduler: traceScheduler,
-    trace: props.tree,
-  });
+    props.tree.rebuild();
+    traceDispatch({
+      type: 'set autogrouping',
+      payload: value,
+    });
+  }, [traceDispatch, traceState.preferences.autogroup, props.tree]);
+
+  const onMissingInstrumentationChange = useCallback(() => {
+    const value = !traceState.preferences.missing_instrumentation;
+    if (!value) {
+      const removeCount = TraceTree.RemoveMissingInstrumentationNodes(props.tree.root);
+      addSuccessMessage(
+        removeCount > 0
+          ? tct(
+              'Missing instrumentation disabled, removed [count] missing instrumentation spans',
+              {
+                count: removeCount,
+              }
+            )
+          : t('Missing instrumentation disabled')
+      );
+    } else {
+      const missingInstrumentationCount = TraceTree.DetectMissingInstrumentation(
+        props.tree.root
+      );
+      addSuccessMessage(
+        missingInstrumentationCount > 0
+          ? tct(
+              'Missing instrumentation enabled, found [count] missing instrumentation spans',
+              {
+                count: missingInstrumentationCount,
+              }
+            )
+          : t('Missing instrumentation enabled')
+      );
+    }
 
-  const isLoading = !!(
-    props.tree.type === 'loading' ||
-    props.tree.type !== 'trace' ||
-    scrollQueueRef.current
-  );
+    props.tree.rebuild();
+    traceDispatch({
+      type: 'set missing instrumentation',
+      payload: value,
+    });
+  }, [traceDispatch, traceState.preferences.missing_instrumentation, props.tree]);
 
   return (
     <Fragment>
@@ -891,6 +1001,15 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
           organization={props.organization}
         />
         <TraceShortcuts />
+        <TracePreferencesDropdown
+          autogroup={
+            traceState.preferences.autogroup.parent &&
+            traceState.preferences.autogroup.sibling
+          }
+          missingInstrumentation={traceState.preferences.missing_instrumentation}
+          onAutogroupChange={onAutogroupChange}
+          onMissingInstrumentationChange={onMissingInstrumentationChange}
+        />
       </TraceToolbar>
       <TraceGrid layout={traceState.preferences.layout} ref={setTraceGridRef}>
         <Trace
@@ -904,20 +1023,20 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) {
           scheduler={traceScheduler}
           forceRerender={forceRender}
           isEmbedded={props.isEmbedded}
-          isLoading={isLoading}
+          isLoading={props.tree.type === 'loading' || onLoadScrollStatus === 'pending'}
         />
 
-        {props.tree.type === 'error' ? (
+        {props.tree.type === 'loading' || onLoadScrollStatus === 'pending' ? (
+          <TraceLoading />
+        ) : props.tree.type === 'error' ? (
           <TraceError />
         ) : props.tree.type === 'empty' ? (
           <TraceEmpty />
-        ) : isLoading ? (
-          <TraceLoading />
         ) : null}
 
         <TraceDrawer
-          replayRecord={props.replayRecord}
-          metaResults={props.metaResults}
+          replay={props.replay}
+          meta={props.meta}
           traceType={shape}
           trace={props.tree}
           traceGridRef={traceGridRef}
@@ -984,7 +1103,7 @@ const TraceInnerLayout = styled('div')`
 const TraceToolbar = styled('div')`
   flex-grow: 0;
   display: grid;
-  grid-template-columns: 1fr min-content min-content;
+  grid-template-columns: 1fr min-content min-content min-content;
   gap: ${space(1)};
 `;
 

+ 174 - 70
static/app/views/performance/newTraceDetails/trace.spec.tsx

@@ -23,6 +23,8 @@ import {
   makeTraceError,
   makeTransaction,
 } from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils';
+import type {TracePreferencesState} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences';
+import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences';
 
 class MockResizeObserver {
   callback: ResizeObserverCallback;
@@ -53,14 +55,34 @@ class MockResizeObserver {
 type Arguments<F extends Function> = F extends (...args: infer A) => any ? A : never;
 type ResponseType = Arguments<typeof MockApiClient.addMockResponse>[0];
 
-function mockQueryString(query: string) {
+function mockQueryString(queryString: string) {
   Object.defineProperty(window, 'location', {
     value: {
-      search: query,
+      search: queryString,
     },
   });
 }
 
+function mockTracePreferences(preferences: Partial<TracePreferencesState>) {
+  const merged: TracePreferencesState = {
+    ...DEFAULT_TRACE_VIEW_PREFERENCES,
+    ...preferences,
+    autogroup: {
+      ...DEFAULT_TRACE_VIEW_PREFERENCES.autogroup,
+      ...preferences.autogroup,
+    },
+    drawer: {
+      ...DEFAULT_TRACE_VIEW_PREFERENCES.drawer,
+      ...preferences.drawer,
+    },
+    list: {
+      ...DEFAULT_TRACE_VIEW_PREFERENCES.list,
+      ...preferences.list,
+    },
+  };
+  localStorage.setItem('trace-view-preferences', JSON.stringify(merged));
+}
+
 function mockTraceResponse(resp?: Partial<ResponseType>) {
   MockApiClient.addMockResponse({
     url: '/organizations/org-slug/events-trace/trace-id/',
@@ -655,19 +677,15 @@ async function completeTestSetup() {
 
 const DRAWER_TABS_TEST_ID = 'trace-drawer-tab';
 const DRAWER_TABS_PIN_BUTTON_TEST_ID = 'trace-drawer-tab-pin-button';
-
-// @ts-expect-error ignore this line
-// eslint-disable-next-line
-const DRAWER_TABS_CONTAINER_TEST_ID = 'trace-drawer-tabs';
 const VISIBLE_TRACE_ROW_SELECTOR = '.TraceRow:not(.Hidden)';
 const ACTIVE_SEARCH_HIGHLIGHT_ROW = '.TraceRow.SearchResult.Highlight:not(.Hidden)';
+
 const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));
 const searchToUpdate = async (): Promise<void> => {
   await wait(500);
 };
-
 const scrollToEnd = async (): Promise<void> => {
-  await wait(1000);
+  await wait(500);
 };
 
 // @ts-expect-error ignore this line
@@ -679,32 +697,42 @@ function printVirtualizedList(container: HTMLElement) {
   )!;
 
   const rows = Array.from(container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR));
+  const searchResultIterator = screen.queryByTestId('trace-search-result-iterator');
   stdout.push(
-    'top:' +
+    'Scroll Container: ' +
+      'top=' +
       scrollContainer.scrollTop +
       ' ' +
-      'left:' +
+      'left=' +
       scrollContainer.scrollLeft +
       ' ' +
-      rows.length
+      rows.length +
+      ' ' +
+      'Search:' +
+      (searchResultIterator?.textContent ?? '')
   );
-  stdout.push('///////////////////');
 
   for (const r of [...rows]) {
-    let t = r.textContent ?? '';
+    const count = (r.querySelector('.TraceChildrenCount') as HTMLElement)?.textContent;
+    const op = (r.querySelector('.TraceOperation') as HTMLElement)?.textContent;
+    const desc = (r.querySelector('.TraceDescription') as HTMLElement)?.textContent;
+    let t = (count ?? '') + ' ' + (op ?? '') + ' — ' + (desc ?? '');
 
     if (r.classList.contains('SearchResult')) {
-      t = 'search ' + t;
+      t = t + ' search';
     }
     if (r.classList.contains('Highlight')) {
-      t = 'highlight ' + t;
+      t = t + ' highlight';
     }
 
     if (document.activeElement === r) {
-      t = '⬅ focused ' + t;
+      t = t + ' ⬅ focused ';
     }
 
-    stdout.push(t);
+    const leftColumn = r.querySelector('.TraceLeftColumnInner') as HTMLElement;
+    const left = Math.round(parseInt(leftColumn.style.paddingLeft, 10) / 10);
+
+    stdout.push(' '.repeat(left) + t);
   }
 
   // This is a debug fn, we need it to log
@@ -738,10 +766,11 @@ function assertHighlightedRowAtIndex(virtualizedContainer: HTMLElement, index: n
   expect(r.indexOf(highlighted_row!)).toBe(index);
 }
 
-describe('trace', () => {
+describe('trace view', () => {
   beforeEach(() => {
     jest.spyOn(console, 'error').mockImplementation(() => {});
     globalThis.ResizeObserver = MockResizeObserver as any;
+    mockQueryString('');
     MockDate.reset();
   });
   afterEach(() => {
@@ -759,7 +788,7 @@ describe('trace', () => {
     expect(await screen.findByText(/assembling the trace/i)).toBeInTheDocument();
   });
 
-  it('renders error state', async () => {
+  it('renders error state if trace fails to load', async () => {
     mockTraceResponse({statusCode: 404});
     mockTraceMetaResponse({statusCode: 404});
     mockTraceTagsResponse({statusCode: 404});
@@ -768,6 +797,21 @@ describe('trace', () => {
     expect(await screen.findByText(/we failed to load your trace/i)).toBeInTheDocument();
   });
 
+  it('renders error state if meta fails to load', async () => {
+    mockTraceResponse({
+      statusCode: 200,
+      body: {
+        transactions: [makeTransaction()],
+        orphan_errors: [],
+      },
+    });
+    mockTraceMetaResponse({statusCode: 404});
+    mockTraceTagsResponse({statusCode: 404});
+
+    render(<TraceView />, {router});
+    expect(await screen.findByText(/we failed to load your trace/i)).toBeInTheDocument();
+  });
+
   it('renders empty state', async () => {
     mockTraceResponse({
       body: {
@@ -788,15 +832,19 @@ describe('trace', () => {
     it('scrolls to trace root', async () => {
       mockQueryString('?node=trace-root');
       const {virtualizedContainer} = await completeTestSetup();
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-      expect(rows[0]).toHaveFocus();
+      await waitFor(() => {
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
+        expect(rows[0]).toHaveFocus();
+      });
     });
 
     it('scrolls to transaction', async () => {
       mockQueryString('?node=txn-1');
       const {virtualizedContainer} = await completeTestSetup();
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-      expect(rows[2]).toHaveFocus();
+      await waitFor(() => {
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
+        expect(rows[2]).toHaveFocus();
+      });
     });
 
     it('scrolls to span that is a child of transaction', async () => {
@@ -805,10 +853,9 @@ describe('trace', () => {
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
 
-      // We need to await a tick because the row is not focused until the next tick
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next tick
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[3]).toHaveFocus();
         expect(rows[3].textContent?.includes('http — request')).toBe(true);
       });
@@ -820,10 +867,9 @@ describe('trace', () => {
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
 
-      // We need to await a tick because the row is not focused until the next tick
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next tick
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[4]).toHaveFocus();
         expect(rows[4].textContent?.includes('Autogrouped')).toBe(true);
       });
@@ -834,10 +880,9 @@ describe('trace', () => {
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
 
-      // We need to await a tick because the row is not focused until the next tick
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next tick
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[5]).toHaveFocus();
         expect(rows[5].textContent?.includes('db — redis')).toBe(true);
       });
@@ -848,11 +893,9 @@ describe('trace', () => {
 
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
-
-      // We need to await a tick because the row is not focused until the next tick
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next tick
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[5]).toHaveFocus();
         expect(rows[5].textContent?.includes('5Autogrouped')).toBe(true);
       });
@@ -864,10 +907,9 @@ describe('trace', () => {
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
 
-      // We need to await a tick because the row is not focused until the next tick
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next tick
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[6]).toHaveFocus();
         expect(rows[6].textContent?.includes('http — request')).toBe(true);
       });
@@ -894,10 +936,9 @@ describe('trace', () => {
       const {virtualizedContainer} = await completeTestSetup();
       await findAllByText(virtualizedContainer, /Autogrouped/i);
 
-      // We need to await a tick because the row is not focused until the next ticks
-      const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
-
       await waitFor(() => {
+        // We need to await a tick because the row is not focused until the next ticks
+        const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
         expect(rows[11]).toHaveFocus();
         expect(rows[11].textContent?.includes('error-title')).toBe(true);
       });
@@ -948,17 +989,76 @@ describe('trace', () => {
       });
     });
 
-    it('triggers search on load', async () => {
-      mockQueryString('?search=transaction-op-5');
-      await pageloadTestSetup();
+    it('does not autogroup if user preference is disabled', async () => {
+      mockTracePreferences({autogroup: {parent: false, sibling: false}});
+      mockQueryString('?node=span-span0&node=txn-1');
 
-      const searchInput = await screen.findByPlaceholderText('Search in trace');
-      expect(searchInput).toHaveValue('transaction-op-5');
+      const {virtualizedContainer} = await completeTestSetup();
 
-      await waitFor(() => {
-        expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent(
-          '1/1'
+      await findAllByText(virtualizedContainer, /process/i);
+      expect(screen.queryByText(/Autogrouped/i)).not.toBeInTheDocument();
+    });
+
+    it('does not inject missing instrumentation if user preference is disabled', async () => {
+      mockTracePreferences({missing_instrumentation: false});
+      mockQueryString('?node=span-span0&node=txn-1');
+
+      const {virtualizedContainer} = await completeTestSetup();
+
+      await findAllByText(virtualizedContainer, /process/i);
+      expect(screen.queryByText(/Missing instrumentation/i)).not.toBeInTheDocument();
+    });
+
+    describe('preferences', () => {
+      it('toggles autogrouping', async () => {
+        mockTracePreferences({autogroup: {parent: true, sibling: true}});
+        mockQueryString('?node=span-span0&node=txn-1');
+
+        const {virtualizedContainer} = await completeTestSetup();
+        await findAllByText(virtualizedContainer, /Autogrouped/i);
+
+        const preferencesDropdownTrigger = screen.getByLabelText('Trace Preferences');
+        await userEvent.click(preferencesDropdownTrigger);
+
+        // Toggle autogrouping off
+        await userEvent.click(await screen.findByText('Autogrouping'));
+        await waitFor(() => {
+          expect(screen.queryByText('Autogrouped')).not.toBeInTheDocument();
+        });
+
+        // Toggle autogrouping on
+        await userEvent.click(await screen.findByText('Autogrouping'));
+        await waitFor(() => {
+          expect(screen.queryAllByText('Autogrouped')).toHaveLength(2);
+        });
+      });
+
+      it('toggles missing instrumentation', async () => {
+        mockTracePreferences({missing_instrumentation: true});
+        mockQueryString('?node=span-span0&node=txn-1');
+
+        const {virtualizedContainer} = await completeTestSetup();
+        await findAllByText(virtualizedContainer, /Missing instrumentation/i);
+
+        const preferencesDropdownTrigger = screen.getByLabelText('Trace Preferences');
+
+        // Toggle missing instrumentation off
+        await userEvent.click(preferencesDropdownTrigger);
+        const missingInstrumentationOption = await screen.findByText(
+          'Missing Instrumentation'
         );
+
+        // Toggle missing instrumentation off
+        await userEvent.click(missingInstrumentationOption);
+        await waitFor(() => {
+          expect(screen.queryByText('Missing instrumentation')).not.toBeInTheDocument();
+        });
+
+        // Toggle missing instrumentation on
+        await userEvent.click(missingInstrumentationOption);
+        await waitFor(() => {
+          expect(screen.queryByText('Missing instrumentation')).toBeInTheDocument();
+        });
       });
     });
 
@@ -970,7 +1070,7 @@ describe('trace', () => {
       expect(searchInput).toHaveValue('transaction-op-999');
 
       await waitFor(() => {
-        expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent(
+        expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent(
           '-/1'
         );
       });
@@ -981,7 +1081,6 @@ describe('trace', () => {
 
     it('if search on load does not match anything, it does not steal focus or highlight first result', async () => {
       mockQueryString('?search=dead&node=txn-5');
-
       const {container} = await pageloadTestSetup();
       const searchInput = await screen.findByPlaceholderText('Search in trace');
       expect(searchInput).toHaveValue('dead');
@@ -1018,8 +1117,7 @@ describe('trace', () => {
       await userEvent.keyboard('{arrowup}');
       await waitFor(() => expect(rows[0]).toHaveFocus());
     });
-    // biome-ignore lint/suspicious/noSkippedTests: Flaky test
-    it.skip('arrow right expands row and fetches data', async () => {
+    it('arrow right expands row and fetches data', async () => {
       const {virtualizedContainer} = await keyboardNavigationTestSetup();
       const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR);
 
@@ -1036,7 +1134,9 @@ describe('trace', () => {
       await waitFor(() => expect(rows[1]).toHaveFocus());
 
       await userEvent.keyboard('{arrowright}');
-      expect(await screen.findByText('special-span')).toBeInTheDocument();
+      await waitFor(() => {
+        expect(screen.queryByText('special-span')).toBeInTheDocument();
+      });
     });
 
     it('arrow left collapses row', async () => {
@@ -1497,10 +1597,10 @@ describe('trace', () => {
         }
       );
 
-      const value = render(<TraceView />, {router});
+      const {container} = render(<TraceView />, {router});
 
       // Awaits for the placeholder rendering rows to be removed
-      expect(await findByText(value.container, /transaction-op-0/i)).toBeInTheDocument();
+      expect(await findByText(container, /transaction-op-0/i)).toBeInTheDocument();
 
       const searchInput = await screen.findByPlaceholderText('Search in trace');
       await userEvent.click(searchInput);
@@ -1508,25 +1608,29 @@ describe('trace', () => {
       fireEvent.change(searchInput, {target: {value: 'op-0'}});
       await searchToUpdate();
 
-      expect(await screen.findByTestId('trace-search-result-iterator')).toHaveTextContent(
-        '1/1'
-      );
+      await waitFor(() => {
+        expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent(
+          '1/1'
+        );
+      });
 
-      const highlighted_row = value.container.querySelector(ACTIVE_SEARCH_HIGHLIGHT_ROW);
       const open = await screen.findAllByRole('button', {name: '+'});
       await userEvent.click(open[0]);
-      expect(await screen.findByText('span-description')).toBeInTheDocument();
-      await searchToUpdate();
 
+      await waitFor(() => {
+        expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent(
+          '1/1'
+        );
+      });
+
+      expect(await screen.findByText('span-description')).toBeInTheDocument();
       expect(spansRequest).toHaveBeenCalled();
 
-      // The search is retriggered, but highlighting of current row is preserved
-      expect(value.container.querySelector(ACTIVE_SEARCH_HIGHLIGHT_ROW)).toBe(
-        highlighted_row
-      );
-      expect(await screen.findByTestId('trace-search-result-iterator')).toHaveTextContent(
-        '1/2'
-      );
+      await waitFor(() => {
+        expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent(
+          '1/2'
+        );
+      });
     });
 
     it('during search, highlighting is persisted on the row', async () => {

+ 8 - 2
static/app/views/performance/newTraceDetails/trace.tsx

@@ -142,6 +142,11 @@ export function Trace({
   const traceStateRef = useRef<TraceReducerState>(traceState);
   traceStateRef.current = traceState;
 
+  const traceStatePreferencesRef = useRef<
+    Pick<TraceReducerState['preferences'], 'autogroup' | 'missing_instrumentation'>
+  >(traceState.preferences);
+  traceStatePreferencesRef.current = traceState.preferences;
+
   useLayoutEffect(() => {
     const onTraceViewChange: TraceEvents['set trace view'] = () => {
       manager.recomputeTimelineIntervals();
@@ -197,6 +202,7 @@ export function Trace({
         .zoom(node, value, {
           api,
           organization,
+          preferences: traceStatePreferencesRef.current,
         })
         .then(() => {
           rerenderRef.current();
@@ -376,7 +382,7 @@ export function Trace({
       className={`
         ${trace.root.space[1] === 0 ? 'Empty' : ''}
         ${trace.indicators.length > 0 ? 'WithIndicators' : ''}
-        ${trace.type !== 'trace' ? 'Loading' : ''}
+        ${trace.type !== 'trace' || isLoading ? 'Loading' : ''}
         ${ConfigStore.get('theme')}`}
     >
       <div
@@ -408,7 +414,7 @@ export function Trace({
         {manager.interval_bars.map((_, i) => {
           const indicatorTimestamp = manager.intervals[i] ?? 0;
 
-          if (trace.type !== 'trace') {
+          if (trace.type !== 'trace' || isLoading) {
             return null;
           }
 

+ 1 - 1
static/app/views/performance/newTraceDetails/traceApi/useReplayTraceMeta.tsx

@@ -87,7 +87,7 @@ export function useReplayTraceMeta(
   const metaResults = useMemo(() => {
     return {
       data: meta.data,
-      isLoading: eventsIsLoading || meta.isLoading,
+      isLoading: eventsIsLoading || meta.status === 'pending',
       errors: meta.errors,
       status: meta.status,
     };

+ 4 - 3
static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx

@@ -12,6 +12,7 @@ import type {
 } from 'sentry/utils/performance/quickTrace/types';
 import {useApiQuery, type UseApiQueryResult} from 'sentry/utils/queryClient';
 import {decodeScalar} from 'sentry/utils/queryString';
+import type RequestError from 'sentry/utils/requestError/requestError';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 
@@ -147,7 +148,7 @@ function makeTraceFromTransaction(
 function useDemoTrace(
   demo: string | undefined,
   organization: {slug: string}
-): UseApiQueryResult<TraceSplitResults<TraceTree.Transaction> | undefined, any> {
+): UseApiQueryResult<TraceSplitResults<TraceTree.Transaction>, RequestError> {
   const demoEventSlug = parseDemoEventSlug(demo);
 
   // When projects don't have performance set up, we allow them to view a demo transaction.
@@ -179,7 +180,7 @@ function useDemoTrace(
   // Casting here since the 'select' option is not available in the useApiQuery hook to transform the data
   // from EventTransaction to TraceSplitResults<TraceFullDetailed>
   return {...demoEventQuery, data} as UseApiQueryResult<
-    TraceSplitResults<TraceTree.Transaction> | undefined,
+    TraceSplitResults<TraceTree.Transaction>,
     any
   >;
 }
@@ -192,7 +193,7 @@ type UseTraceParams = {
 
 export function useTrace(
   options: UseTraceParams
-): UseApiQueryResult<TraceSplitResults<TraceTree.Transaction> | undefined, any> {
+): UseApiQueryResult<TraceSplitResults<TraceTree.Transaction>, RequestError> {
   const filters = usePageFilters();
   const organization = useOrganization();
   const queryParams = useMemo(() => {

+ 8 - 17
static/app/views/performance/newTraceDetails/traceApi/useTraceMeta.spec.tsx

@@ -34,8 +34,7 @@ describe('useTraceMeta', () => {
     jest.spyOn(useOrganization, 'default').mockReturnValue(organization);
   });
 
-  it('Returns merged metaResults', async () => {
-    // Mock the API calls
+  it('Returns merged meta results', async () => {
     MockApiClient.addMockResponse({
       method: 'GET',
       url: '/organizations/org-slug/events-trace-meta/slug1/',
@@ -79,11 +78,10 @@ describe('useTraceMeta', () => {
     expect(result.current).toEqual({
       data: undefined,
       errors: [],
-      isLoading: true,
       status: 'pending',
     });
 
-    await waitFor(() => expect(result.current.isLoading).toBe(false));
+    await waitFor(() => expect(result.current.status === 'success').toBe(true));
 
     expect(result.current).toEqual({
       data: {
@@ -97,13 +95,11 @@ describe('useTraceMeta', () => {
         },
       },
       errors: [],
-      isLoading: false,
       status: 'success',
     });
   });
 
   it('Collects errors from rejected api calls', async () => {
-    // Mock the API calls
     const mockRequest1 = MockApiClient.addMockResponse({
       method: 'GET',
       url: '/organizations/org-slug/events-trace-meta/slug1/',
@@ -129,11 +125,10 @@ describe('useTraceMeta', () => {
     expect(result.current).toEqual({
       data: undefined,
       errors: [],
-      isLoading: true,
       status: 'pending',
     });
 
-    await waitFor(() => expect(result.current.isLoading).toBe(false));
+    await waitFor(() => expect(result.current.status === 'pending').toBe(false));
 
     expect(result.current).toEqual({
       data: {
@@ -144,17 +139,15 @@ describe('useTraceMeta', () => {
         transactiontoSpanChildrenCount: {},
       },
       errors: [expect.any(Error), expect.any(Error), expect.any(Error)],
-      isLoading: false,
-      status: 'success',
+      status: 'error',
     });
 
-    expect(mockRequest1).toHaveBeenCalledTimes(1);
-    expect(mockRequest2).toHaveBeenCalledTimes(1);
-    expect(mockRequest3).toHaveBeenCalledTimes(1);
+    expect(mockRequest1).toHaveBeenCalled();
+    expect(mockRequest2).toHaveBeenCalled();
+    expect(mockRequest3).toHaveBeenCalled();
   });
 
   it('Accumulates metaResults and collects errors from rejected api calls', async () => {
-    // Mock the API calls
     const mockRequest1 = MockApiClient.addMockResponse({
       method: 'GET',
       url: '/organizations/org-slug/events-trace-meta/slug1/',
@@ -192,11 +185,10 @@ describe('useTraceMeta', () => {
     expect(result.current).toEqual({
       data: undefined,
       errors: [],
-      isLoading: true,
       status: 'pending',
     });
 
-    await waitFor(() => expect(result.current.isLoading).toBe(false));
+    await waitFor(() => expect(result.current.status === 'pending').toBe(false));
 
     expect(result.current).toEqual({
       data: {
@@ -207,7 +199,6 @@ describe('useTraceMeta', () => {
         transactiontoSpanChildrenCount: {},
       },
       errors: [expect.any(Error)],
-      isLoading: false,
       status: 'success',
     });
 

+ 28 - 45
static/app/views/performance/newTraceDetails/traceApi/useTraceMeta.tsx

@@ -65,7 +65,7 @@ async function fetchTraceMetaInBatches(
   filters: Partial<PageFilters> = {}
 ) {
   const clonedTraceIds = [...replayTraces];
-  const metaResults: TraceMeta = {
+  const meta: TraceMeta = {
     errors: 0,
     performance_issues: 0,
     projects: 0,
@@ -84,55 +84,39 @@ async function fetchTraceMetaInBatches(
       })
     );
 
-    const updatedData = results.reduce(
-      (acc, result) => {
-        if (result.status === 'fulfilled') {
-          const {
-            errors,
-            performance_issues,
-            projects,
-            transactions,
-            transaction_child_count_map,
-          } = result.value;
-          acc.errors += errors;
-          acc.performance_issues += performance_issues;
-          acc.projects = Math.max(acc.projects, projects);
-          acc.transactions += transactions;
-
-          // Turn the transaction_child_count_map array into a map of transaction id to child count
-          // for more efficient lookups.
-          transaction_child_count_map.forEach(({'transaction.id': id, count}) => {
+    results.reduce((acc, result) => {
+      if (result.status === 'fulfilled') {
+        acc.errors += result.value.errors;
+        acc.performance_issues += result.value.performance_issues;
+        acc.projects = Math.max(acc.projects, result.value.projects);
+        acc.transactions += result.value.transactions;
+
+        // Turn the transaction_child_count_map array into a map of transaction id to child count
+        // for more efficient lookups.
+        result.value.transaction_child_count_map.forEach(
+          ({'transaction.id': id, count}) => {
             acc.transactiontoSpanChildrenCount[id] = count;
-          });
-        } else {
-          apiErrors.push(new Error(result.reason));
-        }
-        return acc;
-      },
-      {...metaResults}
-    );
-
-    metaResults.errors = updatedData.errors;
-    metaResults.performance_issues = updatedData.performance_issues;
-    metaResults.projects = Math.max(updatedData.projects, metaResults.projects);
-    metaResults.transactions = updatedData.transactions;
-    metaResults.transactiontoSpanChildrenCount =
-      updatedData.transactiontoSpanChildrenCount;
+          }
+        );
+      } else {
+        apiErrors.push(new Error(result?.reason));
+      }
+      return acc;
+    }, meta);
   }
 
-  return {metaResults, apiErrors};
+  return {meta, apiErrors};
 }
 
 export type TraceMetaQueryResults = {
   data: TraceMeta | undefined;
   errors: Error[];
-  isLoading: boolean;
   status: QueryStatus;
 };
 
 export function useTraceMeta(replayTraces: ReplayTrace[]): TraceMetaQueryResults {
-  const filters = usePageFilters();
   const api = useApi();
+  const filters = usePageFilters();
   const organization = useOrganization();
 
   const normalizedParams = useMemo(() => {
@@ -147,10 +131,10 @@ export function useTraceMeta(replayTraces: ReplayTrace[]): TraceMetaQueryResults
   // used to query a demo transaction event from the backend.
   const mode = decodeScalar(normalizedParams.demo) ? 'demo' : undefined;
 
-  const {data, isPending, status} = useQuery<
+  const query = useQuery<
     {
       apiErrors: Error[];
-      metaResults: TraceMeta;
+      meta: TraceMeta;
     },
     Error
   >({
@@ -168,12 +152,12 @@ export function useTraceMeta(replayTraces: ReplayTrace[]): TraceMetaQueryResults
 
   const results = useMemo(() => {
     return {
-      data: data?.metaResults,
-      errors: data?.apiErrors || [],
-      isLoading: isPending,
-      status,
+      data: query.data?.meta,
+      errors: query.data?.apiErrors ?? [],
+      status:
+        query.data?.apiErrors?.length === replayTraces.length ? 'error' : query.status,
     };
-  }, [data, isPending, status]);
+  }, [query, replayTraces.length]);
 
   // When projects don't have performance set up, we allow them to view a sample transaction.
   // The backend creates the sample transaction, however the trace is created async, so when the
@@ -190,7 +174,6 @@ export function useTraceMeta(replayTraces: ReplayTrace[]): TraceMetaQueryResults
         transactions: 1,
         transactiontoSpanChildrenCount: {},
       },
-      isLoading: false,
       errors: [],
       status: 'success',
     };

+ 24 - 81
static/app/views/performance/newTraceDetails/traceApi/useTraceTree.spec.tsx

@@ -11,12 +11,7 @@ import * as useApi from 'sentry/utils/useApi';
 import * as useOrganization from 'sentry/utils/useOrganization';
 
 import type {TraceTree} from '../traceModels/traceTree';
-import {
-  makeEvent,
-  makeSpan,
-  makeTraceError,
-  makeTransaction,
-} from '../traceModels/traceTreeTestUtils';
+import {makeTraceError, makeTransaction} from '../traceModels/traceTreeTestUtils';
 
 import type {TraceMetaQueryResults} from './useTraceMeta';
 import {useTraceTree} from './useTraceTree';
@@ -49,10 +44,10 @@ describe('useTraceTree', () => {
   it('returns tree for error case', async () => {
     const {result} = renderHook(() =>
       useTraceTree({
-        traceResults: getMockedTraceResults('error'),
-        metaResults: getMockedMetaResults('error'),
+        trace: getMockedTraceResults('error'),
+        meta: getMockedMetaResults('error'),
         traceSlug: 'test-trace',
-        replayRecord: null,
+        replay: null,
       })
     );
 
@@ -64,10 +59,10 @@ describe('useTraceTree', () => {
   it('returns tree for loading case', async () => {
     const {result} = renderHook(() =>
       useTraceTree({
-        traceResults: getMockedTraceResults('pending'),
-        metaResults: getMockedMetaResults('pending'),
+        trace: getMockedTraceResults('pending'),
+        meta: getMockedMetaResults('pending'),
         traceSlug: 'test-trace',
-        replayRecord: null,
+        replay: null,
       })
     );
 
@@ -77,27 +72,23 @@ describe('useTraceTree', () => {
   });
 
   it('returns tree for empty success case', async () => {
-    const mockedTrace = {
-      transactions: [],
-      orphan_errors: [],
-    };
-
-    const mockedMeta = {
-      errors: 1,
-      performance_issues: 2,
-      projects: 1,
-      transactions: 1,
-      transactiontoSpanChildrenCount: {
-        '1': 1,
-      },
-    };
-
     const {result} = renderHook(() =>
       useTraceTree({
-        traceResults: getMockedTraceResults('success', mockedTrace),
-        metaResults: getMockedMetaResults('success', mockedMeta),
+        trace: getMockedTraceResults('success', {
+          transactions: [],
+          orphan_errors: [],
+        }),
+        meta: getMockedMetaResults('success', {
+          errors: 1,
+          performance_issues: 2,
+          projects: 1,
+          transactions: 1,
+          transactiontoSpanChildrenCount: {
+            '1': 1,
+          },
+        }),
         traceSlug: 'test-trace',
-        replayRecord: null,
+        replay: null,
       })
     );
 
@@ -154,10 +145,10 @@ describe('useTraceTree', () => {
 
     const {result} = renderHook(() =>
       useTraceTree({
-        traceResults: getMockedTraceResults('success', mockedTrace),
-        metaResults: getMockedMetaResults('success', mockedMeta),
+        trace: getMockedTraceResults('success', mockedTrace),
+        meta: getMockedMetaResults('success', mockedMeta),
         traceSlug: 'test-trace',
-        replayRecord: null,
+        replay: null,
       })
     );
 
@@ -166,52 +157,4 @@ describe('useTraceTree', () => {
       expect(result.current.list).toHaveLength(7);
     });
   });
-
-  it('returns tree for non-empty success auto-zoom case', async () => {
-    MockApiClient.addMockResponse({
-      url: `/organizations/org-slug/events/project_slug:event_id/?averageColumn=span.self_time&averageColumn=span.duration`,
-      method: 'GET',
-      body: makeEvent(undefined, [
-        makeSpan({span_id: 'other_child_span'}),
-        makeSpan({span_id: 'child_span'}),
-      ]),
-    });
-
-    const mockedTrace = {
-      transactions: [
-        makeTransaction({
-          event_id: 'event_id',
-          start_timestamp: 0,
-          timestamp: 1,
-          transaction: 'transaction1',
-          project_slug: 'project_slug',
-        }),
-      ],
-      orphan_errors: [],
-    };
-
-    const mockedMeta = {
-      errors: 1,
-      performance_issues: 2,
-      projects: 1,
-      transactions: 1,
-      transactiontoSpanChildrenCount: {
-        '1': 1,
-      },
-    };
-
-    const {result} = renderHook(() =>
-      useTraceTree({
-        traceResults: getMockedTraceResults('success', mockedTrace),
-        metaResults: getMockedMetaResults('success', mockedMeta),
-        traceSlug: 'test-trace',
-        replayRecord: null,
-      })
-    );
-
-    await waitFor(() => {
-      expect(result.current.type).toBe('trace');
-      expect(result.current.list).toHaveLength(4);
-    });
-  });
 });

+ 40 - 58
static/app/views/performance/newTraceDetails/traceApi/useTraceTree.tsx

@@ -1,4 +1,4 @@
-import {useEffect, useMemo, useReducer, useRef, useState} from 'react';
+import {useEffect, useState} from 'react';
 
 import type {TraceSplitResults} from 'sentry/utils/performance/quickTrace/types';
 import type {QueryStatus, UseApiQueryResult} from 'sentry/utils/queryClient';
@@ -7,18 +7,14 @@ import useOrganization from 'sentry/utils/useOrganization';
 import useProjects from 'sentry/utils/useProjects';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 
-import {isTransactionNode} from '../traceGuards';
 import {TraceTree} from '../traceModels/traceTree';
 
 import type {TraceMetaQueryResults} from './useTraceMeta';
 
 type UseTraceTreeParams = {
-  metaResults: TraceMetaQueryResults;
-  replayRecord: ReplayRecord | null;
-  traceResults: UseApiQueryResult<
-    TraceSplitResults<TraceTree.Transaction> | undefined,
-    any
-  >;
+  meta: TraceMetaQueryResults;
+  replay: ReplayRecord | null;
+  trace: UseApiQueryResult<TraceSplitResults<TraceTree.Transaction> | undefined, any>;
   traceSlug?: string;
 };
 
@@ -38,85 +34,71 @@ function getTraceViewQueryStatus(
 }
 
 export function useTraceTree({
-  traceResults,
-  metaResults,
+  trace,
+  meta,
+  replay,
   traceSlug,
-  replayRecord,
 }: UseTraceTreeParams): TraceTree {
   const api = useApi();
   const {projects} = useProjects();
   const organization = useOrganization();
 
   const [tree, setTree] = useState<TraceTree>(TraceTree.Empty());
-  const loadingTraceRef = useRef<TraceTree | null>(null);
-  const [_, rerender] = useReducer(x => (x + 1) % Number.MAX_SAFE_INTEGER, 0);
-
-  const status = useMemo(() => {
-    return getTraceViewQueryStatus(traceResults.status, metaResults.status);
-  }, [traceResults.status, metaResults.status]);
 
   useEffect(() => {
+    const status = getTraceViewQueryStatus(trace.status, meta.status);
+
     if (status === 'error') {
-      const errorTree = TraceTree.Error({
-        project_slug: projects?.[0]?.slug ?? '',
-        event_id: traceSlug,
-      });
-      setTree(errorTree);
+      setTree(t =>
+        t.type === 'error'
+          ? t
+          : TraceTree.Error({
+              project_slug: projects?.[0]?.slug ?? '',
+              event_id: traceSlug,
+            })
+      );
       return;
     }
 
     if (
-      traceResults?.data?.transactions.length === 0 &&
-      traceResults?.data?.orphan_errors.length === 0
+      trace?.data?.transactions.length === 0 &&
+      trace?.data?.orphan_errors.length === 0
     ) {
-      setTree(TraceTree.Empty());
+      setTree(t => (t.type === 'empty' ? t : TraceTree.Empty()));
       return;
     }
 
     if (status === 'pending') {
-      const loadingTrace =
-        loadingTraceRef.current ??
-        TraceTree.Loading({
-          project_slug: projects?.[0]?.slug ?? '',
-          event_id: traceSlug,
-        });
-
-      loadingTraceRef.current = loadingTrace;
-      setTree(loadingTrace);
+      setTree(t =>
+        t.type === 'loading'
+          ? t
+          : TraceTree.Loading({
+              project_slug: projects?.[0]?.slug ?? '',
+              event_id: traceSlug,
+            })
+      );
       return;
     }
 
-    if (traceResults.data && metaResults.data) {
-      const trace = TraceTree.FromTrace(traceResults.data, {
-        meta: metaResults,
-        replayRecord: replayRecord,
+    if (trace.data && meta.data) {
+      const newTree = TraceTree.FromTrace(trace.data, {
+        meta: meta.data,
+        replay: replay,
       });
 
-      // Root frame + 2 nodes
-      const promises: Promise<void>[] = [];
-      const transactions = TraceTree.FindAll(trace.root, c => isTransactionNode(c));
-
-      if (transactions.length <= 3) {
-        for (const c of trace.list) {
-          if (c.canFetch) {
-            promises.push(trace.zoom(c, true, {api, organization}).then(rerender));
-          }
-        }
-      }
-
-      Promise.allSettled(promises).finally(() => {
-        setTree(trace);
-      });
+      setTree(newTree);
+      newTree.build();
+      return;
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [
     api,
     organization,
     projects,
-    replayRecord,
-    status,
-    metaResults.data,
-    traceResults.data,
+    replay,
+    meta.status,
+    trace.status,
+    trace.data,
+    meta.data,
     traceSlug,
   ]);
 

Some files were not shown because too many files changed in this diff