Browse Source

feat(feedback): Buffer requests for the /replay-count/ endpoint (#61431)

This adds a new aggregate-with-a-buffer `useQuery` strategy so that we
can collect (from all over the react tree) all the feedback id which are
shown on the screen, and minimize the number of calls to the
`/replay-count/` endpoint. If an id has been fetched already, then the
result is cached and not fetched again.
There's a short `setTimeout` call to allow for collecting id's that are
rendered "at the same time". Making this longer means that we'll capture
more ids "from the same time", especially on slower machines where the
js execution might be slower. But a longer timeout is in tension with
how fast the UI can respond, it means we'll wait longer before getting
data. It's set to `20ms` which is just over 1 frame if we have a 60fps
screen refresh rate (so just over 1 render pass).

Before, we were making one call to the endpoint for every feedback
displayed, which could be a lot of feedbacks in the case of the list
component.


Fixes https://github.com/getsentry/team-replay/issues/318
Ryan Albrecht 1 year ago
parent
commit
9ca2c7c115

+ 4 - 2
static/app/components/feedback/feedbackItem/feedbackItem.tsx

@@ -18,7 +18,6 @@ import FeedbackViewers from 'sentry/components/feedback/feedbackItem/feedbackVie
 import IssueTrackingSection from 'sentry/components/feedback/feedbackItem/issueTrackingSection';
 import ReplaySection from 'sentry/components/feedback/feedbackItem/replaySection';
 import TagsSection from 'sentry/components/feedback/feedbackItem/tagsSection';
-import useFeedbackHasReplayId from 'sentry/components/feedback/useFeedbackHasReplayId';
 import useMutateFeedback from 'sentry/components/feedback/useMutateFeedback';
 import PanelItem from 'sentry/components/panels/panelItem';
 import {Flex} from 'sentry/components/profiling/flex';
@@ -30,6 +29,7 @@ import {space} from 'sentry/styles/space';
 import type {Event, Group} from 'sentry/types';
 import {GroupStatus} from 'sentry/types';
 import type {FeedbackIssue} from 'sentry/utils/feedback/types';
+import {useReplayCountForFeedbacks} from 'sentry/utils/replayCount/replayCountForFeedbacks';
 import useCopyToClipboard from 'sentry/utils/useCopyToClipboard';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
@@ -42,7 +42,9 @@ interface Props {
 
 export default function FeedbackItem({feedbackItem, eventData, tags}: Props) {
   const organization = useOrganization();
-  const hasReplayId = useFeedbackHasReplayId({feedbackId: feedbackItem.id});
+  const {feedbackHasReplay} = useReplayCountForFeedbacks();
+  const hasReplayId = feedbackHasReplay(feedbackItem.id);
+
   const {markAsRead, resolve} = useMutateFeedback({
     feedbackIds: [feedbackItem.id],
     organization,

+ 3 - 2
static/app/components/feedback/list/feedbackListItem.tsx

@@ -8,7 +8,6 @@ import ProjectAvatar from 'sentry/components/avatar/projectAvatar';
 import Checkbox from 'sentry/components/checkbox';
 import FeedbackItemUsername from 'sentry/components/feedback/feedbackItem/feedbackItemUsername';
 import IssueTrackingSignals from 'sentry/components/feedback/list/issueTrackingSignals';
-import useFeedbackHasReplayId from 'sentry/components/feedback/useFeedbackHasReplayId';
 import InteractionStateLayer from 'sentry/components/interactionStateLayer';
 import Link from 'sentry/components/links/link';
 import {Flex} from 'sentry/components/profiling/flex';
@@ -24,6 +23,7 @@ import {Group} from 'sentry/types/group';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {FeedbackIssue} from 'sentry/utils/feedback/types';
 import {decodeScalar} from 'sentry/utils/queryString';
+import {useReplayCountForFeedbacks} from 'sentry/utils/replayCount/replayCountForFeedbacks';
 import {darkTheme, lightTheme} from 'sentry/utils/theme';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -50,7 +50,8 @@ const FeedbackListItem = forwardRef<HTMLDivElement, Props>(
     const config = useLegacyStore(ConfigStore);
     const organization = useOrganization();
     const isOpen = useIsSelectedFeedback({feedbackItem});
-    const hasReplayId = useFeedbackHasReplayId({feedbackId: feedbackItem.id});
+    const {feedbackHasReplay} = useReplayCountForFeedbacks();
+    const hasReplayId = feedbackHasReplay(feedbackItem.id);
 
     const isCrashReport = feedbackItem.metadata.source === 'crash_report_embed_form';
     const theme = isOpen || config.theme === 'dark' ? darkTheme : lightTheme;

+ 0 - 18
static/app/components/feedback/useFeedbackHasReplayId.ts

@@ -1,18 +0,0 @@
-import useReplaysCount from 'sentry/components/replays/useReplaysCount';
-import {IssueCategory} from 'sentry/types';
-import useOrganization from 'sentry/utils/useOrganization';
-
-interface Props {
-  feedbackId: string;
-}
-
-export default function useFeedbackHasReplayId({feedbackId}: Props) {
-  const organization = useOrganization();
-  const counts = useReplaysCount({
-    organization,
-    issueCategory: IssueCategory.PERFORMANCE, // Feedbacks are in the same dataSource as performance
-    groupIds: [feedbackId],
-  });
-  const hasReplay = Boolean(counts[feedbackId]);
-  return hasReplay;
-}

+ 136 - 0
static/app/utils/api/useAggregatedQueryKeys.tsx

@@ -0,0 +1,136 @@
+import {useCallback, useRef, useState} from 'react';
+
+import {ApiResult} from 'sentry/api';
+import {ApiQueryKey, fetchDataQuery, useQueryClient} from 'sentry/utils/queryClient';
+import useApi from 'sentry/utils/useApi';
+
+const BUFFER_WAIT_MS = 20;
+
+interface Props<QueryKeyAggregate, Data> {
+  /**
+   * Default data that goes into `useState`
+   */
+  defaultData: Data;
+
+  /**
+   * The queryKey reducer/generator.
+   *
+   * Takes the buffered "aggregates" and outputs an ApiQueryKey
+   */
+  genQueryKey: (ids: ReadonlyArray<QueryKeyAggregate>) => ApiQueryKey;
+
+  /**
+   * Data reducer, to integrate new requests with the previous state
+   */
+  reducer: (prev: Data, result: ApiResult<unknown>) => Data;
+
+  /**
+   * Callback should an error happen while fetching or reducing the data
+   */
+  onError?: (error: Error) => void;
+}
+
+/**
+ * A reducer function for similar query calls.
+ *
+ * `useAggregatedQueryKeys` is a reducer for query calls. Instead of individually
+ * fetching a handful of records; you can batch the requests by emitting a new
+ * queryKey.
+ *
+ * EXAMPLE: parallel request like: `GET /api/item/?id=1` & `GET /api/item/?id=2`
+ * would be reduced into a single request `GET /api/item/id=[1,2]`
+ *
+ * This works well with our search endpoints, which have support for querying
+ * multiple ids.
+ *
+ * How it works:
+ * - The hook returns a method called `buffer(aggregates: Array<any>)` and the
+ *   value `data`.
+ * - You will implement the props `getQueryKey(aggregates: Array<any>)` which
+ *   takes the unique list of `aggregates` that have been passed into `buffer()`.
+ *   Your `getQueryKey()` function will be invoked after `buffer()` has stopped
+ *   being called for BUFFER_WAIT_MS, this is a debounce mechanic.
+ * - The new queryKey will be used to fetch some data
+ * - You will implement `reducer(prev: Data, result: ApiResult)` which combines
+ *   `defaultData` with the data that was fetched with the queryKey.
+ */
+export default function useAggregatedQueryKeys<QueryKeyAggregate, Data>({
+  defaultData,
+  genQueryKey,
+  reducer,
+  onError,
+}: Props<QueryKeyAggregate, Data>) {
+  const api = useApi({persistInFlight: true});
+  const queryClient = useQueryClient();
+
+  const buffered = useRef<Set<QueryKeyAggregate>>(new Set());
+  const inFlight = useRef<Set<QueryKeyAggregate>>(new Set());
+  const done = useRef<Set<QueryKeyAggregate>>(new Set());
+  const timer = useRef<null | NodeJS.Timeout>(null);
+
+  const [data, setData] = useState<Data>(defaultData);
+
+  const fetchData = useCallback(async () => {
+    const aggregates = Array.from(buffered.current);
+
+    buffered.current.clear();
+    aggregates.forEach(id => inFlight.current.add(id));
+
+    try {
+      const result = await queryClient.fetchQuery({
+        queryKey: genQueryKey(aggregates),
+        queryFn: fetchDataQuery(api),
+      });
+
+      setData(reducer(data, result));
+
+      aggregates.forEach(id => {
+        inFlight.current.delete(id);
+        done.current.add(id);
+      });
+    } catch (error) {
+      aggregates.forEach(id => {
+        inFlight.current.delete(id);
+        buffered.current.add(id);
+      });
+      onError?.(error);
+    }
+  }, [api, data, genQueryKey, queryClient, reducer, onError]);
+
+  const clearTimer = useCallback(() => {
+    if (timer.current) {
+      clearTimeout(timer.current);
+      timer.current = null;
+    }
+  }, []);
+
+  const setTimer = useCallback(() => {
+    clearTimer();
+    timer.current = setTimeout(() => {
+      fetchData();
+      clearTimer();
+    }, BUFFER_WAIT_MS);
+  }, [clearTimer, fetchData]);
+
+  const buffer = useCallback(
+    (aggregates: ReadonlyArray<QueryKeyAggregate>) => {
+      let needsTimer = false;
+      for (const aggregate of aggregates) {
+        if (
+          !buffered.current.has(aggregate) &&
+          !inFlight.current.has(aggregate) &&
+          !done.current.has(aggregate)
+        ) {
+          buffered.current.add(aggregate);
+          needsTimer = true;
+        }
+      }
+      if (needsTimer) {
+        setTimer();
+      }
+    },
+    [setTimer]
+  );
+
+  return {data, buffer};
+}

+ 164 - 0
static/app/utils/replayCount/replayCountCache.tsx

@@ -0,0 +1,164 @@
+import {createContext, ReactNode, useCallback, useContext, useMemo} from 'react';
+
+import {ApiResult} from 'sentry/api';
+import {Organization} from 'sentry/types';
+import {defined} from 'sentry/utils';
+import useAggregatedQueryKeys from 'sentry/utils/api/useAggregatedQueryKeys';
+import {ApiQueryKey, useQueryClient} from 'sentry/utils/queryClient';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface QueryKeyGenProps {
+  dataSource: string;
+  fieldName: string;
+  organization: Organization;
+  statsPeriod: string;
+}
+
+interface Props {
+  children: ReactNode;
+  queryKeyGenProps: QueryKeyGenProps;
+}
+
+type CountValue = undefined | number;
+type ExistsValue = undefined | boolean;
+type CountState = Record<string, CountValue>;
+type ExistsState = Record<string, ExistsValue>;
+
+interface TContext {
+  getMany: (ids: readonly string[]) => CountState;
+  getOne: (id: string) => CountValue;
+  hasMany: (ids: readonly string[]) => ExistsState;
+  hasOne: (id: string) => ExistsValue;
+}
+
+const DEFAULT_CONTEXT: TContext = {
+  getMany: () => ({}),
+  getOne: () => undefined,
+  hasMany: () => ({}),
+  hasOne: () => undefined,
+};
+
+const ReplayCacheCountContext = createContext<TContext>(DEFAULT_CONTEXT);
+
+function queryKeyGen({
+  dataSource,
+  fieldName,
+  organization,
+  statsPeriod,
+}: QueryKeyGenProps) {
+  return (ids: readonly string[]): ApiQueryKey => [
+    `/organizations/${organization.slug}/replay-count/`,
+    {
+      query: {
+        data_source: dataSource,
+        project: -1,
+        statsPeriod,
+        query: `${fieldName}:[${ids.join(',')}]`,
+      },
+    },
+  ];
+}
+
+function filterKeysInList<V>(obj: Record<string, V>, list: readonly string[]) {
+  return Object.fromEntries(Object.entries(obj).filter(([key, _value]) => key in list));
+}
+
+function boolIfDefined(val: undefined | unknown) {
+  return val === undefined ? undefined : Boolean(val);
+}
+
+function mapToBool<V>(obj: Record<string, V>): Record<string, boolean> {
+  return Object.fromEntries(
+    Object.entries(obj).map(([key, value]) => [key, Boolean(value)])
+  );
+}
+
+function reducer(data: CountState, response: ApiResult) {
+  const [newData] = response;
+  return {...data, ...newData};
+}
+
+function useDefaultData() {
+  const organization = useOrganization();
+  const queryClient = useQueryClient();
+
+  return useMemo(() => {
+    const cache = queryClient.getQueryCache();
+    const queries = cache.findAll({
+      predicate: ({queryKey}) => {
+        const url = queryKey[0];
+        return url === `/organizations/${organization.slug}/replay-count/`;
+      },
+    });
+    const allQueryData = queries
+      .map(({queryKey}) => queryClient.getQueryData<ApiResult>(queryKey))
+      .filter(defined);
+    return allQueryData.reduce(reducer, {});
+  }, [organization, queryClient]);
+}
+
+/**
+ * Base ReactContext for fetching and reducing data from /replay-count/
+ *
+ * Don't use this directly!
+ * Import one of the configured helpers instead:
+ *   - `<ReplayExists>` & `useReplayExists()`
+ *   - `<ReplayCountForIssues>` & `useReplayCountForIssues()`
+ *   - `<ReplayCountForTransactions>` & `useReplayCountForTransactions()`
+ *   - `<ReplayCountForFeedbacks>` & `useReplayCountForFeedbacks()`
+ *
+ * @private
+ */
+export function ReplayCountCache({children, queryKeyGenProps}: Props) {
+  const defaultData = useDefaultData();
+  const cache = useAggregatedQueryKeys<string, CountState>({
+    genQueryKey: queryKeyGen(queryKeyGenProps),
+    reducer,
+    defaultData,
+  });
+
+  const getMany = useCallback(
+    (ids: readonly string[]) => {
+      cache.buffer(ids);
+      return filterKeysInList(cache.data, ids);
+    },
+    [cache]
+  );
+
+  const getOne = useCallback(
+    (id: string) => {
+      cache.buffer([id]);
+      return cache.data[id];
+    },
+    [cache]
+  );
+
+  const hasMany = useCallback(
+    (ids: readonly string[]) => mapToBool(getMany(ids)),
+    [getMany]
+  );
+
+  const hasOne = useCallback((id: string) => boolIfDefined(getOne(id)), [getOne]);
+
+  return (
+    <ReplayCacheCountContext.Provider value={{getOne, getMany, hasOne, hasMany}}>
+      {children}
+    </ReplayCacheCountContext.Provider>
+  );
+}
+
+/**
+ * Base ReactContext for fetching and reducing data from /replay-count/
+ *
+ * Don't use this directly!
+ * Import one of the configured helpers instead:
+ *   - `<ReplayExists>` & `useReplayExists()`
+ *   - `<ReplayCountForIssues>` & `useReplayCountForIssues()`
+ *   - `<ReplayCountForTransactions>` & `useReplayCountForTransactions()`
+ *   - `<ReplayCountForFeedbacks>` & `useReplayCountForFeedbacks()`
+ *
+ * @private
+ */
+export function useReplayCount() {
+  return useContext(ReplayCacheCountContext);
+}

+ 45 - 0
static/app/utils/replayCount/replayCountForFeedbacks.tsx

@@ -0,0 +1,45 @@
+import {ReactNode} from 'react';
+
+import {
+  ReplayCountCache,
+  useReplayCount,
+} from 'sentry/utils/replayCount/replayCountCache';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface Props {
+  children: ReactNode;
+}
+
+/**
+ * React context for whether feedback(s) have a replay assiciated or not.
+ *
+ * You can query & read from the context via `useReplayCountForFeedbacks()`.
+ */
+export function ReplayCountForFeedbacks({children}: Props) {
+  const organization = useOrganization();
+
+  return (
+    <ReplayCountCache
+      queryKeyGenProps={{
+        dataSource: 'search_issues',
+        fieldName: 'issue.id',
+        organization,
+        statsPeriod: '90d',
+      }}
+    >
+      {children}
+    </ReplayCountCache>
+  );
+}
+
+/**
+ * Query results for whether a Feedback has replays associated.
+ */
+export function useReplayCountForFeedbacks() {
+  const {hasOne, hasMany} = useReplayCount();
+
+  return {
+    feedbackHasReplay: hasOne,
+    feedbacksHaveReplay: hasMany,
+  };
+}

+ 47 - 0
static/app/utils/replayCount/replayCountForIssues.tsx

@@ -0,0 +1,47 @@
+import {ReactNode} from 'react';
+
+import {
+  ReplayCountCache,
+  useReplayCount,
+} from 'sentry/utils/replayCount/replayCountCache';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface Props {
+  children: ReactNode;
+}
+
+/**
+ * React context for whether issue(s)/group(s) have a replay assiciated or not.
+ *
+ * You can query & read from the context via `useReplayCountForIssues()`.
+ */
+export function ReplayCountForIssues({children}: Props) {
+  const organization = useOrganization();
+
+  return (
+    <ReplayCountCache
+      queryKeyGenProps={{
+        dataSource: 'discover',
+        fieldName: 'issue.id',
+        organization,
+        statsPeriod: '90d',
+      }}
+    >
+      {children}
+    </ReplayCountCache>
+  );
+}
+
+/**
+ * Query results for whether an Issue/Group has replays associated.
+ */
+export function useReplayCountForIssues() {
+  const {getOne, getMany, hasOne, hasMany} = useReplayCount();
+
+  return {
+    getReplayCountForIssue: getOne,
+    getReplayCountForIssues: getMany,
+    issueHasReplay: hasOne,
+    issuesHaveReplay: hasMany,
+  };
+}

+ 47 - 0
static/app/utils/replayCount/replayCountForTransactions.tsx

@@ -0,0 +1,47 @@
+import {ReactNode} from 'react';
+
+import {
+  ReplayCountCache,
+  useReplayCount,
+} from 'sentry/utils/replayCount/replayCountCache';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface Props {
+  children: ReactNode;
+}
+
+/**
+ * React context for whether transactions(s) have a replay assiciated or not.
+ *
+ * You can query & read from the context via `useReplayCountForTransactions()`.
+ */
+export function ReplayCountForTransactions({children}: Props) {
+  const organization = useOrganization();
+
+  return (
+    <ReplayCountCache
+      queryKeyGenProps={{
+        dataSource: 'discover',
+        fieldName: 'transaction',
+        organization,
+        statsPeriod: '90d',
+      }}
+    >
+      {children}
+    </ReplayCountCache>
+  );
+}
+
+/**
+ * Query results for whether a Transaction has replays associated.
+ */
+export function useReplayCountForTransactions() {
+  const {getOne, getMany, hasOne, hasMany} = useReplayCount();
+
+  return {
+    getReplayCountForTransaction: getOne,
+    getReplayCountForTransactions: getMany,
+    transactionHasReplay: hasOne,
+    transactionsHaveReplay: hasMany,
+  };
+}

+ 45 - 0
static/app/utils/replayCount/replayExists.tsx

@@ -0,0 +1,45 @@
+import {ReactNode} from 'react';
+
+import {
+  ReplayCountCache,
+  useReplayCount,
+} from 'sentry/utils/replayCount/replayCountCache';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface Props {
+  children: ReactNode;
+}
+
+/**
+ * React context for whether replay(s) exist or not.
+ *
+ * You can query & read from the context via `useReplayExists()`.
+ */
+export function ReplayExists({children}: Props) {
+  const organization = useOrganization();
+
+  return (
+    <ReplayCountCache
+      queryKeyGenProps={{
+        dataSource: 'discover',
+        fieldName: 'replay_id',
+        organization,
+        statsPeriod: '90d',
+      }}
+    >
+      {children}
+    </ReplayCountCache>
+  );
+}
+
+/**
+ * Query results for whether a given replayId exists in the database (not deleted, etc)
+ */
+export function useReplayExists() {
+  const {hasOne, hasMany} = useReplayCount();
+
+  return {
+    replayExists: hasOne,
+    replaysExist: hasMany,
+  };
+}

+ 40 - 37
static/app/views/feedback/feedbackListPage.tsx

@@ -19,6 +19,7 @@ import {PageHeadingQuestionTooltip} from 'sentry/components/pageHeadingQuestionT
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
+import {ReplayCountForFeedbacks} from 'sentry/utils/replayCount/replayCountForFeedbacks';
 import useOrganization from 'sentry/utils/useOrganization';
 import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
 
@@ -35,44 +36,46 @@ export default function FeedbackListPage({}: Props) {
     <SentryDocumentTitle title={t('User Feedback')} orgSlug={organization.slug}>
       <FullViewport>
         <FeedbackQueryKeys organization={organization}>
-          <Layout.Header>
-            <Layout.HeaderContent>
-              <Layout.Title>
-                {t('User Feedback')}
-                <PageHeadingQuestionTooltip
-                  title={t(
-                    'The User Feedback Widget allows users to submit feedback quickly and easily any time they encounter something that isn’t working as expected.'
+          <ReplayCountForFeedbacks>
+            <Layout.Header>
+              <Layout.HeaderContent>
+                <Layout.Title>
+                  {t('User Feedback')}
+                  <PageHeadingQuestionTooltip
+                    title={t(
+                      'The User Feedback Widget allows users to submit feedback quickly and easily any time they encounter something that isn’t working as expected.'
+                    )}
+                    docsUrl="https://docs.sentry.io/product/user-feedback/"
+                  />
+                </Layout.Title>
+              </Layout.HeaderContent>
+              <Layout.HeaderActions>
+                <OldFeedbackButton />
+              </Layout.HeaderActions>
+            </Layout.Header>
+            <PageFiltersContainer>
+              <ErrorBoundary>
+                <LayoutGrid>
+                  <FeedbackFilters style={{gridArea: 'filters'}} />
+                  {hasSetupOneFeedback || hasSlug ? (
+                    <Fragment>
+                      <Container style={{gridArea: 'list'}}>
+                        <FeedbackList />
+                      </Container>
+                      <FeedbackSearch style={{gridArea: 'search'}} />
+                      <Container style={{gridArea: 'details'}}>
+                        <FeedbackItemLoader />
+                      </Container>
+                    </Fragment>
+                  ) : (
+                    <SetupContainer>
+                      <FeedbackSetupPanel />
+                    </SetupContainer>
                   )}
-                  docsUrl="https://docs.sentry.io/product/user-feedback/"
-                />
-              </Layout.Title>
-            </Layout.HeaderContent>
-            <Layout.HeaderActions>
-              <OldFeedbackButton />
-            </Layout.HeaderActions>
-          </Layout.Header>
-          <PageFiltersContainer>
-            <ErrorBoundary>
-              <LayoutGrid>
-                <FeedbackFilters style={{gridArea: 'filters'}} />
-                {hasSetupOneFeedback || hasSlug ? (
-                  <Fragment>
-                    <Container style={{gridArea: 'list'}}>
-                      <FeedbackList />
-                    </Container>
-                    <FeedbackSearch style={{gridArea: 'search'}} />
-                    <Container style={{gridArea: 'details'}}>
-                      <FeedbackItemLoader />
-                    </Container>
-                  </Fragment>
-                ) : (
-                  <SetupContainer>
-                    <FeedbackSetupPanel />
-                  </SetupContainer>
-                )}
-              </LayoutGrid>
-            </ErrorBoundary>
-          </PageFiltersContainer>
+                </LayoutGrid>
+              </ErrorBoundary>
+            </PageFiltersContainer>
+          </ReplayCountForFeedbacks>
         </FeedbackQueryKeys>
       </FullViewport>
     </SentryDocumentTitle>