Browse Source

feat(anr): Move thread/thread state to its own section (#46041)

Before:
![Screenshot 2023-03-20 at 12 08 33
PM](https://user-images.githubusercontent.com/63818634/226452318-def7f5fd-9b86-45d3-9603-c499e09e27ea.png)

After:
![Screenshot 2023-03-20 at 12 20 57
PM](https://user-images.githubusercontent.com/63818634/226452305-0424e633-d77f-4288-b132-25580b1041d1.png)
![Screenshot 2023-03-20 at 12 20 13
PM](https://user-images.githubusercontent.com/63818634/226452313-c4f06bdd-efda-4de2-af16-6cbcb0f5df8c.png)
Shruthi 1 year ago
parent
commit
4c487451b0

+ 1 - 0
static/app/components/events/eventEntry.tsx

@@ -133,6 +133,7 @@ function EventEntryContent({
           projectSlug={projectSlug}
           groupingCurrentLevel={groupingCurrentLevel}
           hasHierarchicalGrouping={hasHierarchicalGrouping}
+          organization={organization as Organization}
         />
       ) : (
         <Threads

+ 1 - 44
static/app/components/events/interfaces/threads/index.spec.tsx

@@ -1,4 +1,4 @@
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import {Threads} from 'sentry/components/events/interfaces/threads';
 
@@ -134,47 +134,4 @@ describe('Threads', () => {
       expect(screen.getAllByTestId('line')).toHaveLength(22);
     });
   });
-
-  describe('Thread selector', () => {
-    it('renders thread selector with state', async () => {
-      const threadsEntry = entries[1];
-      render(
-        <Threads
-          data={threadsEntry.data}
-          projectSlug="project-id"
-          event={event}
-          hasHierarchicalGrouping={false}
-        />
-      );
-      expect(
-        screen.getByText('ViewController.captureNSException (RUNNABLE)')
-      ).toBeInTheDocument();
-      await userEvent.click(
-        screen.getByText('ViewController.captureNSException (RUNNABLE)')
-      );
-      expect(screen.getByText('State')).toBeInTheDocument();
-    });
-
-    it('maps android vm states to java vm states', async () => {
-      const threadsEntry = entries[1];
-      threadsEntry.data.values[0].state = 'kWaitingPerformingGc';
-      render(
-        <Threads
-          data={threadsEntry.data}
-          projectSlug="project-id"
-          event={event}
-          hasHierarchicalGrouping={false}
-        />
-      );
-
-      // kWaitingPerformingGc maps to WAITING
-      expect(
-        screen.getByText('ViewController.captureNSException (WAITING)')
-      ).toBeInTheDocument();
-      await userEvent.click(
-        screen.getByText('ViewController.captureNSException (WAITING)')
-      );
-      expect(screen.getByText('State')).toBeInTheDocument();
-    });
-  });
 });

+ 1 - 2
static/app/components/events/interfaces/threads/threadSelector/selectedOption.tsx

@@ -17,8 +17,7 @@ type ThreadInfo = {
 };
 
 function getThreadLabel(details: ThreadInfo) {
-  const threadLabel = details?.label || `<${t('unknown')}>`;
-  return details.state ? `${threadLabel} (${details.state})` : threadLabel;
+  return details?.label || `<${t('unknown')}>`;
 }
 
 const SelectedOption = ({id, details}: Props) => (

+ 31 - 6
static/app/components/events/interfaces/threads/threadSelector/threadStates.tsx

@@ -1,12 +1,13 @@
+import {t} from 'sentry/locale';
 import {defined} from 'sentry/utils';
 
 export enum ThreadStates {
-  RUNNABLE = 'RUNNABLE',
-  TIMED_WAITING = 'TIMED_WAITING',
-  BLOCKED = 'BLOCKED',
-  WAITING = 'WAITING',
-  NEW = 'NEW',
-  TERMINATED = 'TERMINATED',
+  RUNNABLE = 'Runnable',
+  TIMED_WAITING = 'Timed waiting',
+  BLOCKED = 'Blocked',
+  WAITING = 'Waiting',
+  NEW = 'New',
+  TERMINATED = 'Terminated',
 }
 
 type ThreadStatesMap = Record<string, ThreadStates>;
@@ -49,6 +50,30 @@ export const javaThreadStatesMap: ThreadStatesMap = {
   kSuspended: ThreadStates.RUNNABLE, // suspended by GC or debugger
 };
 
+export const THREAD_STATE_TERMS: Record<ThreadStates, string> = {
+  [ThreadStates.RUNNABLE]: t(
+    'A thread executing in the Java virtual machine is in this state.'
+  ),
+  [ThreadStates.WAITING]: t(
+    'A thread that is waiting indefinitely for another thread to perform a particular action is in this state.'
+  ),
+  [ThreadStates.TIMED_WAITING]: t(
+    'A thread that is waiting for another thread to perform an action for up to a specified waiting time is in this state.'
+  ),
+  [ThreadStates.BLOCKED]: t(
+    'A thread that is blocked waiting for a monitor lock is in this state.'
+  ),
+  [ThreadStates.NEW]: t('A thread that has not yet started is in this state.'),
+  [ThreadStates.TERMINATED]: t('A thread that has exited is in this state.'),
+};
+
+export function getThreadStateHelpText(state: keyof typeof THREAD_STATE_TERMS): string {
+  if (!THREAD_STATE_TERMS.hasOwnProperty(state)) {
+    return '';
+  }
+  return THREAD_STATE_TERMS[state];
+}
+
 export function getMappedThreadState(
   state: string | undefined | null
 ): ThreadStates | undefined {

+ 108 - 0
static/app/components/events/interfaces/threadsV2.spec.tsx

@@ -201,6 +201,7 @@ describe('ThreadsV2', function () {
         groupingCurrentLevel: 0,
         hasHierarchicalGrouping: true,
         projectSlug: project.slug,
+        organization,
       };
 
       it('renders', function () {
@@ -628,12 +629,15 @@ describe('ThreadsV2', function () {
                     hasSystemFrames: true,
                   },
                   rawStacktrace: null,
+                  state: 'BLOCKED',
+                  lockReason: 'waiting on tid=1',
                 },
                 {
                   id: 1,
                   current: false,
                   crashed: false,
                   name: null,
+                  state: 'TIMED_WAITING',
                   stacktrace: {
                     frames: [
                       {
@@ -854,6 +858,7 @@ describe('ThreadsV2', function () {
         groupingCurrentLevel: 0,
         hasHierarchicalGrouping: true,
         projectSlug: project.slug,
+        organization,
       };
 
       it('renders', function () {
@@ -866,6 +871,10 @@ describe('ThreadsV2', function () {
         expect(screen.getByRole('radio', {name: 'Full Stack Trace'})).not.toBeChecked();
         expect(screen.getByRole('button', {name: 'Options'})).toBeInTheDocument();
 
+        expect(screen.queryByText('Threads')).not.toBeInTheDocument();
+        expect(screen.queryByText('Thread State')).not.toBeInTheDocument();
+        expect(screen.queryByText('Thread Tags')).not.toBeInTheDocument();
+
         // Stack Trace
         expect(screen.getByRole('heading', {name: 'EXC_BAD_ACCESS'})).toBeInTheDocument();
         expect(
@@ -880,6 +889,105 @@ describe('ThreadsV2', function () {
         expect(container).toSnapshot();
       });
 
+      it('renders thread state and lock reason', function () {
+        const newOrg = {
+          ...organization,
+          features: ['native-stack-trace-v2', 'anr-improvements'],
+        };
+        const newProps = {...props, organization: newOrg};
+        const {container} = render(<ThreadsV2 {...newProps} />, {organization: newOrg});
+        // Title
+        expect(screen.getByTestId('thread-selector')).toBeInTheDocument();
+
+        expect(screen.getByText('Threads')).toBeInTheDocument();
+        expect(screen.getByText('Thread State')).toBeInTheDocument();
+        expect(screen.getByText('Blocked')).toBeInTheDocument();
+        expect(screen.getByText('waiting on tid=1')).toBeInTheDocument();
+        expect(screen.getByText('Thread Tags')).toBeInTheDocument();
+
+        // Actions
+        expect(screen.getByRole('radio', {name: 'Full Stack Trace'})).toBeInTheDocument();
+        expect(screen.getByRole('radio', {name: 'Full Stack Trace'})).not.toBeChecked();
+        expect(screen.getByRole('button', {name: 'Options'})).toBeInTheDocument();
+
+        // Stack Trace
+        expect(screen.getByRole('heading', {name: 'EXC_BAD_ACCESS'})).toBeInTheDocument();
+        expect(
+          screen.getByText(
+            'Attempted to dereference null pointer. Originated at or in a subcall of ViewController.causeCrash(Any) -> ()'
+          )
+        ).toBeInTheDocument();
+
+        expect(screen.getByTestId('stack-trace')).toBeInTheDocument();
+        expect(screen.queryAllByTestId('stack-trace-frame')).toHaveLength(3);
+
+        expect(container).toSnapshot();
+      });
+
+      it('maps android vm states to java vm states', function () {
+        const newEvent = {...event};
+        const threadsEntry = newEvent.entries[1].data as React.ComponentProps<
+          typeof ThreadsV2
+        >['data'];
+        const thread = {
+          id: 0,
+          current: false,
+          crashed: true,
+          name: null,
+          stacktrace: {
+            frames: [
+              {
+                filename: null,
+                absPath: null,
+                module: null,
+                package: '/System/Library/Frameworks/UIKit.framework/UIKit',
+                platform: null,
+                instructionAddr: '0x197885c54',
+                symbolAddr: '0x197885bf4',
+                function: '<redacted>',
+                rawFunction: null,
+                symbol: null,
+                context: [],
+                lineNo: null,
+                colNo: null,
+                inApp: false,
+                trust: null,
+                errors: null,
+                vars: null,
+              },
+            ],
+            registers: {},
+            framesOmitted: null,
+            hasSystemFrames: true,
+          },
+          rawStacktrace: null,
+          state: 'kWaitingPerformingGc',
+        };
+        threadsEntry.values = [
+          {
+            ...thread,
+          },
+          {
+            ...thread,
+            id: 1,
+          },
+        ];
+
+        const newOrg = {
+          ...organization,
+          features: ['native-stack-trace-v2', 'anr-improvements'],
+        };
+        const newProps = {...props, event: newEvent, organization: newOrg};
+        render(<ThreadsV2 {...newProps} />, {organization: newOrg});
+        // Title
+        expect(screen.getByTestId('thread-selector')).toBeInTheDocument();
+
+        expect(screen.getByText('Threads')).toBeInTheDocument();
+        expect(screen.getByText('Thread State')).toBeInTheDocument();
+        // kWaitingPerformingGc maps to Waiting
+        expect(screen.getByText('Waiting')).toBeInTheDocument();
+      });
+
       it('toggle full stack trace button', async function () {
         render(<ThreadsV2 {...props} />, {organization: org});
 

+ 199 - 75
static/app/components/events/interfaces/threadsV2.tsx

@@ -1,13 +1,25 @@
 import {Fragment, useState} from 'react';
+import styled from '@emotion/styled';
 import isNil from 'lodash/isNil';
 
+import {EventDataSection} from 'sentry/components/events/eventDataSection';
+import {
+  getMappedThreadState,
+  getThreadStateHelpText,
+  ThreadStates,
+} from 'sentry/components/events/interfaces/threads/threadSelector/threadStates';
 import Pill from 'sentry/components/pill';
 import Pills from 'sentry/components/pills';
+import QuestionTooltip from 'sentry/components/questionTooltip';
+import TextOverflow from 'sentry/components/textOverflow';
+import {IconClock, IconInfo, IconLock, IconPlay, IconTimer} from 'sentry/icons';
 import {t} from 'sentry/locale';
+import {space} from 'sentry/styles/space';
 import {
   EntryType,
   Event,
   Frame,
+  Organization,
   PlatformType,
   Project,
   STACK_TYPE,
@@ -33,6 +45,7 @@ type Props = Pick<ExceptionProps, 'groupingCurrentLevel' | 'hasHierarchicalGroup
     values?: Array<Thread>;
   };
   event: Event;
+  organization: Organization;
   projectSlug: Project['slug'];
 };
 
@@ -55,12 +68,31 @@ function getIntendedStackView(
   return stacktrace?.hasSystemFrames ? STACK_VIEW.APP : STACK_VIEW.FULL;
 }
 
+export function getThreadStateIcon(state: ThreadStates | undefined) {
+  if (isNil(state)) {
+    return null;
+  }
+  switch (state) {
+    case ThreadStates.BLOCKED:
+      return <IconLock isSolid />;
+    case ThreadStates.TIMED_WAITING:
+      return <IconTimer />;
+    case ThreadStates.WAITING:
+      return <IconClock />;
+    case ThreadStates.RUNNABLE:
+      return <IconPlay />;
+    default:
+      return <IconInfo />;
+  }
+}
+
 export function ThreadsV2({
   data,
   event,
   projectSlug,
   hasHierarchicalGrouping,
   groupingCurrentLevel,
+  organization,
 }: Props) {
   const threads = data.values ?? [];
 
@@ -112,7 +144,14 @@ export function ThreadsV2({
   }
 
   function renderPills() {
-    const {id, name, current, crashed} = activeThread ?? {};
+    const {
+      id,
+      name,
+      current,
+      crashed,
+      state: threadState,
+      lockReason,
+    } = activeThread ?? {};
 
     if (isNil(id) || !name) {
       return null;
@@ -128,6 +167,8 @@ export function ThreadsV2({
             {crashed ? t('yes') : t('no')}
           </Pill>
         )}
+        {!isNil(threadState) && <Pill name={t('state')} value={threadState} />}
+        {!isNil(lockReason) && <Pill name={t('lock reason')} value={lockReason} />}
       </Pills>
     );
   }
@@ -199,84 +240,167 @@ export function ThreadsV2({
   }
 
   const platform = getPlatform();
+  const threadStateDisplay = getMappedThreadState(activeThread?.state);
 
   return (
-    <TraceEventDataSection
-      type={EntryType.THREADS}
-      stackType={STACK_TYPE.ORIGINAL}
-      projectSlug={projectSlug}
-      eventId={event.id}
-      recentFirst={isStacktraceNewestFirst()}
-      fullStackTrace={stackView === STACK_VIEW.FULL}
-      title={
-        hasMoreThanOneThread && activeThread ? (
-          <ThreadSelector
-            threads={threads}
-            activeThread={activeThread}
-            event={event}
-            onChange={thread => {
-              setState({
-                ...state,
-                activeThread: thread,
-              });
-            }}
-            exception={exception}
-            fullWidth
-          />
-        ) : (
-          <PermalinkTitle>{t('Stack Trace')}</PermalinkTitle>
-        )
-      }
-      platform={platform}
-      hasMinified={
-        !!exception?.values?.find(value => value.rawStacktrace) ||
-        !!activeThread?.rawStacktrace
-      }
-      hasVerboseFunctionNames={
-        !!exception?.values?.some(
-          value =>
-            !!value.stacktrace?.frames?.some(
-              frame =>
-                !!frame.rawFunction &&
-                !!frame.function &&
-                frame.rawFunction !== frame.function
-            )
-        ) ||
-        !!activeThread?.stacktrace?.frames?.some(
-          frame =>
-            !!frame.rawFunction &&
-            !!frame.function &&
-            frame.rawFunction !== frame.function
-        )
-      }
-      hasAbsoluteFilePaths={
-        !!exception?.values?.some(
-          value => !!value.stacktrace?.frames?.some(frame => !!frame.filename)
-        ) || !!activeThread?.stacktrace?.frames?.some(frame => !!frame.filename)
-      }
-      hasAbsoluteAddresses={
-        !!exception?.values?.some(
-          value => !!value.stacktrace?.frames?.some(frame => !!frame.instructionAddr)
-        ) || !!activeThread?.stacktrace?.frames?.some(frame => !!frame.instructionAddr)
-      }
-      hasAppOnlyFrames={
-        !!exception?.values?.some(
-          value => !!value.stacktrace?.frames?.some(frame => frame.inApp !== true)
-        ) || !!activeThread?.stacktrace?.frames?.some(frame => frame.inApp !== true)
-      }
-      hasNewestFirst={
-        !!exception?.values?.some(value => (value.stacktrace?.frames ?? []).length > 1) ||
-        (activeThread?.stacktrace?.frames ?? []).length > 1
-      }
-      stackTraceNotFound={stackTraceNotFound}
-      wrapTitle={false}
-    >
-      {childrenProps => (
+    <Fragment>
+      {hasMoreThanOneThread && organization.features.includes('anr-improvements') && (
         <Fragment>
-          {renderPills()}
-          {renderContent(childrenProps)}
+          <Grid>
+            <EventDataSection type={EntryType.THREADS} title={t('Threads')}>
+              {activeThread && (
+                <Wrapper>
+                  <ThreadSelector
+                    threads={threads}
+                    activeThread={activeThread}
+                    event={event}
+                    onChange={thread => {
+                      setState({
+                        ...state,
+                        activeThread: thread,
+                      });
+                    }}
+                    exception={exception}
+                  />
+                </Wrapper>
+              )}
+            </EventDataSection>
+            {activeThread && activeThread.state && (
+              <EventDataSection type={EntryType.THREAD_STATE} title={t('Thread State')}>
+                <ThreadStateWrapper>
+                  {getThreadStateIcon(threadStateDisplay)}
+                  <ThreadState>{threadStateDisplay}</ThreadState>
+                  {threadStateDisplay && (
+                    <QuestionTooltip
+                      position="top"
+                      size="xs"
+                      containerDisplayMode="block"
+                      title={getThreadStateHelpText(threadStateDisplay)}
+                    />
+                  )}
+                  {<LockReason>{activeThread?.lockReason}</LockReason>}
+                </ThreadStateWrapper>
+              </EventDataSection>
+            )}
+          </Grid>
+          <EventDataSection type={EntryType.THREAD_TAGS} title={t('Thread Tags')}>
+            {renderPills()}
+          </EventDataSection>
         </Fragment>
       )}
-    </TraceEventDataSection>
+      <TraceEventDataSection
+        type={EntryType.THREADS}
+        stackType={STACK_TYPE.ORIGINAL}
+        projectSlug={projectSlug}
+        eventId={event.id}
+        recentFirst={isStacktraceNewestFirst()}
+        fullStackTrace={stackView === STACK_VIEW.FULL}
+        title={
+          hasMoreThanOneThread &&
+          activeThread &&
+          !organization.features.includes('anr-improvements') ? (
+            <ThreadSelector
+              threads={threads}
+              activeThread={activeThread}
+              event={event}
+              onChange={thread => {
+                setState({
+                  ...state,
+                  activeThread: thread,
+                });
+              }}
+              exception={exception}
+              fullWidth
+            />
+          ) : (
+            <PermalinkTitle>
+              {hasMoreThanOneThread ? t('Thread Stack Trace') : t('Stack Trace')}
+            </PermalinkTitle>
+          )
+        }
+        platform={platform}
+        hasMinified={
+          !!exception?.values?.find(value => value.rawStacktrace) ||
+          !!activeThread?.rawStacktrace
+        }
+        hasVerboseFunctionNames={
+          !!exception?.values?.some(
+            value =>
+              !!value.stacktrace?.frames?.some(
+                frame =>
+                  !!frame.rawFunction &&
+                  !!frame.function &&
+                  frame.rawFunction !== frame.function
+              )
+          ) ||
+          !!activeThread?.stacktrace?.frames?.some(
+            frame =>
+              !!frame.rawFunction &&
+              !!frame.function &&
+              frame.rawFunction !== frame.function
+          )
+        }
+        hasAbsoluteFilePaths={
+          !!exception?.values?.some(
+            value => !!value.stacktrace?.frames?.some(frame => !!frame.filename)
+          ) || !!activeThread?.stacktrace?.frames?.some(frame => !!frame.filename)
+        }
+        hasAbsoluteAddresses={
+          !!exception?.values?.some(
+            value => !!value.stacktrace?.frames?.some(frame => !!frame.instructionAddr)
+          ) || !!activeThread?.stacktrace?.frames?.some(frame => !!frame.instructionAddr)
+        }
+        hasAppOnlyFrames={
+          !!exception?.values?.some(
+            value => !!value.stacktrace?.frames?.some(frame => frame.inApp !== true)
+          ) || !!activeThread?.stacktrace?.frames?.some(frame => frame.inApp !== true)
+        }
+        hasNewestFirst={
+          !!exception?.values?.some(
+            value => (value.stacktrace?.frames ?? []).length > 1
+          ) || (activeThread?.stacktrace?.frames ?? []).length > 1
+        }
+        stackTraceNotFound={stackTraceNotFound}
+        wrapTitle={false}
+      >
+        {childrenProps => (
+          <Fragment>
+            {!organization.features.includes('anr-improvements') && renderPills()}
+            {renderContent(childrenProps)}
+          </Fragment>
+        )}
+      </TraceEventDataSection>
+    </Fragment>
   );
 }
+
+const Grid = styled('div')`
+  display: grid;
+  grid-template-columns: auto 1fr;
+`;
+
+const ThreadStateWrapper = styled('div')`
+  display: flex;
+  position: relative;
+  flex-direction: row;
+  align-items: flex-start;
+  gap: ${space(0.5)};
+`;
+
+const ThreadState = styled(TextOverflow)`
+  max-width: 100%;
+  text-align: left;
+  font-weight: bold;
+`;
+
+const LockReason = styled(TextOverflow)`
+  font-weight: 400;
+  color: ${p => p.theme.gray300};
+`;
+
+const Wrapper = styled('div')`
+  align-items: center;
+  flex-wrap: wrap;
+  flex-grow: 1;
+  justify-content: flex-start;
+`;

+ 3 - 0
static/app/types/event.tsx

@@ -147,6 +147,7 @@ export interface Thread {
   id: number;
   rawStacktrace: RawStacktrace;
   stacktrace: StacktraceType | null;
+  lockReason?: string | null;
   name?: string | null;
   state?: string | null;
 }
@@ -261,6 +262,8 @@ export enum EntryType {
   HPKP = 'hpkp',
   BREADCRUMBS = 'breadcrumbs',
   THREADS = 'threads',
+  THREAD_STATE = 'thread-state',
+  THREAD_TAGS = 'thread-tags',
   DEBUGMETA = 'debugmeta',
   SPANS = 'spans',
   RESOURCES = 'resources',