fix(ddm): display all code locations

giovanni-guidini · Dec 19, 2023 · 94de45b · 94de45b
1 parent d9b0216
commit 94de45b
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 41 deletions.
diff --git a/static/app/utils/metrics/index.tsx b/static/app/utils/metrics/index.tsx
@@ -125,9 +125,10 @@ export type MetricCodeLocationFrame = {
 };
 
 export type MetricMetaCodeLocation = {
-  frames: MetricCodeLocationFrame[];
   mri: string;
   timestamp: number;
+  codeLocations?: MetricCodeLocationFrame[];
+  frames?: MetricCodeLocationFrame[];
 };
 export function getDdmUrl(
   orgSlug: string,

diff --git a/static/app/utils/metrics/useMetricsCodeLocations.tsx b/static/app/utils/metrics/useMetricsCodeLocations.tsx
@@ -3,15 +3,15 @@ import {useApiQuery} from 'sentry/utils/queryClient';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 
-type ApiResponse = {codeLocations: MetricMetaCodeLocation[]};
+type ApiResponse = {
+  metrics: MetricMetaCodeLocation[];
+};
 
 export function useMetricsCodeLocations(mri: string | undefined) {
   const organization = useOrganization();
   const {selection} = usePageFilters();
 
-  const {data, isLoading, isError, refetch} = useApiQuery<{
-    codeLocations: MetricMetaCodeLocation[];
-  }>(
+  const {data, isLoading, isError, refetch} = useApiQuery<ApiResponse>(
     [
       `/organizations/${organization.slug}/ddm/meta/`,
       {
@@ -28,29 +28,51 @@ export function useMetricsCodeLocations(mri: string | undefined) {
     }
   );
 
-  if (
-    !data ||
-    !Array.isArray(data?.codeLocations) ||
-    !Array.isArray(data?.codeLocations[0]?.frames)
-  ) {
+  if (!data) {
     return {data, isLoading};
   }
 
+  mapToNewResponseShape(data);
   deduplicateCodeLocations(data);
   sortCodeLocations(data);
 
   return {data, isLoading, isError, refetch};
 }
 
+const mapToNewResponseShape = (data: ApiResponse) => {
+  if (data.metrics) {
+    return;
+  }
+  // @ts-expect-error
+  data.metrics = (data.codeLocations ?? [])?.map(codeLocation => {
+    return {
+      mri: codeLocation.mri,
+      timestamp: codeLocation.timestamp,
+      codeLocations: (codeLocation.frames ?? []).map(frame => {
+        return {
+          function: frame.function,
+          module: frame.module,
+          filename: frame.filename,
+          absPath: frame.absPath,
+          lineNo: frame.lineNo,
+          preContext: frame.preContext,
+          contextLine: frame.contextLine,
+          postContext: frame.postContext,
+        };
+      }),
+    };
+  });
+};
+
 const sortCodeLocations = (data: ApiResponse) => {
-  data.codeLocations.sort((a, b) => {
+  data.metrics.sort((a, b) => {
     return b.timestamp - a.timestamp;
   });
 };
 
 const deduplicateCodeLocations = (data: ApiResponse) => {
-  data.codeLocations = data.codeLocations.filter((element, index) => {
-    return !data.codeLocations.slice(0, index).some(e => equalCodeLocations(e, element));
+  data.metrics = data.metrics.filter((element, index) => {
+    return !data.metrics.slice(0, index).some(e => equalCodeLocations(e, element));
   });
 };
 
@@ -59,8 +81,8 @@ const equalCodeLocations = (a: MetricMetaCodeLocation, b: MetricMetaCodeLocation
     return false;
   }
 
-  const aFrame = JSON.stringify(a.frames?.[0] ?? {});
-  const bFrame = JSON.stringify(b.frames?.[0] ?? {});
+  const aCodeLocation = JSON.stringify(a.codeLocations?.[0] ?? {});
+  const bCodeLocation = JSON.stringify(b.codeLocations?.[0] ?? {});
 
-  return aFrame === bFrame;
+  return aCodeLocation === bCodeLocation;
 };
diff --git a/static/app/views/ddm/codeLocations.tsx b/static/app/views/ddm/codeLocations.tsx
@@ -14,7 +14,7 @@ import {space} from 'sentry/styles/space';
 import {Frame} from 'sentry/types';
 import {useMetricsCodeLocations} from 'sentry/utils/metrics/useMetricsCodeLocations';
 
-import {MetricCodeLocationFrame, MetricMetaCodeLocation} from '../../utils/metrics/index';
+import {MetricCodeLocationFrame} from '../../utils/metrics/index';
 
 export function CodeLocations({mri}: {mri: string}) {
   const {data, isLoading, isError, refetch} = useMetricsCodeLocations(mri);
@@ -27,7 +27,7 @@ export function CodeLocations({mri}: {mri: string}) {
     return <LoadingError onRetry={refetch} />;
   }
 
-  if (!Array.isArray(data?.codeLocations) || data?.codeLocations.length === 0) {
+  if (!Array.isArray(data?.metrics) || data?.metrics.length === 0) {
     return (
       <CenterContent>
         <EmptyMessage
@@ -40,26 +40,26 @@ export function CodeLocations({mri}: {mri: string}) {
     );
   }
 
-  const codeLocations = data?.codeLocations ?? [];
+  const codeLocations = data.metrics[0].codeLocations ?? [];
 
   // We only want to show the first 5 code locations
-  const reversedCodeLocations = codeLocations.slice(0, 5);
+  const codeLocationsToShow = codeLocations.slice(0, 5);
   return (
     <CodeLocationsWrapper>
-      {reversedCodeLocations.map((location, index) => (
+      {codeLocationsToShow.slice(0, 5).map((location, index) => (
         <CodeLocation
           key={`location-${index}`}
           codeLocation={location}
           isFirst={index === 0}
-          isLast={index === reversedCodeLocations.length - 1}
+          isLast={index === codeLocationsToShow.length - 1}
         />
       ))}
     </CodeLocationsWrapper>
   );
 }
 
 type CodeLocationProps = {
-  codeLocation: MetricMetaCodeLocation;
+  codeLocation: MetricCodeLocationFrame;
   isFirst?: boolean;
   isLast?: boolean;
 };
@@ -71,12 +71,7 @@ function CodeLocation({codeLocation, isFirst, isLast}: CodeLocationProps) {
     setShowContext(prevState => !prevState);
   }, []);
 
-  const frameToShow = codeLocation.frames[0];
-  if (!frameToShow) {
-    return null;
-  }
-
-  const hasContext = !!frameToShow.contextLine;
+  const hasContext = !!codeLocation.contextLine;
 
   return (
     <CodeLocationWrapper>
@@ -95,18 +90,17 @@ function CodeLocation({codeLocation, isFirst, isLast}: CodeLocationProps) {
           <DefaultLineTitleWrapper>
             <LeftLineTitle>
               <DefaultTitle
-                frame={frameToShow as Frame}
+                frame={codeLocation as Frame}
                 isHoverPreviewed={false}
                 platform="other"
               />
             </LeftLineTitle>
             <DefaultLineActionButtons>
               <CopyToClipboardButton
-                text={`${frameToShow.filename}:${frameToShow.lineNo}`}
+                text={`${codeLocation.filename}:${codeLocation.lineNo}`}
                 size="zero"
                 iconSize="xs"
                 borderless
-                onClick={(event: React.MouseEvent) => event.stopPropagation()}
               />
               <ToggleCodeLocationContextButton
                 disabled={!hasContext}
@@ -117,7 +111,7 @@ function CodeLocation({codeLocation, isFirst, isLast}: CodeLocationProps) {
           </DefaultLineTitleWrapper>
         </DefaultLine>
         {showContext && hasContext && (
-          <CodeLocationContext frame={frameToShow} isLast={isLast} />
+          <CodeLocationContext codeLocation={codeLocation} isLast={isLast} />
         )}
       </DefaultLineWrapper>
     </CodeLocationWrapper>
@@ -152,29 +146,29 @@ function ToggleCodeLocationContextButton({
 }
 
 type CodeLocationContextProps = {
-  frame: MetricCodeLocationFrame;
+  codeLocation: MetricCodeLocationFrame;
   isLast?: boolean;
 };
 
-function CodeLocationContext({frame, isLast}: CodeLocationContextProps) {
-  const lineNo = frame.lineNo ?? 0;
+function CodeLocationContext({codeLocation, isLast}: CodeLocationContextProps) {
+  const lineNo = codeLocation.lineNo ?? 0;
 
   const preContextLines: [number, string][] = useMemo(
-    () => frame.preContext?.map((line, index) => [lineNo - 5 + index, line]) ?? [],
-    [frame.preContext, lineNo]
+    () => codeLocation.preContext?.map((line, index) => [lineNo - 5 + index, line]) ?? [],
+    [codeLocation.preContext, lineNo]
   );
 
   const postContextLines: [number, string][] = useMemo(
-    () => frame.postContext?.map((line, index) => [lineNo + index, line]) ?? [],
-    [frame.postContext, lineNo]
+    () => codeLocation.postContext?.map((line, index) => [lineNo + index, line]) ?? [],
+    [codeLocation.postContext, lineNo]
   );
 
   return (
     <SourceContextWrapper isLast={isLast}>
       {preContextLines.map(line => (
         <ContextLine key={`pre-${line[1]}`} line={line} isActive={false} />
       ))}
-      <ContextLine line={[lineNo, frame.contextLine ?? '']} isActive />
+      <ContextLine line={[lineNo, codeLocation.contextLine ?? '']} isActive />
       {postContextLines.map(line => (
         <ContextLine key={`post-${line[1]}`} line={line} isActive={false} />
       ))}