diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index e67122828cc17e..0e01e05b385f00 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -1,19 +1,14 @@ import {useMemo} from 'react'; -import styled from '@emotion/styled'; -import {ExternalLink, Link} from 'sentry/components/core/link'; -import {Tooltip} from 'sentry/components/core/tooltip'; +import {LinkButton} from '@sentry/scraps/button/linkButton'; + 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 {t} from 'sentry/locale'; import {useLocation} from 'sentry/utils/useLocation'; import useOrganization from 'sentry/utils/useOrganization'; import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; -import { - useFindNextTrace, - useFindPreviousTrace, -} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces'; +import {useFindAdjacentTrace} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces'; import {useTraceStateDispatch} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; import {getTraceDetailsUrl} from 'sentry/views/performance/traceDetails/utils'; @@ -42,25 +37,36 @@ export function TraceLinkNavigationButton({ : currentTraceStartTimestamp + LINKED_TRACE_MAX_DURATION; // Latest end time of next trace (+ 1h) const { - available: isPreviousTraceAvailable, - id: previousTraceSpanId, - trace: previousTraceId, - isLoading: isPreviousTraceLoading, - } = useFindPreviousTrace({ - direction, - previousTraceEndTimestamp: currentTraceStartTimestamp, - previousTraceStartTimestamp: linkedTraceWindowTimestamp, - attributes, - }); + adjacentTraceEndTimestamp, + adjacentTraceStartTimestamp, + iconDirection, + ariaLabel, + } = useMemo(() => { + if (direction === 'previous') { + return { + adjacentTraceEndTimestamp: currentTraceStartTimestamp, + adjacentTraceStartTimestamp: linkedTraceWindowTimestamp, + iconDirection: 'left' as const, + ariaLabel: t('Previous Trace'), + }; + } + return { + adjacentTraceEndTimestamp: linkedTraceWindowTimestamp, + adjacentTraceStartTimestamp: currentTraceStartTimestamp, + iconDirection: 'right' as const, + ariaLabel: t('Next Trace'), + }; + }, [direction, currentTraceStartTimestamp, linkedTraceWindowTimestamp]); const { - id: nextTraceSpanId, - trace: nextTraceId, - isLoading: isNextTraceLoading, - } = useFindNextTrace({ + available: isTraceAvailable, + id: traceSpanId, + trace: traceId, + isLoading: isTraceLoading, + } = useFindAdjacentTrace({ direction, - nextTraceEndTimestamp: linkedTraceWindowTimestamp, - nextTraceStartTimestamp: currentTraceStartTimestamp, + adjacentTraceEndTimestamp, + adjacentTraceStartTimestamp, attributes, }); @@ -78,112 +84,21 @@ export function TraceLinkNavigationButton({ }); } - if ( - direction === 'previous' && - previousTraceId && - !isPreviousTraceLoading && - isPreviousTraceAvailable - ) { - return ( - - ), - } - )} - > - closeSpanDetailsDrawer()} - to={getTraceDetailsUrl({ - traceSlug: previousTraceId, - spanId: previousTraceSpanId, - dateSelection, - timestamp: linkedTraceWindowTimestamp, - location, - organization, - })} - > - - {t('Go to Previous Trace')} - - - ); - } - - if (direction === 'next' && !isNextTraceLoading && nextTraceId && nextTraceSpanId) { - return ( - - ), - } - )} - > - - {t('Go to Next Trace')} - - - - ); - } - - // 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 ; -} - -export function TraceLinkNavigationButtonPlaceHolder() { - return  ; + return ( + } + aria-label={ariaLabel} + onClick={closeSpanDetailsDrawer} + disabled={!traceId || isTraceLoading || !isTraceAvailable} + to={getTraceDetailsUrl({ + traceSlug: traceId ?? '', + spanId: traceSpanId, + dateSelection, + timestamp: linkedTraceWindowTimestamp, + location, + organization, + })} + /> + ); } - -const PlaceHolderText = styled('span')` - padding: ${space(0.5)} ${space(0.5)}; - visibility: hidden; -`; - -const StyledTooltip = styled(Tooltip)` - padding: ${space(0.5)} ${space(0.5)}; - text-decoration: underline dotted - ${p => (p.disabled ? p.theme.gray300 : p.theme.gray300)}; -`; - -const TraceLink = styled(Link)` - font-weight: ${p => p.theme.fontWeight.normal}; - padding: ${space(0.25)} ${space(0.5)}; - display: flex; - align-items: center; - - color: ${p => p.theme.subText}; - :hover { - color: ${p => p.theme.subText}; - } -`; - -const TraceLinkText = styled('span')` - line-height: normal; -`; diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx new file mode 100644 index 00000000000000..ec742b7056e216 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx @@ -0,0 +1,65 @@ +import {ButtonBar} from '@sentry/scraps/button'; +import {ExternalLink} from '@sentry/scraps/link'; +import {Tooltip} from '@sentry/scraps/tooltip'; + +import {tct} from 'sentry/locale'; +import useOrganization from 'sentry/utils/useOrganization'; +import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; +import {isTraceItemDetailsResponse} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; +import {TraceLinkNavigationButton} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton'; + +export function TraceLinksNavigation({ + rootEventResults, + source, +}: { + rootEventResults: TraceRootEventQueryResults; + source: string; +}) { + const organization = useOrganization(); + const showLinkedTraces = + organization?.features.includes('trace-view-linked-traces') && + // Don't show the linked traces buttons when the waterfall is embedded in the replay + // detail page, as it already contains all traces of the replay session. + source !== 'replay'; + + if ( + !showLinkedTraces || + !isTraceItemDetailsResponse(rootEventResults.data) || + !rootEventResults.data.timestamp + ) { + return null; + } + + return ( + + ), + } + )} + > + + + + + + ); +} diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts index a230bc84a86eee..b92342b6c53ac1 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts @@ -6,132 +6,57 @@ import {useSpans} from 'sentry/views/insights/common/queries/useDiscover'; import type {ConnectedTraceConnection} from './traceLinkNavigationButton'; /** - * Find the next trace by looking using the spans endpoint to query for a trace - * linking to the current trace as its previous trace. + * Find an adjacent trace (next or previous) by querying the spans endpoint. + * For 'next' traces: looks for a trace linking to the current trace as its previous trace. + * For 'previous' traces: looks for the trace specified in the previous_trace attribute. */ -export function useFindNextTrace({ +export function useFindAdjacentTrace({ direction, attributes, - nextTraceEndTimestamp, - nextTraceStartTimestamp, + adjacentTraceEndTimestamp, + adjacentTraceStartTimestamp, }: { + adjacentTraceEndTimestamp: number; + adjacentTraceStartTimestamp: number; attributes: TraceItemResponseAttribute[]; direction: ConnectedTraceConnection; - nextTraceEndTimestamp: number; - nextTraceStartTimestamp: number; -}): {isLoading: boolean; id?: string; trace?: string} { - const {currentTraceId, currentSpanId, projectId, environment} = useMemo(() => { - let _currentTraceId: string | undefined; - let _currentSpanId: string | undefined; - let _projectId: number | undefined; - let _environment: string | undefined; - - for (const a of attributes) { - if (a.name === 'trace' && a.type === 'str') { - _currentTraceId = a.value; - } else if (a.name === 'transaction.span_id' && a.type === 'str') { - _currentSpanId = a.value; - } else if (a.name === 'project_id' && a.type === 'int') { - _projectId = a.value; - } else if (a.name === 'environment' && a.type === 'str') { - _environment = a.value; - } - } - return { - currentTraceId: _currentTraceId, - currentSpanId: _currentSpanId, - projectId: _projectId, - environment: _environment, - }; - }, [attributes]); - - const {data, isError, isPending} = useSpans( - { - search: `sentry.previous_trace:${currentTraceId}-${currentSpanId}-1`, - fields: ['id', 'trace'], - limit: 1, - enabled: direction === 'next' && !!projectId, - projectIds: projectId ? [projectId] : [], - pageFilters: { - environments: environment ? [environment] : [], - projects: projectId ? [projectId] : [], - datetime: { - start: nextTraceStartTimestamp - ? new Date(nextTraceStartTimestamp * 1000).toISOString() - : '', - end: nextTraceEndTimestamp - ? new Date(nextTraceEndTimestamp * 1000).toISOString() - : '', - period: null, - utc: true, - }, - }, - queryWithoutPageFilters: true, - }, - `api.insights.trace-panel-${direction}-trace-link` - ); - - const spanId = data?.[0]?.id; - const traceId = data?.[0]?.trace; - - const nextTraceData = useMemo(() => { - if (!spanId || !traceId || isError || isPending) { - return { - id: undefined, - trace: undefined, - isLoading: isPending, - }; - } - return { - id: spanId, - trace: traceId, - isLoading: false, - }; - }, [spanId, traceId, isError, isPending]); - - return nextTraceData; -} - -export function useFindPreviousTrace({ - direction, - attributes, - previousTraceEndTimestamp, - previousTraceStartTimestamp, -}: { - attributes: TraceItemResponseAttribute[]; - direction: ConnectedTraceConnection; - previousTraceEndTimestamp: number; - previousTraceStartTimestamp: number; }): { available: boolean; isLoading: boolean; - sampled: boolean; id?: string; trace?: string; } { const { projectId, environment, - previousTraceId, - previousTraceSpanId, - hasPreviousTraceLink, - previousTraceSampled, + currentTraceId, + currentSpanId, + adjacentTraceId, + adjacentTraceSpanId, + hasAdjacentTraceLink, + adjacentTraceSampled, } = useMemo(() => { let _projectId: number | undefined = undefined; let _environment: string | undefined = undefined; - let _previousTraceAttribute: TraceItemResponseAttribute | undefined = undefined; + let _currentTraceId: string | undefined; + let _currentSpanId: string | undefined; + let _adjacentTraceAttribute: TraceItemResponseAttribute | undefined = undefined; for (const a of attributes ?? []) { if (a.name === 'project_id' && a.type === 'int') { _projectId = a.value; } else if (a.name === 'environment' && a.type === 'str') { _environment = a.value; + } else if (a.name === 'trace' && a.type === 'str') { + _currentTraceId = a.value; + } else if (a.name === 'transaction.span_id' && a.type === 'str') { + _currentSpanId = a.value; } else if (a.name === 'previous_trace' && a.type === 'str') { - _previousTraceAttribute = a; + _adjacentTraceAttribute = a; } } - const _hasPreviousTraceLink = typeof _previousTraceAttribute?.value === 'string'; + const _hasAdjacentTraceLink = typeof _adjacentTraceAttribute?.value === 'string'; // In case the attribute value does not conform to `[traceId]-[spanId]-[sampledFlag]`, // the split operation will return an array with different length or unexpected contents. @@ -142,37 +67,51 @@ export function useFindPreviousTrace({ // we handle that case gracefully. Likewise we handle the case of getting an empty result. // So all in all, this should be safe and we don't have to do further validation on the // attribute content. - const [_previousTraceId, _previousTraceSpanId, _previousTraceSampledFlag] = - _hasPreviousTraceLink ? _previousTraceAttribute?.value.split('-') || [] : []; + const [_adjacentTraceId, _adjacentTraceSpanId, _adjacentTraceSampledFlag] = + _hasAdjacentTraceLink ? _adjacentTraceAttribute?.value.split('-') || [] : []; return { projectId: _projectId, environment: _environment, - hasPreviousTraceLink: _hasPreviousTraceLink, - previousTraceSampled: _previousTraceSampledFlag === '1', - previousTraceId: _previousTraceId, - previousTraceSpanId: _previousTraceSpanId, + currentTraceId: _currentTraceId, + currentSpanId: _currentSpanId, + hasAdjacentTraceLink: _hasAdjacentTraceLink, + adjacentTraceSampled: _adjacentTraceSampledFlag === '1', + adjacentTraceId: _adjacentTraceId, + adjacentTraceSpanId: _adjacentTraceSpanId, }; }, [attributes]); + const searchQuery = + direction === 'next' + ? `sentry.previous_trace:${currentTraceId}-${currentSpanId}-1` + : `id:${adjacentTraceSpanId} trace:${adjacentTraceId}`; + + const enabled = + direction === 'next' + ? !!projectId + : hasAdjacentTraceLink && + adjacentTraceSampled && + !!adjacentTraceSpanId && + !!adjacentTraceId; + const {data, isError, isPending} = useSpans( { - search: `id:${previousTraceSpanId} trace:${previousTraceId}`, + search: searchQuery, fields: ['id', 'trace'], limit: 1, - enabled: - direction === 'previous' && - hasPreviousTraceLink && - previousTraceSampled && - !!previousTraceSpanId && - !!previousTraceId, + enabled, projectIds: projectId ? [projectId] : [], pageFilters: { environments: environment ? [environment] : [], projects: projectId ? [projectId] : [], datetime: { - start: new Date(previousTraceStartTimestamp * 1000).toISOString(), - end: new Date(previousTraceEndTimestamp * 1000).toISOString(), + start: adjacentTraceStartTimestamp + ? new Date(adjacentTraceStartTimestamp * 1000).toISOString() + : '', + end: adjacentTraceEndTimestamp + ? new Date(adjacentTraceEndTimestamp * 1000).toISOString() + : '', period: null, utc: true, }, @@ -182,11 +121,33 @@ export function useFindPreviousTrace({ `api.insights.trace-panel-${direction}-trace-link` ); - return { - trace: previousTraceId, - id: previousTraceSpanId, - available: !!data?.[0]?.id && !isError, - sampled: previousTraceSampled, - isLoading: isPending, - }; + const spanId = data?.[0]?.id; + const traceId = data?.[0]?.trace; + + return useMemo(() => { + if (direction === 'next') { + return { + id: spanId, + trace: traceId, + available: !!spanId && !!traceId && !isError, + isLoading: isPending, + }; + } + + return { + trace: adjacentTraceId, + id: adjacentTraceSpanId, + available: !!data?.[0]?.id && !isError, + isLoading: isPending, + }; + }, [ + direction, + spanId, + traceId, + adjacentTraceId, + adjacentTraceSpanId, + data, + isError, + isPending, + ]); } diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index 80be77f24a0555..0b5c17df96c8f8 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -1,6 +1,5 @@ import type React from 'react'; import { - Fragment, useCallback, useEffect, useLayoutEffect, @@ -36,11 +35,7 @@ import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; import useProjects from 'sentry/utils/useProjects'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; -import {isTraceItemDetailsResponse} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; -import { - TraceLinkNavigationButton, - TraceLinkNavigationButtonPlaceHolder, -} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton'; +import {TraceLinksNavigation} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation'; import {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {TraceOpenInExploreButton} from 'sentry/views/performance/newTraceDetails/traceOpenInExploreButton'; import {traceGridCssVariables} from 'sentry/views/performance/newTraceDetails/traceWaterfallStyles'; @@ -141,12 +136,6 @@ export function TraceWaterfall(props: TraceWaterfallProps) { flushSync(rerender); }, []); - const showLinkedTraces = - organization?.features.includes('trace-view-linked-traces') && - // Don't show the linked traces buttons when the waterfall is embedded in the replay - // detail page, as it already contains all traces of the replay session. - props.source !== 'replay'; - useEffect(() => { trackAnalytics('performance_views.trace_view_v1_page_load', { organization: props.organization, @@ -740,6 +729,10 @@ export function TraceWaterfall(props: TraceWaterfallProps) { + - {showLinkedTraces && ( - - {isTraceItemDetailsResponse(props.rootEventResults.data) && - props.rootEventResults.data.timestamp ? ( - - - - - ) : ( - - )} - - )} ); } @@ -839,16 +807,6 @@ const TraceToolbar = styled('div')` gap: ${space(1)}; `; -const TraceLinksNavigationContainer = styled('div')` - display: flex; - justify-content: space-between; - flex-direction: row; - - &:not(:empty) { - margin-top: ${space(1)}; - } -`; - export const TraceGrid = styled('div')<{ layout: 'drawer bottom' | 'drawer left' | 'drawer right'; }>`