[CLEAN] Synthetic Benchmark PR #102472 - feat: Consolidate adjacent traces & move next to search

[tomerqodo]

User description

Benchmark PR getsentry#102472

Type: Clean (correct implementation)

Original PR Title: feat: Consolidate adjacent traces & move next to search
Original PR Description: ## Description

This is a follow up to getsentry#96510. The new approach is that the buttons are close to the other interaction buttons, so the user doesn't have to go and find this at the very bottom of the page. Since the search and the "Open in Explore" are already trace related, the user doesn't have to switch "context" and the buttons wouldn't need any additional text in it, except the tooltip if users are curious what it is.

Significant changes

• useFindPreviousTrace and useFindNextTrace have been consolidated into useFindAdjacentTrace, which reduces code duplication
• The tooltip has been moved to the outer most ButtonBar, since it wouldn't need a tooltip for each button
• A own component has been created for the Buttons so it can be easily moved around

📹

The UI reacts a little slow in the GIF since my computer was a little low on resources at this point in time

Original PR URL: getsentry#102472

---

PR Type

Enhancement

---

Description

• Consolidate useFindNextTrace and useFindPreviousTrace into unified useFindAdjacentTrace hook

• Move trace navigation buttons to toolbar with shared tooltip and ButtonBar component

• Simplify button rendering using LinkButton component with icon and aria-label

• Extract trace links navigation logic into dedicated TraceLinksNavigation component

---

Diagram Walkthrough

flowchart LR
  A["useFindNextTrace<br/>useFindPreviousTrace"] -->|consolidate| B["useFindAdjacentTrace"]
  C["TraceLinkNavigationButton<br/>with Tooltips"] -->|simplify| D["LinkButton<br/>in ButtonBar"]
  E["Bottom of page<br/>trace links"] -->|move| F["Toolbar<br/>with Search & Explore"]
  B --> D
  D --> F

Loading

File Walkthrough

Relevant files

Enhancement

useFindLinkedTraces.ts Consolidate adjacent trace finding logic                                  static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts Consolidate useFindNextTrace and useFindPreviousTrace into single useFindAdjacentTrace function Unify parameter naming with adjacentTrace* prefix for both directions Implement direction-based logic for search queries and enabled conditions Return consistent object structure with available, isLoading, id, and trace fields +81/-120 traceLinkNavigationButton.tsx Simplify button rendering with LinkButton component            static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx Replace individual tooltip and styled link components with LinkButton from scraps Simplify button rendering to single component without conditional logic Remove placeholder component and styling code Use unified useFindAdjacentTrace hook instead of two separate hooks Add iconDirection and ariaLabel computed values based on direction +48/-133 traceLinksNavigation.tsx Extract trace links navigation into dedicated component    static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx Create new component to wrap trace navigation buttons in ButtonBar with shared tooltip Consolidate feature flag and source checks into single component Move tooltip with documentation link to container level Render both previous and next trace buttons together with merged button styling +65/-0    traceWaterfall.tsx Move trace navigation buttons to toolbar                                  static/app/views/performance/newTraceDetails/traceWaterfall.tsx Remove showLinkedTraces state logic and move to TraceLinksNavigation component Replace bottom-of-page trace links container with toolbar-integrated TraceLinksNavigation Remove TraceLinkNavigationButton and TraceLinkNavigationButtonPlaceHolder imports Remove TraceLinksNavigationContainer styled component Simplify layout by eliminating fragment and conditional rendering at bottom +5/-47

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The PR adds UI navigation actions and span queries without adding any audit logging, which may be acceptable for non-critical UI interactions but cannot be confirmed from this diff. Referred Code return ( <Tooltip position="top" delay={400} isHoverable title={tct( `Go to the previous or next trace of the same session. [link:Learn More]`, { link: ( <ExternalLink href="https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces" /> ), } )} > <ButtonBar merged gap="0"> <TraceLinkNavigationButton direction="previous" attributes={rootEventResults.data.attributes} currentTraceStartTimestamp={ new Date(rootEventResults.data.timestamp).getTime() / 1000 } ... (clipped 12 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Limited error handling: The hook relies on isError from useSpans but surfaces only availability/loading to consumers without conveying error context, which may be acceptable for UI but reduces debuggability. Referred Code const spanId = data?.[0]?.id; const traceId = data?.[0]?.trace; return useMemo(() => { if (direction === 'next') { return { id: spanId, trace: traceId, available: !!spanId && !!traceId && !isError, isLoading: isPending, }; } return { trace: adjacentTraceId, id: adjacentTraceSpanId, available: !!data?.[0]?.id && !isError, isLoading: isPending, }; }, [ direction, ... (clipped 9 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid generating an invalid URL Conditionally set the to prop on LinkButton to undefined when traceId is missing to avoid generating an invalid URL for a disabled button. static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx [88-102] <LinkButton size="xs" icon={<IconChevron direction={iconDirection} />} aria-label={ariaLabel} onClick={closeSpanDetailsDrawer} disabled={!traceId || isTraceLoading || !isTraceAvailable} - to={getTraceDetailsUrl({ - traceSlug: traceId ?? '', - spanId: traceSpanId, - dateSelection, - timestamp: linkedTraceWindowTimestamp, - location, - organization, - })} + to={ + traceId + ? getTraceDetailsUrl({ + traceSlug: traceId, + spanId: traceSpanId, + dateSelection, + timestamp: linkedTraceWindowTimestamp, + location, + organization, + }) + : undefined + } /> Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that a potentially invalid URL is generated for the to prop when the button is disabled. The proposed fix prevents this, improving code robustness.	Low
General	Avoid redundant timestamp calculation Calculate the currentTraceStartTimestamp value once and store it in a variable to avoid redundant computation for the two TraceLinkNavigationButton components. static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx [48-61] -<TraceLinkNavigationButton - direction="previous" - attributes={rootEventResults.data.attributes} - currentTraceStartTimestamp={ - new Date(rootEventResults.data.timestamp).getTime() / 1000 - } -/> -<TraceLinkNavigationButton - direction="next" - attributes={rootEventResults.data.attributes} - currentTraceStartTimestamp={ - new Date(rootEventResults.data.timestamp).getTime() / 1000 - } -/> +const currentTraceStartTimestamp = + new Date(rootEventResults.data.timestamp).getTime() / 1000; +return ( + <Tooltip + position="top" + delay={400} + isHoverable + title={tct( + `Go to the previous or next trace of the same session. [link:Learn More]`, + { + link: ( + <ExternalLink href="https://docs.sentry.io/concepts/key-terms/tracing/trace-view/#previous-and-next-traces" /> + ), + } + )} + > + <ButtonBar merged gap="0"> + <TraceLinkNavigationButton + direction="previous" + attributes={rootEventResults.data.attributes} + currentTraceStartTimestamp={currentTraceStartTimestamp} + /> + <TraceLinkNavigationButton + direction="next" + attributes={rootEventResults.data.attributes} + currentTraceStartTimestamp={currentTraceStartTimestamp} + /> + </ButtonBar> + </Tooltip> +); + Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies a duplicated calculation and proposes a valid refactor that improves code readability and maintainability by removing the redundancy.	Low
More