Browse Source

feat(replays): Deeplink search results when searching DOM clicks (#47379)

Update `getInitialTimeOffset()` (and turn it into a react hook) so that
it will:
1. check to see if there is a `?query=` param in the url on the replay
details page
2. if there is, and if that query includes a `click.*` key. example
`click.tag:button`
3. then request a list of timestamps from the server. each timestamp is
a time when that searched-for element was clicked.
4. finally, set the current player time to be the first timestamp we get
back

The list of timestamps can be paginated, but we're just fetching the
first page because we only care about the first instance.

Why are we doing this round-trip to the server anyway? It seems like
once we get all the rrweb data we could simply grok through it in the
browser instead. Well, in order to do that we'd need to parse the search
query, and deal with all the boolean logic in the browser. Keeping that
in sync with the backend would be annoying and could have subtle errors.
Better to delegate.

Fixes https://github.com/getsentry/sentry/issues/47312
Ryan Albrecht 1 year ago
parent
commit
10160d8b9c

+ 4 - 4
static/app/components/events/eventReplay/replayPreview.tsx

@@ -40,9 +40,9 @@ function ReplayPreview({orgSlug, replaySlug, event}: Props) {
 
   const startTimestampMs = replayRecord?.started_at.getTime() ?? 0;
 
-  const initialTimeOffset = useMemo(() => {
+  const initialTimeOffsetMs = useMemo(() => {
     if (eventTimestamp && startTimestampMs) {
-      return relativeTimeInMs(eventTimestamp, startTimestampMs) / 1000;
+      return relativeTimeInMs(eventTimestamp, startTimestampMs);
     }
 
     return 0;
@@ -91,7 +91,7 @@ function ReplayPreview({orgSlug, replaySlug, event}: Props) {
     query: {
       referrer: getRouteStringFromRoutes(routes),
       t_main: 'console',
-      t: initialTimeOffset,
+      t: initialTimeOffsetMs / 1000,
     },
   };
 
@@ -99,7 +99,7 @@ function ReplayPreview({orgSlug, replaySlug, event}: Props) {
     <ReplayContextProvider
       isFetching={fetching}
       replay={replay}
-      initialTimeOffset={initialTimeOffset}
+      initialTimeOffsetMs={initialTimeOffsetMs}
     >
       <PlayerContainer data-test-id="player-container">
         <StaticPanel>

+ 4 - 4
static/app/components/replays/replayContext.tsx

@@ -192,7 +192,7 @@ type Props = {
   /**
    * Time, in seconds, when the video should start
    */
-  initialTimeOffset?: number;
+  initialTimeOffsetMs?: number;
 
   /**
    * Override return fields for testing
@@ -212,7 +212,7 @@ function updateSavedReplayConfig(config: ReplayConfig) {
 
 export function Provider({
   children,
-  initialTimeOffset = 0,
+  initialTimeOffsetMs = 0,
   isFetching,
   replay,
   value = {},
@@ -495,8 +495,8 @@ export function Provider({
 
   // Only on pageload: set the initial playback timestamp
   useEffect(() => {
-    if (initialTimeOffset && events && replayerRef.current) {
-      setCurrentTime(initialTimeOffset * 1000);
+    if (initialTimeOffsetMs && events && replayerRef.current) {
+      setCurrentTime(initialTimeOffsetMs);
     }
 
     return () => {

+ 45 - 0
static/app/utils/replays/fetchReplayClicks.tsx

@@ -0,0 +1,45 @@
+import * as Sentry from '@sentry/react';
+
+import type {Client} from 'sentry/api';
+
+type Props = {
+  api: Client;
+  orgSlug: string;
+  projectSlug: string;
+  query: string;
+  replayId: string;
+};
+
+type NodeMarker = {
+  node_id: number;
+  timestamp: string;
+};
+
+async function fetchReplayClicks({api, orgSlug, projectSlug, query, replayId}: Props) {
+  const path = `/projects/${orgSlug}/${projectSlug}/replays/${replayId}/clicks/`;
+  try {
+    const [{data}, _textStatus, resp] = await api.requestPromise(path, {
+      includeAllArgs: true,
+      query: {
+        query,
+      },
+    });
+
+    const pageLinks = resp?.getResponseHeader('Link') ?? '';
+
+    return {
+      fetchError: undefined,
+      pageLinks,
+      clicks: data as NodeMarker[],
+    };
+  } catch (error) {
+    Sentry.captureException(error);
+    return {
+      fetchError: error,
+      pageLinks: null,
+      clicks: [],
+    };
+  }
+}
+
+export default fetchReplayClicks;

+ 224 - 0
static/app/utils/replays/hooks/useInitialTimeOffsetMs.spec.tsx

@@ -0,0 +1,224 @@
+import {initializeOrg} from 'sentry-test/initializeOrg';
+import {reactHooks} from 'sentry-test/reactTestingLibrary';
+
+import fetchReplayClicks from 'sentry/utils/replays/fetchReplayClicks';
+import useInitialTimeOffsetMs from 'sentry/utils/replays/hooks/useInitialTimeOffsetMs';
+import {useLocation} from 'sentry/utils/useLocation';
+
+jest.mock('sentry/utils/useLocation');
+jest.mock('sentry/utils/replays/fetchReplayClicks');
+
+const MockUseLocation = useLocation as jest.MockedFunction<typeof useLocation>;
+const MockFetchReplayClicks = fetchReplayClicks as jest.MockedFunction<
+  typeof fetchReplayClicks
+>;
+
+const {organization, project} = initializeOrg();
+const replay = TestStubs.ReplayRecord();
+
+const NOON = '2023-04-14T12:00:00';
+const FIVE_PAST_NOON = '2023-04-14T12:05:00';
+
+function mockQuery(query: Record<string, string>) {
+  MockUseLocation.mockReturnValue({
+    pathname: '',
+    search: '',
+    query,
+    hash: '',
+    state: undefined,
+    action: 'PUSH',
+    key: '',
+  });
+}
+describe('useInitialTimeOffsetMs', () => {
+  beforeEach(() => {
+    MockUseLocation.mockClear();
+    MockFetchReplayClicks.mockClear();
+  });
+
+  describe('fromOffset', () => {
+    it('should return an offset, in ms, if `t` exists in the query', async () => {
+      const offsetInSeconds = 23;
+      mockQuery({t: String(offsetInSeconds)});
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: undefined,
+        },
+      });
+      await waitForNextUpdate();
+
+      expect(result.current).toBe(23 * 1000);
+    });
+
+    it('should prefer reading `t` over the other qs params', async () => {
+      const offsetInSeconds = 23;
+      mockQuery({
+        t: String(offsetInSeconds),
+        event_t: FIVE_PAST_NOON,
+        query: 'click.tag:button',
+      });
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: undefined,
+        },
+      });
+      await waitForNextUpdate();
+
+      expect(result.current).toBe(23 * 1000);
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  describe('fromEventTimestamp', () => {
+    it('should calculate the difference between an event timestamp and the replay start timestamp', async () => {
+      const noon = '2023-04-14T12:00:00';
+      const fivePastNoon = '2023-04-14T12:05:00';
+
+      mockQuery({event_t: fivePastNoon});
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: new Date(noon).getTime(),
+        },
+      });
+      await waitForNextUpdate();
+
+      // Expecting 5 minutes difference, in ms
+      expect(result.current).toBe(5 * 60 * 1000);
+    });
+
+    it('should return 0 offset if there is no replayStartTimetsamp, then recalculate when the startTimestamp appears', async () => {
+      mockQuery({event_t: FIVE_PAST_NOON});
+
+      const {result, rerender, waitForNextUpdate} = reactHooks.renderHook(
+        useInitialTimeOffsetMs,
+        {
+          initialProps: {
+            orgSlug: organization.slug,
+            replaySlug: `${project.slug}:${replay.id}`,
+            replayStartTimestampMs: undefined,
+          },
+        }
+      );
+
+      await waitForNextUpdate();
+      expect(result.current).toBe(0);
+
+      rerender({
+        orgSlug: organization.slug,
+        replaySlug: `${project.slug}:${replay.id}`,
+        replayStartTimestampMs: new Date(NOON).getTime(),
+      });
+      await waitForNextUpdate();
+
+      // Expecting 5 minutes difference, in ms
+      expect(result.current).toBe(5 * 60 * 1000);
+    });
+
+    it('should prefer reading `event_t` over the other search query params', async () => {
+      mockQuery({
+        event_t: FIVE_PAST_NOON,
+        query: 'click.tag:button',
+      });
+      MockFetchReplayClicks.mockResolvedValue({
+        fetchError: undefined,
+        pageLinks: '',
+        clicks: [],
+      });
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: new Date(NOON).getTime(),
+        },
+      });
+      await waitForNextUpdate();
+
+      expect(result.current).toBe(5 * 60 * 1000);
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  describe('fromListPageQuery', () => {
+    it('should skip this strategy if there is no `click.*` term in the query', async () => {
+      mockQuery({query: 'user.email:*@sentry.io'});
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: new Date(NOON).getTime(),
+        },
+      });
+      await waitForNextUpdate();
+
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(0);
+      expect(result.current).toBe(0);
+    });
+
+    it('should request a list of click results, and calculate the offset from the first result', async () => {
+      mockQuery({query: 'click.tag:button'});
+      MockFetchReplayClicks.mockResolvedValue({
+        fetchError: undefined,
+        pageLinks: '',
+        clicks: [{node_id: 7, timestamp: FIVE_PAST_NOON}],
+      });
+
+      const {result, waitForNextUpdate} = reactHooks.renderHook(useInitialTimeOffsetMs, {
+        initialProps: {
+          orgSlug: organization.slug,
+          replaySlug: `${project.slug}:${replay.id}`,
+          replayStartTimestampMs: new Date(NOON).getTime(),
+        },
+      });
+      await waitForNextUpdate();
+
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(1);
+      // Expecting 5 minutes difference, in ms
+      expect(result.current).toBe(5 * 60 * 1000);
+    });
+
+    it('should not call call fetch twice when props change', async () => {
+      mockQuery({query: 'click.tag:button'});
+      MockFetchReplayClicks.mockResolvedValue({
+        fetchError: undefined,
+        pageLinks: '',
+        clicks: [{node_id: 7, timestamp: FIVE_PAST_NOON}],
+      });
+
+      const {result, rerender, waitForNextUpdate} = reactHooks.renderHook(
+        useInitialTimeOffsetMs,
+        {
+          initialProps: {
+            orgSlug: organization.slug,
+            replaySlug: `${project.slug}:${replay.id}`,
+            replayStartTimestampMs: undefined,
+          },
+        }
+      );
+      await waitForNextUpdate();
+
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(0);
+      expect(result.current).toBe(0);
+
+      rerender({
+        orgSlug: organization.slug,
+        replaySlug: `${project.slug}:${replay.id}`,
+        replayStartTimestampMs: new Date(NOON).getTime(),
+      });
+      await waitForNextUpdate();
+
+      expect(MockFetchReplayClicks).toHaveBeenCalledTimes(1);
+      expect(result.current).toBe(5 * 60 * 1000);
+    });
+  });
+});

+ 165 - 0
static/app/utils/replays/hooks/useInitialTimeOffsetMs.tsx

@@ -0,0 +1,165 @@
+import {useEffect, useMemo, useState} from 'react';
+import first from 'lodash/first';
+
+import fetchReplayClicks from 'sentry/utils/replays/fetchReplayClicks';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import useApi from 'sentry/utils/useApi';
+import {useLocation} from 'sentry/utils/useLocation';
+
+export type TimeOffsetLocationQueryParams = {
+  /**
+   * The time when the event happened.
+   * Anything that can be parsed by `new Date()`; for example a timestamp in ms
+   * or an ISO 8601 formatted string.
+   */
+  event_t?: string;
+
+  /**
+   * The query that was used on the index page. If it includes `click.*` fields
+   * then we will use those to lookup a list of `offsetMs` values
+   */
+  query?: string;
+
+  /**
+   * A specific offset into the replay. Number of seconds.
+   * Should be less than the duration of the replay
+   */
+  t?: string;
+};
+
+type Opts = {
+  /**
+   * The organization name you'll see in the browser url
+   */
+  orgSlug: string;
+  /**
+   * The concatenation of: `${projectSlug}:${replayId}`
+   */
+  replaySlug: string;
+
+  /**
+   * The start timestamp of the replay.
+   * Used to calculate the offset into the replay from an event timestamp
+   */
+  replayStartTimestampMs?: number;
+};
+
+function fromOffset({offsetSec}) {
+  if (offsetSec === undefined) {
+    // Not using this strategy
+    return undefined;
+  }
+
+  return Number(offsetSec) * 1000;
+}
+
+function fromEventTimestamp({eventTimestamp, replayStartTimestampMs}) {
+  if (eventTimestamp === undefined) {
+    // Not using this strategy
+    return undefined;
+  }
+
+  if (replayStartTimestampMs !== undefined) {
+    const eventTimestampMs = new Date(eventTimestamp).getTime();
+    if (eventTimestampMs >= replayStartTimestampMs) {
+      return eventTimestampMs - replayStartTimestampMs;
+    }
+  }
+  // The strategy failed, default to something safe
+  return 0;
+}
+
+async function fromListPageQuery({
+  api,
+  listPageQuery,
+  orgSlug,
+  replaySlug,
+  replayStartTimestampMs,
+}) {
+  if (listPageQuery === undefined) {
+    // Not using this strategy
+    return undefined;
+  }
+
+  // Check if there is even any `click.*` fields in the query string
+  const search = new MutableSearch(listPageQuery);
+  const isClickSearch = search.getFilterKeys().some(key => key.startsWith('click.'));
+  if (!isClickSearch) {
+    // There was a search, but not for clicks, so lets skip this strategy.
+    return undefined;
+  }
+
+  if (replayStartTimestampMs === undefined) {
+    // Using the strategy, but we must wait for replayStartTimestampMs to appear
+    return 0;
+  }
+
+  const [projectSlug, replayId] = replaySlug.split(':');
+
+  const results = await fetchReplayClicks({
+    api,
+    orgSlug,
+    projectSlug,
+    replayId,
+    query: listPageQuery,
+  });
+  if (!results.clicks.length) {
+    return 0;
+  }
+  try {
+    const firstTimestamp = first(results.clicks)!.timestamp;
+    const firstTimestmpMs = new Date(firstTimestamp).getTime();
+    return firstTimestmpMs - replayStartTimestampMs;
+  } catch {
+    return 0;
+  }
+}
+
+function useInitialTimeOffsetMs({orgSlug, replaySlug, replayStartTimestampMs}: Opts) {
+  const api = useApi();
+  const {
+    query: {event_t: eventTimestamp, query: listPageQuery, t: offsetSec},
+  } = useLocation<TimeOffsetLocationQueryParams>();
+  const [timestamp, setTimestamp] = useState<undefined | number>(undefined);
+
+  // The different strategies for getting a time offset into the replay (what
+  // time to start the replay at)
+  // Each strategy should return time in milliseconds
+  const offsetTimeMs = useMemo(() => fromOffset({offsetSec}), [offsetSec]);
+  const eventTimeMs = useMemo(
+    () => fromEventTimestamp({eventTimestamp, replayStartTimestampMs}),
+    [eventTimestamp, replayStartTimestampMs]
+  );
+  const queryTimeMs = useMemo(
+    () =>
+      eventTimestamp === undefined
+        ? fromListPageQuery({
+            api,
+            listPageQuery,
+            orgSlug,
+            replaySlug,
+            replayStartTimestampMs,
+          })
+        : undefined,
+    [api, eventTimestamp, listPageQuery, orgSlug, replaySlug, replayStartTimestampMs]
+  );
+
+  useEffect(() => {
+    Promise.resolve(undefined)
+      .then(definedOrDefault(offsetTimeMs))
+      .then(definedOrDefault(eventTimeMs))
+      .then(definedOrDefault(queryTimeMs))
+      .then(definedOrDefault(0))
+      .then(setTimestamp);
+  }, [offsetTimeMs, eventTimeMs, queryTimeMs]);
+
+  return timestamp;
+}
+
+function definedOrDefault<T>(dflt: T | undefined | Promise<T | undefined>) {
+  return (val: T | undefined) => {
+    return val ?? dflt;
+  };
+}
+
+export default useInitialTimeOffsetMs;

+ 3 - 3
static/app/views/performance/transactionSummary/utils.tsx

@@ -196,9 +196,9 @@ export function generateReplayLink(routes: PlainRoute<any>[]) {
     }
 
     const transactionTimestamp = new Date(tableRow.timestamp).getTime();
-
-    const transactionStartTimestamp =
-      transactionTimestamp - (tableRow['transaction.duration'] as number);
+    const transactionStartTimestamp = tableRow['transaction.duration']
+      ? transactionTimestamp - (tableRow['transaction.duration'] as number)
+      : undefined;
 
     return {
       pathname: `/organizations/${organization.slug}/replays/${replaySlug}/`,

+ 13 - 18
static/app/views/replays/details.tsx

@@ -11,6 +11,9 @@ import {
   useReplayContext,
 } from 'sentry/components/replays/replayContext';
 import {t} from 'sentry/locale';
+import useInitialTimeOffsetMs, {
+  TimeOffsetLocationQueryParams,
+} from 'sentry/utils/replays/hooks/useInitialTimeOffsetMs';
 import useReplayData from 'sentry/utils/replays/hooks/useReplayData';
 import useReplayLayout from 'sentry/utils/replays/hooks/useReplayLayout';
 import useReplayPageview from 'sentry/utils/replays/hooks/useReplayPageview';
@@ -18,33 +21,29 @@ import useOrganization from 'sentry/utils/useOrganization';
 import ReplaysLayout from 'sentry/views/replays/detail/layout';
 import Page from 'sentry/views/replays/detail/page';
 import type {ReplayRecord} from 'sentry/views/replays/types';
-import {getInitialTimeOffset} from 'sentry/views/replays/utils';
 
 type Props = RouteComponentProps<
   {replaySlug: string},
   {},
   any,
-  {event_t: string; t: number}
+  TimeOffsetLocationQueryParams
 >;
 
-function ReplayDetails({
-  location: {
-    query: {
-      event_t: eventTimestamp, // Timestamp of the event or activity that was selected
-      t: initialTimeOffset, // Time, in seconds, where the video should start
-    },
-  },
-  params: {replaySlug},
-}: Props) {
+function ReplayDetails({params: {replaySlug}}: Props) {
   useReplayPageview('replay.details-time-spent');
-  const {slug: orgSlug} = useOrganization();
+  const organization = useOrganization();
+  const {slug: orgSlug} = organization;
 
   const {fetching, onRetry, replay, replayRecord, fetchError} = useReplayData({
     replaySlug,
     orgSlug,
   });
 
-  const startTimestampMs = replayRecord?.started_at.getTime() ?? 0;
+  const initialTimeOffsetMs = useInitialTimeOffsetMs({
+    orgSlug,
+    replaySlug,
+    replayStartTimestampMs: replayRecord?.started_at.getTime(),
+  });
 
   if (fetchError) {
     if (fetchError.statusText === 'Not Found') {
@@ -110,11 +109,7 @@ function ReplayDetails({
     <ReplayContextProvider
       isFetching={fetching}
       replay={replay}
-      initialTimeOffset={getInitialTimeOffset({
-        eventTimestamp,
-        initialTimeOffset,
-        startTimestampMs,
-      })}
+      initialTimeOffsetMs={initialTimeOffsetMs}
     >
       <LoadedDetails orgSlug={orgSlug} replayRecord={replayRecord} />
     </ReplayContextProvider>

+ 0 - 59
static/app/views/replays/utils.spec.tsx

@@ -1,59 +0,0 @@
-import {getInitialTimeOffset} from 'sentry/views/replays/utils';
-
-describe('getInitialTimeOffset', () => {
-  it('should return the initialTimeOffset if provided', () => {
-    expect(
-      getInitialTimeOffset({
-        initialTimeOffset: 123,
-        eventTimestamp: '1666047045297',
-        startTimestampMs: 1666047046000,
-      })
-    ).toBe(123);
-  });
-
-  it('should return 0 if no required params are provided', () => {
-    expect(getInitialTimeOffset({startTimestampMs: 1666047046000})).toBe(0);
-
-    expect(getInitialTimeOffset({})).toBe(0);
-
-    expect(getInitialTimeOffset({eventTimestamp: '1666047045297'})).toBe(0);
-  });
-
-  it('should return 0 if the eventTimestamp is not the correct format', () => {
-    expect(getInitialTimeOffset({eventTimestamp: '123'})).toBe(0);
-  });
-
-  it('should return 0 if the eventTimestamp is not within the range of the replay', () => {
-    expect(
-      getInitialTimeOffset({
-        eventTimestamp: '1666047045297',
-        startTimestampMs: 1666047046000,
-      })
-    ).toBe(0);
-  });
-
-  it('should return the correct initialTimeOffset if the eventTimestamp is within the range of the replay', () => {
-    expect(
-      getInitialTimeOffset({
-        eventTimestamp: '1666047046000',
-        startTimestampMs: 1666047045000,
-      })
-    ).toBe(1);
-  });
-
-  it('should return the correct initialTimeOffset if the eventTimestamp is the correct format', () => {
-    expect(
-      getInitialTimeOffset({
-        eventTimestamp: '1666047045297',
-        startTimestampMs: 1666047045000,
-      })
-    ).toBe(0.297);
-
-    expect(
-      getInitialTimeOffset({
-        eventTimestamp: '2022-10-17T22:50:46.000Z',
-        startTimestampMs: 1666047045000,
-      })
-    ).toBe(1);
-  });
-});

+ 0 - 40
static/app/views/replays/utils.tsx

@@ -1,40 +0,0 @@
-import {relativeTimeInMs} from 'sentry/components/replays/utils';
-import {defined} from 'sentry/utils';
-
-export const getInitialTimeOffset = ({
-  eventTimestamp,
-  initialTimeOffset,
-  startTimestampMs,
-}: {
-  eventTimestamp?: string;
-  initialTimeOffset?: number;
-  startTimestampMs?: number;
-}) => {
-  if (defined(initialTimeOffset)) {
-    return initialTimeOffset;
-  }
-
-  // If the user has navigated to the replay from an event, then we want to
-  // start the video at the time of the event.
-  if (defined(eventTimestamp) && defined(startTimestampMs)) {
-    // check if the event timestamp is the correct format
-    let eventTimestampMs = 0;
-    if (eventTimestamp.length === 13) {
-      eventTimestampMs = Number(eventTimestamp);
-    } else {
-      eventTimestampMs = new Date(eventTimestamp).getTime();
-    }
-
-    // check if our event timestamp is within the range of the replay
-    if (eventTimestampMs >= startTimestampMs) {
-      initialTimeOffset = relativeTimeInMs(eventTimestampMs, startTimestampMs) / 1000;
-    }
-  }
-
-  // if the event timestamp is not the correct format or no timestamps are provided, default to the start of the replay
-  if (!defined(initialTimeOffset) || Number.isNaN(initialTimeOffset)) {
-    initialTimeOffset = 0;
-  }
-
-  return initialTimeOffset;
-};