From 1eb1f9d15f6edc6b9755342fe1fbfd674ac4fb0a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 21 Jul 2025 16:14:44 +0200 Subject: [PATCH 01/15] wip --- .../traceLinkNavigationButton.tsx | 54 ++++++++++++++----- .../traceLinksNavigation/useFindNextTrace.ts | 5 ++ .../newTraceDetails/traceWaterfall.tsx | 22 +++++--- 3 files changed, 61 insertions(+), 20 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 09e27fb96a4238..27f49ef5ce5874 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -13,6 +13,7 @@ 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 type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; 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'; @@ -23,15 +24,17 @@ export type ConnectedTraceConnection = 'previous' | 'next'; const LINKED_TRACE_MAX_DURATION = 3600; // 1h in seconds function useIsTraceAvailable( - traceID?: SpanLink['trace_id'], + traceID?: string, linkedTraceTimestamp?: number ): { isAvailable: boolean; isLoading: boolean; } { + // eslint-disable-next-line no-console + console.log('xx useIsTraceAvailable', {traceID, linkedTraceTimestamp}); const trace = useTrace({ traceSlug: traceID, - timestamp: linkedTraceTimestamp, + // timestamp: linkedTraceTimestamp, }); const isAvailable = useMemo(() => { @@ -51,14 +54,14 @@ function useIsTraceAvailable( type TraceLinkNavigationButtonProps = { currentTraceTimestamps: {end?: number; start?: number}; direction: ConnectedTraceConnection; + attributes?: TraceItemResponseAttribute[]; isLoading?: boolean; projectID?: string; - traceContext?: TraceContextType; }; export function TraceLinkNavigationButton({ direction, - traceContext, + attributes, isLoading, projectID, currentTraceTimestamps, @@ -74,35 +77,62 @@ export function TraceLinkNavigationButton({ ? 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 currentTraceID = attributes?.find(a => a.name === 'trace_id' && a.type === 'str') + ?.value as string | undefined; + + const previousTraceAttribute = attributes?.find( + a => a.name === 'previous_trace' && a.type === 'str' ); + const [previousTraceID, previousTraceSpanID, previousTraceSampledFlag] = + typeof previousTraceAttribute?.value === 'string' + ? previousTraceAttribute?.value.split('-') || [] + : []; + + const previousTraceSampled = previousTraceSampledFlag === '1'; + + // eslint-disable-next-line no-console + console.log('xx previous trace', { + previousTraceSampled, + previousTraceID, + previousTraceSpanID, + }); + const nextTraceData = useFindNextTrace({ direction, - currentTraceID: traceContext?.trace_id, + currentTraceID, linkedTraceStartTimestamp: currentTraceTimestamps.end, linkedTraceEndTimestamp: linkedTraceTimestamp, projectID, }); + // eslint-disable-next-line no-console + console.log('xx next trace', { + nextTraceData, + }); + const dateSelection = useMemo( () => normalizeDateTimeParams(location.query), [location.query] ); const {isAvailable: isLinkedTraceAvailable} = useIsTraceAvailable( - direction === 'previous' ? previousTraceLink?.trace_id : nextTraceData?.trace_id, + direction === 'previous' ? previousTraceID : nextTraceData?.trace_id, linkedTraceTimestamp ); + // eslint-disable-next-line no-console + console.log('xx isLinkedTraceAvailable', { + isLinkedTraceAvailable, + }); + 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; } - if (previousTraceLink && isLinkedTraceAvailable) { + if (previousTraceID && isLinkedTraceAvailable) { return ( { trackAnalytics('performance_views.trace_view_v1_page_load', { organization: props.organization, @@ -811,25 +818,24 @@ export function TraceWaterfall(props: TraceWaterfallProps) { traceEventView={props.traceEventView} /> - {showLinkedTraces && !isTraceItemDetailsResponse(props.rootEventResults.data) && ( + {showLinkedTraces && isTraceItemDetailsResponse(props.rootEventResults.data) && ( From 7dfc1ba9ff13400137d85f921d79838a9492a992 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 22 Jul 2025 11:32:30 +0200 Subject: [PATCH 02/15] . --- .../traceLinksNavigation/useFindNextTrace.ts | 184 ++++++++++++------ 1 file changed, 124 insertions(+), 60 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts index a8716411e5ba50..afd07acb448aea 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts @@ -1,27 +1,32 @@ import {useMemo} from 'react'; import type {TraceContextType} from 'sentry/components/events/interfaces/spans/types'; +import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse'; import type {EventTransaction} from 'sentry/types/event'; -import {useApiQueries} from 'sentry/utils/queryClient'; +import {DiscoverDatasets} from 'sentry/utils/discover/types'; +import {useApiQueries, useApiQuery} from 'sentry/utils/queryClient'; import {MutableSearch} from 'sentry/utils/tokenizeSearch'; import useOrganization from 'sentry/utils/useOrganization'; -import {useSpans} from 'sentry/views/insights/common/queries/useDiscover'; -import {SpanFields} from 'sentry/views/insights/types'; +import type {TraceResult} from 'sentry/views/explore/hooks/useTraces'; import {useIsEAPTraceEnabled} from 'sentry/views/performance/newTraceDetails/useIsEAPTraceEnabled'; import type {ConnectedTraceConnection} from './traceLinkNavigationButton'; +interface TraceResults { + data: TraceResult[]; + meta: any; +} + /** - * A temporary solution for getting "next trace" data. - * As traces currently only include information about the previous trace, the next trace is obtained by - * getting all root spans in a certain timespan (e.g. 1h) and searching the root span which has the - * current trace as it's "previous trace". + * Updated solution for getting "next trace" data using the same endpoint and query mechanism + * as the explore view. This uses the /traces/ endpoint with EAP dataset for better performance + * and consistency. */ export function useFindNextTrace({ direction, currentTraceID, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - currentSpanId, + + // currentSpanId, linkedTraceStartTimestamp, linkedTraceEndTimestamp, projectID, @@ -33,77 +38,136 @@ export function useFindNextTrace({ linkedTraceStartTimestamp?: number; projectID?: string; }): TraceContextType | undefined { - // const previousTraceAttributeValueOnNextTraceRootSpan = `${currentTraceID}-${currentSpanId}-1`; + const organization = useOrganization(); + const isEAP = useIsEAPTraceEnabled(); - const {data: indexedSpans} = useSpans( - { - limit: direction === 'next' && projectID ? 100 : 1, - noPagination: true, - pageFilters: { - projects: projectID ? [Number(projectID)] : [], - environments: [], - datetime: { - period: null, - utc: null, - start: linkedTraceStartTimestamp - ? new Date(linkedTraceStartTimestamp * 1000).toISOString() - : null, - end: linkedTraceEndTimestamp - ? new Date(linkedTraceEndTimestamp * 1000).toISOString() - : null, + // Build query using MutableSearch like explore view does + const query = useMemo(() => { + const search = new MutableSearch(''); + + // Only search in the specific project if provided + if (projectID) { + search.addFilterValue('project', projectID); + } + // search.addFilterValue('previous_trace', `${currentTraceID}-${currentSpanId}-1`); + + return search.formatString(); + }, [projectID]); + + // Use the same traces endpoint and patterns as explore view + const tracesQuery = useApiQuery( + [ + `/organizations/${organization.slug}/traces/`, + { + query: { + project: projectID ? [Number(projectID)] : [], + environment: [], + ...normalizeDateTimeParams({ + period: null, + utc: null, + start: linkedTraceStartTimestamp + ? new Date(linkedTraceStartTimestamp * 1000).toISOString() + : null, + end: linkedTraceEndTimestamp + ? new Date(linkedTraceEndTimestamp * 1000).toISOString() + : null, + }), + dataset: DiscoverDatasets.SPANS_EAP, + query, + sort: '-timestamp', + per_page: direction === 'next' ? 100 : 1, + breakdownSlices: 40, }, }, - search: MutableSearch.fromQueryObject({is_transaction: 1}), - fields: [SpanFields.TRANSACTION_SPAN_ID, SpanFields.PROJECT_ID, SpanFields.PROJECT], - }, - 'api.trace-view.linked-traces' + ], + { + staleTime: 0, + refetchOnWindowFocus: false, + refetchOnMount: false, + retry: false, + enabled: Boolean( + isEAP && + currentTraceID && + linkedTraceStartTimestamp && + linkedTraceEndTimestamp && + direction === 'next' + ), + } ); - const traceData = indexedSpans.map(span => ({ - projectSlug: span.project, - eventId: span['transaction.span_id'], - })); + // Extract potential trace data for root event fetching + const traceData = useMemo(() => { + if (!tracesQuery.data?.data) { + return []; + } + return tracesQuery.data.data.map(trace => ({ + projectSlug: trace.project ?? undefined, + eventId: undefined, // We'll need to find the root span event ID + traceId: trace.trace, + })); + }, [tracesQuery.data]); + + // Fetch root events for traces to check for trace links const rootEvents = useTraceRootEvents(traceData); - const nextTrace = rootEvents.find(rootEvent => { - const traceContext = rootEvent.data?.contexts?.trace; - const hasMatchingLink = traceContext?.links?.some( - link => - link.attributes?.['sentry.link.type'] === `previous_trace` && - link.trace_id === currentTraceID - ); + // Find the next trace based on trace links + const nextTrace = useMemo(() => { + const foundRootEvent = rootEvents.find(rootEvent => { + const traceContext = rootEvent.data?.contexts?.trace; + const hasMatchingLink = traceContext?.links?.some( + link => + link.attributes?.['sentry.link.type'] === 'previous_trace' && + link.trace_id === currentTraceID + ); + + return hasMatchingLink; + }); - return hasMatchingLink; - }); + return foundRootEvent?.data?.contexts.trace; + }, [rootEvents, currentTraceID]); - return nextTrace?.data?.contexts.trace; + return nextTrace; } // Similar to `useTraceRootEvent` but allows fetching data for "more than one" trace data function useTraceRootEvents( - traceData: Array<{eventId?: string; projectSlug?: string}> | null + traceData: Array<{eventId?: string; projectSlug?: string; traceId?: string}> | null ) { const organization = useOrganization(); const isEAP = useIsEAPTraceEnabled(); const queryKeys = useMemo(() => { - if (!traceData) { + if (!traceData || isEAP) { return []; } - return traceData.map( - trace => - [ - `/organizations/${organization.slug}/events/${trace?.projectSlug}:${trace.eventId}/`, - {query: {referrer: 'trace-details-summary'}}, - ] as const - ); - }, [traceData, organization.slug]); - - return useApiQueries(queryKeys, { - // 10 minutes - staleTime: 1000 * 60 * 10, - enabled: Array.isArray(traceData) && traceData.length > 0 && !isEAP, - }); + // For now, we'll need to find root spans for each trace to get the event data + // This is a limitation until we have a better way to get trace link data directly + return traceData + .filter(trace => trace.projectSlug && trace.traceId) + .map( + trace => + [ + `/organizations/${organization.slug}/events/`, + { + query: { + field: ['contexts.trace', 'id', 'project'], + query: `trace:${trace.traceId} is_transaction:1`, + sort: '-timestamp', + per_page: 1, + referrer: 'trace-links-navigation', + }, + }, + ] as const + ); + }, [traceData, organization.slug, isEAP]); + + return useApiQueries<{data: EventTransaction[]}>(queryKeys, { + staleTime: 1000 * 60 * 10, // 10 minutes + enabled: queryKeys.length > 0, + }).map(result => ({ + ...result, + data: result.data?.data?.[0], // Get the first (root) transaction + })); } From 4d92ef259748275b58ac9a9454328a0959470d8d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 12:06:06 +0200 Subject: [PATCH 03/15] wip - both links work --- .../insights/common/queries/useDiscover.ts | 16 +- .../insights/common/queries/useSpansQuery.tsx | 2 +- .../traceLinkNavigationButton.tsx | 234 +++++++-------- .../traceLinksNavigation/useFindNextTrace.ts | 281 ++++++++---------- .../newTraceDetails/traceWaterfall.tsx | 13 +- 5 files changed, 252 insertions(+), 294 deletions(-) diff --git a/static/app/views/insights/common/queries/useDiscover.ts b/static/app/views/insights/common/queries/useDiscover.ts index e257c4f2069eb9..32a0e3bc756a4b 100644 --- a/static/app/views/insights/common/queries/useDiscover.ts +++ b/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 { 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 = >, Respo orderby ); - const result = useWrappedDiscoverQuery({ + const queryFn = options.queryWithoutPageFilters + ? useWrappedDiscoverQueryWithoutPageFilters + : useWrappedDiscoverQuery; + + const result = queryFn({ eventView, initialData: [], limit, diff --git a/static/app/views/insights/common/queries/useSpansQuery.tsx b/static/app/views/insights/common/queries/useSpansQuery.tsx index 65679b5aebc6d5..7f384e81af7559 100644 --- a/static/app/views/insights/common/queries/useSpansQuery.tsx +++ b/static/app/views/insights/common/queries/useSpansQuery.tsx @@ -303,7 +303,7 @@ export function useWrappedDiscoverQuery(props: WrappedDiscoverQueryProps) return useWrappedDiscoverQueryBase({...props, pageFiltersReady}); } -function useWrappedDiscoverQueryWithoutPageFilters( +export function useWrappedDiscoverQueryWithoutPageFilters( props: WrappedDiscoverQueryProps ) { return useWrappedDiscoverQueryBase({...props, pageFiltersReady: true}); diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 27f49ef5ce5874..2b16421f73a3af 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -3,10 +3,6 @@ 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'; @@ -14,56 +10,27 @@ import {space} from 'sentry/styles/space'; import {useLocation} from 'sentry/utils/useLocation'; import useOrganization from 'sentry/utils/useOrganization'; import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; -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 { + useFindNextTrace, + useFindPreviousTrace, +} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace'; 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?: string, - linkedTraceTimestamp?: number -): { - isAvailable: boolean; - isLoading: boolean; -} { - // eslint-disable-next-line no-console - console.log('xx useIsTraceAvailable', {traceID, linkedTraceTimestamp}); - 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}; direction: ConnectedTraceConnection; attributes?: TraceItemResponseAttribute[]; isLoading?: boolean; - projectID?: string; }; export function TraceLinkNavigationButton({ direction, attributes, - isLoading, - projectID, + isLoading: isWaterfallLoading, currentTraceTimestamps, }: TraceLinkNavigationButtonProps) { const organization = useOrganization(); @@ -77,38 +44,27 @@ export function TraceLinkNavigationButton({ ? currentTraceTimestamps.end + LINKED_TRACE_MAX_DURATION // Latest end time of next trace (+ 1h) : undefined; - const currentTraceID = attributes?.find(a => a.name === 'trace_id' && a.type === 'str') - ?.value as string | undefined; - - const previousTraceAttribute = attributes?.find( - a => a.name === 'previous_trace' && a.type === 'str' - ); - - const [previousTraceID, previousTraceSpanID, previousTraceSampledFlag] = - typeof previousTraceAttribute?.value === 'string' - ? previousTraceAttribute?.value.split('-') || [] - : []; - - const previousTraceSampled = previousTraceSampledFlag === '1'; - - // eslint-disable-next-line no-console - console.log('xx previous trace', { - previousTraceSampled, - previousTraceID, - previousTraceSpanID, - }); - - const nextTraceData = useFindNextTrace({ + const { + available: isPreviousTraceAvailable, + id: previousTraceSpanId, + trace: previousTraceId, + sampled: previousTraceSampled, + isLoading: isPreviousTraceLoading, + } = useFindPreviousTrace({ direction, - currentTraceID, - linkedTraceStartTimestamp: currentTraceTimestamps.end, - linkedTraceEndTimestamp: linkedTraceTimestamp, - projectID, + linkedTraceTimestamp, + attributes, }); - // eslint-disable-next-line no-console - console.log('xx next trace', { - nextTraceData, + const { + id: nextTraceSpanId, + trace: nextTraceId, + isLoading: isNextTraceLoading, + } = useFindNextTrace({ + direction, + nextTraceEndTimestamp: currentTraceTimestamps.end ?? 0, + nextTraceStartTimestamp: linkedTraceTimestamp ?? 0, + attributes, }); const dateSelection = useMemo( @@ -116,60 +72,76 @@ export function TraceLinkNavigationButton({ [location.query] ); - const {isAvailable: isLinkedTraceAvailable} = useIsTraceAvailable( - direction === 'previous' ? previousTraceID : nextTraceData?.trace_id, - linkedTraceTimestamp - ); + if (isWaterfallLoading) { + return ; + } - // eslint-disable-next-line no-console - console.log('xx isLinkedTraceAvailable', { - isLinkedTraceAvailable, - }); + if (direction === 'previous' && previousTraceId && !isPreviousTraceLoading) { + if (isPreviousTraceAvailable) { + return ( + + ), + } + )} + > + + + {t('Go to Previous Trace')} + + + ); + } - 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; - } + if (!previousTraceSampled) { + return ( + + {t('Previous trace not sampled')} + + ); + } - if (previousTraceID && isLinkedTraceAvailable) { - return ( - - ), - } - )} - > - - - {t('Go to Previous Trace')} - - - ); + {t('Previous trace not available')} + + ); + } } - if (nextTraceData?.trace_id && nextTraceData.span_id && isLinkedTraceAvailable) { + if (direction === 'next' && !isNextTraceLoading && nextTraceId && nextTraceSpanId) { return ( - {t('Previous trace not available')} - - ); - } + // 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 ; +} - // If there is no linked trace or an undefined sampling decision - return null; +function PlaceHolder() { + return  ; } +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)}; `; diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts index afd07acb448aea..80b6f76ff689e4 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts @@ -1,173 +1,152 @@ import {useMemo} from 'react'; -import type {TraceContextType} from 'sentry/components/events/interfaces/spans/types'; -import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse'; -import type {EventTransaction} from 'sentry/types/event'; -import {DiscoverDatasets} from 'sentry/utils/discover/types'; -import {useApiQueries, useApiQuery} from 'sentry/utils/queryClient'; -import {MutableSearch} from 'sentry/utils/tokenizeSearch'; -import useOrganization from 'sentry/utils/useOrganization'; -import type {TraceResult} from 'sentry/views/explore/hooks/useTraces'; -import {useIsEAPTraceEnabled} from 'sentry/views/performance/newTraceDetails/useIsEAPTraceEnabled'; +import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; +import {useSpans} from 'sentry/views/insights/common/queries/useDiscover'; +import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; +import {isEmptyTrace} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import type {ConnectedTraceConnection} from './traceLinkNavigationButton'; -interface TraceResults { - data: TraceResult[]; - meta: any; -} - /** - * Updated solution for getting "next trace" data using the same endpoint and query mechanism - * as the explore view. This uses the /traces/ endpoint with EAP dataset for better performance - * and consistency. + * Find the next trace by looking using the spans endpoint to query for a trace + * linking to the current trace as its previous trace. */ export function useFindNextTrace({ direction, - currentTraceID, - - // currentSpanId, - linkedTraceStartTimestamp, - linkedTraceEndTimestamp, - projectID, + attributes, + nextTraceEndTimestamp, + nextTraceStartTimestamp, }: { direction: ConnectedTraceConnection; - currentSpanId?: string; - currentTraceID?: string; - linkedTraceEndTimestamp?: number; - linkedTraceStartTimestamp?: number; - projectID?: string; -}): TraceContextType | undefined { - const organization = useOrganization(); - const isEAP = useIsEAPTraceEnabled(); - - // Build query using MutableSearch like explore view does - const query = useMemo(() => { - const search = new MutableSearch(''); - - // Only search in the specific project if provided - if (projectID) { - search.addFilterValue('project', projectID); - } - // search.addFilterValue('previous_trace', `${currentTraceID}-${currentSpanId}-1`); - - return search.formatString(); - }, [projectID]); - - // Use the same traces endpoint and patterns as explore view - const tracesQuery = useApiQuery( - [ - `/organizations/${organization.slug}/traces/`, - { - query: { - project: projectID ? [Number(projectID)] : [], - environment: [], - ...normalizeDateTimeParams({ - period: null, - utc: null, - start: linkedTraceStartTimestamp - ? new Date(linkedTraceStartTimestamp * 1000).toISOString() - : null, - end: linkedTraceEndTimestamp - ? new Date(linkedTraceEndTimestamp * 1000).toISOString() - : null, - }), - dataset: DiscoverDatasets.SPANS_EAP, - query, - sort: '-timestamp', - per_page: direction === 'next' ? 100 : 1, - breakdownSlices: 40, + nextTraceEndTimestamp: number; + nextTraceStartTimestamp: number; + attributes?: TraceItemResponseAttribute[]; +}): {isLoading: boolean; id?: string; trace?: string} { + const currentTraceId = attributes?.find(a => a.name === 'trace' && a.type === 'str') + ?.value as string | undefined; + + const currentSpanId = attributes?.find( + a => a.name === 'transaction.span_id' && a.type === 'str' + )?.value as string | undefined; + + const projectId = attributes?.find(a => a.name === 'project_id' && a.type === 'int') + ?.value as number | undefined; + + const environment = attributes?.find(a => a.name === 'environment' && a.type === 'str') + ?.value as string | undefined; + + const {data, isError, isPending} = useSpans( + { + search: `sentry.previous_trace:${currentTraceId}-${currentSpanId}-1`, + fields: ['id', 'trace'], + limit: 1, + enabled: direction === 'next' && !!projectId, + projectIds: [projectId ?? 0], + pageFilters: { + environments: [environment ?? ''], + projects: [projectId ?? 0], + datetime: { + start: nextTraceStartTimestamp + ? new Date(nextTraceStartTimestamp * 1000).toISOString() + : '', + end: nextTraceEndTimestamp + ? new Date(nextTraceEndTimestamp * 1000).toISOString() + : '', + period: '1d', + utc: true, }, }, - ], - { - staleTime: 0, - refetchOnWindowFocus: false, - refetchOnMount: false, - retry: false, - enabled: Boolean( - isEAP && - currentTraceID && - linkedTraceStartTimestamp && - linkedTraceEndTimestamp && - direction === 'next' - ), - } + queryWithoutPageFilters: true, + }, + `api.performance.trace-panel-${direction}-trace-link` ); - // Extract potential trace data for root event fetching - const traceData = useMemo(() => { - if (!tracesQuery.data?.data) { - return []; + const nextTraceData = useMemo(() => { + if (!data?.[0]?.id || !data?.[0]?.trace || isError || isPending) { + return { + id: undefined, + trace: undefined, + isLoading: isPending, + }; } - - return tracesQuery.data.data.map(trace => ({ - projectSlug: trace.project ?? undefined, - eventId: undefined, // We'll need to find the root span event ID - traceId: trace.trace, - })); - }, [tracesQuery.data]); - - // Fetch root events for traces to check for trace links - const rootEvents = useTraceRootEvents(traceData); - - // Find the next trace based on trace links - const nextTrace = useMemo(() => { - const foundRootEvent = rootEvents.find(rootEvent => { - const traceContext = rootEvent.data?.contexts?.trace; - const hasMatchingLink = traceContext?.links?.some( - link => - link.attributes?.['sentry.link.type'] === 'previous_trace' && - link.trace_id === currentTraceID - ); - - return hasMatchingLink; - }); - - return foundRootEvent?.data?.contexts.trace; - }, [rootEvents, currentTraceID]); - - return nextTrace; + return { + id: data[0].id, + trace: data[0].trace, + isLoading: false, + }; + }, [data, isError, isPending]); + + return nextTraceData; } -// Similar to `useTraceRootEvent` but allows fetching data for "more than one" trace data -function useTraceRootEvents( - traceData: Array<{eventId?: string; projectSlug?: string; traceId?: string}> | null -) { - const organization = useOrganization(); - const isEAP = useIsEAPTraceEnabled(); +export function useFindPreviousTrace({ + direction, + attributes, + linkedTraceTimestamp, +}: { + direction: ConnectedTraceConnection; + attributes?: TraceItemResponseAttribute[]; + linkedTraceTimestamp?: number; +}): { + available: boolean; + isLoading: boolean; + sampled: boolean; + id?: string; + trace?: string; +} { + const previousTraceAttribute = useMemo( + () => attributes?.find(a => a.name === 'previous_trace' && a.type === 'str'), + [attributes] + ); + + const hasPreviousTraceLink = typeof previousTraceAttribute?.value === 'string'; + + const [previousTraceId, previousTraceSpanId, previousTraceSampledFlag] = + hasPreviousTraceLink ? previousTraceAttribute?.value.split('-') || [] : []; - const queryKeys = useMemo(() => { - if (!traceData || isEAP) { - return []; + const sampled = previousTraceSampledFlag === '1'; + + const queryFn = + direction === 'previous' && hasPreviousTraceLink && sampled + ? useIsTraceAvailable + : () => ({ + isAvailable: false, + isLoading: false, + }); + + const {isAvailable, isLoading} = queryFn(previousTraceId, linkedTraceTimestamp); + + return { + trace: previousTraceId, + id: previousTraceSpanId, + available: isAvailable, + sampled, + isLoading, + }; +} + +function useIsTraceAvailable( + traceID?: string, + linkedTraceTimestamp?: number +): { + isAvailable: boolean; + isLoading: boolean; +} { + const trace = useTrace({ + traceSlug: traceID, + timestamp: linkedTraceTimestamp, + }); + + const isAvailable = useMemo(() => { + if (!traceID) { + return false; } - // For now, we'll need to find root spans for each trace to get the event data - // This is a limitation until we have a better way to get trace link data directly - return traceData - .filter(trace => trace.projectSlug && trace.traceId) - .map( - trace => - [ - `/organizations/${organization.slug}/events/`, - { - query: { - field: ['contexts.trace', 'id', 'project'], - query: `trace:${trace.traceId} is_transaction:1`, - sort: '-timestamp', - per_page: 1, - referrer: 'trace-links-navigation', - }, - }, - ] as const - ); - }, [traceData, organization.slug, isEAP]); - - return useApiQueries<{data: EventTransaction[]}>(queryKeys, { - staleTime: 1000 * 60 * 10, // 10 minutes - enabled: queryKeys.length > 0, - }).map(result => ({ - ...result, - data: result.data?.data?.[0], // Get the first (root) transaction - })); + return Boolean(trace.data && !isEmptyTrace(trace.data)); + }, [traceID, trace]); + + return { + isAvailable, + isLoading: trace.isLoading, + }; } diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index 29b51f14d8dbf6..56d96e789fd50a 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -142,14 +142,11 @@ export function TraceWaterfall(props: TraceWaterfallProps) { const {timestamp} = useTraceQueryParams(); - const showLinkedTraces = organization?.features.includes('trace-view-linked-traces'); - - // eslint-disable-next-line no-console - console.log('xx showLinkedTraces', showLinkedTraces); - // eslint-disable-next-line no-console - console.log('xx', props.rootEventResults.data); - // eslint-disable-next-line no-console - console.log('cc', isTraceItemDetailsResponse(props.rootEventResults.data)); + 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', { From e3e74a197d82143af1c4050156500ba6f468acc3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 12:35:10 +0200 Subject: [PATCH 04/15] always useSpans, make lookup more efficient --- .../traceLinkNavigationButton.tsx | 21 ++- .../traceLinksNavigation/useFindNextTrace.ts | 135 ++++++++++-------- 2 files changed, 85 insertions(+), 71 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 2b16421f73a3af..ab01d4f4efe184 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -21,9 +21,9 @@ export type ConnectedTraceConnection = 'previous' | 'next'; const LINKED_TRACE_MAX_DURATION = 3600; // 1h in seconds type TraceLinkNavigationButtonProps = { - currentTraceTimestamps: {end?: number; start?: number}; + attributes: TraceItemResponseAttribute[]; + currentTraceTimestamps: {end: number; start: number}; direction: ConnectedTraceConnection; - attributes?: TraceItemResponseAttribute[]; isLoading?: boolean; }; @@ -37,12 +37,10 @@ export function TraceLinkNavigationButton({ 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 = + const linkedTraceWindowTimestamp = 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; + : currentTraceTimestamps.end + LINKED_TRACE_MAX_DURATION; // Latest end time of next trace (+ 1h) const { available: isPreviousTraceAvailable, @@ -52,7 +50,8 @@ export function TraceLinkNavigationButton({ isLoading: isPreviousTraceLoading, } = useFindPreviousTrace({ direction, - linkedTraceTimestamp, + previousTraceEndTimestamp: currentTraceTimestamps.start, + previousTraceStartTimestamp: linkedTraceWindowTimestamp, attributes, }); @@ -62,8 +61,8 @@ export function TraceLinkNavigationButton({ isLoading: isNextTraceLoading, } = useFindNextTrace({ direction, - nextTraceEndTimestamp: currentTraceTimestamps.end ?? 0, - nextTraceStartTimestamp: linkedTraceTimestamp ?? 0, + nextTraceEndTimestamp: currentTraceTimestamps.end, + nextTraceStartTimestamp: linkedTraceWindowTimestamp, attributes, }); @@ -102,7 +101,7 @@ export function TraceLinkNavigationButton({ traceSlug: previousTraceId, spanId: previousTraceSpanId, dateSelection, - timestamp: linkedTraceTimestamp, + timestamp: linkedTraceWindowTimestamp, location, organization, })} @@ -166,7 +165,7 @@ export function TraceLinkNavigationButton({ traceSlug: nextTraceId, spanId: nextTraceSpanId, dateSelection, - timestamp: linkedTraceTimestamp, + timestamp: linkedTraceWindowTimestamp, location, organization, })} diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts index 80b6f76ff689e4..77e7bc497ffa4c 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts @@ -2,8 +2,6 @@ import {useMemo} from 'react'; import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; import {useSpans} from 'sentry/views/insights/common/queries/useDiscover'; -import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; -import {isEmptyTrace} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import type {ConnectedTraceConnection} from './traceLinkNavigationButton'; @@ -17,23 +15,35 @@ export function useFindNextTrace({ nextTraceEndTimestamp, nextTraceStartTimestamp, }: { + attributes: TraceItemResponseAttribute[]; direction: ConnectedTraceConnection; nextTraceEndTimestamp: number; nextTraceStartTimestamp: number; - attributes?: TraceItemResponseAttribute[]; }): {isLoading: boolean; id?: string; trace?: string} { - const currentTraceId = attributes?.find(a => a.name === 'trace' && a.type === 'str') - ?.value as string | undefined; - - const currentSpanId = attributes?.find( - a => a.name === 'transaction.span_id' && a.type === 'str' - )?.value as string | undefined; - - const projectId = attributes?.find(a => a.name === 'project_id' && a.type === 'int') - ?.value as number | undefined; - - const environment = attributes?.find(a => a.name === 'environment' && a.type === 'str') - ?.value as string | undefined; + 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( { @@ -52,7 +62,7 @@ export function useFindNextTrace({ end: nextTraceEndTimestamp ? new Date(nextTraceEndTimestamp * 1000).toISOString() : '', - period: '1d', + period: '2h', utc: true, }, }, @@ -82,11 +92,13 @@ export function useFindNextTrace({ export function useFindPreviousTrace({ direction, attributes, - linkedTraceTimestamp, + previousTraceEndTimestamp, + previousTraceStartTimestamp, }: { + attributes: TraceItemResponseAttribute[]; direction: ConnectedTraceConnection; - attributes?: TraceItemResponseAttribute[]; - linkedTraceTimestamp?: number; + previousTraceEndTimestamp: number; + previousTraceStartTimestamp: number; }): { available: boolean; isLoading: boolean; @@ -94,10 +106,27 @@ export function useFindPreviousTrace({ id?: string; trace?: string; } { - const previousTraceAttribute = useMemo( - () => attributes?.find(a => a.name === 'previous_trace' && a.type === 'str'), - [attributes] - ); + const {projectId, environment, previousTraceAttribute} = useMemo(() => { + let _projectId: number | undefined = undefined; + let _environment: string | undefined = undefined; + let _previousTraceAttribute: 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 === 'previous_trace' && a.type === 'str') { + _previousTraceAttribute = a; + } + } + + return { + projectId: _projectId, + environment: _environment, + previousTraceAttribute: _previousTraceAttribute, + }; + }, [attributes]); const hasPreviousTraceLink = typeof previousTraceAttribute?.value === 'string'; @@ -106,47 +135,33 @@ export function useFindPreviousTrace({ const sampled = previousTraceSampledFlag === '1'; - const queryFn = - direction === 'previous' && hasPreviousTraceLink && sampled - ? useIsTraceAvailable - : () => ({ - isAvailable: false, - isLoading: false, - }); - - const {isAvailable, isLoading} = queryFn(previousTraceId, linkedTraceTimestamp); + const {data, isError, isPending} = useSpans( + { + search: `id:${previousTraceSpanId} trace:${previousTraceId}`, + fields: ['id', 'trace'], + limit: 1, + enabled: direction === 'previous' && hasPreviousTraceLink && sampled, + projectIds: [projectId ?? 0], + pageFilters: { + environments: [environment ?? ''], + projects: [projectId ?? 0], + datetime: { + start: new Date(previousTraceStartTimestamp * 1000).toISOString(), + end: new Date(previousTraceEndTimestamp * 1000).toISOString(), + period: '2h', + utc: true, + }, + }, + queryWithoutPageFilters: true, + }, + `api.performance.trace-panel-${direction}-trace-link` + ); return { trace: previousTraceId, id: previousTraceSpanId, - available: isAvailable, + available: !!data?.[0]?.id && !isError, sampled, - isLoading, - }; -} - -function useIsTraceAvailable( - traceID?: string, - 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, + isLoading: isPending, }; } From d61cc7a8f8d923815d8e769fb7281bfd87269c94 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 13:08:43 +0200 Subject: [PATCH 05/15] remove layout shift --- .../traceLinkNavigationButton.tsx | 14 ++---- .../traceLinksNavigation/useFindNextTrace.ts | 4 +- .../newTraceDetails/traceWaterfall.tsx | 50 +++++++++++-------- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index ab01d4f4efe184..1f42358a955efb 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -24,13 +24,11 @@ type TraceLinkNavigationButtonProps = { attributes: TraceItemResponseAttribute[]; currentTraceTimestamps: {end: number; start: number}; direction: ConnectedTraceConnection; - isLoading?: boolean; }; export function TraceLinkNavigationButton({ direction, attributes, - isLoading: isWaterfallLoading, currentTraceTimestamps, }: TraceLinkNavigationButtonProps) { const organization = useOrganization(); @@ -61,8 +59,8 @@ export function TraceLinkNavigationButton({ isLoading: isNextTraceLoading, } = useFindNextTrace({ direction, - nextTraceEndTimestamp: currentTraceTimestamps.end, - nextTraceStartTimestamp: linkedTraceWindowTimestamp, + nextTraceEndTimestamp: linkedTraceWindowTimestamp, + nextTraceStartTimestamp: currentTraceTimestamps.end, attributes, }); @@ -71,10 +69,6 @@ export function TraceLinkNavigationButton({ [location.query] ); - if (isWaterfallLoading) { - return ; - } - if (direction === 'previous' && previousTraceId && !isPreviousTraceLoading) { if (isPreviousTraceAvailable) { return ( @@ -180,10 +174,10 @@ export function TraceLinkNavigationButton({ // 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 ; + return ; } -function PlaceHolder() { +export function TraceLinkNavigationButtonPlaceHolder() { return  ; } diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts index 77e7bc497ffa4c..992062f3f8f1fc 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts @@ -62,7 +62,7 @@ export function useFindNextTrace({ end: nextTraceEndTimestamp ? new Date(nextTraceEndTimestamp * 1000).toISOString() : '', - period: '2h', + period: null, utc: true, }, }, @@ -148,7 +148,7 @@ export function useFindPreviousTrace({ datetime: { start: new Date(previousTraceStartTimestamp * 1000).toISOString(), end: new Date(previousTraceEndTimestamp * 1000).toISOString(), - period: '2h', + period: null, utc: true, }, }, diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index 56d96e789fd50a..fd76c44e12b6a7 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -1,5 +1,6 @@ import type React from 'react'; import { + Fragment, useCallback, useEffect, useLayoutEffect, @@ -38,7 +39,10 @@ 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} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton'; +import { + TraceLinkNavigationButton, + TraceLinkNavigationButtonPlaceHolder, +} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton'; 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'; @@ -815,26 +819,32 @@ export function TraceWaterfall(props: TraceWaterfallProps) { traceEventView={props.traceEventView} /> - {showLinkedTraces && isTraceItemDetailsResponse(props.rootEventResults.data) && ( + {showLinkedTraces && ( - - + {isTraceItemDetailsResponse(props.rootEventResults.data) ? ( + + + + + ) : ( + + )} )} From c2010a79af1f76059920c70701532016c3867a09 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 13:12:25 +0200 Subject: [PATCH 06/15] rename file --- .../{useFindNextTrace.ts => useFindLinkedTraces.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename static/app/views/performance/newTraceDetails/traceLinksNavigation/{useFindNextTrace.ts => useFindLinkedTraces.ts} (100%) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts similarity index 100% rename from static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts rename to static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts From d855a71b7027bb63661e081b4748430368739adf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 13:16:56 +0200 Subject: [PATCH 07/15] fix import --- .../traceLinksNavigation/traceLinkNavigationButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 1f42358a955efb..5d2689ddc946a5 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -13,7 +13,7 @@ import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTra import { useFindNextTrace, useFindPreviousTrace, -} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace'; +} from 'sentry/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces'; import {getTraceDetailsUrl} from 'sentry/views/performance/traceDetails/utils'; export type ConnectedTraceConnection = 'previous' | 'next'; From d1b93722574e3fb7b85bc1206e044978da4cab9e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 13:30:33 +0200 Subject: [PATCH 08/15] better fallback value handling --- .../traceLinksNavigation/useFindLinkedTraces.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts index 992062f3f8f1fc..75ea4e3548a579 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts @@ -51,10 +51,10 @@ export function useFindNextTrace({ fields: ['id', 'trace'], limit: 1, enabled: direction === 'next' && !!projectId, - projectIds: [projectId ?? 0], + projectIds: projectId ? [projectId] : [], pageFilters: { - environments: [environment ?? ''], - projects: [projectId ?? 0], + environments: environment ? [environment] : [], + projects: projectId ? [projectId] : [], datetime: { start: nextTraceStartTimestamp ? new Date(nextTraceStartTimestamp * 1000).toISOString() @@ -141,10 +141,10 @@ export function useFindPreviousTrace({ fields: ['id', 'trace'], limit: 1, enabled: direction === 'previous' && hasPreviousTraceLink && sampled, - projectIds: [projectId ?? 0], + projectIds: projectId ? [projectId] : [], pageFilters: { - environments: [environment ?? ''], - projects: [projectId ?? 0], + environments: environment ? [environment] : [], + projects: projectId ? [projectId] : [], datetime: { start: new Date(previousTraceStartTimestamp * 1000).toISOString(), end: new Date(previousTraceEndTimestamp * 1000).toISOString(), From 5ee940ecf3903364454d3ee476add72f9c1e0162 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 13:46:34 +0200 Subject: [PATCH 09/15] simplify current trace timestamp --- .../traceLinkNavigationButton.tsx | 14 +++++++------- .../newTraceDetails/traceWaterfall.tsx | 16 ++++++---------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 5d2689ddc946a5..5b0fa99e0cceca 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -22,23 +22,23 @@ const LINKED_TRACE_MAX_DURATION = 3600; // 1h in seconds type TraceLinkNavigationButtonProps = { attributes: TraceItemResponseAttribute[]; - currentTraceTimestamps: {end: number; start: number}; + currentTraceStartTimestamp: number; direction: ConnectedTraceConnection; }; export function TraceLinkNavigationButton({ direction, attributes, - currentTraceTimestamps, + 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 linkedTraceWindowTimestamp = - direction === 'previous' && currentTraceTimestamps.start - ? currentTraceTimestamps.start - LINKED_TRACE_MAX_DURATION // Earliest start time of previous trace (- 1h) - : currentTraceTimestamps.end + LINKED_TRACE_MAX_DURATION; // Latest end time of next trace (+ 1h) + 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, @@ -48,7 +48,7 @@ export function TraceLinkNavigationButton({ isLoading: isPreviousTraceLoading, } = useFindPreviousTrace({ direction, - previousTraceEndTimestamp: currentTraceTimestamps.start, + previousTraceEndTimestamp: currentTraceStartTimestamp, previousTraceStartTimestamp: linkedTraceWindowTimestamp, attributes, }); @@ -60,7 +60,7 @@ export function TraceLinkNavigationButton({ } = useFindNextTrace({ direction, nextTraceEndTimestamp: linkedTraceWindowTimestamp, - nextTraceStartTimestamp: currentTraceTimestamps.end, + nextTraceStartTimestamp: currentTraceStartTimestamp, attributes, }); diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index fd76c44e12b6a7..21877665662d88 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -826,20 +826,16 @@ export function TraceWaterfall(props: TraceWaterfallProps) { ) : ( From 151f8373d369c74e3de8c70f23ba93e791ec55a7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Jul 2025 14:30:52 +0200 Subject: [PATCH 10/15] knip and one more cursor nit --- static/app/components/events/interfaces/spans/types.tsx | 2 +- .../views/performance/newTraceDetails/traceWaterfall.tsx | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/static/app/components/events/interfaces/spans/types.tsx b/static/app/components/events/interfaces/spans/types.tsx index e73c8f695981c7..6ae84f73451bed 100644 --- a/static/app/components/events/interfaces/spans/types.tsx +++ b/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 & {'sentry.link.type'?: AttributeValue}; diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index 21877665662d88..8c7a666f7c4fd6 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -821,20 +821,21 @@ export function TraceWaterfall(props: TraceWaterfallProps) { {showLinkedTraces && ( - {isTraceItemDetailsResponse(props.rootEventResults.data) ? ( + {isTraceItemDetailsResponse(props.rootEventResults.data) && + props.rootEventResults.data?.timestamp ? ( From 7ea7c11c35ed51e7c9c6faf099c38e26cc3f8dc3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Jul 2025 10:24:23 +0200 Subject: [PATCH 11/15] dispatch close details drawer (doesn't work yet) --- .../traceLinkNavigationButton.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index 5b0fa99e0cceca..f6b1dbbe34bf06 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -14,6 +14,7 @@ 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'; @@ -69,6 +70,15 @@ export function TraceLinkNavigationButton({ [location.query] ); + const traceDispatch = useTraceStateDispatch(); + + function closeSpanDetailsDrawer() { + traceDispatch({ + type: 'minimize drawer', + payload: false, + }); + } + if (direction === 'previous' && previousTraceId && !isPreviousTraceLoading) { if (isPreviousTraceAvailable) { return ( @@ -91,6 +101,7 @@ export function TraceLinkNavigationButton({ > closeSpanDetailsDrawer()} to={getTraceDetailsUrl({ traceSlug: previousTraceId, spanId: previousTraceSpanId, @@ -155,6 +166,7 @@ export function TraceLinkNavigationButton({ > Date: Tue, 29 Jul 2025 11:16:31 +0200 Subject: [PATCH 12/15] optimize useMemo usage --- .../traceLinksNavigation/useFindLinkedTraces.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts index 75ea4e3548a579..facd7ad1c2cdce 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts @@ -71,8 +71,11 @@ export function useFindNextTrace({ `api.performance.trace-panel-${direction}-trace-link` ); + const spanId = data?.[0]?.id; + const traceId = data?.[0]?.trace; + const nextTraceData = useMemo(() => { - if (!data?.[0]?.id || !data?.[0]?.trace || isError || isPending) { + if (!spanId || !traceId || isError || isPending) { return { id: undefined, trace: undefined, @@ -80,11 +83,11 @@ export function useFindNextTrace({ }; } return { - id: data[0].id, - trace: data[0].trace, + id: spanId, + trace: traceId, isLoading: false, }; - }, [data, isError, isPending]); + }, [spanId, traceId, isError, isPending]); return nextTraceData; } From 7d7cb5c70d3f0f65b8d11e74bd6a186a3462519f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Jul 2025 11:39:32 +0200 Subject: [PATCH 13/15] futher optimize `useFindPreviousTrace` memoization --- .../useFindLinkedTraces.ts | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts index facd7ad1c2cdce..915074b8431f46 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts @@ -109,7 +109,14 @@ export function useFindPreviousTrace({ id?: string; trace?: string; } { - const {projectId, environment, previousTraceAttribute} = useMemo(() => { + const { + projectId, + environment, + previousTraceId, + previousTraceSpanId, + hasPreviousTraceLink, + previousTraceSampled, + } = useMemo(() => { let _projectId: number | undefined = undefined; let _environment: string | undefined = undefined; let _previousTraceAttribute: TraceItemResponseAttribute | undefined = undefined; @@ -124,26 +131,41 @@ export function useFindPreviousTrace({ } } + const _hasPreviousTraceLink = typeof _previousTraceAttribute?.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. + // For cases where we only get partial, empty or too long arrays, we should be safe because we + // only take the first three elements. If any of the elements are empty or undefined, we'll + // disable the query (see below). + // Worst-case, we get invalid ids and query for those. Since we check for `isError` below, + // 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('-') || [] : []; + return { projectId: _projectId, environment: _environment, - previousTraceAttribute: _previousTraceAttribute, + hasPreviousTraceLink: _hasPreviousTraceLink, + previousTraceSampled: _previousTraceSampledFlag === '1', + previousTraceId: _previousTraceId, + previousTraceSpanId: _previousTraceSpanId, }; }, [attributes]); - const hasPreviousTraceLink = typeof previousTraceAttribute?.value === 'string'; - - const [previousTraceId, previousTraceSpanId, previousTraceSampledFlag] = - hasPreviousTraceLink ? previousTraceAttribute?.value.split('-') || [] : []; - - const sampled = previousTraceSampledFlag === '1'; - const {data, isError, isPending} = useSpans( { search: `id:${previousTraceSpanId} trace:${previousTraceId}`, fields: ['id', 'trace'], limit: 1, - enabled: direction === 'previous' && hasPreviousTraceLink && sampled, + enabled: + direction === 'previous' && + hasPreviousTraceLink && + previousTraceSampled && + !!previousTraceSpanId && + !!previousTraceId, projectIds: projectId ? [projectId] : [], pageFilters: { environments: environment ? [environment] : [], @@ -164,7 +186,7 @@ export function useFindPreviousTrace({ trace: previousTraceId, id: previousTraceSpanId, available: !!data?.[0]?.id && !isError, - sampled, + sampled: previousTraceSampled, isLoading: isPending, }; } From 3a82ea22689920fb623c221a9cf7641c71ff5863 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Jul 2025 12:07:03 +0200 Subject: [PATCH 14/15] actually close the drawer lol --- .../traceLinksNavigation/traceLinkNavigationButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx index f6b1dbbe34bf06..c4f3ce0e109b09 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -75,7 +75,7 @@ export function TraceLinkNavigationButton({ function closeSpanDetailsDrawer() { traceDispatch({ type: 'minimize drawer', - payload: false, + payload: true, }); } From 0201f893b3677f14dbbbc4cf2ec4fe6f132e07a0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Jul 2025 12:09:49 +0200 Subject: [PATCH 15/15] fix inconsistent attribute handling --- .../app/views/performance/newTraceDetails/traceWaterfall.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index 8c7a666f7c4fd6..3110c56753a528 100644 --- a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx @@ -822,7 +822,7 @@ export function TraceWaterfall(props: TraceWaterfallProps) { {showLinkedTraces && ( {isTraceItemDetailsResponse(props.rootEventResults.data) && - props.rootEventResults.data?.timestamp ? ( + props.rootEventResults.data.timestamp ? (