Skip to content

Commit 290e7c9

Browse files
ZylphrexZylphrex
authored
feat(explore): Add support for count_unique in eap (#89558)
This adds support for the `count_unique` aggregate in eap enabled views. Closes EXP-7
1 parent 2815b3d commit 290e7c9

File tree

13 files changed

+416
-45
lines changed

13 files changed

+416
-45
lines changed

‎static/app/utils/fields/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ export const ALLOWED_EXPLORE_VISUALIZE_FIELDS: SpanIndexedField[] = [
829829

830830
export const ALLOWED_EXPLORE_VISUALIZE_AGGREGATES: AggregationKey[] = [
831831
AggregationKey.COUNT, // DO NOT RE-ORDER: the first element is used as the default
832+
AggregationKey.COUNT_UNIQUE,
832833
AggregationKey.AVG,
833834
AggregationKey.P50,
834835
AggregationKey.P75,

‎static/app/views/alerts/rules/metric/eapField.spec.tsx‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,18 +82,6 @@ describe('EAPField', () => {
8282
}
8383

8484
render(<Component />);
85-
expect(fieldsMock).toHaveBeenCalledWith(
86-
`/organizations/${organization.slug}/spans/fields/`,
87-
expect.objectContaining({
88-
query: expect.objectContaining({type: 'number'}),
89-
})
90-
);
91-
expect(fieldsMock).toHaveBeenCalledWith(
92-
`/organizations/${organization.slug}/spans/fields/`,
93-
expect.objectContaining({
94-
query: expect.objectContaining({type: 'string'}),
95-
})
96-
);
9785

9886
// switch from count(spans) -> max(span.duration)
9987
await userEvent.click(screen.getByText('count'));
@@ -111,4 +99,37 @@ describe('EAPField', () => {
11199
expect(screen.getByText('count')).toBeInTheDocument();
112100
expect(screen.getByText('spans')).toBeInTheDocument();
113101
});
102+
103+
it('defaults count_unique argument to span.op', async function () {
104+
function Component() {
105+
const [aggregate, setAggregate] = useState('count(span.duration)');
106+
return (
107+
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP} enabled>
108+
<EAPField aggregate={aggregate} onChange={setAggregate} />
109+
</SpanTagsProvider>
110+
);
111+
}
112+
113+
render(<Component />);
114+
115+
// switch from count(spans) -> count_unique(span.op)
116+
await userEvent.click(screen.getByText('count'));
117+
await userEvent.click(await screen.findByText('count_unique'));
118+
expect(screen.getByText('count_unique')).toBeInTheDocument();
119+
expect(screen.getByText('span.op')).toBeInTheDocument();
120+
121+
// switch from count_unique(span.op) -> avg(span.self_time)
122+
await userEvent.click(screen.getByText('count_unique'));
123+
await userEvent.click(await screen.findByText('avg'));
124+
await userEvent.click(screen.getByText('span.duration'));
125+
await userEvent.click(await screen.findByText('span.self_time'));
126+
expect(screen.getByText('avg')).toBeInTheDocument();
127+
expect(screen.getByText('span.self_time')).toBeInTheDocument();
128+
129+
// switch from avg(span.self_time) -> count_unique(span.op)
130+
await userEvent.click(screen.getByText('avg'));
131+
await userEvent.click(await screen.findByText('count_unique'));
132+
expect(screen.getByText('count_unique')).toBeInTheDocument();
133+
expect(screen.getByText('span.op')).toBeInTheDocument();
134+
});
114135
});

‎static/app/views/alerts/rules/metric/eapField.tsx‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,17 @@ import {space} from 'sentry/styles/space';
77
import type {TagCollection} from 'sentry/types/group';
88
import {defined} from 'sentry/utils';
99
import {parseFunction, prettifyTagKey} from 'sentry/utils/discover/fields';
10-
import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
10+
import {AggregationKey, ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
1111
import {
1212
DEFAULT_VISUALIZATION,
13-
DEFAULT_VISUALIZATION_AGGREGATE,
1413
DEFAULT_VISUALIZATION_FIELD,
14+
updateVisualizeAggregate,
1515
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
1616
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';
1717

18+
export const DEFAULT_EAP_AGGREGATION = 'count';
1819
export const DEFAULT_EAP_FIELD = 'span.duration';
19-
export const DEFAULT_EAP_METRICS_ALERT_FIELD = `count(${DEFAULT_EAP_FIELD})`;
20+
export const DEFAULT_EAP_METRICS_ALERT_FIELD = `${DEFAULT_EAP_AGGREGATION}(${DEFAULT_EAP_FIELD})`;
2021

2122
interface Props {
2223
aggregate: string;
@@ -48,7 +49,10 @@ function EAPField({aggregate, onChange}: Props) {
4849
// compatibility, we don't want to lock down all `count` queries immediately.
4950
const lockOptions = aggregate === DEFAULT_VISUALIZATION;
5051

51-
const {tags: storedTags} = useSpanTags('number');
52+
const {tags: storedStringTags} = useSpanTags('string');
53+
const {tags: storedNumberTags} = useSpanTags('number');
54+
const storedTags =
55+
aggregation === AggregationKey.COUNT_UNIQUE ? storedStringTags : storedNumberTags;
5256
const numberTags: TagCollection = useMemo(() => {
5357
const availableTags: TagCollection = storedTags;
5458
if (field && !defined(storedTags[field])) {
@@ -84,13 +88,14 @@ function EAPField({aggregate, onChange}: Props) {
8488

8589
const handleOperationChange = useCallback(
8690
(option: any) => {
87-
const newAggregate =
88-
option.value === DEFAULT_VISUALIZATION_AGGREGATE
89-
? DEFAULT_VISUALIZATION
90-
: `${option.value}(${field || DEFAULT_EAP_FIELD})`;
91+
const newAggregate = updateVisualizeAggregate({
92+
newAggregate: option.value,
93+
oldAggregate: aggregation || DEFAULT_EAP_AGGREGATION,
94+
oldArgument: field || DEFAULT_EAP_FIELD,
95+
});
9196
onChange(newAggregate, {});
9297
},
93-
[field, onChange]
98+
[aggregation, field, onChange]
9499
);
95100

96101
// As SelectControl does not support an options size limit out of the box

‎static/app/views/dashboards/datasetConfig/spans.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
doDiscoverQuery,
2222
} from 'sentry/utils/discover/genericDiscoverQuery';
2323
import {DiscoverDatasets} from 'sentry/utils/discover/types';
24-
import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
24+
import {AggregationKey, ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
2525
import type {MEPState} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
2626
import type {OnDemandControlContext} from 'sentry/utils/performance/contexts/onDemandControl';
2727
import {
@@ -177,15 +177,22 @@ function _isNotNumericTag(option: FieldValueOption) {
177177
return true;
178178
}
179179

180-
function filterAggregateParams(option: FieldValueOption) {
180+
function filterAggregateParams(option: FieldValueOption, fieldValue?: QueryFieldValue) {
181181
// Allow for unknown values to be used for aggregate functions
182182
// This supports showing the tag value even if it's not in the current
183183
// set of tags.
184184
if ('unknown' in option.value.meta && option.value.meta.unknown) {
185185
return true;
186186
}
187+
188+
const expectedDataType =
189+
fieldValue?.kind === 'function' &&
190+
fieldValue?.function[0] === AggregationKey.COUNT_UNIQUE
191+
? 'string'
192+
: 'number';
193+
187194
if ('dataType' in option.value.meta) {
188-
return option.value.meta.dataType === 'number';
195+
return option.value.meta.dataType === expectedDataType;
189196
}
190197
return true;
191198
}

‎static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,19 @@ describe('Visualize', () => {
4242
return {tags, isLoading: false};
4343
}
4444

45+
const tags: TagCollection = {
46+
'span.op': {
47+
key: 'span.op',
48+
name: 'span.op',
49+
},
50+
'span.description': {
51+
key: 'span.description',
52+
name: 'span.description',
53+
},
54+
};
55+
4556
return {
46-
tags: {},
57+
tags,
4758
isLoading: false,
4859
};
4960
});
@@ -1379,4 +1390,70 @@ describe('Visualize', () => {
13791390
'spans'
13801391
);
13811392
});
1393+
1394+
it('defaults count_unique argument to span.op', async function () {
1395+
render(
1396+
<WidgetBuilderProvider>
1397+
<Visualize />
1398+
</WidgetBuilderProvider>,
1399+
{
1400+
organization,
1401+
router: RouterFixture({
1402+
location: LocationFixture({
1403+
query: {
1404+
dataset: WidgetType.SPANS,
1405+
displayType: DisplayType.LINE,
1406+
yAxis: ['count(span.duration)'],
1407+
},
1408+
}),
1409+
}),
1410+
}
1411+
);
1412+
1413+
expect(
1414+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1415+
).toBeEnabled();
1416+
1417+
expect(
1418+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1419+
).toHaveTextContent('count');
1420+
expect(
1421+
await screen.findByRole('button', {name: 'Column Selection'})
1422+
).toHaveTextContent('spans');
1423+
1424+
await userEvent.click(
1425+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1426+
);
1427+
await userEvent.click(await screen.findByRole('option', {name: 'count_unique'}));
1428+
1429+
expect(
1430+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1431+
).toHaveTextContent('count_unique');
1432+
expect(
1433+
await screen.findByRole('button', {name: 'Column Selection'})
1434+
).toHaveTextContent('span.op');
1435+
1436+
await userEvent.click(
1437+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1438+
);
1439+
await userEvent.click(await screen.findByRole('option', {name: 'avg'}));
1440+
1441+
expect(
1442+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1443+
).toHaveTextContent('avg');
1444+
expect(
1445+
await screen.findByRole('button', {name: 'Column Selection'})
1446+
).toHaveTextContent('span.duration');
1447+
1448+
await userEvent.click(
1449+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1450+
);
1451+
await userEvent.click(await screen.findByRole('option', {name: 'count_unique'}));
1452+
expect(
1453+
await screen.findByRole('button', {name: 'Aggregate Selection'})
1454+
).toHaveTextContent('count_unique');
1455+
expect(
1456+
await screen.findByRole('button', {name: 'Column Selection'})
1457+
).toHaveTextContent('span.op');
1458+
});
13821459
});

‎static/app/views/dashboards/widgetBuilder/components/visualize/selectRow.tsx‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
parseFunction,
1717
type QueryFieldValue,
1818
} from 'sentry/utils/discover/fields';
19+
import {AggregationKey} from 'sentry/utils/fields';
1920
import useOrganization from 'sentry/utils/useOrganization';
2021
import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base';
2122
import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
@@ -36,6 +37,7 @@ import {
3637
DEFAULT_VISUALIZATION_AGGREGATE,
3738
DEFAULT_VISUALIZATION_FIELD,
3839
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
40+
import {SpanIndexedField} from 'sentry/views/insights/types';
3941

4042
type AggregateFunction = [
4143
AggregationKeyWithAlias,
@@ -266,6 +268,7 @@ export function SelectRow({
266268
openColumnSelect();
267269
} else {
268270
if (currentField.kind === FieldValueKind.FUNCTION) {
271+
const originalFunction = currentField.function[0];
269272
// Handle setting an aggregate from an aggregate
270273
currentField.function[0] = parseAggregateFromValueKey(
271274
dropdownSelection.value as string
@@ -279,6 +282,30 @@ export function SelectRow({
279282
) {
280283
currentField.function[1] = DEFAULT_VISUALIZATION_FIELD;
281284

285+
// Wipe out the remaining parameters that are unnecessary
286+
for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) {
287+
currentField.function[i + 1] = undefined;
288+
}
289+
} else if (
290+
// when switching to the count_unique aggregate, we want to reset the
291+
// field to the default
292+
state.dataset === WidgetType.SPANS &&
293+
currentField.function[0] === AggregationKey.COUNT_UNIQUE
294+
) {
295+
currentField.function[1] = SpanIndexedField.SPAN_OP;
296+
297+
// Wipe out the remaining parameters that are unnecessary
298+
for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) {
299+
currentField.function[i + 1] = undefined;
300+
}
301+
} else if (
302+
// when switching away from the count_unique aggregate, we want to reset the
303+
// field to the default
304+
state.dataset === WidgetType.SPANS &&
305+
originalFunction === AggregationKey.COUNT_UNIQUE
306+
) {
307+
currentField.function[1] = DEFAULT_VISUALIZATION_FIELD;
308+
282309
// Wipe out the remaining parameters that are unnecessary
283310
for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) {
284311
currentField.function[i + 1] = undefined;

‎static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -968,8 +968,8 @@ describe('WidgetBuilder', function () {
968968
await screen.findByText('Sort by a y-axis');
969969
await selectEvent.openMenu(screen.getAllByText('count(\u2026)')[1]!);
970970

971-
// 12 options in the dropdown
972-
expect(screen.queryAllByTestId('menu-list-item-label')).toHaveLength(12);
971+
// Check the number of options in the dropdown
972+
expect(screen.queryAllByTestId('menu-list-item-label')).toHaveLength(13);
973973

974974
// Appears once in the y-axis section, dropdown, and in the sort by field
975975
expect(await screen.findAllByText('count(\u2026)')).toHaveLength(3);

‎static/app/views/explore/contexts/pageParamsContext/visualizes.tsx‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import type {Organization} from 'sentry/types/organization';
55
import {defined} from 'sentry/utils';
66
import {parseFunction} from 'sentry/utils/discover/fields';
77
import {
8+
AggregationKey,
89
ALLOWED_EXPLORE_VISUALIZE_AGGREGATES,
910
ALLOWED_EXPLORE_VISUALIZE_FIELDS,
1011
} from 'sentry/utils/fields';
1112
import {decodeList} from 'sentry/utils/queryString';
1213
import {ChartType} from 'sentry/views/insights/common/components/chart';
14+
import {SpanIndexedField} from 'sentry/views/insights/types';
1315

1416
export const MAX_VISUALIZES = 4;
1517

@@ -100,3 +102,34 @@ export function updateLocationWithVisualizes(
100102
delete location.query.visualize;
101103
}
102104
}
105+
106+
export function updateVisualizeAggregate({
107+
newAggregate,
108+
oldAggregate,
109+
oldArgument,
110+
}: {
111+
newAggregate: string;
112+
oldAggregate: string;
113+
oldArgument: string;
114+
}): string {
115+
// the default aggregate only has 1 allowed field
116+
if (newAggregate === DEFAULT_VISUALIZATION_AGGREGATE) {
117+
return DEFAULT_VISUALIZATION;
118+
}
119+
120+
// count_unique uses a different set of fields
121+
if (newAggregate === AggregationKey.COUNT_UNIQUE) {
122+
// The general thing to do here is to valid the the argument type
123+
// and carry the argument if it's the same type, reset to a default
124+
// if it's not the same type. Just hard coding it for now for simplicity
125+
// as `count_unique` is the only aggregate that takes a string.
126+
return `${AggregationKey.COUNT_UNIQUE}(${SpanIndexedField.SPAN_OP})`;
127+
}
128+
129+
// switching away from count_unique means we need to reset the field
130+
if (oldAggregate === AggregationKey.COUNT_UNIQUE) {
131+
return `${newAggregate}(${DEFAULT_VISUALIZATION_FIELD})`;
132+
}
133+
134+
return `${newAggregate}(${oldArgument})`;
135+
}

0 commit comments

Comments
 (0)