Browse Source

fix(issues): Trace timeline cleanup, useMemo events (#65100)

Scott Cooper 1 year ago
parent
commit
75de404b41

+ 4 - 4
static/app/views/issueDetails/traceTimeline/traceLink.tsx

@@ -19,7 +19,7 @@ interface TraceLinkProps {
 
 export function TraceLink({event}: TraceLinkProps) {
   const organization = useOrganization();
-  const {data} = useTraceTimelineEvents({event});
+  const {traceEvents} = useTraceTimelineEvents({event});
   const traceTarget = generateTraceTarget(event, organization);
 
   if (!event.contexts?.trace?.trace_id) {
@@ -49,11 +49,11 @@ export function TraceLink({event}: TraceLinkProps) {
     >
       <span>
         {t('View Full Trace')}
-        {data.length > 0 && (
+        {traceEvents.length > 0 && (
           <Fragment>
-            {data.length >= 100
+            {traceEvents.length >= 100
               ? t(' (100+ issues)')
-              : tn(' (%s issue)', ' (%s issues)', data.length)}
+              : tn(' (%s issue)', ' (%s issues)', traceEvents.length)}
           </Fragment>
         )}
       </span>

+ 6 - 4
static/app/views/issueDetails/traceTimeline/traceTimeline.tsx

@@ -24,14 +24,14 @@ export function TraceTimeline({event}: TraceTimelineProps) {
   const timelineRef = useRef<HTMLDivElement>(null);
   const {width} = useDimensions({elementRef: timelineRef});
   const hasFeature = hasTraceTimelineFeature(organization);
-  const {isError, isLoading, data} = useTraceTimelineEvents({event}, hasFeature);
+  const {isError, isLoading, traceEvents} = useTraceTimelineEvents({event}, hasFeature);
 
   const hasTraceId = !!event.contexts?.trace?.trace_id;
 
   let timelineStatus: string | undefined;
   if (hasFeature) {
     if (hasTraceId && !isLoading) {
-      timelineStatus = data.length > 1 ? 'shown' : 'empty';
+      timelineStatus = traceEvents.length > 1 ? 'shown' : 'empty';
     } else if (!hasTraceId) {
       timelineStatus = 'no_trace_id';
     }
@@ -42,10 +42,12 @@ export function TraceTimeline({event}: TraceTimelineProps) {
     return null;
   }
 
-  const noEvents = !isLoading && data.length === 0;
+  const noEvents = !isLoading && traceEvents.length === 0;
   // Timelines with only the current event are not useful
   const onlySelfEvent =
-    !isLoading && data.length > 0 && data.every(item => item.id === event.id);
+    !isLoading &&
+    traceEvents.length > 0 &&
+    traceEvents.every(item => item.id === event.id);
   if (isError || noEvents || onlySelfEvent) {
     // display empty placeholder to reduce layout shift
     return <div style={{height: 38}} data-test-id="trace-timeline-empty" />;

+ 30 - 29
static/app/views/issueDetails/traceTimeline/traceTimelineEvents.tsx

@@ -1,4 +1,4 @@
-import {Fragment} from 'react';
+import {Fragment, useMemo} from 'react';
 import styled from '@emotion/styled';
 import color from 'color';
 
@@ -24,7 +24,7 @@ interface TraceTimelineEventsProps {
 }
 
 export function TraceTimelineEvents({event, width}: TraceTimelineEventsProps) {
-  const {startTimestamp, endTimestamp, data} = useTraceTimelineEvents({event});
+  const {startTimestamp, endTimestamp, traceEvents} = useTraceTimelineEvents({event});
   let paddedStartTime = startTimestamp;
   let paddedEndTime = endTimestamp;
   // Duration is 0, pad both sides, this is how we end up with 1 dot in the middle
@@ -36,12 +36,11 @@ export function TraceTimelineEvents({event, width}: TraceTimelineEventsProps) {
   const durationMs = paddedEndTime - paddedStartTime;
 
   const totalColumns = Math.floor(width / PARENT_WIDTH);
-  const eventsByColumn = getEventsByColumn(
-    durationMs,
-    data,
-    totalColumns,
-    paddedStartTime
+  const eventsByColumn = useMemo(
+    () => getEventsByColumn(traceEvents, durationMs, totalColumns, paddedStartTime),
+    [durationMs, traceEvents, totalColumns, paddedStartTime]
   );
+
   const columnSize = width / totalColumns;
 
   // If the duration is less than 2 minutes, show seconds
@@ -54,7 +53,7 @@ export function TraceTimelineEvents({event, width}: TraceTimelineEventsProps) {
     <Fragment>
       {/* Add padding to the total columns, 1 column of padding on each side */}
       <TimelineColumns style={{gridTemplateColumns: `repeat(${totalColumns + 2}, 1fr)`}}>
-        {Array.from(eventsByColumn.entries()).map(([column, colEvents]) => {
+        {eventsByColumn.map(([column, colEvents]) => {
           // Calculate the timestamp range that this column represents
           const timeRange = getChunkTimeRange(
             paddedStartTime,
@@ -139,35 +138,33 @@ function NodeGroup({
   timeRange: [number, number];
 }) {
   const totalSubColumns = Math.floor(columnSize / CHILD_WIDTH);
-  const durationMs = timeRange[1] - timeRange[0];
-  const eventsByColumn = getEventsByColumn(
-    durationMs,
-    colEvents,
-    totalSubColumns,
-    timeRange[0]
-  );
-
-  const columns = Array.from(eventsByColumn.keys());
-  const minColumn = Math.min(...columns);
-  const maxColumn = Math.max(...columns);
+  const {eventsByColumn, columns} = useMemo(() => {
+    const durationMs = timeRange[1] - timeRange[0];
+    const eventColumns = getEventsByColumn(
+      colEvents,
+      durationMs,
+      totalSubColumns,
+      timeRange[0]
+    );
+    return {
+      eventsByColumn: eventColumns,
+      columns: eventColumns.map<number>(([column]) => column).sort(),
+    };
+  }, [colEvents, totalSubColumns, timeRange]);
 
   return (
     <Fragment>
       <TimelineColumns style={{gridTemplateColumns: `repeat(${totalSubColumns}, 1fr)`}}>
-        {Array.from(eventsByColumn.entries()).map(([column, groupEvents]) => {
+        {eventsByColumn.map(([column, groupEvents]) => {
           const isCurrentNode = groupEvents.some(e => e.id === currentEventId);
           return (
-            <EventColumn
-              key={`${column}-currrent-event`}
-              style={{gridColumn: Math.floor(column)}}
-            >
-              {isCurrentNode && (
+            <EventColumn key={`${column}-currrent-event`} style={{gridColumn: column}}>
+              {isCurrentNode ? (
                 <CurrentNodeContainer aria-label={t('Current Event')}>
                   <CurrentNodeRing />
                   <CurrentIconNode />
                 </CurrentNodeContainer>
-              )}
-              {!isCurrentNode &&
+              ) : (
                 groupEvents
                   .slice(0, 5)
                   .map(groupEvent =>
@@ -176,7 +173,8 @@ function NodeGroup({
                     ) : (
                       <PerformanceIconNode key={groupEvent.id} />
                     )
-                  )}
+                  )
+              )}
             </EventColumn>
           );
         })}
@@ -193,7 +191,10 @@ function NodeGroup({
         >
           <TooltipHelper
             style={{
-              gridColumn: columns.length > 1 ? `${minColumn} / ${maxColumn}` : columns[0],
+              gridColumn:
+                columns.length > 1
+                  ? `${columns.at(0)} / ${columns.at(-1)}`
+                  : columns.at(0)!,
               width: 8 * columns.length,
             }}
             data-test-id={`trace-timeline-tooltip-${currentColumn}`}

+ 8 - 2
static/app/views/issueDetails/traceTimeline/useTraceTimelineEvents.tsx

@@ -36,7 +36,13 @@ interface UseTraceTimelineEventsOptions {
 export function useTraceTimelineEvents(
   {event}: UseTraceTimelineEventsOptions,
   isEnabled: boolean = true
-) {
+): {
+  endTimestamp: number;
+  isError: boolean;
+  isLoading: boolean;
+  startTimestamp: number;
+  traceEvents: TimelineEvent[];
+} {
   const organization = useOrganization();
   const {start, end} = getTraceTimeRangeFromEvent(event);
 
@@ -150,7 +156,7 @@ export function useTraceTimelineEvents(
   ]);
 
   return {
-    data: eventData.data,
+    traceEvents: eventData.data,
     startTimestamp: eventData.startTimestamp,
     endTimestamp: eventData.endTimestamp,
     isLoading: isLoadingIssuePlatform || isLoadingDiscover,

+ 3 - 3
static/app/views/issueDetails/traceTimeline/utils.tsx

@@ -7,11 +7,11 @@ function getEventTimestamp(start: number, event: TimelineEvent) {
 }
 
 export function getEventsByColumn(
-  durationMs: number,
   events: TimelineEvent[],
+  durationMs: number,
   totalColumns: number,
   start: number
-) {
+): Array<[column: number, events: TimelineEvent[]]> {
   const eventsByColumn = events.reduce((map, event) => {
     const columnPositionCalc =
       Math.floor((getEventTimestamp(start, event) / durationMs) * (totalColumns - 1)) + 1;
@@ -27,7 +27,7 @@ export function getEventsByColumn(
     return map;
   }, new Map<number, TimelineEvent[]>());
 
-  return eventsByColumn;
+  return Array.from(eventsByColumn.entries());
 }
 
 export function getChunkTimeRange(