Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ignore me] fix(ddm): display all code locations #23

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion static/app/utils/metrics/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
52 changes: 37 additions & 15 deletions static/app/utils/metrics/useMetricsCodeLocations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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/`,
{
Expand All @@ -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));
});
};

Expand All @@ -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;
};
44 changes: 19 additions & 25 deletions static/app/views/ddm/codeLocations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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;
};
Expand All @@ -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>
Expand All @@ -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}
Expand All @@ -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>
Expand Down Expand Up @@ -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} />
))}
Expand Down
Loading