Browse Source

feat(span-view): Use quick trace results to render flame icon on spans (#25563)

This is the first of the PRs to make use of the quick trace results to render
errors on the span view instead of making additional discover queries. We
cannot remove them all in one go in order to preserve functionality as the
rollout for quick trace is still in progress

Previously, the only indication was that the span op was coloured red. This
change introduces a flame icon on the divider line to indicate when an error
occurred within a span.
Tony Xiao 3 years ago
parent
commit
60d6126e9c

+ 61 - 23
static/app/components/events/interfaces/spans/spanBar.tsx

@@ -8,8 +8,10 @@ import {ROW_HEIGHT} from 'app/components/performance/waterfall/constants';
 import {Row, RowCell, RowCellContainer} from 'app/components/performance/waterfall/row';
 import {Row, RowCell, RowCellContainer} from 'app/components/performance/waterfall/row';
 import {DurationPill, RowRectangle} from 'app/components/performance/waterfall/rowBar';
 import {DurationPill, RowRectangle} from 'app/components/performance/waterfall/rowBar';
 import {
 import {
+  DividerContainer,
   DividerLine,
   DividerLine,
   DividerLineGhostContainer,
   DividerLineGhostContainer,
+  ErrorBadge,
 } from 'app/components/performance/waterfall/rowDivider';
 } from 'app/components/performance/waterfall/rowDivider';
 import {
 import {
   OperationName,
   OperationName,
@@ -39,6 +41,8 @@ import {defined} from 'app/utils';
 import {TableDataRow} from 'app/utils/discover/discoverQuery';
 import {TableDataRow} from 'app/utils/discover/discoverQuery';
 import * as QuickTraceContext from 'app/utils/performance/quickTrace/quickTraceContext';
 import * as QuickTraceContext from 'app/utils/performance/quickTrace/quickTraceContext';
 import {QuickTraceContextChildrenProps} from 'app/utils/performance/quickTrace/quickTraceContext';
 import {QuickTraceContextChildrenProps} from 'app/utils/performance/quickTrace/quickTraceContext';
+import {TraceError} from 'app/utils/performance/quickTrace/types';
+import {isTraceFull} from 'app/utils/performance/quickTrace/utils';
 
 
 import * as CursorGuideHandler from './cursorGuideHandler';
 import * as CursorGuideHandler from './cursorGuideHandler';
 import * as DividerHandlerManager from './dividerHandlerManager';
 import * as DividerHandlerManager from './dividerHandlerManager';
@@ -57,6 +61,7 @@ import {
   getSpanID,
   getSpanID,
   getSpanOperation,
   getSpanOperation,
   isEventFromBrowserJavaScriptSDK,
   isEventFromBrowserJavaScriptSDK,
+  isGapSpan,
   isOrphanSpan,
   isOrphanSpan,
   isOrphanTreeDepth,
   isOrphanTreeDepth,
   SpanBoundsType,
   SpanBoundsType,
@@ -478,14 +483,17 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
   }
   }
 
 
   renderTitle(
   renderTitle(
-    scrollbarManagerChildrenProps: ScrollbarManager.ScrollbarManagerChildrenProps
+    scrollbarManagerChildrenProps: ScrollbarManager.ScrollbarManagerChildrenProps,
+    errors: TraceError[]
   ) {
   ) {
     const {generateContentSpanBarRef} = scrollbarManagerChildrenProps;
     const {generateContentSpanBarRef} = scrollbarManagerChildrenProps;
     const {span, treeDepth, spanErrors} = this.props;
     const {span, treeDepth, spanErrors} = this.props;
 
 
     const operationName = getSpanOperation(span) ? (
     const operationName = getSpanOperation(span) ? (
       <strong>
       <strong>
-        <OperationName spanErrors={spanErrors}>{getSpanOperation(span)}</OperationName>
+        <OperationName errored={errors.length + spanErrors.length > 0}>
+          {getSpanOperation(span)}
+        </OperationName>
         {` \u2014 `}
         {` \u2014 `}
       </strong>
       </strong>
     ) : (
     ) : (
@@ -722,7 +730,7 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
         <DividerLine
         <DividerLine
           showDetail
           showDetail
           style={{
           style={{
-            position: 'relative',
+            position: 'absolute',
           }}
           }}
         />
         />
       );
       );
@@ -734,7 +742,7 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
       <DividerLine
       <DividerLine
         ref={addDividerLineRef()}
         ref={addDividerLineRef()}
         style={{
         style={{
-          position: 'relative',
+          position: 'absolute',
         }}
         }}
         onMouseEnter={() => {
         onMouseEnter={() => {
           dividerHandlerChildrenProps.setHover(true);
           dividerHandlerChildrenProps.setHover(true);
@@ -755,6 +763,25 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
     );
     );
   }
   }
 
 
+  getRelatedErrors(quickTrace: QuickTraceContextChildrenProps): TraceError[] {
+    if (!quickTrace) {
+      return [];
+    }
+
+    const {span} = this.props;
+    const {currentEvent} = quickTrace;
+
+    if (isGapSpan(span) || !currentEvent || !isTraceFull(currentEvent)) {
+      return [];
+    }
+
+    return currentEvent.errors.filter(error => error.span === span.span_id);
+  }
+
+  renderErrorBadge(errors: TraceError[]): React.ReactNode {
+    return errors.length ? <ErrorBadge /> : null;
+  }
+
   renderWarningText({warningText}: {warningText?: string} = {}) {
   renderWarningText({warningText}: {warningText?: string} = {}) {
     if (!warningText) {
     if (!warningText) {
       return null;
       return null;
@@ -770,9 +797,11 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
   renderHeader({
   renderHeader({
     scrollbarManagerChildrenProps,
     scrollbarManagerChildrenProps,
     dividerHandlerChildrenProps,
     dividerHandlerChildrenProps,
+    quickTrace,
   }: {
   }: {
     dividerHandlerChildrenProps: DividerHandlerManager.DividerHandlerManagerChildrenProps;
     dividerHandlerChildrenProps: DividerHandlerManager.DividerHandlerManagerChildrenProps;
     scrollbarManagerChildrenProps: ScrollbarManager.ScrollbarManagerChildrenProps;
     scrollbarManagerChildrenProps: ScrollbarManager.ScrollbarManagerChildrenProps;
+    quickTrace: QuickTraceContextChildrenProps;
   }) {
   }) {
     const {span, spanBarColour, spanBarHatch, spanNumber} = this.props;
     const {span, spanBarColour, spanBarHatch, spanNumber} = this.props;
     const startTimestamp: number = span.start_timestamp;
     const startTimestamp: number = span.start_timestamp;
@@ -783,6 +812,7 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
     const {dividerPosition, addGhostDividerLineRef} = dividerHandlerChildrenProps;
     const {dividerPosition, addGhostDividerLineRef} = dividerHandlerChildrenProps;
     const displaySpanBar = defined(bounds.left) && defined(bounds.width);
     const displaySpanBar = defined(bounds.left) && defined(bounds.width);
     const durationDisplay = getDurationDisplay(bounds);
     const durationDisplay = getDurationDisplay(bounds);
+    const errors = this.getRelatedErrors(quickTrace);
 
 
     return (
     return (
       <RowCellContainer showDetail={this.state.showDetail}>
       <RowCellContainer showDetail={this.state.showDetail}>
@@ -797,9 +827,12 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
             this.toggleDisplayDetail();
             this.toggleDisplayDetail();
           }}
           }}
         >
         >
-          {this.renderTitle(scrollbarManagerChildrenProps)}
+          {this.renderTitle(scrollbarManagerChildrenProps, errors)}
         </RowCell>
         </RowCell>
-        {this.renderDivider(dividerHandlerChildrenProps)}
+        <DividerContainer>
+          {this.renderDivider(dividerHandlerChildrenProps)}
+          {this.renderErrorBadge(errors)}
+        </DividerContainer>
         <RowCell
         <RowCell
           data-type="span-row-cell"
           data-type="span-row-cell"
           showDetail={this.state.showDetail}
           showDetail={this.state.showDetail}
@@ -873,24 +906,29 @@ class SpanBar extends React.Component<SpanBarProps, SpanBarState> {
         showBorder={this.state.showDetail}
         showBorder={this.state.showDetail}
         data-test-id="span-row"
         data-test-id="span-row"
       >
       >
-        <ScrollbarManager.Consumer>
-          {scrollbarManagerChildrenProps => {
-            return (
-              <DividerHandlerManager.Consumer>
-                {(
-                  dividerHandlerChildrenProps: DividerHandlerManager.DividerHandlerManagerChildrenProps
-                ) =>
-                  this.renderHeader({
-                    dividerHandlerChildrenProps,
-                    scrollbarManagerChildrenProps,
-                  })
-                }
-              </DividerHandlerManager.Consumer>
-            );
-          }}
-        </ScrollbarManager.Consumer>
         <QuickTraceContext.Consumer>
         <QuickTraceContext.Consumer>
-          {quickTrace => this.renderDetail({isVisible: isSpanVisible, quickTrace})}
+          {quickTrace => (
+            <React.Fragment>
+              <ScrollbarManager.Consumer>
+                {scrollbarManagerChildrenProps => {
+                  return (
+                    <DividerHandlerManager.Consumer>
+                      {(
+                        dividerHandlerChildrenProps: DividerHandlerManager.DividerHandlerManagerChildrenProps
+                      ) =>
+                        this.renderHeader({
+                          dividerHandlerChildrenProps,
+                          scrollbarManagerChildrenProps,
+                          quickTrace,
+                        })
+                      }
+                    </DividerHandlerManager.Consumer>
+                  );
+                }}
+              </ScrollbarManager.Consumer>
+              {this.renderDetail({isVisible: isSpanVisible, quickTrace})}
+            </React.Fragment>
+          )}
         </QuickTraceContext.Consumer>
         </QuickTraceContext.Consumer>
       </Row>
       </Row>
     );
     );

+ 32 - 0
static/app/components/performance/waterfall/rowDivider.tsx

@@ -1,5 +1,14 @@
+import React from 'react';
 import styled from '@emotion/styled';
 import styled from '@emotion/styled';
 
 
+import {IconFire} from 'app/icons';
+import space from 'app/styles/space';
+
+export const DividerContainer = styled('div')`
+  position: relative;
+  min-width: 1px;
+`;
+
 export const DividerLine = styled('div')<{showDetail?: boolean}>`
 export const DividerLine = styled('div')<{showDetail?: boolean}>`
   background-color: ${p => (p.showDetail ? p.theme.textColor : p.theme.border)};
   background-color: ${p => (p.showDetail ? p.theme.textColor : p.theme.border)};
   position: absolute;
   position: absolute;
@@ -39,3 +48,26 @@ export const DividerLineGhostContainer = styled('div')`
   width: 100%;
   width: 100%;
   height: 100%;
   height: 100%;
 `;
 `;
+
+const BadgeBorder = styled('div')`
+  position: absolute;
+  margin: ${space(0.25)};
+  left: -11px;
+  background: ${p => p.theme.background};
+  width: ${space(3)};
+  height: ${space(3)};
+  border: 1px solid ${p => p.theme.red300};
+  border-radius: 50%;
+  z-index: ${p => p.theme.zIndex.traceView.dividerLine};
+  display: flex;
+  align-items: center;
+  justify-content: center;
+`;
+
+export function ErrorBadge() {
+  return (
+    <BadgeBorder>
+      <IconFire color="red300" size="xs" />
+    </BadgeBorder>
+  );
+}

+ 2 - 2
static/app/components/performance/waterfall/rowTitle.tsx

@@ -23,6 +23,6 @@ export const RowTitle = styled('div')`
   align-items: center;
   align-items: center;
 `;
 `;
 
 
-export const OperationName = styled('span')<{spanErrors: any[]}>`
-  color: ${p => (p.spanErrors.length ? p.theme.error : 'inherit')};
+export const OperationName = styled('span')<{errored: boolean}>`
+  color: ${p => (p.errored ? p.theme.error : 'inherit')};
 `;
 `;

+ 9 - 1
static/app/utils/performance/quickTrace/quickTraceQuery.tsx

@@ -8,6 +8,7 @@ import {QuickTraceQueryChildrenProps} from 'app/utils/performance/quickTrace/typ
 import {
 import {
   flattenRelevantPaths,
   flattenRelevantPaths,
   getTraceTimeRangeFromEvent,
   getTraceTimeRangeFromEvent,
+  isCurrentEvent,
 } from 'app/utils/performance/quickTrace/utils';
 } from 'app/utils/performance/quickTrace/utils';
 
 
 type QueryProps = Omit<DiscoverQueryProps, 'api' | 'eventView'> & {
 type QueryProps = Omit<DiscoverQueryProps, 'api' | 'eventView'> & {
@@ -26,6 +27,7 @@ export default function QuickTraceQuery({children, event, ...props}: QueryProps)
           error: null,
           error: null,
           trace: [],
           trace: [],
           type: 'empty',
           type: 'empty',
+          currentEvent: null,
         })}
         })}
       </React.Fragment>
       </React.Fragment>
     );
     );
@@ -55,6 +57,7 @@ export default function QuickTraceQuery({children, event, ...props}: QueryProps)
                   return children({
                   return children({
                     ...traceFullResults,
                     ...traceFullResults,
                     trace,
                     trace,
+                    currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
                   });
                   });
                 } catch {
                 } catch {
                   // let this fall through and check the next subtrace
                   // let this fall through and check the next subtrace
@@ -68,7 +71,11 @@ export default function QuickTraceQuery({children, event, ...props}: QueryProps)
               traceLiteResults.error === null &&
               traceLiteResults.error === null &&
               traceLiteResults.trace !== null
               traceLiteResults.trace !== null
             ) {
             ) {
-              return children(traceLiteResults);
+              const {trace} = traceLiteResults;
+              return children({
+                ...traceLiteResults,
+                currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
+              });
             }
             }
 
 
             return children({
             return children({
@@ -83,6 +90,7 @@ export default function QuickTraceQuery({children, event, ...props}: QueryProps)
               // that means there were other transactions in the trace, but the current
               // that means there were other transactions in the trace, but the current
               // event could not be found
               // event could not be found
               type: traceFullResults.traces?.length ? 'missing' : 'empty',
               type: traceFullResults.traces?.length ? 'missing' : 'empty',
+              currentEvent: null,
             });
             });
           }}
           }}
         </TraceFullQuery>
         </TraceFullQuery>

+ 4 - 1
static/app/utils/performance/quickTrace/types.tsx

@@ -98,7 +98,10 @@ export type BaseTraceChildrenProps = Omit<
 
 
 export type QuickTrace = EmptyQuickTrace | PartialQuickTrace | FullQuickTrace;
 export type QuickTrace = EmptyQuickTrace | PartialQuickTrace | FullQuickTrace;
 
 
-export type QuickTraceQueryChildrenProps = BaseTraceChildrenProps & QuickTrace;
+export type QuickTraceQueryChildrenProps = BaseTraceChildrenProps &
+  QuickTrace & {
+    currentEvent: QuickTraceEvent | null;
+  };
 
 
 export type TraceMeta = {
 export type TraceMeta = {
   projects: number;
   projects: number;

+ 9 - 1
static/app/utils/performance/quickTrace/utils.tsx

@@ -23,7 +23,7 @@ export function isTransaction(event: Event): event is EventTransaction {
  * An event can be an error or a transaction. We need to check whether the current
  * An event can be an error or a transaction. We need to check whether the current
  * event id is in the list of errors as well
  * event id is in the list of errors as well
  */
  */
-function isCurrentEvent(
+export function isCurrentEvent(
   event: TraceFull | QuickTraceEvent,
   event: TraceFull | QuickTraceEvent,
   currentEvent: Event
   currentEvent: Event
 ): boolean {
 ): boolean {
@@ -298,3 +298,11 @@ export function filterTrace(
     []
     []
   );
   );
 }
 }
+
+export function isTraceFull(transaction): transaction is TraceFull {
+  return Boolean((transaction as TraceFull).event_id);
+}
+
+export function isTraceFullDetailed(transaction): transaction is TraceFullDetailed {
+  return Boolean((transaction as TraceFullDetailed).event_id);
+}

+ 1 - 1
static/app/views/performance/compare/spanBar.tsx

@@ -191,7 +191,7 @@ class SpanBar extends React.Component<Props, State> {
 
 
     const operationName = getSpanOperation(span) ? (
     const operationName = getSpanOperation(span) ? (
       <strong>
       <strong>
-        <OperationName spanErrors={[]}>{getSpanOperation(span)}</OperationName>
+        <OperationName errored={false}>{getSpanOperation(span)}</OperationName>
         {` \u2014 `}
         {` \u2014 `}
       </strong>
       </strong>
     ) : (
     ) : (

+ 0 - 28
static/app/views/performance/traceDetails/styles.tsx

@@ -7,7 +7,6 @@ import {SecondaryHeader} from 'app/components/events/interfaces/spans/header';
 import {Panel} from 'app/components/panels';
 import {Panel} from 'app/components/panels';
 import Pills from 'app/components/pills';
 import Pills from 'app/components/pills';
 import SearchBar from 'app/components/searchBar';
 import SearchBar from 'app/components/searchBar';
-import {IconFire} from 'app/icons';
 import overflowEllipsis from 'app/styles/overflowEllipsis';
 import overflowEllipsis from 'app/styles/overflowEllipsis';
 import space from 'app/styles/space';
 import space from 'app/styles/space';
 import {Organization} from 'app/types';
 import {Organization} from 'app/types';
@@ -69,33 +68,6 @@ export const TransactionBarTitleContent = styled('span')`
   margin-left: ${space(0.75)};
   margin-left: ${space(0.75)};
 `;
 `;
 
 
-export const DividerContainer = styled('div')`
-  position: relative;
-`;
-
-const BadgeBorder = styled('div')<{showDetail: boolean}>`
-  position: absolute;
-  margin: ${space(0.25)};
-  left: -11.5px;
-  background: ${p => (p.showDetail ? p.theme.textColor : p.theme.background)};
-  width: ${space(3)};
-  height: ${space(3)};
-  border: 1px solid ${p => p.theme.red300};
-  border-radius: 50%;
-  z-index: ${p => p.theme.zIndex.traceView.dividerLine};
-  display: flex;
-  align-items: center;
-  justify-content: center;
-`;
-
-export function ErrorBadge({showDetail}: {showDetail: boolean}) {
-  return (
-    <BadgeBorder showDetail={showDetail}>
-      <IconFire color="red300" size="xs" />
-    </BadgeBorder>
-  );
-}
-
 export const ErrorMessageTitle = styled('div')`
 export const ErrorMessageTitle = styled('div')`
   display: flex;
   display: flex;
   justify-content: space-between;
   justify-content: space-between;

+ 7 - 6
static/app/views/performance/traceDetails/transactionBar.tsx

@@ -10,8 +10,10 @@ import {ROW_HEIGHT} from 'app/components/performance/waterfall/constants';
 import {Row, RowCell, RowCellContainer} from 'app/components/performance/waterfall/row';
 import {Row, RowCell, RowCellContainer} from 'app/components/performance/waterfall/row';
 import {DurationPill, RowRectangle} from 'app/components/performance/waterfall/rowBar';
 import {DurationPill, RowRectangle} from 'app/components/performance/waterfall/rowBar';
 import {
 import {
+  DividerContainer,
   DividerLine,
   DividerLine,
   DividerLineGhostContainer,
   DividerLineGhostContainer,
+  ErrorBadge,
 } from 'app/components/performance/waterfall/rowDivider';
 } from 'app/components/performance/waterfall/rowDivider';
 import {
 import {
   OperationName,
   OperationName,
@@ -34,12 +36,12 @@ import {
 import Tooltip from 'app/components/tooltip';
 import Tooltip from 'app/components/tooltip';
 import {Organization} from 'app/types';
 import {Organization} from 'app/types';
 import {TraceFullDetailed} from 'app/utils/performance/quickTrace/types';
 import {TraceFullDetailed} from 'app/utils/performance/quickTrace/types';
+import {isTraceFullDetailed} from 'app/utils/performance/quickTrace/utils';
 import Projects from 'app/utils/projects';
 import Projects from 'app/utils/projects';
 
 
-import {DividerContainer, ErrorBadge, TransactionBarTitleContent} from './styles';
+import {TransactionBarTitleContent} from './styles';
 import TransactionDetail from './transactionDetail';
 import TransactionDetail from './transactionDetail';
 import {TraceInfo, TraceRoot, TreeDepth} from './types';
 import {TraceInfo, TraceRoot, TreeDepth} from './types';
-import {isTraceFullDetailed} from './utils';
 
 
 const MARGIN_LEFT = 0;
 const MARGIN_LEFT = 0;
 
 
@@ -214,7 +216,7 @@ class TransactionBar extends React.Component<Props, State> {
         </Projects>
         </Projects>
         <TransactionBarTitleContent>
         <TransactionBarTitleContent>
           <strong>
           <strong>
-            <OperationName spanErrors={transaction.errors}>
+            <OperationName errored={transaction.errors.length > 0}>
               {transaction['transaction.op']}
               {transaction['transaction.op']}
             </OperationName>
             </OperationName>
             {' \u2014 '}
             {' \u2014 '}
@@ -225,7 +227,7 @@ class TransactionBar extends React.Component<Props, State> {
     ) : (
     ) : (
       <TransactionBarTitleContent>
       <TransactionBarTitleContent>
         <strong>
         <strong>
-          <OperationName spanErrors={[]}>Trace</OperationName>
+          <OperationName errored={false}>Trace</OperationName>
           {' \u2014 '}
           {' \u2014 '}
         </strong>
         </strong>
         {transaction.traceSlug}
         {transaction.traceSlug}
@@ -320,13 +322,12 @@ class TransactionBar extends React.Component<Props, State> {
 
 
   renderErrorBadge() {
   renderErrorBadge() {
     const {transaction} = this.props;
     const {transaction} = this.props;
-    const {showDetail} = this.state;
 
 
     if (!isTraceFullDetailed(transaction) || !transaction.errors.length) {
     if (!isTraceFullDetailed(transaction) || !transaction.errors.length) {
       return null;
       return null;
     }
     }
 
 
-    return <ErrorBadge showDetail={showDetail} />;
+    return <ErrorBadge />;
   }
   }
 
 
   renderRectangle() {
   renderRectangle() {

+ 0 - 4
static/app/views/performance/traceDetails/utils.tsx

@@ -63,10 +63,6 @@ export function getTraceInfo(traces: TraceFullDetailed[]) {
   );
   );
 }
 }
 
 
-export function isTraceFullDetailed(transaction): transaction is TraceFullDetailed {
-  return Boolean((transaction as TraceFullDetailed).event_id);
-}
-
 export function isRootTransaction(trace: TraceFullDetailed): boolean {
 export function isRootTransaction(trace: TraceFullDetailed): boolean {
   // Root transactions has no parent_span_id
   // Root transactions has no parent_span_id
   return trace.parent_span_id === null;
   return trace.parent_span_id === null;