Browse Source

feat(performance): add suspect functions to perf transaction summary (#44323)

## Summary 

- Moves profiling `functionsTable` into `suspectFunctionsTable` 
- Adds `suspectFunctionsTable` to Performance > Transaction Summary

~~@olivier-w I've simply added this to the bottom of the page, but its
well below the fold. I'm not sure where we intended on putting it.~~
We've moved the table between Suspect Tags + Related Issues for now.


![dev getsentry
net_7999_organizations_specto-dev_performance_summary__project=6625302
query= referrer=performance-transaction-summary statsPeriod=24h
transaction=timeout_transaction
unselectedSeries=p100%28%29](https://user-images.githubusercontent.com/7349258/218118490-ac9ad476-b491-4bd9-a2d8-7b7a0ebd065c.png)
Elias Hussary 2 years ago
parent
commit
4352ae1345

+ 6 - 1
static/app/components/profiling/functionsTable.spec.tsx → static/app/components/profiling/suspectFunctions/functionsTable.spec.tsx

@@ -2,7 +2,7 @@ import {ReactElement, useEffect} from 'react';
 
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import {FunctionsTable} from 'sentry/components/profiling/functionsTable';
+import {FunctionsTable} from 'sentry/components/profiling/suspectFunctions/functionsTable';
 import ProjectsStore from 'sentry/stores/projectsStore';
 
 const project = TestStubs.Project();
@@ -21,6 +21,7 @@ describe('FunctionsTable', function () {
     render(
       <TestContext>
         <FunctionsTable
+          analyticsPageSource="profiling_transaction"
           isLoading
           error={null}
           functions={[]}
@@ -36,6 +37,7 @@ describe('FunctionsTable', function () {
     render(
       <TestContext>
         <FunctionsTable
+          analyticsPageSource="profiling_transaction"
           isLoading={false}
           error={null}
           functions={[]}
@@ -65,6 +67,7 @@ describe('FunctionsTable', function () {
     render(
       <TestContext>
         <FunctionsTable
+          analyticsPageSource="profiling_transaction"
           isLoading={false}
           error={null}
           functions={[func]}
@@ -107,6 +110,7 @@ describe('FunctionsTable', function () {
     render(
       <TestContext>
         <FunctionsTable
+          analyticsPageSource="profiling_transaction"
           isLoading={false}
           error={null}
           functions={[func]}
@@ -140,6 +144,7 @@ describe('FunctionsTable', function () {
     render(
       <TestContext>
         <FunctionsTable
+          analyticsPageSource="profiling_transaction"
           isLoading={false}
           error={null}
           functions={[func]}

+ 9 - 4
static/app/components/profiling/functionsTable.tsx → static/app/components/profiling/suspectFunctions/functionsTable.tsx

@@ -20,10 +20,11 @@ import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 
 interface FunctionsTableProps {
+  analyticsPageSource: 'performance_transaction' | 'profiling_transaction';
   error: string | null;
   functions: SuspectFunction[];
   isLoading: boolean;
-  project: Project;
+  project: Project | undefined;
   sort: string;
 }
 
@@ -51,6 +52,10 @@ function FunctionsTable(props: FunctionsTableProps) {
   }, [props.sort]);
 
   const functions: TableDataRow[] = useMemo(() => {
+    const project = props.project;
+    if (!project) {
+      return [];
+    }
     return props.functions.map(func => {
       const {worst, examples, ...rest} = func;
 
@@ -66,11 +71,11 @@ function FunctionsTable(props: FunctionsTableProps) {
             onClick: () =>
               trackAdvancedAnalyticsEvent('profiling_views.go_to_flamegraph', {
                 organization,
-                source: 'profiling_transaction.suspect_functions_table',
+                source: `${props.analyticsPageSource}.suspect_functions_table`,
               }),
             target: generateProfileFlamechartRouteWithQuery({
               orgSlug: organization.slug,
-              projectSlug: props.project.slug,
+              projectSlug: project.slug,
               profileId,
               query: {
                 // specify the frame to focus, the flamegraph will switch
@@ -83,7 +88,7 @@ function FunctionsTable(props: FunctionsTableProps) {
         }),
       };
     });
-  }, [organization, props.project.slug, props.functions]);
+  }, [organization, props.project, props.functions, props.analyticsPageSource]);
 
   const generateSortLink = useCallback(
     (column: TableColumnKey) => {

+ 111 - 0
static/app/components/profiling/suspectFunctions/suspectFunctionsTable.tsx

@@ -0,0 +1,111 @@
+import {Fragment, useCallback, useMemo, useState} from 'react';
+import {browserHistory} from 'react-router';
+import styled from '@emotion/styled';
+
+import {CompactSelect} from 'sentry/components/compactSelect';
+import Pagination from 'sentry/components/pagination';
+import {FunctionsTable} from 'sentry/components/profiling/suspectFunctions/functionsTable';
+import {t} from 'sentry/locale';
+import space from 'sentry/styles/space';
+import {Project} from 'sentry/types';
+import {useFunctions} from 'sentry/utils/profiling/hooks/useFunctions';
+import {decodeScalar} from 'sentry/utils/queryString';
+import {useLocation} from 'sentry/utils/useLocation';
+import usePageFilters from 'sentry/utils/usePageFilters';
+
+interface SuspectFunctionsTableProps {
+  analyticsPageSource: 'performance_transaction' | 'profiling_transaction';
+  project: Project | undefined;
+  transaction: string;
+}
+
+const FUNCTIONS_CURSOR_NAME = 'functionsCursor';
+
+export function SuspectFunctionsTable({
+  analyticsPageSource,
+  project,
+  transaction,
+}: SuspectFunctionsTableProps) {
+  const {selection} = usePageFilters();
+  const [functionType, setFunctionType] = useState<'application' | 'system' | 'all'>(
+    'application'
+  );
+  const location = useLocation();
+  const functionsCursor = useMemo(
+    () => decodeScalar(location.query.functionsCursor),
+    [location.query.functionsCursor]
+  );
+  const functionsSort = useMemo(
+    () => decodeScalar(location.query.functionsSort, '-p99'),
+    [location.query.functionsSort]
+  );
+
+  const handleFunctionsCursor = useCallback((cursor, pathname, query) => {
+    browserHistory.push({
+      pathname,
+      query: {...query, [FUNCTIONS_CURSOR_NAME]: cursor},
+    });
+  }, []);
+
+  const functionsQuery = useFunctions({
+    cursor: functionsCursor,
+    project,
+    query: '', // TODO: This doesnt support the same filters
+    selection,
+    transaction,
+    sort: functionsSort,
+    functionType,
+  });
+  return (
+    <Fragment>
+      <TableHeader>
+        <CompactSelect
+          triggerProps={{prefix: t('Suspect Functions'), size: 'xs'}}
+          value={functionType}
+          options={[
+            {
+              label: t('All'),
+              value: 'all' as const,
+            },
+            {
+              label: t('Application'),
+              value: 'application' as const,
+            },
+            {
+              label: t('System'),
+              value: 'system' as const,
+            },
+          ]}
+          onChange={({value}) => setFunctionType(value)}
+        />
+        <StyledPagination
+          pageLinks={
+            functionsQuery.isFetched ? functionsQuery.data?.[0]?.pageLinks : null
+          }
+          onCursor={handleFunctionsCursor}
+          size="xs"
+        />
+      </TableHeader>
+      <FunctionsTable
+        analyticsPageSource={analyticsPageSource}
+        error={functionsQuery.isError ? functionsQuery.error.message : null}
+        isLoading={functionsQuery.isLoading}
+        functions={
+          functionsQuery.isFetched ? functionsQuery.data?.[0].functions ?? [] : []
+        }
+        project={project}
+        sort={functionsSort}
+      />
+    </Fragment>
+  );
+}
+
+const TableHeader = styled('div')`
+  display: flex;
+  justify-content: space-between;
+  margin-bottom: ${space(1)};
+`;
+
+const StyledPagination = styled(Pagination)`
+  margin: 0 0 0 ${space(1)};
+`;

+ 1 - 0
static/app/utils/analytics/profilingAnalyticsEvents.tsx

@@ -13,6 +13,7 @@ type ProfilingEventSource =
   | 'transaction_hovercard.suspect_function'
   | 'events.profile_event_context'
   | 'profiling_transaction.suspect_functions_table'
+  | 'performance_transaction.suspect_functions_table'
   | 'discover.table';
 
 interface EventPayloadWithProjectDetails {

+ 7 - 4
static/app/utils/profiling/hooks/useFunctions.tsx

@@ -16,7 +16,7 @@ type FunctionsResult = {
 };
 
 interface UseFunctionsOptions {
-  project: Project;
+  project: Project | undefined;
   query: string;
   sort: string;
   transaction: string | null;
@@ -39,7 +39,7 @@ function useFunctions({
   const api = useApi();
   const organization = useOrganization();
 
-  const path = `/projects/${organization.slug}/${project.slug}/profiling/functions/`;
+  const path = `/projects/${organization.slug}/${project?.slug}/profiling/functions/`;
   const fetchFunctionsOptions = {
     functionType,
     query,
@@ -54,9 +54,12 @@ function useFunctions({
   const queryFn = () => {
     if (
       !defined(fetchFunctionsOptions.selection) ||
-      !defined(fetchFunctionsOptions.transaction)
+      !defined(fetchFunctionsOptions.transaction) ||
+      !defined(project)
     ) {
-      throw Error('selection and transaction arguments required for fetchFunctions');
+      throw Error(
+        'selection, transaction and project arguments required for fetchFunctions'
+      );
     }
 
     return fetchFunctions(

+ 4 - 0
static/app/views/performance/transactionSummary/transactionOverview/content.spec.tsx

@@ -129,6 +129,10 @@ describe('Transaction Summary Content', function () {
         },
       ],
     });
+    MockApiClient.addMockResponse({
+      url: `/projects/org-slug/project-slug/profiling/functions/`,
+      body: {functions: []},
+    });
   });
 
   afterEach(function () {

+ 7 - 0
static/app/views/performance/transactionSummary/transactionOverview/content.tsx

@@ -13,6 +13,7 @@ import SearchBar from 'sentry/components/events/searchBar';
 import * as Layout from 'sentry/components/layouts/thirds';
 import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
+import {SuspectFunctionsTable} from 'sentry/components/profiling/suspectFunctions/suspectFunctionsTable';
 import Tooltip from 'sentry/components/tooltip';
 import {MAX_QUERY_LENGTH} from 'sentry/constants';
 import {IconWarning} from 'sentry/icons';
@@ -403,6 +404,12 @@ function SummaryContent({
           transactionName={transactionName}
           currentFilter={spanOperationBreakdownFilter}
         />
+
+        <SuspectFunctionsTable
+          project={project}
+          transaction={transactionName}
+          analyticsPageSource="performance_transaction"
+        />
         <RelatedIssues
           organization={organization}
           location={location}

+ 4 - 0
static/app/views/performance/transactionSummary/transactionOverview/index.spec.tsx

@@ -436,6 +436,10 @@ describe('Performance > TransactionSummary', function () {
         },
       ],
     });
+    MockApiClient.addMockResponse({
+      url: `/projects/org-slug/project-slug/profiling/functions/`,
+      body: {functions: []},
+    });
 
     jest.spyOn(MEPSetting, 'get').mockImplementation(() => MEPState.auto);
   });

+ 5 - 71
static/app/views/profiling/profileSummary/content.tsx

@@ -1,4 +1,4 @@
-import {useCallback, useMemo, useState} from 'react';
+import {useCallback, useMemo} from 'react';
 import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
 import {Location} from 'history';
@@ -6,13 +6,12 @@ import {Location} from 'history';
 import {CompactSelect} from 'sentry/components/compactSelect';
 import * as Layout from 'sentry/components/layouts/thirds';
 import Pagination from 'sentry/components/pagination';
-import {FunctionsTable} from 'sentry/components/profiling/functionsTable';
 import {ProfileEventsTable} from 'sentry/components/profiling/profileEventsTable';
+import {SuspectFunctionsTable} from 'sentry/components/profiling/suspectFunctions/suspectFunctionsTable';
 import {mobile} from 'sentry/data/platformCategories';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {PageFilters, Project} from 'sentry/types';
-import {useFunctions} from 'sentry/utils/profiling/hooks/useFunctions';
 import {
   formatSort,
   useProfileEvents,
@@ -20,8 +19,6 @@ import {
 import {decodeScalar} from 'sentry/utils/queryString';
 import {ProfileCharts} from 'sentry/views/profiling/landing/profileCharts';
 
-const FUNCTIONS_CURSOR_NAME = 'functionsCursor';
-
 interface ProfileSummaryContentProps {
   location: Location;
   project: Project;
@@ -41,16 +38,6 @@ function ProfileSummaryContent(props: ProfileSummaryContentProps) {
     [props.location.query.cursor]
   );
 
-  const functionsCursor = useMemo(
-    () => decodeScalar(props.location.query.functionsCursor),
-    [props.location.query.functionsCursor]
-  );
-
-  const functionsSort = useMemo(
-    () => decodeScalar(props.location.query.functionsSort, '-p99'),
-    [props.location.query.functionsSort]
-  );
-
   const sort = formatSort<ProfilingFieldType>(
     decodeScalar(props.location.query.sort),
     fields,
@@ -69,27 +56,6 @@ function ProfileSummaryContent(props: ProfileSummaryContentProps) {
     referrer: 'api.profiling.profile-summary-table',
   });
 
-  const [functionType, setFunctionType] = useState<'application' | 'system' | 'all'>(
-    'application'
-  );
-
-  const functionsQuery = useFunctions({
-    cursor: functionsCursor,
-    project: props.project,
-    query: '', // TODO: This doesnt support the same filters
-    selection: props.selection,
-    transaction: props.transaction,
-    sort: functionsSort,
-    functionType,
-  });
-
-  const handleFunctionsCursor = useCallback((cursor, pathname, query) => {
-    browserHistory.push({
-      pathname,
-      query: {...query, [FUNCTIONS_CURSOR_NAME]: cursor},
-    });
-  }, []);
-
   const handleFilterChange = useCallback(
     value => {
       browserHistory.push({
@@ -126,42 +92,10 @@ function ProfileSummaryContent(props: ProfileSummaryContentProps) {
         isLoading={profiles.status === 'loading'}
         sort={sort}
       />
-      <TableHeader>
-        <CompactSelect
-          triggerProps={{prefix: t('Suspect Functions'), size: 'xs'}}
-          value={functionType}
-          options={[
-            {
-              label: t('All'),
-              value: 'all' as const,
-            },
-            {
-              label: t('Application'),
-              value: 'application' as const,
-            },
-            {
-              label: t('System'),
-              value: 'system' as const,
-            },
-          ]}
-          onChange={({value}) => setFunctionType(value)}
-        />
-        <StyledPagination
-          pageLinks={
-            functionsQuery.isFetched ? functionsQuery.data?.[0]?.pageLinks : null
-          }
-          onCursor={handleFunctionsCursor}
-          size="xs"
-        />
-      </TableHeader>
-      <FunctionsTable
-        error={functionsQuery.isError ? functionsQuery.error.message : null}
-        isLoading={functionsQuery.isLoading}
-        functions={
-          functionsQuery.isFetched ? functionsQuery.data?.[0].functions ?? [] : []
-        }
+      <SuspectFunctionsTable
         project={props.project}
-        sort={functionsSort}
+        transaction={props.transaction}
+        analyticsPageSource="profiling_transaction"
       />
     </Layout.Main>
   );