feat(perf): Improve transaction error alerts (#36436)

Improve the associated error alerts on the transaction details page

If a transaction has associated errors the transaction details page shows alerts
at the top of the page, and in the span tree. This PR improves their display in two ways:

1. Groups the errors by level and span
2. Sets the alert's level to the highest cumulative error level

Before:

- 2 errors in 'ui.load'
- 3 errors in 'fetch'

The alert is red.

After:

- 2 warnings in 'ui.load'
- 2 warnings in 'fetch'
- 1 info in 'fetch'

The alert is yellow, since there's nothing higher than an error.

fixtures/js-stubs/span.js

@@ -0,0 +1,11 @@
+export function Span(params = {}) {
+  return {
+    timestamp: 1657201239.51,
+    start_timestamp: 1657201239.503,
+    op: 'ui.load',
+    span_id: 'a385d9fd52e0c4bc',
+    parent_span_id: 'bdf1a9fae2062311',
+    trace_id: '4d5c2e2102234a7d94102b4f1e41c2bb',
+    ...params,
+  };
+}

fixtures/js-stubs/traceError.js

@@ -0,0 +1,12 @@
+export function TraceError(params = {}) {
+  return {
+    event_id: '08384ee83c9145e79b5f6fbed5c37a51',
+    issue_id: 62,
+    span: 'bdf1a9fae2062311',
+    project_id: 8,
+    project_slug: 'santry',
+    title: 'Error: Something went wrong',
+    level: 'error',
+    ...params,
+  };
+}

fixtures/js-stubs/types.tsx

@@ -118,6 +118,7 @@ type TestStubFixtures = {
   ShortIdQueryResult: OverridableStub;
   SourceMapArchive: OverridableStub;
   SourceMapArtifact: OverridableStub;
+  Span: OverridableStub;
   Subscriptions: OverridableStubList;
   TagValues: OverridableStubList;
   Tags: OverridableStubList;
@@ -127,6 +128,7 @@ type TestStubFixtures = {
   TeamIssuesReviewed: SimpleStub;
   TeamResolutionTime: SimpleStub;
   Tombstones: OverridableStubList;
+  TraceError: OverridableStub;
   UpdateSdkAndEnableIntegrationSuggestion: SimpleStub;
   User: OverridableStub;
   UserDetails: OverridableStub;

static/app/components/events/interfaces/spans/index.tsx

@@ -5,11 +5,9 @@ import {Observer} from 'mobx-react';
 
 import Alert from 'sentry/components/alert';
 import GuideAnchor from 'sentry/components/assistant/guideAnchor';
-import List from 'sentry/components/list';
-import ListItem from 'sentry/components/list/listItem';
 import {Panel} from 'sentry/components/panels';
 import SearchBar from 'sentry/components/searchBar';
-import {t, tct, tn} from 'sentry/locale';
+import {t, tn} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {Organization} from 'sentry/types';
 import {EventTransaction} from 'sentry/types/event';
@@ -21,9 +19,10 @@ import withOrganization from 'sentry/utils/withOrganization';
 
 import * as AnchorLinkManager from './anchorLinkManager';
 import Filter from './filter';
+import TraceErrorList from './traceErrorList';
 import TraceView from './traceView';
 import {FocusedSpanIDMap, ParsedTraceType} from './types';
-import {parseTrace, scrollToSpan} from './utils';
+import {getCumulativeAlertLevelFromErrors, parseTrace, scrollToSpan} from './utils';
 import WaterfallModel from './waterfallModel';
 
 type Props = {
@@ -92,64 +91,25 @@ class SpansInterface extends PureComponent<Props, State> {
             errors.length
           );
 
-    // mapping from span ids to the span op and the number of errors in that span
-    const errorsMap: {
-      [spanId: string]: {errorsCount: number; operation: string};
-    } = {};
-
-    errors.forEach(error => {
-      if (!errorsMap[error.span]) {
-        // first check of the error belongs to the root span
-        if (parsedTrace.rootSpanID === error.span) {
-          errorsMap[error.span] = {
-            operation: parsedTrace.op,
-            errorsCount: 0,
-          };
-        } else {
-          // since it does not belong to the root span, check if it belongs
-          // to one of the other spans in the transaction
-          const span = parsedTrace.spans.find(s => s.span_id === error.span);
-          if (!span?.op) {
-            return;
-          }
-
-          errorsMap[error.span] = {
-            operation: span.op,
-            errorsCount: 0,
-          };
-        }
-      }
-
-      errorsMap[error.span].errorsCount++;
-    });
-
     return (
       <AlertContainer>
-        <Alert type="error">
+        <Alert type={getCumulativeAlertLevelFromErrors(errors)}>
           <ErrorLabel>{label}</ErrorLabel>
+
           <AnchorLinkManager.Consumer>
             {({scrollToHash}) => (
-              <List symbol="bullet">
-                {Object.entries(errorsMap).map(([spanId, {operation, errorsCount}]) => (
-                  <ListItem key={spanId}>
-                    {tct('[errors] [link]', {
-                      errors: tn('%s error in ', '%s errors in ', errorsCount),
-                      link: (
-                        <ErrorLink
-                          onClick={scrollToSpan(
-                            spanId,
-                            scrollToHash,
-                            this.props.location,
-                            this.props.organization
-                          )}
-                        >
-                          {operation}
-                        </ErrorLink>
-                      ),
-                    })}
-                  </ListItem>
-                ))}
-              </List>
+              <TraceErrorList
+                trace={parsedTrace}
+                errors={errors}
+                onClickSpan={(event, spanId) => {
+                  return scrollToSpan(
+                    spanId,
+                    scrollToHash,
+                    this.props.location,
+                    this.props.organization
+                  )(event);
+                }}
+              />
             )}
           </AnchorLinkManager.Consumer>
         </Alert>
@@ -233,13 +193,6 @@ const Container = styled('div')<{hasErrors: boolean}>`
   `}
 `;
 
-const ErrorLink = styled('a')`
-  color: ${p => p.theme.textColor};
-  :hover {
-    color: ${p => p.theme.textColor};
-  }
-`;
-
 const Search = styled('div')`
   display: grid;
   gap: ${space(2)};

static/app/components/events/interfaces/spans/spanDetail.tsx

@@ -48,7 +48,13 @@ import {transactionSummaryRouteWithQuery} from 'sentry/views/performance/transac
 import * as SpanEntryContext from './context';
 import InlineDocs from './inlineDocs';
 import {ParsedTraceType, ProcessedSpanType, rawSpanKeys, RawSpanType} from './types';
-import {getTraceDateTimeRange, isGapSpan, isOrphanSpan, scrollToSpan} from './utils';
+import {
+  getCumulativeAlertLevelFromErrors,
+  getTraceDateTimeRange,
+  isGapSpan,
+  isOrphanSpan,
+  scrollToSpan,
+} from './utils';
 
 const DEFAULT_ERRORS_VISIBLE = 5;
 
@@ -285,11 +291,11 @@ class SpanDetail extends Component<Props, State> {
       : relatedErrors.slice(0, DEFAULT_ERRORS_VISIBLE);
 
     return (
-      <Alert type="error" system>
+      <Alert type={getCumulativeAlertLevelFromErrors(relatedErrors)} system>
         <ErrorMessageTitle>
           {tn(
-            'An error event occurred in this transaction.',
-            '%s error events occurred in this transaction.',
+            'An error event occurred in this span.',
+            '%s error events occurred in this span.',
             relatedErrors.length
           )}
         </ErrorMessageTitle>

static/app/components/events/interfaces/spans/traceErrorList.tsx

@@ -0,0 +1,63 @@
+import {memo} from 'react';
+import styled from '@emotion/styled';
+import flatten from 'lodash/flatten';
+import groupBy from 'lodash/groupBy';
+
+import List from 'sentry/components/list';
+import ListItem from 'sentry/components/list/listItem';
+import {tct, tn} from 'sentry/locale';
+import {TraceError} from 'sentry/utils/performance/quickTrace/types';
+
+import {ParsedTraceType, SpanType} from './types';
+
+interface TraceErrorListProps {
+  errors: TraceError[];
+  onClickSpan: (event: React.MouseEvent, spanId: SpanType['span_id']) => void;
+  trace: ParsedTraceType;
+}
+
+function TraceErrorList({trace, errors, onClickSpan}: TraceErrorListProps) {
+  return (
+    <List symbol="bullet" data-test-id="trace-error-list">
+      {flatten(
+        Object.entries(groupBy(errors, 'span')).map(([spanId, spanErrors]) => {
+          return Object.entries(groupBy(spanErrors, 'level')).map(
+            ([level, spanLevelErrors]) => (
+              <ListItem key={`${spanId}-${level}`}>
+                {tct('[errors] [link]', {
+                  errors: tn(
+                    '%s %s error in ',
+                    '%s %s errors in ',
+                    spanLevelErrors.length,
+                    level === 'error' ? '' : level // Prevents awkward "error errors" copy if the level is "error"
+                  ),
+                  link: (
+                    <ErrorLink onClick={event => onClickSpan(event, spanId)}>
+                      {findSpanById(trace, spanId).op}
+                    </ErrorLink>
+                  ),
+                })}
+              </ListItem>
+            )
+          );
+        })
+      )}
+    </List>
+  );
+}
+
+const ErrorLink = styled('a')`
+  color: ${p => p.theme.textColor};
+  :hover {
+    color: ${p => p.theme.textColor};
+  }
+`;
+
+// Given a span ID, find the associated span. It might be the trace itself
+// (which is technically a type of span) or a specific span associated with
+// the trace
+const findSpanById = (trace: ParsedTraceType, spanId: SpanType['span_id']) => {
+  return trace.spans.find(span => span.span_id === spanId && span?.op) || trace;
+};
+
+export default memo(TraceErrorList);

static/app/components/events/interfaces/spans/utils.tsx

@@ -2,6 +2,7 @@ import {browserHistory} from 'react-router';
 import {Location} from 'history';
 import isNumber from 'lodash/isNumber';
 import isString from 'lodash/isString';
+import maxBy from 'lodash/maxBy';
 import set from 'lodash/set';
 import moment from 'moment';
 
@@ -13,8 +14,10 @@ import {Organization} from 'sentry/types';
 import {EntryType, EventTransaction} from 'sentry/types/event';
 import {assert} from 'sentry/types/utils';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
+import {TraceError} from 'sentry/utils/performance/quickTrace/types';
 import {WEB_VITAL_DETAILS} from 'sentry/utils/performance/vitals/constants';
 import {getPerformanceTransaction} from 'sentry/utils/performanceForSentry';
+import {Theme} from 'sentry/utils/theme';
 
 import {MERGE_LABELS_THRESHOLD_PERCENT} from './constants';
 import {
@@ -852,3 +855,37 @@ export function isSpanIdFocused(spanId: string, focusedSpanIds: FocusedSpanIDMap
     Object.values(focusedSpanIds).some(relatedSpans => relatedSpans.has(spanId))
   );
 }
+
+export function getCumulativeAlertLevelFromErrors(
+  errors?: Pick<TraceError, 'level'>[]
+): keyof Theme['alert'] | undefined {
+  const highestErrorLevel = maxBy(
+    errors || [],
+    error => ERROR_LEVEL_WEIGHTS[error.level]
+  )?.level;
+
+  if (!highestErrorLevel) {
+    return undefined;
+  }
+  return ERROR_LEVEL_TO_ALERT_TYPE[highestErrorLevel];
+}
+
+// Maps the six known error levels to one of three Alert component types
+const ERROR_LEVEL_TO_ALERT_TYPE: Record<TraceError['level'], keyof Theme['alert']> = {
+  fatal: 'error',
+  error: 'error',
+  default: 'error',
+  warning: 'warning',
+  sample: 'info',
+  info: 'info',
+};
+
+// Allows sorting errors according to their level of severity
+const ERROR_LEVEL_WEIGHTS: Record<TraceError['level'], number> = {
+  fatal: 5,
+  error: 4,
+  default: 4,
+  warning: 3,
+  sample: 2,
+  info: 1,
+};

tests/js/spec/components/events/interfaces/spans/traceErrorList.spec.tsx

@@ -0,0 +1,84 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import TraceErrorList from 'sentry/components/events/interfaces/spans/traceErrorList';
+import {parseTrace} from 'sentry/components/events/interfaces/spans/utils';
+
+describe('TraceErrorList', () => {
+  it('aggregates errors by span and level', () => {
+    const event = TestStubs.Event({
+      entries: [
+        {
+          type: 'spans',
+          data: [
+            TestStubs.Span({
+              op: '/api/fetchitems',
+              span_id: '42118aba',
+            }),
+          ],
+        },
+      ],
+    });
+
+    const errors = [
+      TestStubs.TraceError({
+        event_id: '1',
+        span: '42118aba',
+        level: 'warning',
+      }),
+      TestStubs.TraceError({
+        event_id: '2',
+        span: '42118aba',
+        level: 'warning',
+      }),
+      TestStubs.TraceError({
+        event_id: '3',
+        span: '42118aba',
+        level: 'error',
+      }),
+    ];
+
+    render(
+      <TraceErrorList trace={parseTrace(event)} errors={errors} onClickSpan={jest.fn()} />
+    );
+
+    const listItems = screen.getAllByRole('listitem');
+    expect(listItems).toHaveLength(2);
+    expect(listItems[0]).toHaveTextContent('2 warning errors in /api/fetchitems');
+    expect(listItems[1]).toHaveTextContent('1 error in /api/fetchitems');
+  });
+
+  it('groups span-less errors under the transaction', () => {
+    const event = TestStubs.Event({
+      contexts: {
+        trace: {
+          op: '/path',
+        },
+      },
+      entries: [
+        {
+          type: 'spans',
+          data: [
+            TestStubs.Span({
+              op: '/api/fetchitems',
+              span_id: '42118aba',
+            }),
+          ],
+        },
+      ],
+    });
+
+    const errors = [
+      TestStubs.TraceError({
+        event_id: '1',
+        level: 'warning',
+      }),
+    ];
+
+    render(
+      <TraceErrorList trace={parseTrace(event)} errors={errors} onClickSpan={jest.fn()} />
+    );
+
+    const listItem = screen.getByRole('listitem');
+    expect(listItem).toHaveTextContent('1 warning error in /path');
+  });
+});

tests/js/spec/components/events/interfaces/spans/utils.spec.tsx

@@ -0,0 +1,29 @@
+import {getCumulativeAlertLevelFromErrors} from 'sentry/components/events/interfaces/spans/utils';
+
+describe('getCumulativeAlertLevelFromErrors', () => {
+  it('returns undefined for an empty array', () => {
+    expect(getCumulativeAlertLevelFromErrors([])).toBeUndefined();
+  });
+
+  it('returns the alert level of the first error if only one error is provided', () => {
+    expect(getCumulativeAlertLevelFromErrors([{level: 'error'}])).toBe('error');
+  });
+
+  it('returns the highest alert level for a set of severe errors', () => {
+    expect(getCumulativeAlertLevelFromErrors([{level: 'fatal'}, {level: 'info'}])).toBe(
+      'error'
+    );
+  });
+
+  it('returns the highest alert level for a set of non-severe errors', () => {
+    expect(getCumulativeAlertLevelFromErrors([{level: 'warning'}, {level: 'info'}])).toBe(
+      'warning'
+    );
+  });
+
+  it('returns the highest alert level for a set of info errors', () => {
+    expect(getCumulativeAlertLevelFromErrors([{level: 'info'}, {level: 'info'}])).toBe(
+      'info'
+    );
+  });
+});