From a3578d90018c51128504e87460d7f6354ba2d739 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 18 Mar 2025 16:30:07 -0700 Subject: [PATCH 1/3] feat(issues): Add overlays to each trace row Currently, we have one big overlay that hijacks any interaction and links you to the trace view. Now each row will have its own overlay and link you to that specific span. --- .../interfaces/performance/eventTraceView.tsx | 45 +--- .../performance/spanEvidenceTraceView.tsx | 50 ----- .../newTraceDetails/issuesTraceWaterfall.tsx | 48 +++-- .../issuesTraceWaterfallOverlay.tsx | 196 ++++++++++++++++++ 4 files changed, 232 insertions(+), 107 deletions(-) create mode 100644 static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx diff --git a/static/app/components/events/interfaces/performance/eventTraceView.tsx b/static/app/components/events/interfaces/performance/eventTraceView.tsx index 04a609df902f93..005f37f333a276 100644 --- a/static/app/components/events/interfaces/performance/eventTraceView.tsx +++ b/static/app/components/events/interfaces/performance/eventTraceView.tsx @@ -1,17 +1,13 @@ import {Fragment, useMemo} from 'react'; import styled from '@emotion/styled'; -import type {LocationDescriptor} from 'history'; import ButtonBar from 'sentry/components/buttonBar'; import {LinkButton} from 'sentry/components/core/button'; -import Link from 'sentry/components/links/link'; import {generateTraceTarget} from 'sentry/components/quickTrace/utils'; import {t} from 'sentry/locale'; import type {Event} from 'sentry/types/event'; import {type Group, IssueCategory} from 'sentry/types/group'; import type {Organization} from 'sentry/types/organization'; -import {defined} from 'sentry/utils'; -import {trackAnalytics} from 'sentry/utils/analytics'; import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams'; import {useLocation} from 'sentry/utils/useLocation'; import {SectionKey} from 'sentry/views/issueDetails/streamline/context'; @@ -19,6 +15,7 @@ import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSectio import {TraceIssueEvent} from 'sentry/views/issueDetails/traceTimeline/traceIssue'; import {useTraceTimelineEvents} from 'sentry/views/issueDetails/traceTimeline/useTraceTimelineEvents'; import {IssuesTraceWaterfall} from 'sentry/views/performance/newTraceDetails/issuesTraceWaterfall'; +import {getTraceLinkForIssue} from 'sentry/views/performance/newTraceDetails/issuesTraceWaterfallOverlay'; import {useIssuesTraceTree} from 'sentry/views/performance/newTraceDetails/traceApi/useIssuesTraceTree'; import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; import {useTraceMeta} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; @@ -59,15 +56,9 @@ interface EventTraceViewInnerProps { event: Event; organization: Organization; traceId: string; - traceTarget: LocationDescriptor; } -function EventTraceViewInner({ - event, - organization, - traceId, - traceTarget, -}: EventTraceViewInnerProps) { +function EventTraceViewInner({event, organization, traceId}: EventTraceViewInnerProps) { const timestamp = new Date(event.dateReceived).getTime() / 1e3; const trace = useTrace({ @@ -115,34 +106,11 @@ function EventTraceViewInner({ replay={null} event={event} /> - { - trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { - organization, - }); - }} - /> ); } -function getHrefFromTraceTarget(traceTarget: LocationDescriptor) { - if (typeof traceTarget === 'string') { - return traceTarget; - } - - const searchParams = new URLSearchParams(); - for (const key in traceTarget.query) { - if (defined(traceTarget.query[key])) { - searchParams.append(key, traceTarget.query[key]); - } - } - - return `${traceTarget.pathname}?${searchParams.toString()}`; -} - function OneOtherIssueEvent({event}: {event: Event}) { const {isLoading, oneOtherIssueEvent} = useTraceTimelineEvents({event}); useRouteAnalyticsParams(oneOtherIssueEvent ? {has_related_trace_issue: true} : {}); @@ -163,12 +131,6 @@ const IssuesTraceContainer = styled('div')` position: relative; `; -const IssuesTraceOverlayContainer = styled(Link)` - position: absolute; - inset: 0; - z-index: 10; -`; - interface EventTraceViewProps { event: Event; group: Group; @@ -216,7 +178,7 @@ export function EventTraceView({group, event, organization}: EventTraceViewProps @@ -231,7 +193,6 @@ export function EventTraceView({group, event, organization}: EventTraceViewProps event={event} organization={organization} traceId={traceId} - traceTarget={traceTarget} /> )} diff --git a/static/app/components/events/interfaces/performance/spanEvidenceTraceView.tsx b/static/app/components/events/interfaces/performance/spanEvidenceTraceView.tsx index 11f73b42578800..64dc56c1a7121c 100644 --- a/static/app/components/events/interfaces/performance/spanEvidenceTraceView.tsx +++ b/static/app/components/events/interfaces/performance/spanEvidenceTraceView.tsx @@ -1,19 +1,12 @@ import {lazy, Suspense, useMemo} from 'react'; import styled from '@emotion/styled'; -import type {LocationDescriptor} from 'history'; -import Link from 'sentry/components/links/link'; -import {generateTraceTarget} from 'sentry/components/quickTrace/utils'; import type {Event} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; -import {defined} from 'sentry/utils'; -import {trackAnalytics} from 'sentry/utils/analytics'; -import {useLocation} from 'sentry/utils/useLocation'; import {useIssuesTraceTree} from 'sentry/views/performance/newTraceDetails/traceApi/useIssuesTraceTree'; import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; import {useTraceMeta} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; import {useTraceRootEvent} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; -import {TraceViewSources} from 'sentry/views/performance/newTraceDetails/traceHeader/breadcrumbs'; import { loadTraceViewPreferences, type TracePreferencesState, @@ -28,21 +21,6 @@ const LazyIssuesTraceWaterfall = lazy(() => ) ); -function getHrefFromTraceTarget(traceTarget: LocationDescriptor) { - if (typeof traceTarget === 'string') { - return traceTarget; - } - - const searchParams = new URLSearchParams(); - for (const key in traceTarget.query) { - if (defined(traceTarget.query[key])) { - searchParams.append(key, traceTarget.query[key]); - } - } - - return `${traceTarget.pathname}?${searchParams.toString()}`; -} - const DEFAULT_ISSUE_DETAILS_TRACE_VIEW_PREFERENCES: TracePreferencesState = { drawer: { minimized: true, @@ -78,7 +56,6 @@ export function SpanEvidenceTraceView({ }: SpanEvidenceTraceViewProps) { const timestamp = new Date(event.dateReceived).getTime() / 1e3; - const location = useLocation(); const trace = useTrace({ timestamp, traceSlug: traceId, @@ -104,19 +81,6 @@ export function SpanEvidenceTraceView({ return null; } - const traceTarget = generateTraceTarget( - event, - organization, - { - ...location, - query: { - ...location.query, - groupId: event.groupID, - }, - }, - TraceViewSources.ISSUE_DETAILS - ); - return ( - { - trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { - organization, - }); - }} - /> ); @@ -153,9 +109,3 @@ export function SpanEvidenceTraceView({ const IssuesTraceContainer = styled('div')` position: relative; `; - -const IssuesTraceOverlayContainer = styled(Link)` - position: absolute; - inset: 0; - z-index: 10; -`; diff --git a/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx b/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx index f7e813ac34a5d9..b2b79f782cb3e6 100644 --- a/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx @@ -1,4 +1,3 @@ -import type React from 'react'; import { Fragment, useCallback, @@ -16,6 +15,7 @@ import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import useOrganization from 'sentry/utils/useOrganization'; import useProjects from 'sentry/utils/useProjects'; +import {IssueTraceWaterfallOverlay} from 'sentry/views/performance/newTraceDetails/issuesTraceWaterfallOverlay'; import { isSpanNode, isTraceErrorNode, @@ -58,6 +58,7 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) { const organization = useOrganization(); const traceState = useTraceState(); const traceDispatch = useTraceStateDispatch(); + const containerRef = useRef(null); const [forceRender, rerender] = useReducer(x => (x + 1) % Number.MAX_SAFE_INTEGER, 0); @@ -318,21 +319,32 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) { : 8 } > - - + + + + - + {props.tree.type === 'loading' || onLoadScrollStatus === 'pending' ? ( @@ -370,3 +382,9 @@ const IssuesTraceGrid = styled(TraceGrid)<{ Math.min(Math.max(p.rowCount, MIN_ROW_COUNT), MAX_ROW_COUNT) * ROW_HEIGHT + HEADER_HEIGHT}px; `; + +const IssuesTraceContainer = styled('div')` + position: relative; + height: 100%; + width: 100%; +`; diff --git a/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx new file mode 100644 index 00000000000000..4a2eaa862dea99 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx @@ -0,0 +1,196 @@ +import {useEffect, useMemo, useState} from 'react'; +import styled from '@emotion/styled'; +import Color from 'color'; +import type {LocationDescriptor} from 'history'; +import * as qs from 'query-string'; + +import Link from 'sentry/components/links/link'; +import {generateTraceTarget} from 'sentry/components/quickTrace/utils'; +import type {Event} from 'sentry/types/event'; +import {defined} from 'sentry/utils'; +import {trackAnalytics} from 'sentry/utils/analytics'; +import {useLocation} from 'sentry/utils/useLocation'; +import useOrganization from 'sentry/utils/useOrganization'; +import {isCollapsedNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; +import {TraceViewSources} from 'sentry/views/performance/newTraceDetails/traceHeader/breadcrumbs'; +import type {IssuesTraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/issuesTraceTree'; +import {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; + +import type {VirtualizedViewManager} from './traceRenderers/virtualizedViewManager'; + +interface RowPosition { + height: number; + left: number; + pathToNode: ReturnType; + top: number; + width: number; +} + +interface TraceOverlayProps { + containerRef: React.RefObject; + event: Event; + groupId: string | undefined; + tree: IssuesTraceTree; + viewManager: VirtualizedViewManager; +} + +/** + * Renders a overlay over each row in the trace waterfall that blocks interaction with the row. + * Instead, the overlay will link to the full trace view for the row. + */ +export function IssueTraceWaterfallOverlay({ + containerRef, + event, + groupId, + tree, + viewManager, +}: TraceOverlayProps) { + const organization = useOrganization(); + const [rowPositions, setRowPositions] = useState(null); + const location = useLocation(); + + const traceTarget = useMemo( + () => + generateTraceTarget( + event, + organization, + { + ...location, + query: { + ...location.query, + ...(groupId ? {groupId} : {}), + }, + }, + TraceViewSources.ISSUE_DETAILS + ), + [event, organization, location, groupId] + ); + + useEffect(() => { + const measurePositions = () => { + const container = containerRef.current; + + if (!container) { + return; + } + + const rows = document.querySelectorAll('.TraceRow:not(.Hidden)'); + const newPositions: RowPosition[] = []; + const containerRect = container.getBoundingClientRect(); + + // Rows should match the number of rows in the tree + if (rows.length === 0 || rows.length !== tree.list.length) { + setRowPositions(null); + return; + } + + rows.forEach((row, index) => { + const node = tree.list[index]; + if (!node || isCollapsedNode(node)) { + return; + } + + const pathToNode = TraceTree.PathToNode(node); + + if (!pathToNode) { + return; + } + + const rect = row.getBoundingClientRect(); + newPositions.push({ + top: rect.top - containerRect.top, + left: rect.left - containerRect.left, + width: rect.width, + height: rect.height, + pathToNode, + }); + }); + + setRowPositions(newPositions); + }; + + viewManager.row_measurer.on('row measure end', measurePositions); + return () => { + viewManager.row_measurer.off('row measure end', measurePositions); + }; + }, [viewManager, containerRef, tree]); + + const baseLink = getTraceLinkForIssue(traceTarget); + + return ( + + {rowPositions?.length ? ( + rowPositions.map(pos => ( + { + trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { + organization, + }); + }} + style={{ + top: `${pos.top}px`, + left: `${pos.left}px`, + width: '100%', + height: `${pos.height}px`, + }} + /> + )) + ) : ( + { + trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { + organization, + }); + }} + style={{inset: 0}} + /> + )} + + ); +} + +export function getTraceLinkForIssue( + traceTarget: LocationDescriptor, + pathToNode?: ReturnType +) { + if (typeof traceTarget === 'string') { + return traceTarget; + } + + const searchParams = new URLSearchParams(); + for (const key in traceTarget.query) { + if (key === 'node' && pathToNode) { + continue; + } + if (defined(traceTarget.query[key])) { + searchParams.append(key, traceTarget.query[key]); + } + } + + let pathSearchParams = ''; + if (pathToNode) { + pathSearchParams = `&${qs.stringify({node: pathToNode})}`; + } + + return `${traceTarget.pathname}?${searchParams.toString()}${pathSearchParams}`; +} + +const OverlayWrapper = styled(Link)` + position: absolute; + inset: 0; + z-index: 10; + overflow: hidden; +`; + +const IssuesTraceOverlayContainer = styled(Link)` + position: absolute; + display: block; + pointer-events: auto; + + &:hover { + background: ${p => Color(p.theme.backgroundSecondary).alpha(0.3).toString()}; + } +`; From 4606aae994d4998505d99c32b89e6b904d2e7738 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 18 Mar 2025 16:40:56 -0700 Subject: [PATCH 2/3] use qs for the whole thing --- .../issuesTraceWaterfallOverlay.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx index 4a2eaa862dea99..0e9f74f83267e5 100644 --- a/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx +++ b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx @@ -160,22 +160,19 @@ export function getTraceLinkForIssue( return traceTarget; } - const searchParams = new URLSearchParams(); + const searchParams: Record = {}; for (const key in traceTarget.query) { - if (key === 'node' && pathToNode) { - continue; - } if (defined(traceTarget.query[key])) { - searchParams.append(key, traceTarget.query[key]); + searchParams[key] = traceTarget.query[key]; } } - let pathSearchParams = ''; if (pathToNode) { - pathSearchParams = `&${qs.stringify({node: pathToNode})}`; + // Override the node query param from traceTarget.query + searchParams.node = pathToNode; } - return `${traceTarget.pathname}?${searchParams.toString()}${pathSearchParams}`; + return `${traceTarget.pathname}?${qs.stringify(searchParams)}`; } const OverlayWrapper = styled(Link)` @@ -191,6 +188,6 @@ const IssuesTraceOverlayContainer = styled(Link)` pointer-events: auto; &:hover { - background: ${p => Color(p.theme.backgroundSecondary).alpha(0.3).toString()}; + background: ${p => Color(p.theme.gray300).alpha(0.1).toString()}; } `; From c9dfaf6315b4344e76e296da0d028552a7b6041e Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 18 Mar 2025 16:58:58 -0700 Subject: [PATCH 3/3] Remove nested anchor tags, cleanup fallback --- .../issuesTraceWaterfallOverlay.tsx | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx index 0e9f74f83267e5..d875e4bae3aa66 100644 --- a/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx +++ b/static/app/views/performance/newTraceDetails/issuesTraceWaterfallOverlay.tsx @@ -1,4 +1,4 @@ -import {useEffect, useMemo, useState} from 'react'; +import {useCallback, useEffect, useMemo, useState} from 'react'; import styled from '@emotion/styled'; import Color from 'color'; import type {LocationDescriptor} from 'history'; @@ -115,39 +115,34 @@ export function IssueTraceWaterfallOverlay({ }; }, [viewManager, containerRef, tree]); + const handleLinkClick = useCallback(() => { + trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { + organization, + }); + }, [organization]); + const baseLink = getTraceLinkForIssue(traceTarget); return ( - - {rowPositions?.length ? ( - rowPositions.map(pos => ( - { - trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { - organization, - }); - }} - style={{ - top: `${pos.top}px`, - left: `${pos.left}px`, - width: '100%', - height: `${pos.height}px`, - }} - /> - )) - ) : ( + + + {rowPositions?.map(pos => ( { - trackAnalytics('issue_details.view_full_trace_waterfall_clicked', { - organization, - }); + key={pos.pathToNode[0]!} + to={getTraceLinkForIssue(traceTarget, pos.pathToNode)} + onClick={handleLinkClick} + style={{ + top: `${pos.top}px`, + left: `${pos.left}px`, + width: '100%', + height: `${pos.height}px`, }} - style={{inset: 0}} /> - )} + ))} ); } @@ -175,13 +170,19 @@ export function getTraceLinkForIssue( return `${traceTarget.pathname}?${qs.stringify(searchParams)}`; } -const OverlayWrapper = styled(Link)` +const OverlayWrapper = styled('div')` position: absolute; inset: 0; z-index: 10; overflow: hidden; `; +const FallbackOverlayContainer = styled(Link)` + position: absolute; + display: block; + pointer-events: auto; +`; + const IssuesTraceOverlayContainer = styled(Link)` position: absolute; display: block;