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

[tomerqodo]

User description

Benchmark PR getsentry#102472

Type: Corrupted (contains bugs)

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

• Simplify trace navigation button component using LinkButton from scraps library

• Extract trace links navigation into dedicated TraceLinksNavigation component with shared tooltip

• Move trace navigation buttons from bottom of page to toolbar near search and explore buttons

---

Diagram Walkthrough

flowchart LR
  A["useFindNextTrace<br/>useFindPreviousTrace"] -->|consolidate| B["useFindAdjacentTrace"]
  C["TraceLinkNavigationButton<br/>with individual tooltips"] -->|simplify| D["TraceLinkNavigationButton<br/>with LinkButton"]
  E["Bottom of page<br/>TraceLinksNavigationContainer"] -->|move| F["TraceToolbar<br/>TraceLinksNavigation"]
  B -->|used by| D
  D -->|wrapped in| F

Loading

File Walkthrough

Relevant files

Enhancement

useFindLinkedTraces.ts Consolidate adjacent trace finding logic                                  static/app/views/performance/newTraceDetails/traceLinksNavigation/useFindLinkedTraces.ts Consolidate two separate hooks (useFindNextTrace and useFindPreviousTrace) into single useFindAdjacentTrace hook Rename parameters to use adjacentTrace prefix instead of nextTrace/previousTrace for consistency Unify query logic with direction-based conditional for search query and enabled conditions Simplify return type by removing sampled field and using consistent structure for both directions Add useMemo wrapper for return value to optimize performance +81/-120 traceLinkNavigationButton.tsx Simplify button component with LinkButton                                static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx Replace custom styled components with LinkButton from @sentry/scraps/button library Remove individual tooltips and styling logic from button component Simplify component to use single useFindAdjacentTrace hook instead of two separate hooks Consolidate direction-specific logic using useMemo for timestamp and icon direction Remove placeholder component and conditional rendering logic Reduce component from ~120 lines to ~30 lines +48/-133 traceLinksNavigation.tsx New component for trace links navigation                                  static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinksNavigation.tsx New component that wraps trace navigation buttons with shared tooltip and button bar Handles feature flag check and source validation for showing linked traces Consolidates tooltip logic previously duplicated in individual buttons Uses ButtonBar from scraps library for consistent button grouping Manages rendering of both previous and next trace buttons together +65/-0    traceWaterfall.tsx Relocate trace navigation to toolbar                                          static/app/views/performance/newTraceDetails/traceWaterfall.tsx Move trace navigation buttons from bottom of page to toolbar near search and explore buttons Replace TraceLinksNavigationContainer with TraceLinksNavigation component in toolbar Remove feature flag and conditional rendering logic from waterfall component Remove unused imports and styled components Simplify component by delegating trace links logic to dedicated component +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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing auditing: The new hooks and navigation actions perform trace lookups and navigation without adding any audit logging for these potentially sensitive user actions. Referred Code const searchQuery = direction === 'next' ? `sentry.previous_trace:${currentTraceId}-${currentSpanId}-1` : `id:${adjacentTraceSpanId} trace:${adjacentTraceId}`; const enabled = direction === 'next' ? !!currentTraceId && !!currentSpanId : hasAdjacentTraceLink && adjacentTraceSampled && !!adjacentTraceSpanId && !!adjacentTraceId; const {data, isError, isPending} = useSpans( { search: searchQuery, fields: ['id', 'trace'], limit: 1, enabled, projectIds: projectId ? [projectId] : [], pageFilters: { ... (clipped 18 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: Error handling gaps: The hook relies on isError/isPending but does not surface or log contextual errors or handle malformed adjacent trace attributes beyond disabling queries, which may hinder debugging. Referred Code const {data, isError, isPending} = useSpans( { search: searchQuery, fields: ['id', 'trace'], limit: 1, enabled, projectIds: projectId ? [projectId] : [], pageFilters: { environments: environment ? [environment] : [], projects: projectId ? [projectId] : [], datetime: { start: adjacentTraceStartTimestamp ? new Date(adjacentTraceStartTimestamp * 1000).toISOString() : '', end: adjacentTraceEndTimestamp ? new Date(adjacentTraceEndTimestamp * 1000).toISOString() : '', period: null, utc: true, }, }, ... (clipped 35 lines) 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: Query input validation: The search query is built from attributes (trace/span IDs) without explicit validation/sanitization, which could risk malformed queries if upstream data is unexpected. Referred Code const searchQuery = direction === 'next' ? `sentry.previous_trace:${currentTraceId}-${currentSpanId}-1` : `id:${adjacentTraceSpanId} trace:${adjacentTraceId}`; const enabled = direction === 'next' ? !!currentTraceId && !!currentSpanId : hasAdjacentTraceLink && adjacentTraceSampled && !!adjacentTraceSpanId && !!adjacentTraceId; const {data, isError, isPending} = useSpans( { search: searchQuery, fields: ['id', 'trace'], limit: 1, enabled, projectIds: projectId ? [projectId] : [], pageFilters: { ... (clipped 6 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
High-level	The trace navigation logic is broken The trace navigation feature is broken due to two bugs: the "previous" and "next" button icons are swapped, and the navigation URL uses the current trace's timestamp instead of the adjacent trace's, causing incorrect view centering. Examples: static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx [45-58] if (direction === 'previous') { return { adjacentTraceEndTimestamp: currentTraceStartTimestamp, adjacentTraceStartTimestamp: linkedTraceWindowTimestamp, iconDirection: 'right' as const, ariaLabel: t('Previous Trace'), }; } return { adjacentTraceEndTimestamp: linkedTraceWindowTimestamp, ... (clipped 4 lines) static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx [94-101] to={getTraceDetailsUrl({ traceSlug: traceId ?? '', spanId: traceSpanId, dateSelection, timestamp: currentTraceStartTimestamp, location, organization, })} Solution Walkthrough: Before: // file: static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx const { iconDirection } = useMemo(() => { if (direction === 'previous') { return { // ... iconDirection: 'right' as const, // Incorrect: should be 'left' }; } return { // ... iconDirection: 'left' as const, // Incorrect: should be 'right' }; }, ...); return ( <LinkButton // ... to={getTraceDetailsUrl({ // ... timestamp: currentTraceStartTimestamp, // Incorrect: uses current trace's timestamp })} /> ); After: // file: static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx const { iconDirection } = useMemo(() => { if (direction === 'previous') { return { // ... iconDirection: 'left' as const, // Corrected }; } return { // ... iconDirection: 'right' as const, // Corrected }; }, ...); return ( <LinkButton // ... to={getTraceDetailsUrl({ // ... timestamp: linkedTraceWindowTimestamp, // Corrected: uses calculated timestamp for adjacent trace })} /> ); Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies two critical bugs in the trace navigation logic—swapped icons and an incorrect timestamp—that break the feature's functionality and user experience.	High
Possible issue	Correct chevron icon direction Change the iconDirection for the 'previous' trace button from 'right' to 'left' to correctly indicate backward navigation. static/app/views/performance/newTraceDetails/traceLinksNavigation/traceLinkNavigationButton.tsx [49] -iconDirection: 'right' as const, +iconDirection: 'left' as const, Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a UI bug where the navigation icon for the 'previous' trace button points in the wrong direction, which would confuse users.	Low
More