Skip to content

Commit 168cf45

Browse files
authored
feat(explore): Map input to known attribute in equations (#95589)
When users type the name of an attribute without the explicit tag syntax, we want to map it to the explicit tag syntax so it's a valid column whenever possible.
1 parent bcb733f commit 168cf45

File tree

9 files changed

+195
-25
lines changed

9 files changed

+195
-25
lines changed

static/app/components/arithmeticBuilder/context.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ interface ArithmeticBuilderContextData {
1414
focusOverride: FocusOverride | null;
1515
functionArguments: FunctionArgument[];
1616
getFieldDefinition: (key: string) => FieldDefinition | null;
17+
getSuggestedKey?: (key: string) => string | null;
1718
}
1819

1920
export const ArithmeticBuilderContext = createContext<ArithmeticBuilderContextData>({

static/app/components/arithmeticBuilder/index.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,26 @@ interface ArithmeticBuilderProps {
1818
functionArguments: FunctionArgument[];
1919
getFieldDefinition: (key: string) => FieldDefinition | null;
2020
className?: string;
21+
'data-test-id'?: string;
2122
disabled?: boolean;
23+
/**
24+
* This is used when a user types in a search key and submits the token.
25+
* The submission happens when the user types a colon or presses enter.
26+
* When this happens, this function is used to try to map the user input
27+
* to a known column.
28+
*/
29+
getSuggestedKey?: (key: string) => string | null;
2230
setExpression?: (expression: Expression) => void;
2331
}
2432

2533
export function ArithmeticBuilder({
34+
'data-test-id': dataTestId,
2635
expression,
2736
setExpression,
2837
aggregations,
2938
functionArguments,
3039
getFieldDefinition,
40+
getSuggestedKey,
3141
className,
3242
disabled,
3343
}: ArithmeticBuilderProps) {
@@ -45,16 +55,24 @@ export function ArithmeticBuilder({
4555
}),
4656
functionArguments,
4757
getFieldDefinition,
58+
getSuggestedKey,
4859
};
49-
}, [state, dispatch, aggregations, functionArguments, getFieldDefinition]);
60+
}, [
61+
state,
62+
dispatch,
63+
aggregations,
64+
functionArguments,
65+
getFieldDefinition,
66+
getSuggestedKey,
67+
]);
5068

5169
return (
5270
<PanelProvider>
5371
<ArithmeticBuilderContext value={contextValue}>
5472
<Wrapper
5573
className={className}
5674
aria-disabled={disabled}
57-
data-test-id="arithmetic-builder"
75+
data-test-id={dataTestId ?? 'arithmetic-builder'}
5876
state={state.expression.isValid ? 'valid' : 'invalid'}
5977
>
6078
<TokenGrid tokens={state.expression.tokens} />

static/app/components/arithmeticBuilder/token/function.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ function InternalInput({
120120
updateSelectionIndex();
121121
}, [updateSelectionIndex]);
122122

123-
const {dispatch, functionArguments, getFieldDefinition} = useArithmeticBuilder();
123+
const {dispatch, functionArguments, getFieldDefinition, getSuggestedKey} =
124+
useArithmeticBuilder();
124125

125126
const parameterDefinition = useMemo(
126127
() => getFieldDefinition(token.function)?.parameters?.[argumentIndex],
@@ -182,7 +183,16 @@ function InternalInput({
182183
);
183184

184185
const onInputCommit = useCallback(() => {
185-
const value = inputValue.trim() || attribute.attribute;
186+
let value = inputValue.trim() || attribute.attribute;
187+
188+
if (
189+
defined(getSuggestedKey) &&
190+
parameterDefinition &&
191+
parameterDefinition.kind === 'column'
192+
) {
193+
value = getSuggestedKey(value) ?? value;
194+
}
195+
186196
dispatch({
187197
text: `${token.function}(${value})`,
188198
type: 'REPLACE_TOKEN',
@@ -192,7 +202,16 @@ function InternalInput({
192202
},
193203
});
194204
resetInputValue();
195-
}, [dispatch, state, token, attribute, inputValue, resetInputValue]);
205+
}, [
206+
dispatch,
207+
state,
208+
token,
209+
attribute,
210+
inputValue,
211+
resetInputValue,
212+
getSuggestedKey,
213+
parameterDefinition,
214+
]);
196215

197216
const onInputEscape = useCallback(() => {
198217
resetInputValue();

static/app/components/arithmeticBuilder/token/index.spec.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ const getSpanFieldDefinition = (key: string) => {
3636
return getFieldDefinition(key, 'span', argument?.kind);
3737
};
3838

39+
const getSuggestedKey = (key: string) => {
40+
switch (key) {
41+
case 'duration':
42+
case 'self_time':
43+
case 'op':
44+
case 'description':
45+
return `span.${key}`;
46+
default:
47+
return null;
48+
}
49+
};
50+
3951
interface TokensProp {
4052
expression: string;
4153
dispatch?: Dispatch<ArithmeticBuilderAction>;
@@ -62,6 +74,7 @@ function Tokens(props: TokensProp) {
6274
aggregations,
6375
functionArguments,
6476
getFieldDefinition: getSpanFieldDefinition,
77+
getSuggestedKey,
6578
}}
6679
>
6780
<TokenGrid tokens={state.expression.tokens} />
@@ -518,6 +531,38 @@ describe('token', function () {
518531
).toBeInTheDocument();
519532
});
520533

534+
it('maps key to suggested key on enter', async function () {
535+
render(<Tokens expression="avg(span.duration)" />);
536+
537+
expect(
538+
await screen.findByRole('row', {
539+
name: 'avg(span.duration)',
540+
})
541+
).toBeInTheDocument();
542+
543+
const input = screen.getByRole('combobox', {
544+
name: 'Select an attribute',
545+
});
546+
expect(input).toBeInTheDocument();
547+
548+
await userEvent.click(input);
549+
expect(input).toHaveFocus();
550+
expect(input).toHaveAttribute('placeholder', 'span.duration');
551+
expect(input).toHaveValue('');
552+
553+
await userEvent.type(input, 'self_time{Enter}');
554+
555+
const lastInput = getLastInput();
556+
await waitFor(() => expect(lastInput).toHaveFocus());
557+
await userEvent.type(lastInput, '{Escape}');
558+
559+
expect(
560+
await screen.findByRole('row', {
561+
name: 'avg(span.self_time)',
562+
})
563+
).toBeInTheDocument();
564+
});
565+
521566
it('doesnt change argument on enter if input is empty', async function () {
522567
render(<Tokens expression="avg(span.duration)" />);
523568

static/app/views/explore/components/traceItemSearchQueryBuilder.tsx

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useMemo} from 'react';
1+
import {useMemo} from 'react';
22

33
import {getHasTag} from 'sentry/components/events/searchBar';
44
import type {EAPSpanSearchQueryBuilderProps} from 'sentry/components/performance/spanSearchQueryBuilder';
@@ -7,6 +7,7 @@ import {t} from 'sentry/locale';
77
import {SavedSearchType, type TagCollection} from 'sentry/types/group';
88
import type {AggregationKey} from 'sentry/utils/fields';
99
import {FieldKind, getFieldDefinition} from 'sentry/utils/fields';
10+
import {useExploreSuggestedAttribute} from 'sentry/views/explore/hooks/useExploreSuggestedAttribute';
1011
import {useGetTraceItemAttributeValues} from 'sentry/views/explore/hooks/useGetTraceItemAttributeValues';
1112
import {LOGS_FILTER_KEY_SECTIONS} from 'sentry/views/explore/logs/constants';
1213
import {TraceItemDataset} from 'sentry/views/explore/types';
@@ -74,24 +75,10 @@ export function useSearchQueryBuilderProps({
7475
projectIds: projects,
7576
});
7677

77-
const getSuggestedFilterKey = useCallback(
78-
(key: string) => {
79-
// prioritize exact matches first
80-
if (filterTags.hasOwnProperty(key)) {
81-
return key;
82-
}
83-
84-
// try to see if there's numeric attribute by the same name
85-
const explicitNumberTag = `tags[${key},number]`;
86-
if (filterTags.hasOwnProperty(explicitNumberTag)) {
87-
return explicitNumberTag;
88-
}
89-
90-
// give up, and fall back to the default behaviour
91-
return null;
92-
},
93-
[filterTags]
94-
);
78+
const getSuggestedAttribute = useExploreSuggestedAttribute({
79+
numberAttributes,
80+
stringAttributes,
81+
});
9582

9683
return {
9784
placeholder: placeholderText,
@@ -104,7 +91,7 @@ export function useSearchQueryBuilderProps({
10491
getFilterTokenWarning,
10592
searchSource,
10693
filterKeySections,
107-
getSuggestedFilterKey,
94+
getSuggestedFilterKey: getSuggestedAttribute,
10895
getTagValues: getTraceItemAttributeValues,
10996
disallowUnsupportedFilters: true,
11097
recentSearches: itemTypeToRecentSearches(itemType),
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import {useCallback} from 'react';
2+
3+
import type {TagCollection} from 'sentry/types/group';
4+
5+
interface UseExploreSuggestedAttributeOptions {
6+
numberAttributes: TagCollection;
7+
stringAttributes: TagCollection;
8+
}
9+
10+
export function useExploreSuggestedAttribute({
11+
numberAttributes,
12+
stringAttributes,
13+
}: UseExploreSuggestedAttributeOptions) {
14+
return useCallback(
15+
(key: string): string | null => {
16+
if (stringAttributes.hasOwnProperty(key)) {
17+
return key;
18+
}
19+
20+
if (numberAttributes.hasOwnProperty(key)) {
21+
return key;
22+
}
23+
24+
const explicitStringAttribute = `tags[${key},string]`;
25+
if (stringAttributes.hasOwnProperty(explicitStringAttribute)) {
26+
return explicitStringAttribute;
27+
}
28+
29+
const explicitNumberAttribute = `tags[${key},number]`;
30+
if (numberAttributes.hasOwnProperty(explicitNumberAttribute)) {
31+
return explicitNumberAttribute;
32+
}
33+
34+
return null;
35+
},
36+
[numberAttributes, stringAttributes]
37+
);
38+
}

static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {initializeOrg} from 'sentry-test/initializeOrg';
12
import {
23
act,
34
renderGlobalModal,
@@ -57,6 +58,11 @@ const numberTags: TagCollection = {
5758
name: 'span.self_time',
5859
kind: FieldKind.MEASUREMENT,
5960
},
61+
'tags[foo,number]': {
62+
key: 'tags[foo,number]',
63+
name: 'foo',
64+
kind: FieldKind.MEASUREMENT,
65+
},
6066
};
6167

6268
describe('AggregateColumnEditorModal', function () {
@@ -263,6 +269,47 @@ describe('AggregateColumnEditorModal', function () {
263269
{yAxes: ['count(span.duration)']},
264270
]);
265271
});
272+
273+
it('allows adding an equation', async function () {
274+
const {organization} = initializeOrg({
275+
organization: {
276+
features: ['visibility-explore-equations'],
277+
},
278+
});
279+
280+
const onColumnsChange = jest.fn();
281+
282+
renderGlobalModal({organization});
283+
284+
act(() => {
285+
openModal(
286+
modalProps => (
287+
<AggregateColumnEditorModal
288+
{...modalProps}
289+
columns={[{groupBy: 'geo.country'}, new Visualize(DEFAULT_VISUALIZATION)]}
290+
onColumnsChange={onColumnsChange}
291+
stringTags={stringTags}
292+
numberTags={numberTags}
293+
/>
294+
),
295+
{onClose: jest.fn()}
296+
);
297+
});
298+
299+
await userEvent.click(screen.getByRole('button', {name: 'Add a Column'}));
300+
await userEvent.click(screen.getByRole('menuitemradio', {name: 'Equation'}));
301+
302+
await userEvent.click(screen.getByRole('combobox', {name: 'Add a term'}));
303+
304+
await userEvent.keyboard('avg(foo{Enter}*5{Escape}');
305+
306+
await userEvent.click(screen.getByRole('button', {name: 'Apply'}));
307+
expect(onColumnsChange).toHaveBeenCalledWith([
308+
{groupBy: 'geo.country'},
309+
{yAxes: ['count(span.duration)']},
310+
{yAxes: ['equation|avg(tags[foo,number]) * 5']},
311+
]);
312+
});
266313
});
267314

268315
function expectRows(rows: HTMLElement[]) {

static/app/views/explore/tables/aggregateColumnEditorModal.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import {
4848
Visualize,
4949
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
5050
import type {Column} from 'sentry/views/explore/hooks/useDragNDropColumns';
51+
import {useExploreSuggestedAttribute} from 'sentry/views/explore/hooks/useExploreSuggestedAttribute';
5152
import {useGroupByFields} from 'sentry/views/explore/hooks/useGroupByFields';
5253
import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields';
5354

@@ -425,13 +426,20 @@ function EquationSelector({
425426
[onChange, visualize]
426427
);
427428

429+
const getSuggestedAttribute = useExploreSuggestedAttribute({
430+
numberAttributes: numberTags,
431+
stringAttributes: stringTags,
432+
});
433+
428434
return (
429435
<ArithmeticBuilder
436+
data-test-id="editor-visualize-equation"
430437
aggregations={ALLOWED_EXPLORE_VISUALIZE_AGGREGATES}
431438
functionArguments={functionArguments}
432439
getFieldDefinition={getSpanFieldDefinition}
433440
expression={expression}
434441
setExpression={handleExpressionChange}
442+
getSuggestedKey={getSuggestedAttribute}
435443
/>
436444
);
437445
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
Visualize,
3434
} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
3535
import {useTraceItemTags} from 'sentry/views/explore/contexts/spanTagsContext';
36+
import {useExploreSuggestedAttribute} from 'sentry/views/explore/hooks/useExploreSuggestedAttribute';
3637
import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields';
3738

3839
import {
@@ -206,6 +207,11 @@ function VisualizeEquation({
206207
[group, setVisualizes, visualizes]
207208
);
208209

210+
const getSuggestedAttribute = useExploreSuggestedAttribute({
211+
numberAttributes: numberTags,
212+
stringAttributes: stringTags,
213+
});
214+
209215
return (
210216
<ToolbarRow>
211217
<ArithmeticBuilder
@@ -214,6 +220,7 @@ function VisualizeEquation({
214220
getFieldDefinition={getSpanFieldDefinition}
215221
expression={expression}
216222
setExpression={handleExpressionChange}
223+
getSuggestedKey={getSuggestedAttribute}
217224
/>
218225
<Button
219226
borderless

0 commit comments

Comments
 (0)