Skip to content

Commit 36df18e

Browse files
shayna-chAhmed-Labs
shayna-ch
authored and
Ahmed-Labs
committed
ref(groupingInfo): Refactor grouping info to avoid storing redundant data (#102820)
1 parent 8abef94 commit 36df18e

File tree

4 files changed

+127
-34
lines changed

4 files changed

+127
-34
lines changed

‎static/app/components/events/groupingInfo/groupingInfo.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ export default function GroupingInfo({
3939
projectSlug,
4040
});
4141

42-
const variants = groupInfo
43-
? Object.values(groupInfo).sort((a, b) => {
42+
const variants = groupInfo?.variants
43+
? Object.values(groupInfo.variants).sort((a, b) => {
4444
// Sort contributing variants before non-contributing ones
4545
if (a.contributes && !b.contributes) {
4646
return -1;

‎static/app/components/events/groupingInfo/groupingInfoSection.spec.tsx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,59 @@ describe('EventGroupingInfo', () => {
6767
// Should not make grouping-info request
6868
expect(groupingInfoRequest).not.toHaveBeenCalled();
6969
});
70+
71+
it('works with new groupingInfo format', async () => {
72+
groupingInfoRequest = MockApiClient.addMockResponse({
73+
url: `/projects/org-slug/project-slug/events/${event.id}/grouping-info/`,
74+
body: {
75+
grouping_config: 'default:XXXX',
76+
variants: {
77+
app: {
78+
contributes: true,
79+
description: 'variant description',
80+
hash: '123',
81+
hashMismatch: false,
82+
key: 'key',
83+
type: EventGroupVariantType.CHECKSUM,
84+
},
85+
},
86+
},
87+
});
88+
render(<EventGroupingInfoSection {...defaultProps} />);
89+
90+
expect(await screen.findByText('variant description')).toBeInTheDocument();
91+
expect(screen.getByText('123')).toBeInTheDocument();
92+
});
93+
it('gets performance new grouping info from group/event data', async () => {
94+
groupingInfoRequest = MockApiClient.addMockResponse({
95+
url: `/projects/org-slug/project-slug/events/${event.id}/grouping-info/`,
96+
body: {
97+
grouping_config: null,
98+
variants: {
99+
app: {
100+
contributes: true,
101+
description: 'variant description',
102+
hash: '123',
103+
hashMismatch: false,
104+
key: 'key',
105+
type: EventGroupVariantType.CHECKSUM,
106+
},
107+
},
108+
},
109+
});
110+
const perfEvent = EventFixture({
111+
type: 'transaction',
112+
occurrence: {fingerprint: ['123'], evidenceData: {op: 'bad-op'}},
113+
});
114+
const perfGroup = GroupFixture({issueCategory: IssueCategory.PERFORMANCE});
115+
116+
render(
117+
<EventGroupingInfoSection {...defaultProps} event={perfEvent} group={perfGroup} />
118+
);
119+
120+
expect(await screen.findByText('performance problem')).toBeInTheDocument();
121+
expect(screen.getByText('123')).toBeInTheDocument();
122+
// Should not make grouping-info request
123+
expect(groupingInfoRequest).not.toHaveBeenCalled();
124+
});
70125
});

‎static/app/components/events/groupingInfo/groupingSummary.tsx‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,15 @@ export function GroupInfoSummary({
2222
group,
2323
projectSlug,
2424
});
25-
const groupedBy = groupInfo
26-
? Object.values(groupInfo)
25+
const groupedBy = groupInfo?.variants
26+
? Object.values(groupInfo.variants)
2727
.filter(variant => variant.contributes && variant.description !== null)
2828
.map(variant => variant.description!)
2929
.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase()))
3030
.join(', ')
3131
: t('nothing');
3232

33-
const groupingConfig =
34-
showGroupingConfig && groupInfo
35-
? (
36-
Object.values(groupInfo).find(
37-
variant => 'config' in variant && variant.config?.id
38-
) as any
39-
)?.config?.id
40-
: null;
33+
const groupingConfig = showGroupingConfig && groupInfo?.grouping_config;
4134

4235
if (isPending && !hasPerformanceGrouping) {
4336
return (

‎static/app/components/events/groupingInfo/useEventGroupingInfo.tsx‎

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,37 @@ import type {Group} from 'sentry/types/group';
88
import {useApiQuery} from 'sentry/utils/queryClient';
99
import useOrganization from 'sentry/utils/useOrganization';
1010

11-
type EventGroupingInfoResponse = Record<string, EventGroupVariant>;
11+
type EventGroupingInfoResponseOld = Record<string, EventGroupVariant>;
12+
type EventGroupingInfoResponse = {
13+
grouping_config: string | null;
14+
variants: Record<string, EventGroupVariant>;
15+
};
16+
17+
// temporary function to convert the old response structure to the new one
18+
function eventGroupingInfoResponseOldToNew(
19+
old: EventGroupingInfoResponseOld | null
20+
): EventGroupingInfoResponse | null {
21+
const grouping_config = old
22+
? (
23+
Object.values(old).find(
24+
variant => 'config' in variant && variant.config?.id
25+
) as any
26+
)?.config?.id
27+
: null;
28+
return old
29+
? {
30+
grouping_config,
31+
variants: old,
32+
}
33+
: null;
34+
}
35+
36+
// temporary function to check if the respinse is old type
37+
function isOld(
38+
data: EventGroupingInfoResponseOld | EventGroupingInfoResponse | null
39+
): data is EventGroupingInfoResponseOld {
40+
return data ? !('grouping_config' in data) : false;
41+
}
1242

1343
function generatePerformanceGroupInfo({
1444
event,
@@ -27,21 +57,24 @@ function generatePerformanceGroupInfo({
2757

2858
return group
2959
? {
30-
[group.issueType]: {
31-
contributes: true,
32-
description: t('performance problem'),
33-
hash: event.occurrence?.fingerprint[0] || '',
34-
hashMismatch: false,
35-
hint: null,
36-
key: group.issueType,
37-
type: EventGroupVariantType.PERFORMANCE_PROBLEM,
38-
evidence: {
39-
op: evidenceData?.op,
40-
parent_span_ids: evidenceData?.parentSpanIds,
41-
cause_span_ids: evidenceData?.causeSpanIds,
42-
offender_span_ids: evidenceData?.offenderSpanIds,
43-
desc: t('performance problem'),
44-
fingerprint: hash,
60+
grouping_config: null,
61+
variants: {
62+
[group.issueType]: {
63+
contributes: true,
64+
description: t('performance problem'),
65+
hash: event.occurrence?.fingerprint[0] || '',
66+
hashMismatch: false,
67+
hint: null,
68+
key: group.issueType,
69+
type: EventGroupVariantType.PERFORMANCE_PROBLEM,
70+
evidence: {
71+
op: evidenceData?.op,
72+
parent_span_ids: evidenceData?.parentSpanIds,
73+
cause_span_ids: evidenceData?.causeSpanIds,
74+
offender_span_ids: evidenceData?.offenderSpanIds,
75+
desc: t('performance problem'),
76+
fingerprint: hash,
77+
},
4578
},
4679
},
4780
}
@@ -61,14 +94,26 @@ export function useEventGroupingInfo({
6194

6295
const hasPerformanceGrouping = event.occurrence && event.type === 'transaction';
6396

64-
const {data, isPending, isError, isSuccess} = useApiQuery<EventGroupingInfoResponse>(
65-
[`/projects/${organization.slug}/${projectSlug}/events/${event.id}/grouping-info/`],
66-
{enabled: !hasPerformanceGrouping, staleTime: Infinity}
67-
);
97+
const {data, isPending, isError, isSuccess} = useApiQuery<
98+
EventGroupingInfoResponseOld | EventGroupingInfoResponse
99+
>([`/projects/${organization.slug}/${projectSlug}/events/${event.id}/grouping-info/`], {
100+
enabled: !hasPerformanceGrouping,
101+
staleTime: Infinity,
102+
});
68103

69-
const groupInfo = hasPerformanceGrouping
104+
const groupInfoRaw = hasPerformanceGrouping
70105
? generatePerformanceGroupInfo({group, event})
71106
: (data ?? null);
72107

73-
return {groupInfo, isPending, isError, isSuccess, hasPerformanceGrouping};
108+
const groupInfo = isOld(groupInfoRaw)
109+
? eventGroupingInfoResponseOldToNew(groupInfoRaw)
110+
: groupInfoRaw;
111+
112+
return {
113+
groupInfo,
114+
isPending,
115+
isError,
116+
isSuccess,
117+
hasPerformanceGrouping,
118+
};
74119
}

0 commit comments

Comments
 (0)