feat(trace-waterfall): Add previous and next trace buttons (EAP) (#96510)

Reworks the previous implementation of showing previous/next
trace buttons:

- Linked Trace buttons are now compatible with EAP (only)!
- For now, we solely rely on the `sentry.previous_trace` span attribute.
Meaning, we don't use span links at the moment, because span links in
EAP are WIP. In the near future they'll be stored as a JSON blob. For us
to rely on them, we need them to be queryable again though. Therefore,
the SDK currently sends the `sentry.previous_trace` span attribute,
which we can use to search for specific spans on the EAP dataset.
- the buttons are still feature-flagged for the `sentry` and
`sentry-sdks` orgs. We can remove the flags once we dogfooded for a bit
and things seem to work as expected.
- Use the `useSpans` query to find the next trace linking to the current
trace (reverse span link lookup)
- Use the `useSpans` instead of `useTrace` query to check if the
previous trace is actually available (vs. dropped server-side/not
ingested)
- Rework the logic around showing "previous trace not available|sampled"
errors
- Use two dedicated hooks `findPreviousTrace` and `findNextTrace`
instead of once combined
- Add placeholders to avoid the waterfall panel's lower end shifting
upwards once links are loaded (see video)
- This has the inherent disadvantage that the vertical space for the
waterfall becomes smaller. We can explore alternative positions for the
previous/next buttons to get rid of this limitation again. I'd argue
that for now, as long as things are feature flagged, having the
placeholders is better than causing layout shift and repaints.

Author: Lms24

static/app/components/events/interfaces/spans/types.tsx

@@ -202,7 +202,7 @@ export enum TickAlignment {
 
 type AttributeValue = string | number | boolean | string[] | number[] | boolean[];
 
-export type SpanLink = {
+type SpanLink = {
   span_id: string;
   trace_id: string;
   attributes?: Record<string, AttributeValue> & {'sentry.link.type'?: AttributeValue};

static/app/views/insights/common/queries/useDiscover.ts

@@ -5,7 +5,10 @@ import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import type {SamplingMode} from 'sentry/views/explore/hooks/useProgressiveQuery';
-import {useWrappedDiscoverQuery} from 'sentry/views/insights/common/queries/useSpansQuery';
+import {
+  useWrappedDiscoverQuery,
+  useWrappedDiscoverQueryWithoutPageFilters,
+} from 'sentry/views/insights/common/queries/useSpansQuery';
 import type {SpanProperty, SpanResponse} from 'sentry/views/insights/types';
 
 interface UseDiscoverQueryOptions {
@@ -22,6 +25,11 @@ interface UseDiscoverOptions<Fields> {
   orderby?: string | string[];
   pageFilters?: PageFilters;
   projectIds?: number[];
+  /**
+   * If true, the query will be executed without the page filters.
+   * {@link pageFilters} can still be passed and will be used to build the event view on top of the query.
+   */
+  queryWithoutPageFilters?: boolean;
   samplingMode?: SamplingMode;
   /**
    * TODO - ideally this probably would be only `Mutable Search`, but it doesn't handle some situations well
@@ -72,7 +80,11 @@ const useDiscover = <T extends Array<Extract<keyof ResponseType, string>>, Respo
     orderby
   );
 
-  const result = useWrappedDiscoverQuery({
+  const queryFn = options.queryWithoutPageFilters
+    ? useWrappedDiscoverQueryWithoutPageFilters
+    : useWrappedDiscoverQuery;
+
+  const result = queryFn({
     eventView,
     initialData: [],
     limit,

static/app/views/insights/common/queries/useSpansQuery.tsx

@@ -303,7 +303,7 @@ export function useWrappedDiscoverQuery<T>(props: WrappedDiscoverQueryProps<T>)
   return useWrappedDiscoverQueryBase({...props, pageFiltersReady});
 }
 
-function useWrappedDiscoverQueryWithoutPageFilters<T>(
+export function useWrappedDiscoverQueryWithoutPageFilters<T>(
   props: WrappedDiscoverQueryProps<T>
 ) {
   return useWrappedDiscoverQueryBase({...props, pageFiltersReady: true});

static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx

@@ -3,143 +3,149 @@ import styled from '@emotion/styled';
 
 import {ExternalLink, Link} from 'sentry/components/core/link';
 import {Tooltip} from 'sentry/components/core/tooltip';
-import type {
-  SpanLink,
-  TraceContextType,
-} from 'sentry/components/events/interfaces/spans/types';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import {IconChevron} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
-import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace';
-import {isEmptyTrace} from 'sentry/views/performance/newTraceDetails/traceApi/utils';
-import {useFindNextTrace} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace';
+import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails';
+import {
+  useFindNextTrace,
+  useFindPreviousTrace,
+} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces';
+import {useTraceStateDispatch} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider';
 import {getTraceDetailsUrl} from 'sentry/views/performance/traceDetails/utils';
 
 export type ConnectedTraceConnection = 'previous' | 'next';
 
 const LINKED_TRACE_MAX_DURATION = 3600; // 1h in seconds
 
-function useIsTraceAvailable(
-  traceID?: SpanLink['trace_id'],
-  linkedTraceTimestamp?: number
-): {
-  isAvailable: boolean;
-  isLoading: boolean;
-} {
-  const trace = useTrace({
-    traceSlug: traceID,
-    timestamp: linkedTraceTimestamp,
-  });
-
-  const isAvailable = useMemo(() => {
-    if (!traceID) {
-      return false;
-    }
-
-    return Boolean(trace.data && !isEmptyTrace(trace.data));
-  }, [traceID, trace]);
-
-  return {
-    isAvailable,
-    isLoading: trace.isLoading,
-  };
-}
-
 type TraceLinkNavigationButtonProps = {
-  currentTraceTimestamps: {end?: number; start?: number};
+  attributes: TraceItemResponseAttribute[];
+  currentTraceStartTimestamp: number;
   direction: ConnectedTraceConnection;
-  isLoading?: boolean;
-  projectID?: string;
-  traceContext?: TraceContextType;
 };
 
 export function TraceLinkNavigationButton({
   direction,
-  traceContext,
-  isLoading,
-  projectID,
-  currentTraceTimestamps,
+  attributes,
+  currentTraceStartTimestamp,
 }: TraceLinkNavigationButtonProps) {
   const organization = useOrganization();
   const location = useLocation();
 
   // We connect traces over a 1h period - As we don't have timestamps of the linked trace, it is calculated based on this timeframe
-  const linkedTraceTimestamp =
-    direction === 'previous' && currentTraceTimestamps.start
-      ? currentTraceTimestamps.start - LINKED_TRACE_MAX_DURATION // Earliest start time of previous trace (- 1h)
-      : direction === 'next' && currentTraceTimestamps.end
-        ? currentTraceTimestamps.end + LINKED_TRACE_MAX_DURATION // Latest end time of next trace (+ 1h)
-        : undefined;
-
-  const previousTraceLink = traceContext?.links?.find(
-    link => link.attributes?.['sentry.link.type'] === `${direction}_trace`
-  );
+  const linkedTraceWindowTimestamp =
+    direction === 'previous'
+      ? currentTraceStartTimestamp - LINKED_TRACE_MAX_DURATION // Earliest start time of previous trace (- 1h)
+      : currentTraceStartTimestamp + LINKED_TRACE_MAX_DURATION; // Latest end time of next trace (+ 1h)
+
+  const {
+    available: isPreviousTraceAvailable,
+    id: previousTraceSpanId,
+    trace: previousTraceId,
+    sampled: previousTraceSampled,
+    isLoading: isPreviousTraceLoading,
+  } = useFindPreviousTrace({
+    direction,
+    previousTraceEndTimestamp: currentTraceStartTimestamp,
+    previousTraceStartTimestamp: linkedTraceWindowTimestamp,
+    attributes,
+  });
 
-  const nextTraceData = useFindNextTrace({
+  const {
+    id: nextTraceSpanId,
+    trace: nextTraceId,
+    isLoading: isNextTraceLoading,
+  } = useFindNextTrace({
     direction,
-    currentTraceID: traceContext?.trace_id,
-    linkedTraceStartTimestamp: currentTraceTimestamps.end,
-    linkedTraceEndTimestamp: linkedTraceTimestamp,
-    projectID,
+    nextTraceEndTimestamp: linkedTraceWindowTimestamp,
+    nextTraceStartTimestamp: currentTraceStartTimestamp,
+    attributes,
   });
 
   const dateSelection = useMemo(
     () => normalizeDateTimeParams(location.query),
     [location.query]
   );
 
-  const {isAvailable: isLinkedTraceAvailable} = useIsTraceAvailable(
-    direction === 'previous' ? previousTraceLink?.trace_id : nextTraceData?.trace_id,
-    linkedTraceTimestamp
-  );
+  const traceDispatch = useTraceStateDispatch();
 
-  if (isLoading) {
-    // We don't show a placeholder/skeleton here as it would cause layout shifts most of the time.
-    // Most traces don't have a next/previous trace and the hard to avoid layout shift should only occur if the actual button can be shown.
-    return null;
+  function closeSpanDetailsDrawer() {
+    traceDispatch({
+      type: 'minimize drawer',
+      payload: true,
+    });
   }
 
-  if (previousTraceLink && isLinkedTraceAvailable) {
-    return (
-      <StyledTooltip
-        position="right"
-        delay={400}
-        isHoverable
-        title={tct(
-          `This links to the previous trace within the same session. To learn more, [link:read the docs].`,
-          {
-            link: (
-              <ExternalLink
-                href={
-                  'https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces'
-                }
-              />
-            ),
-          }
-        )}
-      >
-        <TraceLink
-          color="gray500"
-          to={getTraceDetailsUrl({
-            traceSlug: previousTraceLink.trace_id,
-            spanId: previousTraceLink.span_id,
-            dateSelection,
-            timestamp: linkedTraceTimestamp,
-            location,
-            organization,
-          })}
+  if (direction === 'previous' && previousTraceId && !isPreviousTraceLoading) {
+    if (isPreviousTraceAvailable) {
+      return (
+        <StyledTooltip
+          position="right"
+          delay={400}
+          isHoverable
+          title={tct(
+            `This links to the previous trace within the same session. To learn more, [link:read the docs].`,
+            {
+              link: (
+                <ExternalLink
+                  href={
+                    'https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces'
+                  }
+                />
+              ),
+            }
+          )}
         >
-          <IconChevron direction="left" />
-          <TraceLinkText>{t('Go to Previous Trace')}</TraceLinkText>
-        </TraceLink>
-      </StyledTooltip>
-    );
+          <TraceLink
+            color="gray500"
+            onClick={() => closeSpanDetailsDrawer()}
+            to={getTraceDetailsUrl({
+              traceSlug: previousTraceId,
+              spanId: previousTraceSpanId,
+              dateSelection,
+              timestamp: linkedTraceWindowTimestamp,
+              location,
+              organization,
+            })}
+          >
+            <IconChevron direction="left" />
+            <TraceLinkText>{t('Go to Previous Trace')}</TraceLinkText>
+          </TraceLink>
+        </StyledTooltip>
+      );
+    }
+
+    if (!previousTraceSampled) {
+      return (
+        <StyledTooltip
+          position="right"
+          title={t(
+            'Trace contains a link to an unsampled trace. Increase traces sample rate in SDK settings to see more connected traces'
+          )}
+        >
+          <TraceLinkText>{t('Previous trace not sampled')}</TraceLinkText>
+        </StyledTooltip>
+      );
+    }
+
+    if (!isPreviousTraceAvailable) {
+      return (
+        <StyledTooltip
+          position="right"
+          title={t(
+            'Trace contains a link to a trace that is not available. This means that the trace was not stored.'
+          )}
+        >
+          <TraceLinkText>{t('Previous trace not available')}</TraceLinkText>
+        </StyledTooltip>
+      );
+    }
   }
 
-  if (nextTraceData?.trace_id && nextTraceData.span_id && isLinkedTraceAvailable) {
+  if (direction === 'next' && !isNextTraceLoading && nextTraceId && nextTraceSpanId) {
     return (
       <StyledTooltip
         position="left"
@@ -160,11 +166,12 @@ export function TraceLinkNavigationButton({
       >
         <TraceLink
           color="gray500"
+          onClick={closeSpanDetailsDrawer}
           to={getTraceDetailsUrl({
-            traceSlug: nextTraceData.trace_id,
-            spanId: nextTraceData.span_id,
+            traceSlug: nextTraceId,
+            spanId: nextTraceSpanId,
             dateSelection,
-            timestamp: linkedTraceTimestamp,
+            timestamp: linkedTraceWindowTimestamp,
             location,
             organization,
           })}
@@ -176,25 +183,23 @@ export function TraceLinkNavigationButton({
     );
   }
 
-  if (previousTraceLink?.sampled === false) {
-    return (
-      <StyledTooltip
-        position="right"
-        title={t(
-          'Trace contains a link to unsampled trace. Increase traces sample rate in SDK settings to see more connected traces'
-        )}
-      >
-        <TraceLinkText>{t('Previous trace not available')}</TraceLinkText>
-      </StyledTooltip>
-    );
-  }
+  // If there's no linked trace, let's render a placeholder for now to avoid layout shifts
+  // We should reconsider the place where we render these buttons, to avoid reducing the
+  // waterfall height permanently
+  return <TraceLinkNavigationButtonPlaceHolder />;
+}
 
-  // If there is no linked trace or an undefined sampling decision
-  return null;
+export function TraceLinkNavigationButtonPlaceHolder() {
+  return <PlaceHolderText>&nbsp;</PlaceHolderText>;
 }
 
+const PlaceHolderText = styled('span')`
+  padding: ${space(0.5)} ${space(0.5)};
+  visibility: hidden;
+`;
+
 const StyledTooltip = styled(Tooltip)`
-  padding: ${space(0.5)} ${space(1)};
+  padding: ${space(0.5)} ${space(0.5)};
   text-decoration: underline dotted
     ${p => (p.disabled ? p.theme.gray300 : p.theme.gray300)};
 `;