Browse Source

feat(trace): add no data message (#67873)

Sometimes we try to fetch data from traces that have no children (we
dont know aot). When the data fetched is empty, we previously just didnt
show anything, so it looked like the UI never updated. With this, we
will now show an empty data row and explain the cause. The new row is
linkable, just like everything else so users can share direct links to
this nodes as well.

![CleanShot 2024-03-28 at 16 50
07@2x](https://github.com/getsentry/sentry/assets/9317857/c3c558c4-f81c-47ca-9ca4-119c1abcf9d0)
Jonas 11 months ago
parent
commit
1eca50e46d

+ 8 - 1
static/app/views/performance/newTraceDetails/guards.tsx

@@ -1,5 +1,6 @@
 import {
   MissingInstrumentationNode,
+  NoDataNode,
   ParentAutogroupNode,
   SiblingAutogroupNode,
   type TraceTree,
@@ -54,7 +55,7 @@ export function isTraceErrorNode(
 export function isRootNode(
   node: TraceTreeNode<TraceTree.NodeValue>
 ): node is TraceTreeNode<null> {
-  return node.value === null;
+  return node.value === null && !(node instanceof NoDataNode);
 }
 
 export function isTraceNode(
@@ -66,6 +67,12 @@ export function isTraceNode(
   );
 }
 
+export function isNoDataNode(
+  node: TraceTreeNode<TraceTree.NodeValue>
+): node is NoDataNode {
+  return node instanceof NoDataNode;
+}
+
 export function shouldAddMissingInstrumentationSpan(sdk: string | undefined): boolean {
   if (!sdk) return true;
   if (sdk.length < 'sentry.javascript.'.length) return true;

+ 67 - 5
static/app/views/performance/newTraceDetails/trace.tsx

@@ -9,7 +9,7 @@ import * as qs from 'query-string';
 
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Placeholder from 'sentry/components/placeholder';
-import {t} from 'sentry/locale';
+import {t, tct} from 'sentry/locale';
 import type {Organization, PlatformKey, Project} from 'sentry/types';
 import {getDuration} from 'sentry/utils/formatters';
 import type {
@@ -33,6 +33,7 @@ import type {
 import {
   isAutogroupedNode,
   isMissingInstrumentationNode,
+  isNoDataNode,
   isParentAutogroupedNode,
   isSpanNode,
   isTraceErrorNode,
@@ -272,6 +273,7 @@ export function Trace({
 
       if (!maybeNode) {
         Sentry.captureMessage('Failled to find and scroll to node in tree');
+        setRender(a => (a + 1) % 2);
         return;
       }
 
@@ -680,10 +682,6 @@ function RenderRow(props: {
   trace_id: string;
 }) {
   const virtualized_index = props.index - props.manager.start_virtualized_index;
-  if (!props.node.value) {
-    return null;
-  }
-
   const rowSearchClassName = `${props.isSearchResult ? 'SearchResult' : ''} ${props.searchResultsIteratorIndex === props.index ? 'Highlight' : ''}`;
 
   if (isAutogroupedNode(props.node)) {
@@ -1250,6 +1248,70 @@ function RenderRow(props: {
     );
   }
 
+  if (isNoDataNode(props.node)) {
+    return (
+      <div
+        key={props.index}
+        ref={r =>
+          props.tabIndex === props.index
+            ? maybeFocusRow(r, props.node, props.previouslyFocusedNodeRef)
+            : null
+        }
+        tabIndex={props.tabIndex === props.index ? 0 : -1}
+        className={`TraceRow ${rowSearchClassName}`}
+        onClick={e => props.onRowClick(e, props.index, props.node)}
+        onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
+        style={{
+          top: props.style.top,
+          height: props.style.height,
+        }}
+      >
+        <div
+          className="TraceLeftColumn"
+          ref={r =>
+            props.manager.registerColumnRef('list', r, virtualized_index, props.node)
+          }
+          style={{
+            width: props.manager.columns.list.width * 100 + '%',
+          }}
+        >
+          <div
+            className="TraceLeftColumnInner"
+            style={{
+              paddingLeft: props.node.depth * props.manager.row_depth_padding,
+            }}
+          >
+            <div className="TraceChildrenCountWrapper">
+              <Connectors node={props.node} manager={props.manager} />
+            </div>
+            <span className="TraceOperation">{t('Empty')}</span>
+            <strong className="TraceEmDash"> — </strong>
+            <span className="TraceDescription">
+              {tct('[type] did not report any span data', {
+                type: props.node.parent
+                  ? isTransactionNode(props.node.parent)
+                    ? 'Transaction'
+                    : isSpanNode(props.node.parent)
+                      ? 'Span'
+                      : ''
+                  : '',
+              })}
+            </span>
+          </div>
+        </div>
+        <div
+          ref={r =>
+            props.manager.registerColumnRef('span_list', r, virtualized_index, props.node)
+          }
+          className={`TraceRightColumn ${props.index % 2 === 0 ? 0 : 'Odd'}`}
+          style={{
+            width: props.manager.columns.span_list.width * 100 + '%',
+          }}
+        />
+      </div>
+    );
+  }
+
   return null;
 }
 

+ 73 - 0
static/app/views/performance/newTraceDetails/traceDrawer/details/noData.tsx

@@ -0,0 +1,73 @@
+import {useRef} from 'react';
+import {useTheme} from '@emotion/react';
+
+import {Button} from 'sentry/components/button';
+import useFeedbackWidget from 'sentry/components/feedback/widget/useFeedbackWidget';
+import {IconGroup} from 'sentry/icons';
+import {t, tct} from 'sentry/locale';
+import type {Organization} from 'sentry/types';
+import {TraceDrawerComponents} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/styles';
+import {
+  makeTraceNodeBarColor,
+  type NoDataNode,
+  type TraceTree,
+  type TraceTreeNode,
+} from 'sentry/views/performance/newTraceDetails/traceTree';
+import {Row} from 'sentry/views/performance/traceDetails/styles';
+
+interface NoDataDetailsProps {
+  node: NoDataNode;
+  onParentClick: (node: TraceTreeNode<TraceTree.NodeValue>) => void;
+  organization: Organization;
+  scrollToNode: (node: TraceTreeNode<TraceTree.NodeValue>) => void;
+}
+
+export function NoDataDetails(props: NoDataDetailsProps) {
+  const theme = useTheme();
+
+  return (
+    <TraceDrawerComponents.DetailContainer>
+      <TraceDrawerComponents.HeaderContainer>
+        <TraceDrawerComponents.IconTitleWrapper>
+          <TraceDrawerComponents.IconBorder
+            backgroundColor={makeTraceNodeBarColor(theme, props.node)}
+          >
+            <IconGroup />
+          </TraceDrawerComponents.IconBorder>
+          <div style={{fontWeight: 'bold'}}>{t('Empty')}</div>
+        </TraceDrawerComponents.IconTitleWrapper>
+
+        <TraceDrawerComponents.Actions>
+          <Button size="xs" onClick={_e => props.scrollToNode(props.node)}>
+            {t('Show in view')}
+          </Button>
+        </TraceDrawerComponents.Actions>
+      </TraceDrawerComponents.HeaderContainer>
+
+      <TraceDrawerComponents.Table className="table key-value">
+        <tbody>
+          <Row title={t('Data quality')}>
+            {tct(
+              'The cause of missing data could be misconfiguration or lack of instrumentation. Send us [feedback] if you are having trouble figuring this out.',
+              {feedback: <InlineFeedbackLink />}
+            )}
+          </Row>
+        </tbody>
+      </TraceDrawerComponents.Table>
+    </TraceDrawerComponents.DetailContainer>
+  );
+}
+
+function InlineFeedbackLink() {
+  const linkref = useRef<HTMLAnchorElement>(null);
+  const feedback = useFeedbackWidget({buttonRef: linkref});
+  return feedback ? (
+    <a href="#" ref={linkref}>
+      {t('feedback')}
+    </a>
+  ) : (
+    <a href="mailto:support@sentry.io?subject=Trace%20does%20not%20contain%20data">
+      {t('feedback')}
+    </a>
+  );
+}

+ 13 - 0
static/app/views/performance/newTraceDetails/traceDrawer/tabs/details.tsx

@@ -3,6 +3,7 @@ import type {VirtualizedViewManager} from 'sentry/views/performance/newTraceDeta
 
 import {
   isMissingInstrumentationNode,
+  isNoDataNode,
   isParentAutogroupedNode,
   isSiblingAutogroupedNode,
   isSpanNode,
@@ -12,6 +13,7 @@ import {
 import type {TraceTree, TraceTreeNode} from '../../traceTree';
 import {ErrorNodeDetails} from '../details/error';
 import {MissingInstrumentationNodeDetails} from '../details/missingInstrumentation';
+import {NoDataDetails} from '../details/noData';
 import {ParentAutogroupNodeDetails} from '../details/parentAutogroup';
 import {SiblingAutogroupNodeDetails} from '../details/siblingAutogroup';
 import {SpanNodeDetails} from '../details/span';
@@ -96,5 +98,16 @@ export default function NodeDetail({
     );
   }
 
+  if (isNoDataNode(node)) {
+    return (
+      <NoDataDetails
+        node={node}
+        onParentClick={onParentClick}
+        organization={organization}
+        scrollToNode={scrollToNode}
+      />
+    );
+  }
+
   throw new Error('Unknown clicked node type');
 }

+ 5 - 0
static/app/views/performance/newTraceDetails/traceTabs.tsx

@@ -9,6 +9,7 @@ import type {
 import {
   isAutogroupedNode,
   isMissingInstrumentationNode,
+  isNoDataNode,
   isSpanNode,
   isTraceErrorNode,
   isTraceNode,
@@ -43,6 +44,10 @@ export function getTraceTabTitle(node: TraceTreeNode<TraceTree.NodeValue>) {
     return t('Trace');
   }
 
+  if (isNoDataNode(node)) {
+    return t('Empty');
+  }
+
   Sentry.captureMessage('Unknown node type in trace drawer');
   return 'Unknown';
 }

+ 37 - 0
static/app/views/performance/newTraceDetails/traceTree.spec.tsx

@@ -20,6 +20,7 @@ import {
   isTransactionNode,
 } from './guards';
 import {
+  NoDataNode,
   ParentAutogroupNode,
   SiblingAutogroupNode,
   TraceTree,
@@ -166,6 +167,14 @@ function assertSiblingAutogroupedNode(
   }
 }
 
+function assertNoDataNode(
+  node: TraceTreeNode<TraceTree.NodeValue>
+): asserts node is NoDataNode {
+  if (!(node instanceof NoDataNode)) {
+    throw new Error('node is not a no data node');
+  }
+}
+
 describe('TreeNode', () => {
   it('expands transaction nodes by default', () => {
     const node = new TraceTreeNode(null, makeTransaction(), {
@@ -594,6 +603,34 @@ describe('TreeNode', () => {
       });
     });
 
+    it('inserts no data node when txn has no span children', async () => {
+      const tree = TraceTree.FromTrace(
+        makeTrace({
+          transactions: [
+            makeTransaction({
+              transaction: '/',
+              project_slug: 'project',
+              event_id: 'event_id',
+            }),
+          ],
+        })
+      );
+
+      MockApiClient.addMockResponse({
+        url: EVENT_REQUEST_URL,
+        method: 'GET',
+        body: makeEvent({}, []),
+      });
+
+      await tree.zoomIn(tree.list[1], true, {
+        api: new MockApiClient(),
+        organization: OrganizationFixture(),
+      });
+
+      assertNoDataNode(tree.list[2]);
+      expect(tree.list.length).toBe(3);
+    });
+
     describe('autogrouped children', () => {
       const tree = TraceTree.FromTrace(
         makeTrace({

+ 45 - 13
static/app/views/performance/newTraceDetails/traceTree.tsx

@@ -23,6 +23,7 @@ import {isRootTransaction} from '../traceDetails/utils';
 import {
   isAutogroupedNode,
   isMissingInstrumentationNode,
+  isNoDataNode,
   isParentAutogroupedNode,
   isRootNode,
   isSiblingAutogroupedNode,
@@ -141,7 +142,8 @@ export declare namespace TraceTree {
     | ChildrenAutogroup
     | null;
 
-  type NodePath = `${'txn' | 'span' | 'ag' | 'trace' | 'ms' | 'error'}:${string}`;
+  type NodePath =
+    `${'txn' | 'span' | 'ag' | 'trace' | 'ms' | 'error' | 'empty'}:${string}`;
 
   type Metadata = {
     event_id: string | undefined;
@@ -239,6 +241,9 @@ export function makeTraceNodeBarColor(
   if (isTraceErrorNode(node)) {
     return theme.red300;
   }
+  if (isNoDataNode(node)) {
+    return theme.yellow300;
+  }
   return pickBarColor('default');
 }
 
@@ -503,11 +508,19 @@ export class TraceTree {
       TraceTreeNode<TraceTree.Span | TraceTree.Transaction>
     > = {};
 
+    // If we've already fetched children, the tree is already assembled
     if (parent.spanChildren.length > 0) {
       parent.zoomedIn = true;
       return parent;
     }
 
+    // If we have no spans, insert an empty node to indicate that there is no data
+    if (!spans.length && !parent.children.length) {
+      parent.zoomedIn = true;
+      parent.spanChildren.push(new NoDataNode(parent));
+      return parent;
+    }
+
     if (parentIsSpan) {
       if (parent.value && 'span_id' in parent.value) {
         lookuptable[parent.value.span_id] = parent as TraceTreeNode<TraceTree.Span>;
@@ -964,10 +977,7 @@ export class TraceTree {
       .then(data => {
         node.fetchStatus = 'resolved';
 
-        const spans = data.entries.find(s => s.type === 'spans');
-        if (!spans) {
-          return data;
-        }
+        const spans = data.entries.find(s => s.type === 'spans') ?? {data: []};
 
         // Remove existing entries from the list
         const index = this._list.indexOf(node);
@@ -979,9 +989,7 @@ export class TraceTree {
         }
 
         // Api response is not sorted
-        if (spans.data) {
-          spans.data.sort((a, b) => a.start_timestamp - b.start_timestamp);
-        }
+        spans.data.sort((a, b) => a.start_timestamp - b.start_timestamp);
 
         TraceTree.FromSpans(node, data, spans.data, {sdk: data.sdk?.name});
 
@@ -1124,8 +1132,12 @@ export class TraceTreeNode<T extends TraceTree.NodeValue> {
     }
   }
 
-  cloneDeep(): TraceTreeNode<T> | ParentAutogroupNode | SiblingAutogroupNode {
-    let clone: TraceTreeNode<T> | ParentAutogroupNode | SiblingAutogroupNode;
+  cloneDeep():
+    | TraceTreeNode<T>
+    | ParentAutogroupNode
+    | SiblingAutogroupNode
+    | NoDataNode {
+    let clone: TraceTreeNode<T> | ParentAutogroupNode | SiblingAutogroupNode | NoDataNode;
 
     if (isParentAutogroupedNode(this)) {
       clone = new ParentAutogroupNode(
@@ -1139,6 +1151,8 @@ export class TraceTreeNode<T extends TraceTree.NodeValue> {
     } else if (isSiblingAutogroupedNode(this)) {
       clone = new SiblingAutogroupNode(this.parent, this.value, this.metadata);
       clone.groupCount = this.groupCount;
+    } else if (isNoDataNode(this)) {
+      clone = new NoDataNode(this.parent);
     } else {
       clone = new TraceTreeNode(this.parent, this.value, this.metadata);
     }
@@ -1316,9 +1330,10 @@ export class TraceTreeNode<T extends TraceTree.NodeValue> {
     this._children = children;
   }
 
-  get spanChildren(): TraceTreeNode<
-    TraceTree.Span | TraceTree.MissingInstrumentationSpan
-  >[] {
+  get spanChildren(): (
+    | TraceTreeNode<TraceTree.Span | TraceTree.MissingInstrumentationSpan>
+    | NoDataNode
+  )[] {
     return this._spanChildren;
   }
 
@@ -1594,6 +1609,15 @@ export class SiblingAutogroupNode extends TraceTreeNode<TraceTree.SiblingAutogro
   }
 }
 
+export class NoDataNode extends TraceTreeNode<null> {
+  constructor(parent: TraceTreeNode<TraceTree.NodeValue> | null) {
+    super(parent, null, {
+      event_id: undefined,
+      project_slug: undefined,
+    });
+  }
+}
+
 function partialTransaction(
   partial: Partial<TraceTree.Transaction>
 ): TraceTree.Transaction {
@@ -1703,6 +1727,10 @@ function nodeToId(n: TraceTreeNode<TraceTree.NodeValue>): TraceTree.NodePath {
     return `error:${n.value.event_id}`;
   }
 
+  if (isNoDataNode(n)) {
+    return `empty:node`;
+  }
+
   if (isRootNode(n)) {
     throw new Error('A path to root node does not exist as the node is virtual');
   }
@@ -1858,6 +1886,10 @@ function printNode(t: TraceTreeNode<TraceTree.NodeValue>, offset: number): strin
     return padding + 'Trace';
   }
 
+  if (isNoDataNode(t)) {
+    return padding + 'No Data';
+  }
+
   if (isTraceErrorNode(t)) {
     return padding + (t.value.event_id || t.value.level) || 'unknown trace error';
   }

+ 36 - 0
static/app/views/performance/newTraceDetails/virtualizedViewManager.spec.tsx

@@ -430,6 +430,42 @@ describe('VirtualizedViewManger', () => {
       expect(manager.list.scrollToRow).toHaveBeenCalledWith(2);
     });
 
+    it('scrolls to empty data node of expanded transaction', async () => {
+      manager.list = makeList();
+
+      const tree = TraceTree.FromTrace(
+        makeTrace({
+          transactions: [
+            makeTransaction({
+              event_id: 'event_id',
+              project_slug: 'project',
+              children: [],
+            }),
+          ],
+        })
+      );
+
+      MockApiClient.addMockResponse({
+        url: EVENT_REQUEST_URL,
+        method: 'GET',
+        body: makeEvent(undefined, []),
+      });
+
+      const result = await manager.scrollToPath(
+        tree,
+        ['empty:node', 'txn:event_id'],
+        () => void 0,
+        {
+          api: api,
+          organization,
+        }
+      );
+
+      expect(tree.list[1].zoomedIn).toBe(true);
+      expect(result?.node).toBe(tree.list[2]);
+      expect(manager.list.scrollToRow).toHaveBeenCalledWith(2);
+    });
+
     it('scrolls to span -> transaction -> span -> transaction', async () => {
       manager.list = makeList();
 

+ 6 - 0
static/app/views/performance/newTraceDetails/virtualizedViewManager.tsx

@@ -11,6 +11,7 @@ import {lightTheme as theme} from 'sentry/utils/theme';
 import {
   isAutogroupedNode,
   isMissingInstrumentationNode,
+  isNoDataNode,
   isParentAutogroupedNode,
   isSiblingAutogroupedNode,
   isSpanNode,
@@ -1074,6 +1075,7 @@ export class VirtualizedViewManager {
         const nextSegment = segments[segments.length - 1];
         if (
           nextSegment?.startsWith('span:') ||
+          nextSegment?.startsWith('empty:') ||
           nextSegment?.startsWith('ag:') ||
           nextSegment?.startsWith('ms:')
         ) {
@@ -2042,6 +2044,10 @@ function findInTreeFromSegment(
       return node.value.event_id === id;
     }
 
+    if (type === 'empty' && isNoDataNode(node)) {
+      return true;
+    }
+
     return false;
   });
 }