ref(trace): address feedback in trace view (#66819)

Addresses some feedback from @gggritso - add cursor pointer to rows, - move txn warning to drawer and fix flashing - increase drawer handle size - persist click selection styles - fix tab title
getsentry · Mar 12, 2024 · 525cec9 · 525cec9
1 parent 8dbc875
commit 525cec9
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 45 deletions.
diff --git a/static/app/components/events/interfaces/spans/newTraceDetailsSpanDetails.tsx b/static/app/components/events/interfaces/spans/newTraceDetailsSpanDetails.tsx
@@ -1,6 +1,7 @@
 import {Fragment, useEffect, useState} from 'react';
 import styled from '@emotion/styled';
 import omit from 'lodash/omit';
+import * as qs from 'query-string';
 
 import {Alert} from 'sentry/components/alert';
 import {Button} from 'sentry/components/button';
@@ -38,6 +39,7 @@ import type {
   TraceErrorOrIssue,
   TraceFullDetailed,
 } from 'sentry/utils/performance/quickTrace/types';
+import {safeURL} from 'sentry/utils/url/safeURL';
 import {useLocation} from 'sentry/utils/useLocation';
 import useProjects from 'sentry/utils/useProjects';
 import {CustomMetricsEventData} from 'sentry/views/ddm/customMetricsEventData';
@@ -474,6 +476,9 @@ function NewTraceDetailsSpanDetail(props: SpanDetailProps) {
                   {profileId}
                 </Row>
               )}
+              <Row title={t('Status')}>{span.status || ''}</Row>
+              <Row title={t('Duration')}>{durationString}</Row>
+              <SpanHTTPInfo span={props.span} />
               <Row
                 title={
                   resolvedModule === ModuleName.DB && span.op?.startsWith('db')
@@ -492,8 +497,6 @@ function NewTraceDetailsSpanDetail(props: SpanDetailProps) {
                   span.description
                 )}
               </Row>
-              <Row title={t('Status')}>{span.status || ''}</Row>
-              <Row title={t('Duration')}>{durationString}</Row>
               <Row title={t('Date Range')}>
                 {getDynamicText({
                   fixed: 'Mar 16, 2020 9:10:12 AM UTC',
@@ -599,6 +602,31 @@ function NewTraceDetailsSpanDetail(props: SpanDetailProps) {
   );
 }
 
+function SpanHTTPInfo({span}: {span: RawSpanType}) {
+  if (span.op === 'http.client' && span.description) {
+    const [method, url] = span.description.split(' ');
+
+    const parsedURL = safeURL(url);
+    const queryString = qs.parse(parsedURL?.search ?? '');
+
+    return parsedURL ? (
+      <Fragment>
+        <Row title={t('HTTP Method')}>{method}</Row>
+        <Row title={t('URL')}>
+          {parsedURL ? parsedURL?.origin + parsedURL?.pathname : 'failed to parse URL'}
+        </Row>
+        <Row title={t('Query')}>
+          {parsedURL
+            ? JSON.stringify(queryString, null, 2)
+            : 'failed to parse query string'}
+        </Row>
+      </Fragment>
+    ) : null;
+  }
+
+  return null;
+}
+
 function RowTimingPrefix({timing}: {timing: SubTimingInfo}) {
   return <OpsDot style={{backgroundColor: timing.color}} />;
 }

diff --git a/static/app/views/performance/newTraceDetails/index.tsx b/static/app/views/performance/newTraceDetails/index.tsx
@@ -55,7 +55,6 @@ import {isTraceNode} from './guards';
 import Trace from './trace';
 import TraceHeader from './traceHeader';
 import {TraceTree, type TraceTreeNode} from './traceTree';
-import TraceWarnings from './traceWarnings';
 import {useTrace} from './useTrace';
 
 const DOCUMENT_TITLE = [t('Trace Details'), t('Performance')].join(' — ');
@@ -206,13 +205,6 @@ function TraceViewContent(props: TraceViewContentProps) {
     rootEvent.status,
   ]);
 
-  const traceType = useMemo(() => {
-    if (props.status !== 'success' || !tree) {
-      return null;
-    }
-    return tree.shape;
-  }, [props.status, tree]);
-
   const [rovingTabIndexState, rovingTabIndexDispatch] = useReducer(
     rovingTabIndexReducer,
     {
@@ -411,7 +403,6 @@ function TraceViewContent(props: TraceViewContentProps) {
       </Layout.Header>
       <Layout.Body>
         <Layout.Main fullWidth>
-          {traceType ? <TraceWarnings type={traceType} /> : null}
           <TraceHeader
             rootEventResults={rootEvent}
             metaResults={props.metaResults}
@@ -445,6 +436,7 @@ function TraceViewContent(props: TraceViewContentProps) {
               manager={viewManager}
             />
             <TraceDrawer
+              trace={tree}
               scrollToNode={scrollToNode}
               manager={viewManager}
               activeTab={activeTab}

diff --git a/static/app/views/performance/newTraceDetails/trace.tsx b/static/app/views/performance/newTraceDetails/trace.tsx
@@ -1204,7 +1204,7 @@ function RenderRow(props: {
             <PlatformIcon
               platform={props.projects[props.node.value.project_slug] ?? 'default'}
             />
-            ;<span className="TraceOperation Errored">{t('Error')}</span>
+            <span className="TraceOperation Errored">{t('Error')}</span>
             <strong className="TraceEmDash Errored"> — </strong>
             <span className="TraceDescription Errored">{props.node.value.title}</span>
           </div>
@@ -1771,7 +1771,9 @@ const TraceStylingWrapper = styled('div')`
       }
     }
 
-    &:focus {
+    &:focus,
+    &[tabindex='0'] {
+      background-color: ${p => p.theme.backgroundTertiary};
       box-shadow: inset 0 0 0 1px ${p => p.theme.blue300} !important;
 
       .TraceLeftColumn {
@@ -1841,6 +1843,7 @@ const TraceStylingWrapper = styled('div')`
     align-items: center;
     will-change: width;
     z-index: 1;
+    cursor: pointer;
 
     &:hover {
       .TraceArrow.Visible {

diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace.tsx
@@ -1,4 +1,4 @@
-import {Fragment} from 'react';
+import {Fragment, useMemo} from 'react';
 import styled from '@emotion/styled';
 import type {Location} from 'history';
 
@@ -20,6 +20,8 @@ import {WEB_VITAL_DETAILS} from 'sentry/utils/performance/vitals/constants';
 import type {UseApiQueryResult} from 'sentry/utils/queryClient';
 import type RequestError from 'sentry/utils/requestError/requestError';
 import Tags from 'sentry/views/discover/tags';
+import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceTree';
+import TraceWarnings from 'sentry/views/performance/newTraceDetails/traceWarnings';
 
 import {getTraceInfo} from '../../../traceDetails/utils';
 
@@ -37,6 +39,7 @@ type TraceFooterProps = {
   rootEventResults: UseApiQueryResult<EventTransaction, RequestError>;
   traceEventView: EventView;
   traces: TraceSplitResults<TraceFullDetailed> | null;
+  tree: TraceTree;
 };
 
 function NoWebVitals() {
@@ -84,6 +87,10 @@ function TraceDataLoading() {
 }
 
 export function TraceLevelDetails(props: TraceFooterProps) {
+  const treeType = useMemo(() => {
+    return props.tree.shape;
+  }, [props.tree]);
+
   if (!props.traces) {
     return <TraceDataLoading />;
   }
@@ -99,34 +106,37 @@ export function TraceLevelDetails(props: TraceFooterProps) {
     .sort();
 
   return rootEvent ? (
-    <TraceFooterWrapper>
-      {webVitals.length > 0 ? (
+    <Fragment>
+      {treeType ? <TraceWarnings type={treeType} /> : null}
+      <TraceFooterWrapper>
+        {webVitals.length > 0 ? (
+          <div style={{flex: 1}}>
+            <EventVitals event={rootEvent} />
+          </div>
+        ) : (
+          <NoWebVitals />
+        )}
         <div style={{flex: 1}}>
-          <EventVitals event={rootEvent} />
+          <Tags
+            generateUrl={(key: string, value: string) => {
+              const url = props.traceEventView.getResultsViewUrlTarget(
+                props.organization.slug,
+                false
+              );
+              url.query = generateQueryWithTag(url.query, {
+                key: formatTagKey(key),
+                value,
+              });
+              return url;
+            }}
+            totalValues={totalNumOfEvents}
+            eventView={props.traceEventView}
+            organization={props.organization}
+            location={props.location}
+          />
         </div>
-      ) : (
-        <NoWebVitals />
-      )}
-      <div style={{flex: 1}}>
-        <Tags
-          generateUrl={(key: string, value: string) => {
-            const url = props.traceEventView.getResultsViewUrlTarget(
-              props.organization.slug,
-              false
-            );
-            url.query = generateQueryWithTag(url.query, {
-              key: formatTagKey(key),
-              value,
-            });
-            return url;
-          }}
-          totalValues={totalNumOfEvents}
-          eventView={props.traceEventView}
-          organization={props.organization}
-          location={props.location}
-        />
-      </div>
-    </TraceFooterWrapper>
+      </TraceFooterWrapper>
+    </Fragment>
   ) : null;
 }
 

diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/traceDrawer.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/traceDrawer.tsx
@@ -33,12 +33,18 @@ const DEFAULT_PANEL_HEIGHT = 200;
 function getTabTitle(node: TraceTreeNode<TraceTree.NodeValue>) {
   if (isTransactionNode(node)) {
     return (
-      t('Transaction: ') + node.value['transaction.op'] + ' - ' + node.value.transaction
+      t('Transaction: ') +
+      node.value['transaction.op'] +
+      (node.value.transaction ? ' - ' + node.value.transaction : '')
     );
   }
 
   if (isSpanNode(node)) {
-    return t('Span: ') + node.value.op + ' - ' + node.value.description;
+    return (
+      t('Span: ') +
+      node.value.op +
+      (node.value.description ? ' - ' + node.value.description : '')
+    );
   }
 
   if (isAutogroupedNode(node)) {
@@ -50,7 +56,7 @@ function getTabTitle(node: TraceTreeNode<TraceTree.NodeValue>) {
   }
 
   if (isTraceErrorNode(node)) {
-    return node.value.title;
+    return node.value.title || 'Error';
   }
 
   return t('Detail');
@@ -65,6 +71,7 @@ type TraceDrawerProps = {
   rootEventResults: UseApiQueryResult<EventTransaction, RequestError>;
   scrollToNode: (node: TraceTreeNode<TraceTree.NodeValue>) => void;
   setActiveTab: (tab: 'trace' | 'node') => void;
+  trace: TraceTree;
   traceEventView: EventView;
   traces: TraceSplitResults<TraceFullDetailed> | null;
 };
@@ -114,6 +121,7 @@ function TraceDrawer(props: TraceDrawerProps) {
       <Content>
         {props.activeTab === 'trace' ? (
           <TraceLevelDetails
+            tree={props.trace}
             rootEventResults={props.rootEventResults}
             organization={props.organization}
             location={props.location}
@@ -139,10 +147,10 @@ function TraceDrawer(props: TraceDrawerProps) {
 
 const ResizeableHandle = styled('div')`
   width: 100%;
-  height: 8px;
+  height: 12px;
   cursor: ns-resize;
   position: absolute;
-  top: -4px;
+  top: -6px;
   left: 0;
   z-index: 1;
 `;