Browse Source

ref(replays): Call useReplaysCount for each list of issues in the app (#41554)

This fixes an N+1 problem where the UI was making a request for each row in the Issue table. Previously it was requesting the count of replays that belong to the specific issue.
Now we're doing one request (per issues table) to get the count of replays for a list of issue ids.


Test Plan:
Issue table still works,
There's 1 request in the network tab, so all the counts pop in at the same time, see the highlighted area in the screen shot:

![Screen Shot 2022-11-17 at 8.27.26 PM.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/ck6V58zD6DexpsEB0MNr/f6330e76-e4aa-4690-a37f-4c52e8e79cf6/Screen%20Shot%202022-11-17%20at%208.27.26%20PM.png)


Follows: https://github.com/getsentry/sentry/pull/41298

Fixes #40730
Ryan Albrecht 2 years ago
parent
commit
55cefbb25f

+ 58 - 68
static/app/components/eventOrGroupExtraDetails.tsx

@@ -12,8 +12,6 @@ import ReplayCount from 'sentry/components/group/issueReplayCount';
 import ProjectBadge from 'sentry/components/idBadge/projectBadge';
 import Link from 'sentry/components/links/link';
 import Placeholder from 'sentry/components/placeholder';
-import ReplayCountContext from 'sentry/components/replays/replayCountContext';
-import useReplaysCount from 'sentry/components/replays/useReplaysCount';
 import {IconChat} from 'sentry/icons';
 import {tct} from 'sentry/locale';
 import space from 'sentry/styles/space';
@@ -57,76 +55,68 @@ function EventOrGroupExtraDetails({
   const showReplayCount =
     organization.features.includes('session-replay-ui') && projectSupportsReplay(project);
 
-  const counts = useReplaysCount({
-    groupIds: id,
-    organization,
-    project,
-  });
-
   return (
-    <ReplayCountContext.Provider value={counts}>
-      <GroupExtra>
-        {inbox && <InboxReason inbox={inbox} showDateAdded={showInboxTime} />}
-        {shortId && (
-          <InboxShortId
-            shortId={shortId}
-            avatar={
-              project && (
-                <ShadowlessProjectBadge project={project} avatarSize={12} hideName />
-              )
+    <GroupExtra>
+      {inbox && <InboxReason inbox={inbox} showDateAdded={showInboxTime} />}
+      {shortId && (
+        <InboxShortId
+          shortId={shortId}
+          avatar={
+            project && (
+              <ShadowlessProjectBadge project={project} avatarSize={12} hideName />
+            )
+          }
+        />
+      )}
+      {isUnhandled && <UnhandledTag />}
+      {!lifetime && !firstSeen && !lastSeen ? (
+        <Placeholder height="14px" width="100px" />
+      ) : (
+        <TimesTag
+          lastSeen={lifetime?.lastSeen || lastSeen}
+          firstSeen={lifetime?.firstSeen || firstSeen}
+        />
+      )}
+      {/* Always display comment count on inbox */}
+      {numComments > 0 && (
+        <CommentsLink to={`${issuesPath}${id}/activity/`} className="comments">
+          <IconChat
+            size="xs"
+            color={
+              subscriptionDetails?.reason === 'mentioned' ? 'successText' : undefined
             }
           />
-        )}
-        {isUnhandled && <UnhandledTag />}
-        {!lifetime && !firstSeen && !lastSeen ? (
-          <Placeholder height="14px" width="100px" />
-        ) : (
-          <TimesTag
-            lastSeen={lifetime?.lastSeen || lastSeen}
-            firstSeen={lifetime?.firstSeen || firstSeen}
-          />
-        )}
-        {/* Always display comment count on inbox */}
-        {numComments > 0 && (
-          <CommentsLink to={`${issuesPath}${id}/activity/`} className="comments">
-            <IconChat
-              size="xs"
-              color={
-                subscriptionDetails?.reason === 'mentioned' ? 'successText' : undefined
-              }
-            />
-            <span>{numComments}</span>
-          </CommentsLink>
-        )}
-        {showReplayCount && <ReplayCount groupId={id} />}
-        {logger && (
-          <LoggerAnnotation>
-            <GlobalSelectionLink
-              to={{
-                pathname: issuesPath,
-                query: {
-                  query: `logger:${logger}`,
-                },
-              }}
-            >
-              {logger}
-            </GlobalSelectionLink>
-          </LoggerAnnotation>
-        )}
-        {annotations?.map((annotation, key) => (
-          <AnnotationNoMargin
-            dangerouslySetInnerHTML={{
-              __html: annotation,
+          <span>{numComments}</span>
+        </CommentsLink>
+      )}
+      {showReplayCount && <ReplayCount groupId={id} />}
+      {logger && (
+        <LoggerAnnotation>
+          <GlobalSelectionLink
+            to={{
+              pathname: issuesPath,
+              query: {
+                query: `logger:${logger}`,
+              },
             }}
-            key={key}
-          />
-        ))}
-
-        {showAssignee && assignedTo && (
-          <div>{tct('Assigned to [name]', {name: assignedTo.name})}</div>
-        )}
-      </GroupExtra>
-    </ReplayCountContext.Provider>
+          >
+            {logger}
+          </GlobalSelectionLink>
+        </LoggerAnnotation>
+      )}
+      {annotations?.map((annotation, key) => (
+        <AnnotationNoMargin
+          dangerouslySetInnerHTML={{
+            __html: annotation,
+          }}
+          key={key}
+        />
+      ))}
+
+      {showAssignee && assignedTo && (
+        <div>{tct('Assigned to [name]', {name: assignedTo.name})}</div>
+      )}
+    </GroupExtra>
   );
 }
 

+ 7 - 2
static/app/components/group/issueReplayCount.tsx

@@ -27,7 +27,7 @@ function IssueReplayCount({groupId}: Props) {
   const countDisplay = count > 50 ? '50+' : count;
 
   return (
-    <Tooltip title={t('This issue has %s replays available to view', countDisplay)}>
+    <StyledTooltip title={t('This issue has %s replays available to view', countDisplay)}>
       <ReplayCountLink
         to={`/organizations/${organization.slug}/issues/${groupId}/replays/`}
         aria-label="replay-count"
@@ -35,10 +35,15 @@ function IssueReplayCount({groupId}: Props) {
         <IconPlay size="xs" />
         {countDisplay}
       </ReplayCountLink>
-    </Tooltip>
+    </StyledTooltip>
   );
 }
 
+const StyledTooltip = styled(Tooltip)`
+  display: flex;
+  align-items: center;
+`;
+
 const ReplayCountLink = styled(Link)`
   display: inline-flex;
   color: ${p => p.theme.gray400};

+ 4 - 3
static/app/components/issues/groupList.tsx

@@ -1,4 +1,4 @@
-import {Component, Fragment} from 'react';
+import {Component} from 'react';
 // eslint-disable-next-line no-restricted-imports
 import {browserHistory, withRouter, WithRouterProps} from 'react-router';
 import isEqual from 'lodash/isEqual';
@@ -12,6 +12,7 @@ import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Pagination from 'sentry/components/pagination';
 import {Panel, PanelBody} from 'sentry/components/panels';
+import IssuesReplayCountProvider from 'sentry/components/replays/issuesReplayCountProvider';
 import {parseSearch, Token} from 'sentry/components/searchSyntax/parser';
 import {treeResultLocator} from 'sentry/components/searchSyntax/utils';
 import StreamGroup, {
@@ -273,7 +274,7 @@ class GroupList extends Component<Props, State> {
         : DEFAULT_STREAM_GROUP_STATS_PERIOD;
 
     return (
-      <Fragment>
+      <IssuesReplayCountProvider groupIds={groups.map(({id}) => id)}>
         <Panel>
           <GroupListHeader withChart={!!withChart} narrowGroups={narrowGroups} />
           <PanelBody>
@@ -304,7 +305,7 @@ class GroupList extends Component<Props, State> {
         {withPagination && (
           <Pagination pageLinks={pageLinks} onCursor={this.handleCursorChange} />
         )}
-      </Fragment>
+      </IssuesReplayCountProvider>
     );
   }
 }

+ 54 - 0
static/app/components/replays/issuesReplayCountProvider.tsx

@@ -0,0 +1,54 @@
+import {ReactNode, useMemo} from 'react';
+import first from 'lodash/first';
+
+import ReplayCountContext from 'sentry/components/replays/replayCountContext';
+import useReplaysCount from 'sentry/components/replays/useReplaysCount';
+import GroupStore from 'sentry/stores/groupStore';
+import type {Group} from 'sentry/types';
+import projectSupportsReplay from 'sentry/utils/replays/projectSupportsReplay';
+import useOrganization from 'sentry/utils/useOrganization';
+
+/**
+ * Component to make it easier to query for replay counts against a list of groups
+ * that exist across many projects.
+ *
+ * Later on when you want to read the fetched data:
+ * ```
+ * import ReplayCountContext from 'sentry/components/replays/replayCountContext';
+ * const count = useContext(ReplayCountContext)[groupId];
+ * ```
+ */
+export default function IssuesReplayCountProvider({
+  children,
+  groupIds,
+}: {
+  children: ReactNode;
+  groupIds: string[];
+}) {
+  const organization = useOrganization();
+
+  // Only ask for the groupIds where the project supports replay.
+  // For projects that don't support replay the count will always be zero.
+  const groups = useMemo(
+    () =>
+      groupIds
+        .map(id => GroupStore.get(id) as Group)
+        .filter(Boolean)
+        .filter(group => projectSupportsReplay(group.project)),
+    [groupIds]
+  );
+
+  // Any project that supports replay will do here.
+  // Project is used to signal if we should/should not do the query at all.
+  const project = first(groups)?.project;
+
+  const counts = useReplaysCount({
+    groupIds,
+    organization,
+    project,
+  });
+
+  return (
+    <ReplayCountContext.Provider value={counts}>{children}</ReplayCountContext.Provider>
+  );
+}

+ 3 - 1
static/app/components/replays/replayCountContext.tsx

@@ -18,4 +18,6 @@ import type useReplaysCount from 'sentry/components/replays/useReplaysCount';
  * const count = useContext(ReplayCountContext)[groupId];
  * ```
  */
-export default createContext<ReturnType<typeof useReplaysCount>>({});
+const ReplayCountContext = createContext<ReturnType<typeof useReplaysCount>>({});
+
+export default ReplayCountContext;

+ 11 - 5
static/app/components/replays/useReplaysCount.tsx

@@ -26,16 +26,19 @@ function useReplaysCount({groupIds, transactionNames, organization, project}: Op
   );
 
   const [condition, fieldName] = useMemo(() => {
-    if (groupIds !== undefined) {
+    if (groupIds === undefined && transactionNames === undefined) {
+      throw new Error('Missing groupId or transactionName in useReplaysCount()');
+    }
+    if (groupIds && groupIds.length) {
       return [`issue.id:[${toArray(groupIds).join(',')}]`, 'issue.id'];
     }
-    if (transactionNames !== undefined) {
+    if (transactionNames && transactionNames.length) {
       return [
         `event.type:transaction transaction:[${toArray(transactionNames).join(',')}]`,
         'transaction',
       ];
     }
-    throw new Error('Missing groupId or transactionName in useReplaysCount()');
+    return [null, null];
   }, [groupIds, transactionNames]);
 
   const eventView = useMemo(
@@ -45,7 +48,7 @@ function useReplaysCount({groupIds, transactionNames, organization, project}: Op
           id: '',
           name: `Errors within replay`,
           version: 2,
-          fields: ['count_unique(replayId)', fieldName],
+          fields: ['count_unique(replayId)', String(fieldName)],
           query: `!replayId:"" ${condition}`,
           projects: [],
         },
@@ -56,6 +59,9 @@ function useReplaysCount({groupIds, transactionNames, organization, project}: Op
 
   const fetchReplayCount = useCallback(async () => {
     try {
+      if (!condition || !fieldName) {
+        return;
+      }
       const [data] = await doDiscoverQuery<TableData>(
         api,
         `/organizations/${organization.slug}/events/`,
@@ -72,7 +78,7 @@ function useReplaysCount({groupIds, transactionNames, organization, project}: Op
     } catch (err) {
       Sentry.captureException(err);
     }
-  }, [api, location, organization.slug, fieldName, eventView]);
+  }, [api, location, organization.slug, condition, fieldName, eventView]);
 
   useEffect(() => {
     const hasSessionReplay =

+ 3 - 3
static/app/views/alerts/rules/issue/previewTable.tsx

@@ -1,4 +1,3 @@
-import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import {indexMembersByProject} from 'sentry/actionCreators/members';
@@ -7,6 +6,7 @@ import GroupListHeader from 'sentry/components/issues/groupListHeader';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Pagination, {CursorHandler} from 'sentry/components/pagination';
 import {Panel, PanelBody} from 'sentry/components/panels';
+import IssuesReplayCountProvider from 'sentry/components/replays/issuesReplayCountProvider';
 import StreamGroup from 'sentry/components/stream/group';
 import {t, tct} from 'sentry/locale';
 import GroupStore from 'sentry/stores/groupStore';
@@ -95,13 +95,13 @@ const PreviewTable = ({
   };
 
   return (
-    <Fragment>
+    <IssuesReplayCountProvider groupIds={previewGroups || []}>
       <Panel>
         <GroupListHeader withChart={false} />
         <PanelBody>{renderBody()}</PanelBody>
       </Panel>
       {renderPagination()}
-    </Fragment>
+    </IssuesReplayCountProvider>
   );
 };
 

+ 12 - 12
static/app/views/issueList/groupListBody.tsx

@@ -2,6 +2,7 @@ import {IndexedMembersByProject} from 'sentry/actionCreators/members';
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {PanelBody} from 'sentry/components/panels';
+import IssuesReplayCountProvider from 'sentry/components/replays/issuesReplayCountProvider';
 import StreamGroup from 'sentry/components/stream/group';
 import GroupStore from 'sentry/stores/groupStore';
 import {Group} from 'sentry/types';
@@ -74,18 +75,17 @@ function GroupListBody({
   }
 
   return (
-    <GroupList
-      {...{
-        groupIds,
-        memberList,
-        query,
-        sort,
-        displayReprocessingLayout,
-        groupStatsPeriod,
-        source: 'group-list',
-        isSavedSearchesOpen,
-      }}
-    />
+    <IssuesReplayCountProvider groupIds={groupIds}>
+      <GroupList
+        groupIds={groupIds}
+        memberList={memberList}
+        query={query}
+        sort={sort}
+        displayReprocessingLayout={displayReprocessingLayout}
+        groupStatsPeriod={groupStatsPeriod}
+        isSavedSearchesOpen={isSavedSearchesOpen}
+      />
+    </IssuesReplayCountProvider>
   );
 }
 

+ 17 - 10
static/app/views/organizationGroupDetails/groupSimilarIssues/similarStackTrace/index.tsx

@@ -10,6 +10,7 @@ import ButtonBar from 'sentry/components/buttonBar';
 import * as Layout from 'sentry/components/layouts/thirds';
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
+import IssuesReplayCountProvider from 'sentry/components/replays/issuesReplayCountProvider';
 import {t} from 'sentry/locale';
 import GroupingStore, {SimilarItem} from 'sentry/stores/groupingStore';
 import space from 'sentry/styles/space';
@@ -176,6 +177,10 @@ class SimilarStackTrace extends Component<Props, State> {
       (similarItems.length > 0 || filteredSimilarItems.length > 0) &&
       isLoadedSuccessfully;
 
+    const groupsIds = similarItems
+      .concat(filteredSimilarItems)
+      .map(({issue}) => issue.id);
+
     return (
       <Layout.Body>
         <Layout.Main fullWidth>
@@ -205,16 +210,18 @@ class SimilarStackTrace extends Component<Props, State> {
             />
           )}
           {hasSimilarItems && (
-            <List
-              items={similarItems}
-              filteredItems={filteredSimilarItems}
-              onMerge={this.handleMerge}
-              orgId={orgId}
-              project={project}
-              groupId={groupId}
-              pageLinks={similarLinks}
-              v2={v2}
-            />
+            <IssuesReplayCountProvider groupIds={groupsIds}>
+              <List
+                items={similarItems}
+                filteredItems={filteredSimilarItems}
+                onMerge={this.handleMerge}
+                orgId={orgId}
+                project={project}
+                groupId={groupId}
+                pageLinks={similarLinks}
+                v2={v2}
+              />
+            </IssuesReplayCountProvider>
           )}
         </Layout.Main>
       </Layout.Body>

+ 5 - 3
static/app/views/organizationGroupDetails/groupSimilarIssues/similarTraceID/list.tsx

@@ -1,4 +1,4 @@
-import {Component, Fragment} from 'react';
+import {Component} from 'react';
 import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
@@ -13,6 +13,7 @@ import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Pagination, {CursorHandler} from 'sentry/components/pagination';
 import {Panel, PanelBody} from 'sentry/components/panels';
+import IssuesReplayCountProvider from 'sentry/components/replays/issuesReplayCountProvider';
 import StreamGroup from 'sentry/components/stream/group';
 import {URL_PARAM} from 'sentry/constants/pageFilters';
 import {tct} from 'sentry/locale';
@@ -172,14 +173,15 @@ class List extends Component<Props, State> {
       );
     }
 
+    const groupIds = this.props.issues.map(({id}) => id);
     return (
-      <Fragment>
+      <IssuesReplayCountProvider groupIds={groupIds}>
         <StyledPanel>
           <GroupListHeader withChart={false} />
           <PanelBody>{this.renderContent()}</PanelBody>
         </StyledPanel>
         <StyledPagination pageLinks={pageLinks} onCursor={this.handleCursorChange} />
-      </Fragment>
+      </IssuesReplayCountProvider>
     );
   }
 }

Some files were not shown because too many files changed in this diff