Skip to content

Commit d9d0f15

Browse files
ZylphrexZylphrex
authored
ref(explore): Consolidate visualize arguments helper (#92212)
This consolidates all the places the visualize arguments are generated into the `useVisualizeFields` helper.
1 parent 347bb82 commit d9d0f15

File tree

4 files changed

+123
-106
lines changed

4 files changed

+123
-106
lines changed

‎static/app/views/explore/hooks/useVisualizeFields.spec.tsx

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@ import {makeTestQueryClient} from 'sentry-test/queryClient';
55
import {renderHook} from 'sentry-test/reactTestingLibrary';
66

77
import type {Organization} from 'sentry/types/organization';
8+
import {parseFunction} from 'sentry/utils/discover/fields';
89
import {DiscoverDatasets} from 'sentry/utils/discover/types';
910
import {QueryClientProvider} from 'sentry/utils/queryClient';
1011
import {useLocation} from 'sentry/utils/useLocation';
1112
import {PageParamsProvider} from 'sentry/views/explore/contexts/pageParamsContext';
12-
import {SpanTagsProvider} from 'sentry/views/explore/contexts/spanTagsContext';
13+
import {
14+
SpanTagsProvider,
15+
useSpanTags,
16+
} from 'sentry/views/explore/contexts/spanTagsContext';
1317
import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields';
1418
import {OrganizationContext} from 'sentry/views/organizationContext';
1519

@@ -32,6 +36,17 @@ function createWrapper(organization: Organization) {
3236
};
3337
}
3438

39+
function useWrapper(yAxis: string) {
40+
const {tags: stringTags} = useSpanTags('string');
41+
const {tags: numberTags} = useSpanTags('number');
42+
return useVisualizeFields({
43+
numberTags,
44+
stringTags,
45+
yAxes: [yAxis],
46+
parsedFunction: parseFunction(yAxis) ?? undefined,
47+
});
48+
}
49+
3550
describe('useVisualizeFields', () => {
3651
const organization = OrganizationFixture();
3752

@@ -46,21 +61,41 @@ describe('useVisualizeFields', () => {
4661
mockedUsedLocation.mockReturnValue(LocationFixture());
4762
});
4863

49-
it('returns a valid list of field options', () => {
50-
const {result} = renderHook(
51-
() =>
52-
useVisualizeFields({
53-
yAxes: ['avg(span.duration)', 'avg(score.ttfb)'],
54-
}),
55-
{
56-
wrapper: createWrapper(organization),
57-
}
58-
);
64+
it('returns numeric fields', () => {
65+
const {result} = renderHook(() => useWrapper('avg(score.ttfb)'), {
66+
wrapper: createWrapper(organization),
67+
});
5968

6069
expect(result.current.map(field => field.value)).toEqual([
6170
'score.ttfb',
6271
'span.duration',
6372
'span.self_time',
6473
]);
6574
});
75+
76+
it('returns numeric fields for count', () => {
77+
const {result} = renderHook(() => useWrapper('count(span.duration)'), {
78+
wrapper: createWrapper(organization),
79+
});
80+
81+
expect(result.current.map(field => field.value)).toEqual(['span.duration']);
82+
});
83+
84+
it('returns string fields for count_unique', () => {
85+
const {result} = renderHook(() => useWrapper('count_unique(foobar)'), {
86+
wrapper: createWrapper(organization),
87+
});
88+
89+
expect(result.current.map(field => field.value)).toEqual(
90+
expect.arrayContaining([
91+
'foobar',
92+
'project',
93+
'span.description',
94+
'span.op',
95+
'timestamp',
96+
'trace',
97+
'transaction',
98+
])
99+
);
100+
});
66101
});

‎static/app/views/explore/hooks/useVisualizeFields.tsx

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,56 @@
11
import {useMemo} from 'react';
22

33
import type {SelectOption} from 'sentry/components/core/compactSelect';
4+
import {t} from 'sentry/locale';
5+
import type {TagCollection} from 'sentry/types/group';
46
import {defined} from 'sentry/utils';
57
import type {ParsedFunction} from 'sentry/utils/discover/fields';
68
import {parseFunction} from 'sentry/utils/discover/fields';
79
import {AggregationKey, FieldKind, prettifyTagKey} from 'sentry/utils/fields';
810
import {AttributeDetails} from 'sentry/views/explore/components/attributeDetails';
911
import {TypeBadge} from 'sentry/views/explore/components/typeBadge';
10-
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';
12+
import {SpanIndexedField} from 'sentry/views/insights/types';
1113

12-
interface Props {
14+
interface UseVisualizeFieldsProps {
15+
numberTags: TagCollection;
16+
stringTags: TagCollection;
1317
/**
1418
* All the aggregates that are in use. The arguments will be extracted
1519
* and injected as options if they are compatible.
1620
*/
1721
yAxes: string[];
18-
/**
19-
* The current aggregate in use. Used to determine what the argument
20-
* types will be compatible.
21-
*/
22-
yAxis?: string;
22+
parsedFunction?: ParsedFunction | null;
2323
}
2424

25-
export function useVisualizeFields({yAxis, yAxes}: Props) {
26-
const {tags: stringTags} = useSpanTags('string');
27-
const {tags: numberTags} = useSpanTags('number');
25+
export function useVisualizeFields({
26+
parsedFunction,
27+
numberTags,
28+
stringTags,
29+
yAxes,
30+
}: UseVisualizeFieldsProps) {
31+
const [kind, tags]: [FieldKind, TagCollection] = useMemo(() => {
32+
if (parsedFunction?.name === AggregationKey.COUNT) {
33+
const countTags: TagCollection = {
34+
[SpanIndexedField.SPAN_DURATION]: {
35+
name: t('spans'),
36+
key: SpanIndexedField.SPAN_DURATION,
37+
},
38+
};
39+
return [FieldKind.MEASUREMENT, countTags];
40+
}
2841

29-
const parsedYAxis = useMemo(() => (yAxis ? parseFunction(yAxis) : undefined), [yAxis]);
42+
if (parsedFunction?.name === AggregationKey.COUNT_UNIQUE) {
43+
return [FieldKind.TAG, stringTags];
44+
}
3045

31-
const tags =
32-
parsedYAxis?.name === AggregationKey.COUNT_UNIQUE ? stringTags : numberTags;
46+
return [FieldKind.MEASUREMENT, numberTags];
47+
}, [parsedFunction?.name, numberTags, stringTags]);
3348

3449
const parsedYAxes: ParsedFunction[] = useMemo(() => {
3550
return yAxes.map(parseFunction).filter(defined);
3651
}, [yAxes]);
3752

3853
const fieldOptions: Array<SelectOption<string>> = useMemo(() => {
39-
const kind =
40-
parsedYAxis?.name === AggregationKey.COUNT_UNIQUE
41-
? FieldKind.TAG
42-
: FieldKind.MEASUREMENT;
43-
4454
const unknownOptions = parsedYAxes
4555
.flatMap(entry => {
4656
return entry.arguments;
@@ -90,7 +100,7 @@ export function useVisualizeFields({yAxis, yAxes}: Props) {
90100
});
91101

92102
return options;
93-
}, [tags, parsedYAxes, parsedYAxis?.name]);
103+
}, [kind, tags, parsedYAxes]);
94104

95105
return fieldOptions;
96106
}

‎static/app/views/explore/multiQueryMode/queryConstructors/visualize.tsx

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ import {defined} from 'sentry/utils';
99
import type {ParsedFunction} from 'sentry/utils/discover/fields';
1010
import {parseFunction} from 'sentry/utils/discover/fields';
1111
import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
12-
import {
13-
DEFAULT_VISUALIZATION_AGGREGATE,
14-
DEFAULT_VISUALIZATION_FIELD,
15-
updateVisualizeAggregate,
16-
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
12+
import {updateVisualizeAggregate} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
13+
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';
1714
import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields';
1815
import {
1916
type ReadableExploreQueryParts,
@@ -31,32 +28,17 @@ type Props = {
3128
};
3229

3330
export function VisualizeSection({query, index}: Props) {
34-
const [yAxis, parsedFunction] = findFirstFunction(query.yAxes);
31+
const {tags: stringTags} = useSpanTags('string');
32+
const {tags: numberTags} = useSpanTags('number');
3533

36-
// We want to lock down the fields dropdown when using count so that we can
37-
// render `count(spans)` for better legibility. However, for backwards
38-
// compatibility, we don't want to lock down all `count` queries immediately.
39-
const lockOptions =
40-
defined(parsedFunction) &&
41-
parsedFunction.name === DEFAULT_VISUALIZATION_AGGREGATE &&
42-
parsedFunction.arguments.length === 1 &&
43-
parsedFunction.arguments[0] === DEFAULT_VISUALIZATION_FIELD;
34+
const parsedFunction = findFirstFunction(query.yAxes);
4435

45-
const countFieldOptions: Array<SelectOption<string>> = useMemo(
46-
() => [
47-
{
48-
label: t('spans'),
49-
value: DEFAULT_VISUALIZATION_FIELD,
50-
textValue: DEFAULT_VISUALIZATION_FIELD,
51-
},
52-
],
53-
[]
54-
);
55-
const defaultFieldOptions: Array<SelectOption<string>> = useVisualizeFields({
36+
const options: Array<SelectOption<string>> = useVisualizeFields({
37+
numberTags,
38+
stringTags,
5639
yAxes: query.yAxes,
57-
yAxis,
40+
parsedFunction,
5841
});
59-
const fieldOptions = lockOptions ? countFieldOptions : defaultFieldOptions;
6042

6143
const updateYAxis = useUpdateQueryAtIndex(index);
6244

@@ -97,13 +79,13 @@ export function VisualizeSection({query, index}: Props) {
9779
/>
9880
<CompactSelect
9981
searchable
100-
options={fieldOptions}
82+
options={options}
10183
value={parsedFunction?.arguments?.[0]}
10284
onChange={newField => {
10385
const newYAxis = `${parsedFunction!.name}(${newField.value})`;
10486
updateYAxis({yAxes: [newYAxis]});
10587
}}
106-
disabled={lockOptions}
88+
disabled={options.length === 1}
10789
/>
10890
</StyledPageFilterBar>
10991
</Fragment>
@@ -113,15 +95,15 @@ export function VisualizeSection({query, index}: Props) {
11395

11496
function findFirstFunction(
11597
yAxes: ReadableExploreQueryParts['yAxes']
116-
): [string | undefined, ParsedFunction | undefined] {
98+
): ParsedFunction | undefined {
11799
for (const yAxis of yAxes) {
118100
const parsed = parseFunction(yAxis);
119101
if (defined(parsed)) {
120-
return [yAxis, parsed];
102+
return parsed;
121103
}
122104
}
123105

124-
return [undefined, undefined];
106+
return undefined;
125107
}
126108

127109
const StyledPageFilterBar = styled(PageFilterBar)`

‎static/app/views/explore/toolbar/toolbarVisualize.tsx

Lines changed: 34 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
updateVisualizeAggregate,
2323
Visualize,
2424
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
25+
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';
2526
import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields';
2627

2728
import {
@@ -136,32 +137,14 @@ function VisualizeDropdown({
136137
yAxis,
137138
label,
138139
}: VisualizeDropdownProps) {
140+
const {tags: stringTags} = useSpanTags('string');
141+
const {tags: numberTags} = useSpanTags('number');
142+
139143
const yAxes: string[] = useMemo(() => {
140144
return visualizes.flatMap(visualize => visualize.yAxes);
141145
}, [visualizes]);
142146

143-
const parsedVisualize = useMemo(() => parseFunction(yAxis)!, [yAxis]);
144-
145-
// We want to lock down the fields dropdown when using count so that we can
146-
// render `count(spans)` for better legibility. However, for backwards
147-
// compatibility, we don't want to lock down all `count` queries immediately.
148-
const lockOptions = yAxis === DEFAULT_VISUALIZATION;
149-
150-
const countFieldOptions: Array<SelectOption<string>> = useMemo(
151-
() => [
152-
{
153-
label: t('spans'),
154-
value: DEFAULT_VISUALIZATION_FIELD,
155-
textValue: DEFAULT_VISUALIZATION_FIELD,
156-
},
157-
],
158-
[]
159-
);
160-
const defaultFieldOptions: Array<SelectOption<string>> = useVisualizeFields({
161-
yAxes,
162-
yAxis,
163-
});
164-
const fieldOptions = lockOptions ? countFieldOptions : defaultFieldOptions;
147+
const parsedFunction = useMemo(() => parseFunction(yAxis)!, [yAxis]);
165148

166149
const aggregateOptions: Array<SelectOption<string>> = useMemo(() => {
167150
return ALLOWED_EXPLORE_VISUALIZE_AGGREGATES.map(aggregate => {
@@ -173,54 +156,61 @@ function VisualizeDropdown({
173156
});
174157
}, []);
175158

176-
const setChartField = useCallback(
177-
({value}: SelectOption<SelectKey>) => {
159+
const fieldOptions: Array<SelectOption<string>> = useVisualizeFields({
160+
numberTags,
161+
stringTags,
162+
yAxes,
163+
parsedFunction,
164+
});
165+
166+
const setYAxis = useCallback(
167+
(newYAxis: string, fields?: string[]) => {
178168
const newVisualizes = visualizes.map((visualize, i) => {
179169
if (i === group) {
180170
const newYAxes = [...visualize.yAxes];
181-
newYAxes[index] = `${parsedVisualize.name}(${value})`;
171+
newYAxes[index] = newYAxis;
182172
visualize = visualize.replace({yAxes: newYAxes});
183173
}
184174
return visualize.toJSON();
185175
});
186-
setVisualizes(newVisualizes, [String(value)]);
176+
setVisualizes(newVisualizes, fields);
187177
},
188-
[group, index, parsedVisualize, setVisualizes, visualizes]
178+
[group, index, setVisualizes, visualizes]
189179
);
190180

191181
const setChartAggregate = useCallback(
192-
({value}: SelectOption<SelectKey>) => {
193-
const newVisualizes = visualizes.map((visualize, i) => {
194-
if (i === group) {
195-
const newYAxes = [...visualize.yAxes];
196-
newYAxes[index] = updateVisualizeAggregate({
197-
newAggregate: value as string,
198-
oldAggregate: parsedVisualize.name,
199-
oldArgument: parsedVisualize.arguments[0]!,
200-
});
201-
visualize = visualize.replace({yAxes: newYAxes});
202-
}
203-
return visualize.toJSON();
182+
(option: SelectOption<SelectKey>) => {
183+
const newYAxis = updateVisualizeAggregate({
184+
newAggregate: option.value as string,
185+
oldAggregate: parsedFunction.name,
186+
oldArgument: parsedFunction.arguments[0]!,
204187
});
205-
setVisualizes(newVisualizes);
188+
setYAxis(newYAxis);
189+
},
190+
[parsedFunction, setYAxis]
191+
);
192+
193+
const setChartField = useCallback(
194+
(option: SelectOption<SelectKey>) => {
195+
setYAxis(`${parsedFunction.name}(${option.value})`, [option.value as string]);
206196
},
207-
[group, index, parsedVisualize, setVisualizes, visualizes]
197+
[parsedFunction.name, setYAxis]
208198
);
209199

210200
return (
211201
<ToolbarRow>
212202
{label && <ChartLabel>{label}</ChartLabel>}
213203
<AggregateCompactSelect
214204
options={aggregateOptions}
215-
value={parsedVisualize.name}
205+
value={parsedFunction.name}
216206
onChange={setChartAggregate}
217207
/>
218208
<ColumnCompactSelect
219209
searchable
220210
options={fieldOptions}
221-
value={parsedVisualize.arguments[0]}
211+
value={parsedFunction.arguments[0]}
222212
onChange={setChartField}
223-
disabled={lockOptions}
213+
disabled={fieldOptions.length === 1}
224214
/>
225215
{canDelete ? (
226216
<Button

0 commit comments

Comments
 (0)