Browse Source

ref(replays): Refactor trace data loading on Replay Details page (#47450)

Incremental loading of trace data.

What this change does is move loading trace data into a react context
provider, so the data can be loaded outside of the Trace tab itself.
This allows us to load the data once per pageload, not every time the
trace tab is opened!

Also, this change iterates over all pages of data to fetch every traceId
that's related to the replay. It could be many ids, and once the loading
process is started it'll eventually complete as long as we stay on the
page. A follow up change could, for example, start loading the trace
data before the trace tab is clicked, so it would render quicker once
the user wants to see it.

Initially I had a loading indicator/counter that would show how many
traces were expected, and how many were loaded already. I pulled this
out
a) Because the PR is slightly simpler without it (still have the
`detailsRequests` state, etc)
b) Turns out that we can load the first page of 50 traces quick enough
to fill up the screen. Then once the screen is filled up it looks ready
to use, and you an actually start clicking into the traces that are
rendered! We could followup with a subtle loader, maybe loading-rows
injected into the table, or a single spinner at the bottom. To be
designed.

Using
https://sentry.dev.getsentry.net:7999/replays/javascript:449205bb6d0b47bfaf1e6407febda9ec/
as an example replay, here are some screen shots of the process:

| Desc | Image |
| --- | --- |
| Details Loading | <img width="1349" alt="loading - with border"
src="https://user-images.githubusercontent.com/187460/232828144-d4d6c2c2-6309-40a9-85db-c8b3f36a22c7.png">
|
| Incremental Loading 1 | <img width="572" alt="loading - partial 1"
src="https://user-images.githubusercontent.com/187460/232828202-1f8a94b5-1f7e-4679-8df7-717a81d23596.png">
|
| Incremental Loading 2 | <img width="571" alt="loading - partial data"
src="https://user-images.githubusercontent.com/187460/232828307-e3446e6b-e2e3-4dd4-816c-57a7232ec051.png">
|
| Loading Complete | <img width="575" alt="loading - all data"
src="https://user-images.githubusercontent.com/187460/232828381-daaffcb5-37a1-4523-aca5-ab0a1f66ea40.png">
|




Fixes https://github.com/getsentry/team-replay/issues/60
Ryan Albrecht 1 year ago
parent
commit
c8b844af07

+ 1 - 4
static/app/views/replays/detail/layout/focusArea.tsx

@@ -26,10 +26,7 @@ function FocusArea({}: Props) {
         />
       );
     case TabKey.trace:
-      if (!replay) {
-        return <Placeholder height="150px" />;
-      }
-      return <Trace organization={organization} replayRecord={replay.getReplay()} />;
+      return <Trace organization={organization} replayRecord={replay?.getReplay()} />;
     case TabKey.issues:
       if (!replay) {
         return <Placeholder height="150px" />;

+ 1 - 1
static/app/views/replays/detail/page.tsx

@@ -19,8 +19,8 @@ import type {ReplayRecord} from 'sentry/views/replays/types';
 type Props = {
   children: ReactNode;
   orgSlug: string;
+  replayRecord: undefined | ReplayRecord;
   crumbs?: Crumb[];
-  replayRecord?: ReplayRecord;
 };
 
 function Page({children, crumbs, orgSlug, replayRecord}: Props) {

+ 2 - 2
static/app/views/replays/detail/trace/index.tsx

@@ -7,7 +7,7 @@ import type {ReplayRecord} from 'sentry/views/replays/types';
 
 type Props = {
   organization: Organization;
-  replayRecord: ReplayRecord;
+  replayRecord: undefined | ReplayRecord;
 };
 
 const features = ['organizations:performance-view'];
@@ -31,7 +31,7 @@ function TraceFeature({organization, replayRecord}: Props) {
       organization={organization}
       renderDisabled={PerfDisabled}
     >
-      <Trace organization={organization} replayRecord={replayRecord} />
+      <Trace replayRecord={replayRecord} />
     </Feature>
   );
 }

+ 250 - 0
static/app/views/replays/detail/trace/replayTransactionContext.tsx

@@ -0,0 +1,250 @@
+import {
+  createContext,
+  ReactNode,
+  useCallback,
+  useContext,
+  useEffect,
+  useMemo,
+  useState,
+} from 'react';
+import {Location} from 'history';
+import sortBy from 'lodash/sortBy';
+
+import {getUtcDateString} from 'sentry/utils/dates';
+import type {TableData} from 'sentry/utils/discover/discoverQuery';
+import EventView from 'sentry/utils/discover/eventView';
+import {doDiscoverQuery} from 'sentry/utils/discover/genericDiscoverQuery';
+import parseLinkHeader, {ParsedHeader} from 'sentry/utils/parseLinkHeader';
+import {TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
+import {
+  getTraceRequestPayload,
+  makeEventView,
+} from 'sentry/utils/performance/quickTrace/utils';
+import useApi from 'sentry/utils/useApi';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import type {ReplayRecord} from 'sentry/views/replays/types';
+
+type Options = {
+  children: ReactNode;
+  replayRecord: undefined | ReplayRecord;
+};
+
+type InternalState = {
+  detailsErrors: Error[];
+  detailsRequests: number;
+  detailsResponses: number;
+  indexComplete: boolean;
+  indexError: undefined | Error;
+  isFetching: boolean;
+  traces: undefined | TraceFullDetailed[];
+};
+
+type ExternalState = {
+  errors: Error[];
+  isFetching: boolean;
+  traces: undefined | TraceFullDetailed[];
+};
+
+const INITIAL_STATE: InternalState = {
+  detailsErrors: [],
+  detailsRequests: 0,
+  detailsResponses: 0,
+  indexComplete: true,
+  indexError: undefined,
+  isFetching: false,
+  traces: undefined,
+};
+
+type TxnContextProps = {
+  eventView: null | EventView;
+  fetchTransactionData: () => void;
+  state: ExternalState;
+};
+
+const TxnContext = createContext<TxnContextProps>({
+  eventView: null,
+  fetchTransactionData: () => {},
+  state: {errors: [], isFetching: false, traces: []},
+});
+
+function ReplayTransactionContext({children, replayRecord}: Options) {
+  const api = useApi();
+  const location = useLocation();
+  const organization = useOrganization();
+
+  const [state, setState] = useState<InternalState>(INITIAL_STATE);
+
+  const orgSlug = organization.slug;
+
+  const listEventView = useMemo(() => {
+    if (!replayRecord) {
+      return null;
+    }
+    const replayId = replayRecord?.id;
+    const projectId = replayRecord?.project_id;
+    const start = getUtcDateString(replayRecord?.started_at.getTime());
+    const end = getUtcDateString(replayRecord?.finished_at.getTime());
+
+    return EventView.fromSavedQuery({
+      id: undefined,
+      name: `Traces in replay ${replayId}`,
+      fields: ['trace', 'count(trace)', 'min(timestamp)'],
+      orderby: 'min_timestamp',
+      query: `replayId:${replayId}`,
+      projects: [Number(projectId)],
+      version: 2,
+      start,
+      end,
+    });
+  }, [replayRecord]);
+
+  const tracePayload = useMemo(() => {
+    const start = getUtcDateString(replayRecord?.started_at.getTime());
+    const end = getUtcDateString(replayRecord?.finished_at.getTime());
+
+    const traceEventView = makeEventView({start, end});
+    return getTraceRequestPayload({eventView: traceEventView, location});
+  }, [replayRecord, location]);
+
+  const fetchSingleTraceData = useCallback(
+    async traceId => {
+      try {
+        const [trace, , _traceResp] = await doDiscoverQuery(
+          api,
+          `/organizations/${orgSlug}/events-trace/${traceId}/`,
+          tracePayload
+        );
+
+        setState(prev => ({
+          ...prev,
+          traces: sortBy(
+            (prev.traces || []).concat(trace as TraceFullDetailed),
+            'start_timestamp'
+          ),
+        }));
+      } catch (error) {
+        setState(prev => ({
+          ...prev,
+          detailsErrors: prev.detailsErrors.concat(error),
+        }));
+      }
+    },
+    [api, orgSlug, tracePayload]
+  );
+
+  const fetchTransactionData = useCallback(async () => {
+    if (!listEventView) {
+      return;
+    }
+    const start = getUtcDateString(replayRecord?.started_at.getTime());
+    const end = getUtcDateString(replayRecord?.finished_at.getTime());
+
+    setState({
+      detailsErrors: [],
+      detailsRequests: 0,
+      detailsResponses: 0,
+      indexComplete: false,
+      indexError: undefined,
+      isFetching: true,
+      traces: [],
+    });
+
+    let cursor = {
+      cursor: '0:0:0',
+      results: true,
+      href: '',
+    } as ParsedHeader;
+    while (cursor.results) {
+      const payload = {
+        ...listEventView.getEventsAPIPayload({
+          start,
+          end,
+          limit: 10,
+        } as unknown as Location),
+        sort: ['min_timestamp', 'trace'],
+        cursor: cursor.cursor,
+      };
+
+      try {
+        const [{data}, , listResp] = await doDiscoverQuery<TableData>(
+          api,
+          `/organizations/${orgSlug}/events/`,
+          payload
+        );
+
+        const traceIds = data.map(({trace}) => String(trace)).filter(Boolean);
+
+        // Do not await results here. Do the fetches async and let the loop continue
+        (async function () {
+          setState(
+            prev =>
+              ({
+                ...prev,
+                detailsRequests: prev.detailsRequests + traceIds.length,
+              } as InternalState)
+          );
+          await Promise.allSettled(traceIds.map(fetchSingleTraceData));
+          setState(
+            prev =>
+              ({
+                ...prev,
+                detailsResponses: prev.detailsResponses + traceIds.length,
+              } as InternalState)
+          );
+        })();
+
+        const pageLinks = listResp?.getResponseHeader('Link') ?? null;
+        cursor = parseLinkHeader(pageLinks)?.next;
+      } catch (indexError) {
+        setState(prev => ({...prev, indexError} as InternalState));
+        cursor = {cursor: '', results: false, href: ''} as ParsedHeader;
+      }
+    }
+
+    setState(prev => ({...prev, indexComplete: true} as InternalState));
+  }, [api, fetchSingleTraceData, listEventView, orgSlug, replayRecord]);
+
+  return (
+    <TxnContext.Provider
+      value={{
+        eventView: listEventView,
+        fetchTransactionData,
+        state: internalToExternalState(state),
+      }}
+    >
+      {children}
+    </TxnContext.Provider>
+  );
+}
+
+function internalToExternalState({
+  detailsErrors,
+  detailsRequests,
+  detailsResponses,
+  indexComplete,
+  indexError,
+  traces,
+}: InternalState): ExternalState {
+  const isComplete = indexComplete && detailsRequests === detailsResponses;
+
+  return {
+    errors: indexError ? [indexError] : detailsErrors,
+    isFetching: !isComplete,
+    traces,
+  };
+}
+
+export default ReplayTransactionContext;
+
+export const useFetchTransactions = () => {
+  const {fetchTransactionData} = useContext(TxnContext);
+
+  useEffect(fetchTransactionData, [fetchTransactionData]);
+};
+
+export const useTransactionData = () => {
+  const {eventView, state} = useContext(TxnContext);
+  const data = useMemo(() => ({eventView, state}), [eventView, state]);
+  return data;
+};

+ 35 - 155
static/app/views/replays/detail/trace/trace.tsx

@@ -1,168 +1,48 @@
-import {useEffect, useState} from 'react';
-import * as Sentry from '@sentry/react';
+import styled from '@emotion/styled';
 
-import EmptyMessage from 'sentry/components/emptyMessage';
-import LoadingError from 'sentry/components/loadingError';
-import LoadingIndicator from 'sentry/components/loadingIndicator';
-import {t} from 'sentry/locale';
-import {Organization} from 'sentry/types';
-import {getUtcDateString} from 'sentry/utils/dates';
-import {TableData} from 'sentry/utils/discover/discoverQuery';
-import EventView from 'sentry/utils/discover/eventView';
-import {doDiscoverQuery} from 'sentry/utils/discover/genericDiscoverQuery';
-import {TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
-import {
-  getTraceRequestPayload,
-  makeEventView,
-} from 'sentry/utils/performance/quickTrace/utils';
-import useApi from 'sentry/utils/useApi';
+import Placeholder from 'sentry/components/placeholder';
 import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
 import TraceView from 'sentry/views/performance/traceDetails/traceView';
-import type {ReplayListLocationQuery, ReplayRecord} from 'sentry/views/replays/types';
+import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
+import {
+  useFetchTransactions,
+  useTransactionData,
+} from 'sentry/views/replays/detail/trace/replayTransactionContext';
+import type {ReplayRecord} from 'sentry/views/replays/types';
 
-type State = {
-  /**
-   * Error, if not null.
-   */
-  // error: QueryError | null;
-  error: any | null;
-  /**
-   * Loading state of this query.
-   */
-  isLoading: boolean;
-  /**
-   * Pagelinks, if applicable. Can be provided to the Pagination component.
-   */
-  pageLinks: string | null;
-  /**
-   * EventView that generates API payload
-   */
-  traceEventView: EventView | null;
-  /**
-   * Data / result.
-   */
-  traces: TraceFullDetailed[] | null;
+type Props = {
+  replayRecord: undefined | ReplayRecord;
 };
 
-interface Props {
-  organization: Organization;
-  replayRecord: ReplayRecord;
-}
-
-const INITIAL_STATE = Object.freeze({
-  error: null,
-  isLoading: true,
-  pageLinks: null,
-  traceEventView: null,
-  traces: null,
-});
-
-export default function Trace({replayRecord, organization}: Props) {
-  const [state, setState] = useState<State>(INITIAL_STATE);
-  const api = useApi();
-  const location = useLocation<ReplayListLocationQuery>();
-
-  const replayId = replayRecord.id;
-  const projectId = replayRecord.project_id;
-  const orgSlug = organization.slug;
-
-  const start = getUtcDateString(replayRecord.started_at.getTime());
-  const end = getUtcDateString(replayRecord.finished_at.getTime());
-
-  useEffect(() => {
-    async function loadTraces() {
-      const eventView = EventView.fromSavedQuery({
-        id: undefined,
-        name: `Traces in replay ${replayId}`,
-        fields: ['trace', 'count(trace)', 'min(timestamp)'],
-        orderby: 'min_timestamp',
-        query: `replayId:${replayId}`,
-        projects: [Number(projectId)],
-        version: 2,
-        start,
-        end,
-      });
-
-      try {
-        const [data, , resp] = await doDiscoverQuery<TableData>(
-          api,
-          `/organizations/${orgSlug}/events/`,
-          eventView.getEventsAPIPayload(location)
-        );
-
-        const traceIds = data.data.map(({trace}) => trace).filter(trace => trace);
-
-        // TODO(replays): Potential performance concerns here if number of traceIds is large
-        const traceDetails = await Promise.allSettled(
-          traceIds.map(traceId =>
-            doDiscoverQuery(
-              api,
-              `/organizations/${orgSlug}/events-trace/${traceId}/`,
-              getTraceRequestPayload({
-                eventView: makeEventView({start, end}),
-                location,
-              })
-            )
-          )
-        );
+function Trace({replayRecord}: Props) {
+  const location = useLocation();
+  const organization = useOrganization();
+  const {state, eventView} = useTransactionData();
 
-        const successfulTraceDetails = traceDetails
-          .map(settled => (settled.status === 'fulfilled' ? settled.value[0] : undefined))
-          .filter(Boolean);
+  useFetchTransactions();
 
-        if (successfulTraceDetails.length !== traceDetails.length) {
-          traceDetails.forEach(trace => {
-            if (trace.status === 'rejected') {
-              Sentry.captureMessage(trace.reason);
-            }
-          });
-        }
-
-        setState(prevState => ({
-          isLoading: false,
-          error: null,
-          traceEventView: eventView,
-          pageLinks: resp?.getResponseHeader('Link') ?? prevState.pageLinks,
-          traces:
-            successfulTraceDetails.flatMap(trace => trace as TraceFullDetailed[]) || [],
-        }));
-      } catch (err) {
-        setState({
-          isLoading: false,
-          error: err,
-          pageLinks: null,
-          traceEventView: null,
-          traces: null,
-        });
-      }
-    }
-
-    loadTraces();
-
-    return () => {};
-  }, [api, replayId, projectId, orgSlug, location, start, end]);
-
-  if (state.isLoading) {
-    return <LoadingIndicator />;
-  }
-
-  if (state.error || !state.traceEventView) {
-    return <LoadingError />;
-  }
-
-  if (!state.traces?.length) {
-    return <EmptyMessage title={t('No traces found')} />;
+  if (!replayRecord || !state.traces?.length) {
+    return <StyledPlaceholder height="100%" />;
   }
 
   return (
-    <TraceView
-      meta={null}
-      traces={state.traces}
-      location={location}
-      organization={organization}
-      traceEventView={state.traceEventView}
-      traceSlug="Replay"
-    />
+    <FluidHeight>
+      <TraceView
+        meta={null}
+        traces={state.traces ?? null}
+        location={location}
+        organization={organization}
+        traceEventView={eventView!}
+        traceSlug="Replay"
+      />
+    </FluidHeight>
   );
-  // TODO(replays): pagination
 }
+
+const StyledPlaceholder = styled(Placeholder)`
+  border: 1px solid ${p => p.theme.border};
+  border-radius: ${p => p.theme.borderRadius};
+`;
+
+export default Trace;

+ 5 - 2
static/app/views/replays/details.tsx

@@ -20,6 +20,7 @@ import useReplayPageview from 'sentry/utils/replays/hooks/useReplayPageview';
 import useOrganization from 'sentry/utils/useOrganization';
 import ReplaysLayout from 'sentry/views/replays/detail/layout';
 import Page from 'sentry/views/replays/detail/page';
+import ReplayTransactionContext from 'sentry/views/replays/detail/trace/replayTransactionContext';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 
 type Props = RouteComponentProps<
@@ -115,12 +116,14 @@ function ReplayDetails({params: {replaySlug}}: Props) {
       replay={replay}
       initialTimeOffsetMs={initialTimeOffsetMs}
     >
-      <LoadedDetails orgSlug={orgSlug} replayRecord={replayRecord} />
+      <ReplayTransactionContext replayRecord={replayRecord}>
+        <DetailsInsideContext orgSlug={orgSlug} replayRecord={replayRecord} />
+      </ReplayTransactionContext>
     </ReplayContextProvider>
   );
 }
 
-function LoadedDetails({
+function DetailsInsideContext({
   orgSlug,
   replayRecord,
 }: {