Browse Source

feat(perf-issues): Add resources section (#39018)

Co-authored-by: k-fish <kevan.fisher@sentry.io>
Ash Anand 2 years ago
parent
commit
d4b9cab7a7

+ 45 - 4
static/app/components/events/eventEntries.tsx

@@ -429,6 +429,42 @@ const EventEntries = ({
   );
 };
 
+function injectResourcesEntry(definedEvent: Event) {
+  const entries = definedEvent.entries;
+  let adjustedEntries: Entry[] = [];
+
+  // This check is to ensure we are not injecting multiple Resources entries
+  const resourcesIndex = entries.findIndex(entry => entry.type === EntryType.RESOURCES);
+  if (resourcesIndex === -1) {
+    const spansIndex = entries.findIndex(entry => entry.type === EntryType.SPANS);
+    const breadcrumbsIndex = entries.findIndex(
+      entry => entry.type === EntryType.BREADCRUMBS
+    );
+
+    // We want the Resources section to appear after Breadcrumbs.
+    // If Breadcrumbs are included on this event, we will inject this entry right after it.
+    // Otherwise, we inject it after the Spans entry.
+    const resourcesEntry: Entry = {type: EntryType.RESOURCES, data: null};
+    if (breadcrumbsIndex > -1) {
+      adjustedEntries = [
+        ...entries.slice(0, breadcrumbsIndex + 1),
+        resourcesEntry,
+        ...entries.slice(breadcrumbsIndex + 1, entries.length),
+      ];
+    } else if (spansIndex > -1) {
+      adjustedEntries = [
+        ...entries.slice(0, spansIndex + 1),
+        resourcesEntry,
+        ...entries.slice(spansIndex + 1, entries.length),
+      ];
+    }
+  }
+
+  if (adjustedEntries.length > 0) {
+    definedEvent.entries = adjustedEntries;
+  }
+}
+
 function Entries({
   definedEvent,
   projectSlug,
@@ -440,15 +476,20 @@ function Entries({
   definedEvent: Event;
   projectSlug: string;
 } & Pick<Props, 'group' | 'organization' | 'route' | 'router'>) {
-  const entries = definedEvent.entries;
-
-  if (!Array.isArray(entries)) {
+  if (!Array.isArray(definedEvent.entries)) {
     return null;
   }
 
+  if (
+    group?.issueCategory === IssueCategory.PERFORMANCE &&
+    organization.features?.includes('performance-issues')
+  ) {
+    injectResourcesEntry(definedEvent);
+  }
+
   return (
     <Fragment>
-      {(entries as Array<Entry>).map((entry, entryIdx) => (
+      {(definedEvent.entries as Array<Entry>).map((entry, entryIdx) => (
         <ErrorBoundary
           key={`entry-${entryIdx}`}
           customComponent={

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

@@ -22,6 +22,9 @@ import {
 } from 'sentry/types';
 import {Entry, EntryType, Event, EventTransaction} from 'sentry/types/event';
 
+import {Resources} from './interfaces/performance/resources';
+import {getResourceDescription, getResourceLinks} from './interfaces/performance/utils';
+
 type Props = Pick<React.ComponentProps<typeof Breadcrumbs>, 'route' | 'router'> & {
   entry: Entry;
   event: Event;
@@ -169,6 +172,17 @@ function EventEntry({
           organization={organization as Organization}
         />
       );
+    case EntryType.RESOURCES:
+      if (!group || !group.issueType) {
+        return null;
+      }
+
+      return (
+        <Resources
+          description={getResourceDescription(group.issueType)}
+          links={getResourceLinks(group.issueType)}
+        />
+      );
     default:
       // this should not happen
       /* eslint no-console:0 */

+ 115 - 3
static/app/components/events/groupEventEntries.spec.tsx

@@ -1,13 +1,16 @@
-import {initializeOrg} from 'sentry-test/initializeOrg';
-import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {initializeData} from 'sentry-test/performance/initializePerformanceData';
+import {act, render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
 
 import type {Error} from 'sentry/components/events/errors';
 import EventEntries from 'sentry/components/events/eventEntries';
+import {Group, IssueCategory} from 'sentry/types';
 import {EntryType, Event} from 'sentry/types/event';
 import {OrganizationContext} from 'sentry/views/organizationContext';
 import {RouteContext} from 'sentry/views/routeContext';
 
-const {organization, project, router} = initializeOrg();
+const {organization, project, router} = initializeData({
+  features: ['performance-issues'],
+});
 
 const api = new MockApiClient();
 
@@ -293,4 +296,113 @@ describe('GroupEventEntries', function () {
       });
     });
   });
+  describe('Rendering', function () {
+    it('renders the Resources section for Performance Issues', function () {
+      const group: Group = TestStubs.Group({issueCategory: IssueCategory.PERFORMANCE});
+
+      const newEvent = {
+        ...event,
+        entries: [{type: EntryType.SPANS, data: []}],
+      };
+
+      render(
+        <OrganizationContext.Provider value={organization}>
+          <EventEntries
+            organization={organization}
+            event={newEvent}
+            project={project}
+            location={location}
+            api={api}
+            group={group}
+          />
+        </OrganizationContext.Provider>
+      );
+
+      const resourcesHeadingText = screen.getByRole('heading', {
+        name: /resources and whatever/i,
+      });
+
+      expect(resourcesHeadingText).toBeInTheDocument();
+    });
+
+    it('injects the resources section in the correct spot', function () {
+      const group: Group = TestStubs.Group({issueCategory: IssueCategory.PERFORMANCE});
+      group.issueCategory = IssueCategory.PERFORMANCE;
+      const sampleBreadcrumb = {
+        type: 'default',
+        timestamp: '2022-09-19T19:29:32.261000Z',
+        level: 'info',
+        message: 'span.css-1hs7lfd.e1b8u3ky1 > svg',
+        category: 'ui.click',
+        data: null,
+        event_id: null,
+      };
+
+      const newEvent = {
+        ...event,
+        title: 'test',
+        perfProblem: {parentSpanIds: ['a'], causeSpanIds: ['a'], offenderSpanIds: ['a']},
+        entries: [
+          {type: EntryType.SPANS, data: [{span_id: 'a'}]},
+          {type: EntryType.BREADCRUMBS, data: {values: [sampleBreadcrumb]}},
+          {type: EntryType.REQUEST, data: {}},
+        ],
+      };
+
+      render(
+        <OrganizationContext.Provider value={organization}>
+          <RouteContext.Provider
+            value={{
+              router,
+              location: router.location,
+              params: {},
+              routes: [],
+            }}
+          >
+            <EventEntries
+              organization={organization}
+              event={newEvent}
+              project={project}
+              location={location}
+              api={api}
+              group={group}
+            />
+          </RouteContext.Provider>
+        </OrganizationContext.Provider>
+      );
+
+      const eventEntriesContainer = screen.getByTestId('event-entries-loading-false');
+      const spanEvidenceHeading = within(eventEntriesContainer).getByRole('heading', {
+        name: /span evidence/i,
+      });
+      const breadcrumbsHeading = within(eventEntriesContainer).getByRole('heading', {
+        name: /breadcrumbs/i,
+      });
+      const resourcesHeadingText = screen.getByRole('heading', {
+        name: /resources and whatever/i,
+      });
+
+      expect(spanEvidenceHeading).toBeInTheDocument();
+      expect(breadcrumbsHeading).toBeInTheDocument();
+      expect(resourcesHeadingText).toBeInTheDocument();
+
+      expect(
+        within(eventEntriesContainer.children[0] as HTMLElement).getByRole('heading', {
+          name: /span evidence/i,
+        })
+      ).toBeInTheDocument();
+
+      expect(
+        within(eventEntriesContainer.children[1] as HTMLElement).getByRole('heading', {
+          name: /breadcrumbs/i,
+        })
+      ).toBeInTheDocument();
+
+      expect(
+        within(eventEntriesContainer.children[2] as HTMLElement).getByRole('heading', {
+          name: /resources and whatever/i,
+        })
+      ).toBeInTheDocument();
+    });
+  });
 });

+ 50 - 0
static/app/components/events/interfaces/performance/resources.tsx

@@ -0,0 +1,50 @@
+import styled from '@emotion/styled';
+
+import {IconDocs} from 'sentry/icons';
+import {t} from 'sentry/locale';
+import space from 'sentry/styles/space';
+
+import EventDataSection from '../../eventDataSection';
+
+export type ResourceLink = {
+  link: string;
+  text: string;
+};
+
+type Props = {
+  description: string;
+  links: ResourceLink[];
+};
+
+// This section provides users with resources on how to resolve an issue
+export function Resources(props: Props) {
+  return (
+    <EventDataSection type="resources-and-whatever" title={t('Resources and Whatever')}>
+      {props.description}
+      <LinkSection>
+        {props.links.map(({link, text}) => (
+          <a key={link} href={link} target="_blank" rel="noreferrer">
+            <IconDocs /> {text}
+          </a>
+        ))}
+      </LinkSection>
+    </EventDataSection>
+  );
+}
+
+const LinkSection = styled('div')`
+  display: grid;
+  grid-template-columns: 1fr 1fr;
+  grid-row-gap: ${space(1)};
+
+  margin-top: ${space(2)};
+
+  a {
+    display: flex;
+    align-items: center;
+  }
+
+  svg {
+    margin-right: ${space(1)};
+  }
+`;

+ 42 - 0
static/app/components/events/interfaces/performance/utils.tsx

@@ -0,0 +1,42 @@
+import {t} from 'sentry/locale';
+import {IssueType} from 'sentry/types';
+
+import {ResourceLink} from './resources';
+
+const RESOURCES_DESCRIPTIONS: Record<IssueType, string> = {
+  [IssueType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES]: t(
+    "N+1 queries are extraneous queries (N) caused by a single, initial query (+1). In the Span Evidence above, we've identified the parent span where the extraneous spans are located, the source span where the queries are initialized, and the extraneous spans themselves. To learn more about how to fix N+1 problems, check out these resources:"
+  ),
+  [IssueType.ERROR]: '',
+};
+
+const RESOURCE_LINKS: Record<IssueType, ResourceLink[]> = {
+  [IssueType.PERFORMANCE_N_PLUS_ONE_DB_QUERIES]: [
+    {
+      text: t('Finding and Fixing Django N+1 Problems'),
+      link: 'https://blog.sentry.io/2020/09/14/finding-and-fixing-django-n-1-problems',
+    },
+    {
+      text: t('Rails Guide: Active Record Query Interface'),
+      link: 'https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations',
+    },
+    {
+      text: t('Django select_related and prefetch_related'),
+      link: 'https://betterprogramming.pub/django-select-related-and-prefetch-related-f23043fd635d',
+    },
+    // TODO: Update this when the blogpost has been written
+    // {
+    //   text: t('[Leave empty for future Sentry post]'),
+    //   link: 'https://sentry.io/',
+    // },
+  ],
+  [IssueType.ERROR]: [],
+};
+
+export function getResourceDescription(issueType: IssueType): string {
+  return RESOURCES_DESCRIPTIONS[issueType];
+}
+
+export function getResourceLinks(issueType: IssueType) {
+  return RESOURCE_LINKS[issueType];
+}

+ 9 - 4
static/app/types/event.tsx

@@ -241,8 +241,7 @@ export enum EntryType {
   THREADS = 'threads',
   DEBUGMETA = 'debugmeta',
   SPANS = 'spans',
-  SPANTREE = 'spantree',
-  PERFORMANCE = 'performance',
+  RESOURCES = 'resources',
 }
 
 type EntryDebugMeta = {
@@ -278,7 +277,7 @@ type EntryStacktrace = {
 
 type EntrySpans = {
   data: any;
-  type: EntryType.SPANS; // data is not used
+  type: EntryType.SPANS;
 };
 
 type EntryMessage = {
@@ -323,6 +322,11 @@ type EntryGeneric = {
   type: EntryType.EXPECTCT | EntryType.EXPECTSTAPLE | EntryType.HPKP;
 };
 
+type EntryResources = {
+  data: any; // Data is unused here
+  type: EntryType.RESOURCES;
+};
+
 export type Entry =
   | EntryDebugMeta
   | EntryBreadcrumbs
@@ -334,7 +338,8 @@ export type Entry =
   | EntryRequest
   | EntryTemplate
   | EntryCsp
-  | EntryGeneric;
+  | EntryGeneric
+  | EntryResources;
 
 // Contexts
 type RuntimeContext = {

+ 1 - 1
static/app/types/group.tsx

@@ -49,7 +49,7 @@ export enum IssueCategory {
 
 export enum IssueType {
   ERROR = 'error',
-  PERFORMANCE_N_PLUS_ONE = 'performance_n_plus_one',
+  PERFORMANCE_N_PLUS_ONE_DB_QUERIES = 'performance_n_plus_one_db_queries',
 }
 
 type CapabilityInfo = {