Просмотр исходного кода

fix(quick-trace): Less frequent component updates (#25867)

This change prevents quick trace query from updating when the event has not
changed. This is especially noticeable on the issues page when going through the
events. Until the next event is finished loading, the previous event may cause
quick trace to re-render making excessive api calls. Sometimes, this even
results in the wrong quick trace being shown for the event.
Tony Xiao 3 лет назад
Родитель
Сommit
cbd0e17cec
1 измененных файлов с 90 добавлено и 81 удалено
  1. 90 81
      static/app/utils/performance/quickTrace/quickTraceQuery.tsx

+ 90 - 81
static/app/utils/performance/quickTrace/quickTraceQuery.tsx

@@ -16,93 +16,102 @@ type QueryProps = Omit<DiscoverQueryProps, 'api' | 'eventView'> & {
   children: (props: QuickTraceQueryChildrenProps) => React.ReactNode;
 };
 
-export default function QuickTraceQuery({children, event, ...props}: QueryProps) {
-  const traceId = event.contexts?.trace?.trace_id;
-
-  if (!traceId) {
-    return (
-      <React.Fragment>
-        {children({
-          isLoading: false,
-          error: null,
-          trace: [],
-          type: 'empty',
-          currentEvent: null,
-        })}
-      </React.Fragment>
-    );
+class QuickTraceQuery extends React.Component<QueryProps> {
+  shouldComponentUpdate(nextProps) {
+    return this.props.event !== nextProps.event;
   }
 
-  const {start, end} = getTraceTimeRangeFromEvent(event);
+  render() {
+    const {children, event, ...props} = this.props;
+    const traceId = event.contexts?.trace?.trace_id;
 
-  return (
-    <TraceLiteQuery
-      eventId={event.id}
-      traceId={traceId}
-      start={start}
-      end={end}
-      {...props}
-    >
-      {traceLiteResults => (
-        <TraceFullQuery
-          eventId={event.id}
-          traceId={traceId}
-          start={start}
-          end={end}
-          {...props}
-        >
-          {traceFullResults => {
-            if (
-              !traceFullResults.isLoading &&
-              traceFullResults.error === null &&
-              traceFullResults.traces !== null
-            ) {
-              for (const subtrace of traceFullResults.traces) {
-                try {
-                  const trace = flattenRelevantPaths(event, subtrace);
-                  return children({
-                    ...traceFullResults,
-                    trace,
-                    currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
-                  });
-                } catch {
-                  // let this fall through and check the next subtrace
-                  // or use the trace lite results
+    if (!traceId) {
+      return (
+        <React.Fragment>
+          {children({
+            isLoading: false,
+            error: null,
+            trace: [],
+            type: 'empty',
+            currentEvent: null,
+          })}
+        </React.Fragment>
+      );
+    }
+
+    const {start, end} = getTraceTimeRangeFromEvent(event);
+
+    return (
+      <TraceLiteQuery
+        eventId={event.id}
+        traceId={traceId}
+        start={start}
+        end={end}
+        {...props}
+      >
+        {traceLiteResults => (
+          <TraceFullQuery
+            eventId={event.id}
+            traceId={traceId}
+            start={start}
+            end={end}
+            {...props}
+          >
+            {traceFullResults => {
+              if (
+                !traceFullResults.isLoading &&
+                traceFullResults.error === null &&
+                traceFullResults.traces !== null
+              ) {
+                for (const subtrace of traceFullResults.traces) {
+                  try {
+                    const trace = flattenRelevantPaths(event, subtrace);
+                    return children({
+                      ...traceFullResults,
+                      trace,
+                      currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
+                    });
+                  } catch {
+                    // let this fall through and check the next subtrace
+                    // or use the trace lite results
+                  }
                 }
               }
-            }
 
-            if (
-              !traceLiteResults.isLoading &&
-              traceLiteResults.error === null &&
-              traceLiteResults.trace !== null
-            ) {
-              const {trace} = traceLiteResults;
+              if (
+                !traceLiteResults.isLoading &&
+                traceLiteResults.error === null &&
+                traceLiteResults.trace !== null
+              ) {
+                const {trace} = traceLiteResults;
+                return children({
+                  ...traceLiteResults,
+                  currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
+                });
+              }
+
               return children({
-                ...traceLiteResults,
-                currentEvent: trace.find(e => isCurrentEvent(e, event)) ?? null,
+                // only use the light results loading state if it didn't error
+                // if it did, we should rely on the full results
+                isLoading: traceLiteResults.error
+                  ? traceFullResults.isLoading
+                  : traceLiteResults.isLoading || traceFullResults.isLoading,
+                // swallow any errors from the light results because we
+                // should rely on the full results in this situations
+                error: traceFullResults.error,
+                trace: [],
+                // if we reach this point but there were some traces in the full results,
+                // that means there were other transactions in the trace, but the current
+                // event could not be found
+                type: traceFullResults.traces?.length ? 'missing' : 'empty',
+                currentEvent: null,
               });
-            }
-
-            return children({
-              // only use the light results loading state if it didn't error
-              // if it did, we should rely on the full results
-              isLoading: traceLiteResults.error
-                ? traceFullResults.isLoading
-                : traceLiteResults.isLoading || traceFullResults.isLoading,
-              // swallow any errors from the light results because we
-              // should rely on the full results in this situations
-              error: traceFullResults.error,
-              trace: [],
-              // if we reach this point but there were some traces in the full results,
-              // that means there were other transactions in the trace, but the current
-              // event could not be found
-              type: traceFullResults.traces?.length ? 'missing' : 'empty',
-              currentEvent: null,
-            });
-          }}
-        </TraceFullQuery>
-      )}
-    </TraceLiteQuery>
-  );
+            }}
+          </TraceFullQuery>
+        )}
+      </TraceLiteQuery>
+    );
+  }
 }
+
+export default QuickTraceQuery;