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/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 09e27fb96a4238..c4f3ce0e109b09 100644 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx @@ -3,87 +3,66 @@ 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( @@ -91,55 +70,82 @@ export function TraceLinkNavigationButton({ [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 ( - - ), - } - )} - > - + ), + } + )} > - - {t('Go to Previous Trace')} - - - ); + closeSpanDetailsDrawer()} + to={getTraceDetailsUrl({ + traceSlug: previousTraceId, + spanId: previousTraceSpanId, + dateSelection, + timestamp: linkedTraceWindowTimestamp, + location, + organization, + })} + > + + {t('Go to Previous Trace')} + + + ); + } + + if (!previousTraceSampled) { + return ( + + {t('Previous trace not sampled')} + + ); + } + + if (!isPreviousTraceAvailable) { + return ( + + {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; +export function TraceLinkNavigationButtonPlaceHolder() { + 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/useFindLinkedTraces.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts new file mode 100644 index 00000000000000..915074b8431f46 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts @@ -0,0 +1,192 @@ +import {useMemo} from 'react'; + +import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; +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. + */ +export function useFindNextTrace({ + direction, + attributes, + nextTraceEndTimestamp, + nextTraceStartTimestamp, +}: { + 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.performance.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, + } = 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; + } + } + + 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, + hasPreviousTraceLink: _hasPreviousTraceLink, + previousTraceSampled: _previousTraceSampledFlag === '1', + previousTraceId: _previousTraceId, + previousTraceSpanId: _previousTraceSpanId, + }; + }, [attributes]); + + const {data, isError, isPending} = useSpans( + { + search: `id:${previousTraceSpanId} trace:${previousTraceId}`, + fields: ['id', 'trace'], + limit: 1, + enabled: + direction === 'previous' && + hasPreviousTraceLink && + previousTraceSampled && + !!previousTraceSpanId && + !!previousTraceId, + projectIds: projectId ? [projectId] : [], + pageFilters: { + environments: environment ? [environment] : [], + projects: projectId ? [projectId] : [], + datetime: { + start: new Date(previousTraceStartTimestamp * 1000).toISOString(), + end: new Date(previousTraceEndTimestamp * 1000).toISOString(), + period: null, + utc: true, + }, + }, + queryWithoutPageFilters: true, + }, + `api.performance.trace-panel-${direction}-trace-link` + ); + + return { + trace: previousTraceId, + id: previousTraceSpanId, + available: !!data?.[0]?.id && !isError, + sampled: previousTraceSampled, + isLoading: isPending, + }; +} diff --git a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts b/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts deleted file mode 100644 index 6b941f165731b1..00000000000000 --- a/static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindNextTrace.ts +++ /dev/null @@ -1,104 +0,0 @@ -import {useMemo} from 'react'; - -import type {TraceContextType} from 'sentry/components/events/interfaces/spans/types'; -import type {EventTransaction} from 'sentry/types/event'; -import {useApiQueries} 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 {useIsEAPTraceEnabled} from 'sentry/views/performance/newTraceDetails/useIsEAPTraceEnabled'; - -import type {ConnectedTraceConnection} from './traceLinkNavigationButton'; - -/** - * 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". - */ -export function useFindNextTrace({ - direction, - currentTraceID, - linkedTraceStartTimestamp, - linkedTraceEndTimestamp, - projectID, -}: { - direction: ConnectedTraceConnection; - currentTraceID?: string; - linkedTraceEndTimestamp?: number; - linkedTraceStartTimestamp?: number; - projectID?: string; -}): TraceContextType | undefined { - 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, - }, - }, - search: MutableSearch.fromQueryObject({is_transaction: 1}), - fields: [SpanFields.TRANSACTION_SPAN_ID, SpanFields.PROJECT_ID, SpanFields.PROJECT], - }, - 'api.trace-view.linked-traces' - ); - - const traceData = indexedSpans.map(span => ({ - projectSlug: span.project, - eventId: span['transaction.span_id'], - })); - - 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 - ); - - return hasMatchingLink; - }); - - return nextTrace?.data?.contexts.trace; -} - -// Similar to `useTraceRootEvent` but allows fetching data for "more than one" trace data -function useTraceRootEvents( - traceData: Array<{eventId?: string; projectSlug?: string}> | null -) { - const organization = useOrganization(); - const isEAP = useIsEAPTraceEnabled(); - - const queryKeys = useMemo(() => { - if (!traceData) { - 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, - }); -} diff --git a/static/app/views/performance/newTraceDetails/traceWaterfall.tsx b/static/app/views/performance/newTraceDetails/traceWaterfall.tsx index b55b0c31b8cf2b..3110c56753a528 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'; @@ -142,7 +146,11 @@ export function TraceWaterfall(props: TraceWaterfallProps) { const {timestamp} = useTraceQueryParams(); - const showLinkedTraces = organization?.features.includes('trace-view-linked-traces'); + 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', { @@ -811,27 +819,29 @@ export function TraceWaterfall(props: TraceWaterfallProps) { traceEventView={props.traceEventView} /> - {showLinkedTraces && !isTraceItemDetailsResponse(props.rootEventResults.data) && ( + {showLinkedTraces && ( - - + {isTraceItemDetailsResponse(props.rootEventResults.data) && + props.rootEventResults.data.timestamp ? ( + + + + + ) : ( + + )} )}