Просмотр исходного кода

fix(trace): distinguish between different error levels (#68790)

Distinguish between different error levels on the trace view.
Jonas 11 месяцев назад
Родитель
Сommit
f99c90439d

+ 40 - 0
static/app/views/performance/newTraceDetails/icons.tsx

@@ -1,6 +1,13 @@
 // These are the icons used in the TraceDetails component - their SVGs path values have been copied
 // and removed from the emotion wrapper which is parsing and compiling unnecessary CSS as the
 // components rerended, which causes frame drops and performance issues that result in white
+import type {
+  TraceError,
+  TracePerformanceIssue,
+} from 'sentry/utils/performance/quickTrace/types';
+
+import type {TraceTree} from './traceModels/traceTree';
+
 // row flashes when scrolling.
 function Chevron(props: {direction: 'up' | 'down' | 'left'}) {
   return (
@@ -30,8 +37,41 @@ function Profile() {
     </svg>
   );
 }
+
+function Warning() {
+  return (
+    <svg viewBox="0 0 16 16">
+      <path d="M13.87 15.26H2.13A2.1 2.1 0 0 1 0 13.16a2.07 2.07 0 0 1 .27-1L6.17 1.8a2.1 2.1 0 0 1 1.27-1 2.11 2.11 0 0 1 2.39 1l5.87 10.31a2.1 2.1 0 0 1-1.83 3.15ZM8 2.24a.44.44 0 0 0-.16 0 .58.58 0 0 0-.37.28L1.61 12.86a.52.52 0 0 0-.08.3.6.6 0 0 0 .6.6h11.74a.54.54 0 0 0 .3-.08.59.59 0 0 0 .22-.82L8.53 2.54a.61.61 0 0 0-.23-.22.54.54 0 0 0-.3-.08Z" />
+      <path d="M8 10.37a.75.75 0 0 1-.75-.75v-3.7a.75.75 0 0 1 1.5 0v3.7a.74.74 0 0 1-.75.75Z" />
+      <circle cx="8" cy="11.79" r=".76" />
+    </svg>
+  );
+}
+
+interface EventTypeIconProps {
+  event: TracePerformanceIssue | TraceError | TraceTree.Profile;
+}
+
+function EventTypeIcon(props: EventTypeIconProps) {
+  if ('profile_id' in props.event) {
+    return <Profile />;
+  }
+
+  switch (props.event.level) {
+    case 'error':
+    case 'fatal': {
+      return <Fire />;
+    }
+    default: {
+      return <Warning />;
+    }
+  }
+}
+
 export const TraceIcons = {
+  Icon: EventTypeIcon,
   Chevron,
   Fire,
   Profile,
+  Warning,
 };

+ 10 - 0
static/app/views/performance/newTraceDetails/index.tsx

@@ -790,6 +790,16 @@ const TraceInnerLayout = styled('div')`
   flex: 1 1 100%;
   padding: 0 ${space(2)} 0 ${space(2)};
   background-color: ${p => p.theme.background};
+
+  --info: ${p => p.theme.purple400};
+  --warning: ${p => p.theme.yellow300};
+  --error: ${p => p.theme.error};
+  --fatal: ${p => p.theme.error};
+  --default: ${p => p.theme.gray300};
+  --unknown: ${p => p.theme.gray300};
+  --profile: ${p => p.theme.purple300};
+  --autogrouped: ${p => p.theme.blue300};
+  --performance-issue: ${p => p.theme.blue300};
 `;
 
 const TraceToolbar = styled('div')`

+ 96 - 81
static/app/views/performance/newTraceDetails/trace.tsx

@@ -102,6 +102,19 @@ const CHILDREN_COUNT_WRAPPER_ORPHANED_CLASSNAME = [
   'Orphaned',
 ].join(' ');
 
+const ERROR_LEVEL_LABELS: Record<keyof Theme['level'], string> = {
+  sample: t('Sample'),
+  info: t('Info'),
+  warning: t('Warning'),
+  // Hardcoded legacy color (orange400). We no longer use orange anywhere
+  // else in the app (except for the chart palette). This needs to be harcoded
+  // here because existing users may still associate orange with the "error" level.
+  error: t('Error'),
+  fatal: t('Fatal'),
+  default: t('Default'),
+  unknown: t('Unknown'),
+};
+
 function maybeFocusRow(
   ref: HTMLDivElement | null,
   node: TraceTreeNode<TraceTree.NodeValue>,
@@ -564,7 +577,7 @@ function RenderRow(props: {
             : null
         }
         tabIndex={props.tabIndex}
-        className={`Autogrouped TraceRow ${rowSearchClassName} ${props.node.has_errors ? 'Errored' : ''}`}
+        className={`Autogrouped TraceRow ${rowSearchClassName} ${props.node.has_errors ? props.node.max_severity : ''}`}
         onClick={e => props.onRowClick(props.node, e, props.index)}
         onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
         style={{
@@ -593,7 +606,6 @@ function RenderRow(props: {
                 status={props.node.fetchStatus}
                 expanded={!props.node.expanded}
                 onClick={e => props.onExpand(e, props.node, !props.node.expanded)}
-                errored={props.node.has_errors}
               >
                 {COUNT_FORMATTER.format(props.node.groupCount)}
               </ChildrenButton>
@@ -645,10 +657,6 @@ function RenderRow(props: {
   }
 
   if (isTransactionNode(props.node)) {
-    const errored =
-      props.node.value.errors.length > 0 ||
-      props.node.value.performance_issues.length > 0;
-
     return (
       <div
         key={props.index}
@@ -658,7 +666,7 @@ function RenderRow(props: {
             : null
         }
         tabIndex={props.tabIndex}
-        className={`TraceRow ${rowSearchClassName} ${errored ? 'Errored' : ''}`}
+        className={`TraceRow ${rowSearchClassName} ${props.node.has_errors ? props.node.max_severity : ''}`}
         onClick={e => props.onRowClick(props.node, e, props.index)}
         onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
         style={{
@@ -704,7 +712,6 @@ function RenderRow(props: {
                       ? props.onZoomIn(e, props.node, !props.node.zoomedIn)
                       : props.onExpand(e, props.node, !props.node.expanded);
                   }}
-                  errored={errored}
                 >
                   {props.node.children.length > 0
                     ? COUNT_FORMATTER.format(props.node.children.length)
@@ -760,7 +767,6 @@ function RenderRow(props: {
   }
 
   if (isSpanNode(props.node)) {
-    const errored = props.node.errors.size > 0 || props.node.performance_issues.size > 0;
     return (
       <div
         key={props.index}
@@ -770,7 +776,7 @@ function RenderRow(props: {
             : null
         }
         tabIndex={props.tabIndex}
-        className={`TraceRow ${rowSearchClassName} ${errored ? 'Errored' : ''}`}
+        className={`TraceRow ${rowSearchClassName} ${props.node.has_errors ? props.node.max_severity : ''}`}
         onClick={e => props.onRowClick(props.node, e, props.index)}
         onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
         style={{
@@ -816,7 +822,6 @@ function RenderRow(props: {
                       ? props.onZoomIn(e, props.node, !props.node.zoomedIn)
                       : props.onExpand(e, props.node, !props.node.expanded)
                   }
-                  errored={errored}
                 >
                   {props.node.children.length > 0
                     ? COUNT_FORMATTER.format(props.node.children.length)
@@ -959,7 +964,7 @@ function RenderRow(props: {
             : null
         }
         tabIndex={props.tabIndex}
-        className={`TraceRow ${rowSearchClassName} ${props.node.has_errors ? 'Errored' : ''}`}
+        className={`TraceRow ${rowSearchClassName} ${props.node.has_errors ? props.node.max_severity : ''}`}
         onClick={e => props.onRowClick(props.node, e, props.index)}
         onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
         style={{
@@ -1044,7 +1049,7 @@ function RenderRow(props: {
             : null
         }
         tabIndex={props.tabIndex}
-        className={`TraceRow ${rowSearchClassName} Errored`}
+        className={`TraceRow ${rowSearchClassName} ${props.node.max_severity}`}
         onClick={e => props.onRowClick(props.node, e, props.index)}
         onKeyDown={event => props.onRowKeyDown(event, props.index, props.node)}
         style={{
@@ -1070,7 +1075,9 @@ function RenderRow(props: {
             <PlatformIcon
               platform={props.projects[props.node.value.project_slug] ?? 'default'}
             />
-            <span className="TraceOperation">{t('Error')}</span>
+            <span className="TraceOperation">
+              {ERROR_LEVEL_LABELS[props.node.value.level ?? 'error']}
+            </span>
             <strong className="TraceEmDash"> — </strong>
             <span className="TraceDescription">{props.node.value.title}</span>
           </div>
@@ -1095,8 +1102,8 @@ function RenderRow(props: {
             virtualizedIndex={virtualized_index}
           >
             {typeof props.node.value.timestamp === 'number' ? (
-              <div className="TraceError">
-                <TraceIcons.Fire />
+              <div className={`TraceIcon ${props.node.value.level}`}>
+                <TraceIcons.Icon event={props.node.value} />
               </div>
             ) : null}
           </InvisibleTraceBar>
@@ -1302,13 +1309,9 @@ function ChildrenButton(props: {
   icon: React.ReactNode;
   onClick: (e: React.MouseEvent) => void;
   status: TraceTreeNode<any>['fetchStatus'] | undefined;
-  errored?: boolean;
 }) {
   return (
-    <button
-      className={`TraceChildrenCount ${props.errored ? 'Errored' : ''}`}
-      onClick={props.onClick}
-    >
+    <button className={`TraceChildrenCount`} onClick={props.onClick}>
       <div className="TraceChildrenCountContent">{props.children}</div>
       <div className="TraceChildrenCountAction">
         {props.icon}
@@ -1374,9 +1377,9 @@ function TraceBar(props: TraceBarProps) {
         ) : null}
         {props.performance_issues.size > 0 ? (
           <PerformanceIssues
+            manager={props.manager}
             node_space={props.node_space}
             performance_issues={props.performance_issues}
-            manager={props.manager}
           />
         ) : null}
       </div>
@@ -1481,8 +1484,8 @@ function PerformanceIssues(props: PerformanceIssuesProps) {
             className="TracePerformanceIssue"
             style={{left: left * 100 + '%', width: width + '%'}}
           >
-            <div className="TraceError" style={{left: 0}}>
-              <TraceIcons.Fire />
+            <div className={`TraceIcon performance_issue`} style={{left: 0}}>
+              <TraceIcons.Icon event={issue} />
             </div>
           </div>
         );
@@ -1523,10 +1526,10 @@ function Errors(props: ErrorsProps) {
         return (
           <div
             key={error.event_id}
-            className="TraceError"
+            className={`TraceIcon ${error.level}`}
             style={{left: left * 100 + '%'}}
           >
-            <TraceIcons.Fire />
+            <TraceIcons.Icon event={error} />
           </div>
         );
       })}
@@ -1561,10 +1564,10 @@ function Profiles(props: ProfilesProps) {
         return (
           <div
             key={profile.profile_id}
-            className="TraceProfile"
+            className="TraceIcon profile"
             style={{left: left * 100 + '%'}}
           >
-            <TraceIcons.Profile />
+            <TraceIcons.Icon event={profile} />
           </div>
         );
       })}
@@ -1906,6 +1909,26 @@ const TraceStylingWrapper = styled('div')`
     --row-background-odd: ${p => p.theme.translucentSurface100};
     --row-background-hover: ${p => p.theme.translucentSurface100};
     --row-background-focused: ${p => p.theme.translucentSurface200};
+    --row-outline: ${p => p.theme.blue300};
+    --row-children-button-border-color: ${p => p.theme.border};
+
+    /* false positive for grid layout */
+    /* stylelint-disable */
+    &.info {
+    }
+    &.warning {
+    }
+    &.error,
+    &.fatal,
+    &.performance_issue {
+      --autogrouped: ${p => p.theme.error};
+      --row-children-button-border-color: ${p => p.theme.error};
+      --row-outline: ${p => p.theme.error};
+    }
+    &.default {
+    }
+    &.unknown {
+    }
 
     &.Hidden {
       position: absolute;
@@ -1921,43 +1944,60 @@ const TraceStylingWrapper = styled('div')`
       }
     }
 
-    .TraceError {
+    .TraceIcon {
       position: absolute;
       top: 50%;
       transform: translate(-50%, -50%) scaleX(var(--inverse-span-scale));
-      background: ${p => p.theme.background};
+      background-color: ${p => p.theme.background};
       width: 18px !important;
       height: 18px !important;
-      background-color: ${p => p.theme.error};
       border-radius: 50%;
       display: flex;
       align-items: center;
       justify-content: center;
 
-      svg {
-        fill: ${p => p.theme.white};
+      &.info {
+        background-color: var(--info);
+      }
+      &.warning {
+        background-color: var(--warning);
+      }
+      &.error,
+      &.fatal {
+        background-color: var(--error);
+      }
+      &.performance_issue {
+        background-color: var(--performance-issue);
+      }
+      &.default {
+        background-color: var(--default);
+      }
+      &.unknown {
+        background-color: var(--unknown);
+      }
+      &.profile {
+        background-color: var(--profile);
       }
-    }
-
-    .TraceProfile {
-      position: absolute;
-      top: 50%;
-      transform: translate(-50%, -50%) scaleX(var(--inverse-span-scale));
-      background: ${p => p.theme.background};
-      width: 18px !important;
-      height: 18px !important;
-      background-color: ${p => p.theme.purple300};
-      border-radius: 50%;
-      display: flex;
-      align-items: center;
-      justify-content: center;
 
       svg {
         width: 12px;
         height: 12px;
-        margin-left: 2px;
         fill: ${p => p.theme.white};
       }
+
+      &.profile svg {
+        margin-left: 2px;
+      }
+
+      &.info,
+      &.warning,
+      &.performance_issue,
+      &.default,
+      &.unknown {
+        svg {
+          transform: translateY(-1px);
+        }
+      }
     }
 
     .TracePerformanceIssue {
@@ -1966,7 +2006,7 @@ const TraceStylingWrapper = styled('div')`
       display: flex;
       align-items: center;
       justify-content: flex-start;
-      background-color: ${p => p.theme.error};
+      background-color: var(--performance-issue);
       height: 16px;
     }
 
@@ -2000,36 +2040,16 @@ const TraceStylingWrapper = styled('div')`
     &:focus,
     &[tabindex='0'] {
       background-color: var(--row-background-focused);
-      box-shadow: inset 0 0 0 1px ${p => p.theme.blue300} !important;
+      box-shadow: inset 0 0 0 1px var(--row-outline) !important;
 
       .TraceLeftColumn {
-        box-shadow: inset 0px 0 0px 1px ${p => p.theme.blue300} !important;
+        box-shadow: inset 0px 0 0px 1px var(--row-outline) !important;
       }
       .TraceRightColumn.Odd {
         background-color: transparent !important;
       }
     }
 
-    &.Errored {
-      color: ${p => p.theme.error};
-
-      .TraceChildrenCount {
-        border: 2px solid ${p => p.theme.error};
-
-        svg {
-          fill: ${p => p.theme.error};
-        }
-      }
-
-      &:focus {
-        box-shadow: inset 0 0 0 1px ${p => p.theme.red300} !important;
-
-        .TraceLeftColumn {
-          box-shadow: inset 0px 0 0px 1px ${p => p.theme.red300} !important;
-        }
-      }
-    }
-
     &.SearchResult {
       background-color: ${p => p.theme.yellow100};
 
@@ -2041,14 +2061,6 @@ const TraceStylingWrapper = styled('div')`
     &.Autogrouped {
       color: ${p => p.theme.blue300};
 
-      &.Errored {
-        color: ${p => p.theme.error};
-
-        .TraceChildrenCount {
-          background-color: ${p => p.theme.error} !important;
-        }
-      }
-
       .TraceDescription {
         font-weight: bold;
       }
@@ -2185,7 +2197,7 @@ const TraceStylingWrapper = styled('div')`
     padding: 0px 4px;
     transition: all 0.15s ease-in-out;
     background: ${p => p.theme.background};
-    border: 2px solid ${p => p.theme.border};
+    border: 2px solid var(--row-children-button-border-color);
     line-height: 0;
     z-index: 1;
     font-size: 10px;
@@ -2238,7 +2250,6 @@ const TraceStylingWrapper = styled('div')`
     svg {
       width: 7px;
       transition: none;
-      fill: ${p => p.theme.textColor};
     }
   }
 
@@ -2254,6 +2265,10 @@ const TraceStylingWrapper = styled('div')`
       transition: none;
     }
 
+    svg {
+      fill: currentColor;
+    }
+
     &.Orphaned {
       .TraceVerticalConnector,
       .TraceVerticalLastChildConnector,

+ 3 - 3
static/app/views/performance/newTraceDetails/traceDrawer/details/error.tsx

@@ -10,11 +10,11 @@ import {
 } from 'sentry/components/groupPreviewTooltip/stackTracePreview';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {generateIssueEventTarget} from 'sentry/components/quickTrace/utils';
-import {IconFire} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import type {EventError} from 'sentry/types';
 import {useApiQuery} from 'sentry/utils/queryClient';
 import {useLocation} from 'sentry/utils/useLocation';
+import {TraceIcons} from 'sentry/views/performance/newTraceDetails/icons';
 import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails';
 import {getTraceTabTitle} from 'sentry/views/performance/newTraceDetails/traceState/traceTabs';
 import {Row, Tags} from 'sentry/views/performance/traceDetails/styles';
@@ -68,10 +68,10 @@ export function ErrorNodeDetails({
           <TraceDrawerComponents.IconBorder
             backgroundColor={makeTraceNodeBarColor(theme, node)}
           >
-            <IconFire size="md" />
+            <TraceIcons.Icon event={node.value} />
           </TraceDrawerComponents.IconBorder>
           <TraceDrawerComponents.TitleText>
-            <div>{t('error')}</div>
+            <div>{node.value.level ?? t('error')}</div>
             <TraceDrawerComponents.TitleOp>
               {' '}
               {node.value.title}

+ 12 - 5
static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx

@@ -176,6 +176,16 @@ function IssueListHeader({node}: {node: TraceTreeNode<TraceTree.NodeValue>}) {
     return {start, end, statsPeriod};
   }, []);
 
+  const [singular, plural] = useMemo((): [string, string] => {
+    const label = [t('Issue'), t('Issues')] as [string, string];
+    for (const event of errors) {
+      if (event.level === 'error' || event.level === 'fatal') {
+        return [t('Error'), t('Errors')];
+      }
+    }
+    return label;
+  }, [errors]);
+
   return (
     <StyledPanelHeader disablePadding>
       <IssueHeading>
@@ -201,7 +211,7 @@ function IssueListHeader({node}: {node: TraceTreeNode<TraceTree.NodeValue>}) {
           : errors.size > 0 && performance_issues.size === 0
             ? tct('[count] [text]', {
                 count: errors.size,
-                text: tn('Error', 'Errors', errors.size),
+                text: errors.size > 1 ? plural : singular,
               })
             : performance_issues.size > 0 && errors.size === 0
               ? tct('[count] [text]', {
@@ -217,7 +227,7 @@ function IssueListHeader({node}: {node: TraceTreeNode<TraceTree.NodeValue>}) {
                   {
                     errors: errors.size,
                     performance_issues: performance_issues.size,
-                    errorsText: tn('Error', 'Errors', errors.size),
+                    errorsText: errors.size > 1 ? plural : singular,
                     performanceIssuesText: tn(
                       'performance issue',
                       'performance issues',
@@ -271,15 +281,12 @@ const UsersHeading = styled(Heading)`
 `;
 
 const StyledPanel = styled(Panel)`
-  margin-bottom: 0;
-  border: 1px solid ${p => p.theme.red200};
   container-type: inline-size;
 `;
 
 const StyledPanelHeader = styled(PanelHeader)`
   padding-top: ${space(1)};
   padding-bottom: ${space(1)};
-  border-bottom: 1px solid ${p => p.theme.red200};
 `;
 
 const StyledLoadingIndicatorWrapper = styled('div')`

+ 26 - 3
static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx

@@ -256,12 +256,19 @@ export function makeTraceNodeBarColor(
   if (isMissingInstrumentationNode(node)) {
     return theme.gray300;
   }
-  if (isTraceErrorNode(node)) {
-    return theme.red300;
-  }
   if (isNoDataNode(node)) {
     return theme.yellow300;
   }
+  if (isTraceErrorNode(node)) {
+    // Theme defines this as orange, yet everywhere in our product we show red for errors
+    if (node.value.level === 'error' || node.value.level === 'fatal') {
+      return theme.red300;
+    }
+    if (node.value.level) {
+      return theme.level[node.value.level] ?? theme.red300;
+    }
+    return theme.red300;
+  }
   return pickBarColor('default');
 }
 
@@ -1553,6 +1560,22 @@ export class TraceTreeNode<T extends TraceTree.NodeValue = TraceTree.NodeValue>
     return this._spanChildren;
   }
 
+  private _max_severity: keyof Theme['level'] | undefined;
+  get max_severity(): keyof Theme['level'] {
+    if (this._max_severity) {
+      return this._max_severity;
+    }
+
+    for (const error of this.errors) {
+      if (error.level === 'error' || error.level === 'fatal') {
+        this._max_severity = error.level;
+        return this.max_severity;
+      }
+    }
+
+    return 'default';
+  }
+
   /**
    * Invalidate the visual data used to render the tree, forcing it
    * to be recalculated on the next render. This is useful when for example

+ 1 - 3
static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx

@@ -484,9 +484,7 @@ export class VirtualizedViewManager {
 
       // Holding shift key allows for horizontal scrolling
       const distance = event.shiftKey ? event.deltaY : event.deltaX;
-
-      // Prevent vertical scroll when holding shiftkey
-      if (event.shiftKey) {
+      if (event.shiftKey || Math.abs(event.deltaX) !== 0) {
         event.preventDefault();
       }