Skip to content

Commit dd7fb18

Browse files
narsaynorathnarsaynorath
authored
feat(tracemetrics): Use reference map to render equations (#112817)
Updates the references hook to return a reference map where the keys are the query label (A, B, etc) and the values are the aggregate which represent that query for use in equations. If there's a filter, we convert it to the `_if` format. e.g. a count on metric A for `geo.country:Canada` would look like ``` count_if(`geo.country:Canada`,value,metricA,distribution,none) ``` The equation builder will use the reference map to turn the input expression into the A, B, etc form and render that form. Any updates to that will call a callback that "resolves" the equation to its original aggregate form so we can use it to make downstream `events-timeseries` request. I added an effect to pick up on when the reference map changes, because if I'm plotting `A + B` and `B` changes, I'd want to call the callback so we continue to plot the correct data.
1 parent 287adbf commit dd7fb18

File tree

11 files changed

+340
-43
lines changed

11 files changed

+340
-43
lines changed

‎static/app/views/explore/metrics/equationBuilder.spec.tsx‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
2+
3+
import {EquationBuilder} from 'sentry/views/explore/metrics/equationBuilder';
4+
5+
describe('EquationBuilder', () => {
6+
it('takes an equation and represents the equation using the provided reference map', async () => {
7+
const expression = 'count(metricA) + sum(metricB)';
8+
const referenceMap = {
9+
A: 'count(metricA)',
10+
F: 'sum(metricB)',
11+
};
12+
13+
render(
14+
<EquationBuilder
15+
expression={expression}
16+
referenceMap={referenceMap}
17+
handleExpressionChange={() => {}}
18+
/>
19+
);
20+
21+
const tokens = await screen.findAllByRole('row');
22+
23+
// tokens are flanked by empty text nodes for cursor movement,
24+
// and '+' is encoded as 'op:0'
25+
expect(tokens.map(token => token.getAttribute('aria-label'))).toEqual([
26+
'',
27+
'A',
28+
'',
29+
'op:0',
30+
'',
31+
'F',
32+
'',
33+
]);
34+
});
35+
36+
it('calls the handleExpressionChange callback when the expression changes', async () => {
37+
const expression = '';
38+
39+
const handleExpressionChange = jest.fn();
40+
41+
render(
42+
<EquationBuilder
43+
expression={expression}
44+
referenceMap={{A: 'count(value,metricA,distribution,none)'}}
45+
handleExpressionChange={handleExpressionChange}
46+
/>
47+
);
48+
49+
// Typing the reference de-references it into the metric call
50+
await userEvent.type(
51+
await screen.findByRole('combobox', {name: 'Add a term'}),
52+
'A * 2'
53+
);
54+
55+
expect(handleExpressionChange).toHaveBeenCalledWith(
56+
expect.objectContaining({
57+
text: 'count(value,metricA,distribution,none) * 2',
58+
})
59+
);
60+
});
61+
});

‎static/app/views/explore/metrics/equationBuilder.tsx‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import {useCallback, useEffect, useMemo, useRef, useTransition} from 'react';
2+
import isEqual from 'lodash/isEqual';
3+
4+
import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder';
5+
import {Expression} from 'sentry/components/arithmeticBuilder/expression';
6+
import {
7+
isTokenFreeText,
8+
isTokenReference,
9+
} from 'sentry/components/arithmeticBuilder/token';
10+
import {tokenizeExpression} from 'sentry/components/arithmeticBuilder/tokenizer';
11+
12+
/**
13+
* Takes an expression and map of references and returns the internal string representation that uses the references.
14+
*/
15+
function unresolveExpression(
16+
expression: string,
17+
referenceMap: Record<string, string> = {}
18+
): string {
19+
// Reverse the keys and values of the reference map, duplicates keep the first reference found
20+
const reversedReferenceMap = Object.entries(referenceMap).reduce(
21+
(reversedMap: Record<string, string>, [key, value]) => {
22+
if (!reversedMap[value]) {
23+
reversedMap[value] = key;
24+
}
25+
return reversedMap;
26+
},
27+
{}
28+
);
29+
30+
const tokens = tokenizeExpression(expression);
31+
return tokens
32+
.map(token => {
33+
if (token.text in reversedReferenceMap) {
34+
return reversedReferenceMap[token.text] ?? token.text;
35+
}
36+
return token.text;
37+
})
38+
.join(' ');
39+
}
40+
41+
/**
42+
* Resolves the expression using references into a format that is compatible with our querying endpoints.
43+
*/
44+
function resolveExpression(
45+
expression: Expression,
46+
referenceMap: Record<string, string> = {}
47+
): Expression {
48+
const newTokens = expression.tokens
49+
.map(token => {
50+
if (referenceMap && isTokenReference(token) && token.text in referenceMap) {
51+
return referenceMap[token.text];
52+
}
53+
if (!isTokenFreeText(token)) {
54+
return token.text;
55+
}
56+
return null;
57+
})
58+
.filter(Boolean);
59+
60+
return new Expression(newTokens.join(' '));
61+
}
62+
63+
/**
64+
* A component that takes an equation in full resolved form and allows
65+
* the user to edit it using "references" to refer to the different components
66+
* of the equation.
67+
*
68+
* The references are used to resolve the equation into a format that is
69+
* compatible with our querying endpoints.
70+
*/
71+
export function EquationBuilder({
72+
expression,
73+
referenceMap,
74+
handleExpressionChange,
75+
}: {
76+
expression: string;
77+
handleExpressionChange: (expression: Expression) => void;
78+
referenceMap?: Record<string, string>;
79+
}) {
80+
const [_, startTransition] = useTransition();
81+
const references = useMemo(
82+
() => new Set(Object.keys(referenceMap ?? {})),
83+
[referenceMap]
84+
);
85+
86+
// Tracks the reference map that `expression` was last resolved against.
87+
// When referenceMap changes externally, expression still contains values
88+
// resolved against the previous map until we re-resolve and the parent updates.
89+
const expressionMapRef = useRef(referenceMap);
90+
const mapChanged = !isEqual(expressionMapRef.current, referenceMap);
91+
92+
const internalExpression = unresolveExpression(
93+
expression,
94+
mapChanged ? expressionMapRef.current : referenceMap
95+
);
96+
97+
// When the reference map changes, re-resolve the expression and invoke the callback
98+
useEffect(() => {
99+
if (!isEqual(expressionMapRef.current, referenceMap)) {
100+
const expr = new Expression(
101+
internalExpression,
102+
new Set(Object.keys(expressionMapRef.current ?? {}))
103+
);
104+
const resolved = resolveExpression(expr, referenceMap);
105+
106+
// Check the validity of the internal expression, not the resolved one because
107+
// there are issues with validating it with the new _if aggregation format and
108+
// this check just needs to ensure the structure is valid.
109+
if (expr.isValid) {
110+
handleExpressionChange(resolved);
111+
expressionMapRef.current = referenceMap;
112+
}
113+
}
114+
}, [referenceMap, internalExpression, handleExpressionChange]);
115+
116+
const handleInternalExpressionChange = useCallback(
117+
(newExpression: Expression) => {
118+
startTransition(() => {
119+
if (newExpression.isValid) {
120+
handleExpressionChange(resolveExpression(newExpression, referenceMap));
121+
}
122+
});
123+
},
124+
[handleExpressionChange, referenceMap]
125+
);
126+
127+
return (
128+
<ArithmeticBuilder
129+
aggregations={[]}
130+
expression={internalExpression}
131+
functionArguments={[]}
132+
getFieldDefinition={() => null}
133+
references={references}
134+
setExpression={handleInternalExpressionChange}
135+
/>
136+
);
137+
}

‎static/app/views/explore/metrics/hooks/useMetricReferences.spec.tsx‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import type {ReactNode} from 'react';
2+
3+
import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
4+
5+
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
6+
import {encodeMetricQueryParams} from 'sentry/views/explore/metrics/metricQuery';
7+
import {
8+
MultiMetricsQueryParamsProvider,
9+
useMultiMetricsQueryParams,
10+
} from 'sentry/views/explore/metrics/multiMetricsQueryParams';
11+
import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
12+
import {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
13+
14+
import {useMetricReferences} from './useMetricReferences';
15+
16+
function Wrapper({children}: {children: ReactNode}) {
17+
return <MultiMetricsQueryParamsProvider>{children}</MultiMetricsQueryParamsProvider>;
18+
}
19+
20+
describe('useMetricReferences', () => {
21+
it('returns _if form for metrics with a filter and plain yAxis for metrics without', () => {
22+
const metricWithFilter = encodeMetricQueryParams({
23+
metric: {name: 'metric_a', type: 'counter', unit: 'none'},
24+
queryParams: new ReadableQueryParams({
25+
extrapolate: true,
26+
mode: Mode.SAMPLES,
27+
query: 'status:ok',
28+
cursor: '',
29+
fields: ['id', 'timestamp'],
30+
sortBys: [{field: 'timestamp', kind: 'desc'}],
31+
aggregateCursor: '',
32+
aggregateFields: [new VisualizeFunction('count(value,metric_a,counter,none)')],
33+
aggregateSortBys: [{field: 'count(value,metric_a,counter,none)', kind: 'desc'}],
34+
}),
35+
});
36+
37+
const metricWithoutFilter = encodeMetricQueryParams({
38+
metric: {name: 'metric_b', type: 'counter', unit: 'none'},
39+
queryParams: new ReadableQueryParams({
40+
extrapolate: true,
41+
mode: Mode.SAMPLES,
42+
query: '',
43+
cursor: '',
44+
fields: ['id', 'timestamp'],
45+
sortBys: [{field: 'timestamp', kind: 'desc'}],
46+
aggregateCursor: '',
47+
aggregateFields: [new VisualizeFunction('sum(value,metric_b,counter,none)')],
48+
aggregateSortBys: [{field: 'sum(value,metric_b,counter,none)', kind: 'desc'}],
49+
}),
50+
});
51+
52+
const {result} = renderHookWithProviders(
53+
() => ({
54+
references: useMetricReferences(),
55+
queries: useMultiMetricsQueryParams(),
56+
}),
57+
{
58+
additionalWrapper: Wrapper,
59+
initialRouterConfig: {
60+
location: {
61+
pathname: '/',
62+
query: {
63+
metric: [metricWithFilter, metricWithoutFilter],
64+
},
65+
},
66+
},
67+
}
68+
);
69+
70+
// Metric A has a filter, so it gets the _if form
71+
expect(result.current.references.A).toBe(
72+
'count_if(`status:ok`,value,metric_a,counter,none)'
73+
);
74+
75+
// Metric B has no filter, so the yAxis is returned as-is
76+
expect(result.current.references.B).toBe('sum(value,metric_b,counter,none)');
77+
});
78+
});

‎static/app/views/explore/metrics/hooks/useMetricReferences.tsx‎

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

3+
import {parseFunction} from 'sentry/utils/discover/fields';
34
import {useMultiMetricsQueryParams} from 'sentry/views/explore/metrics/multiMetricsQueryParams';
5+
import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
46
import {isVisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
57
import {getVisualizeLabel} from 'sentry/views/explore/toolbar/toolbarVisualize';
68

9+
function resolveMetricReference(metricConfig: ReadableQueryParams): string {
10+
if (metricConfig.query) {
11+
// convert the aggregate to an "if" format
12+
const visualize = metricConfig.visualizes[0]!;
13+
const parsed = parseFunction(visualize.yAxis);
14+
if (parsed) {
15+
return `${parsed.name}_if(\`${metricConfig.query}\`,${parsed.arguments.join(',')})`;
16+
}
17+
}
18+
return metricConfig.visualizes[0]?.yAxis!;
19+
}
20+
721
export function useMetricReferences() {
822
const metricQueries = useMultiMetricsQueryParams();
923

1024
return useMemo(() => {
11-
return new Set(
25+
return Object.fromEntries(
1226
metricQueries
1327
.filter(metricQuery =>
1428
metricQuery.queryParams.visualizes.some(isVisualizeFunction)
1529
)
16-
.map((metricQuery, index) => metricQuery.label ?? getVisualizeLabel(index, false))
30+
.map((metricQuery, index) => [
31+
metricQuery.label ?? getVisualizeLabel(index, false),
32+
resolveMetricReference(metricQuery.queryParams),
33+
])
1734
);
1835
}, [metricQueries]);
1936
}

‎static/app/views/explore/metrics/hooks/useMetricTimeseries.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
useQueryParamsGroupBys,
1717
useQueryParamsSearch,
1818
} from 'sentry/views/explore/queryParams/context';
19+
import {isVisualizeEquation} from 'sentry/views/explore/queryParams/visualize';
1920
import {useSortedTimeSeries} from 'sentry/views/insights/common/queries/useSortedTimeSeries';
2021

2122
interface UseMetricTimeseriesOptions {
@@ -68,7 +69,12 @@ function useMetricTimeseriesImpl({
6869
yAxis,
6970
interval,
7071
fields: [...groupBys, ...yAxis],
71-
enabled: enabled && Boolean(traceMetric.name),
72+
enabled:
73+
enabled &&
74+
(Boolean(traceMetric.name) ||
75+
visualizes.some(
76+
visualize => isVisualizeEquation(visualize) && visualize.expression.text
77+
)),
7278
topEvents,
7379
orderby: sortBys.map(formatSort),
7480
...queryExtras,

‎static/app/views/explore/metrics/hooks/useSaveMetricsMultiQuery.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ import {getTitleFromLocation} from 'sentry/views/explore/contexts/pageParamsCont
1313
import {useInvalidateSavedQueries} from 'sentry/views/explore/hooks/useGetSavedQueries';
1414
import {useMultiMetricsQueryParams} from 'sentry/views/explore/metrics/multiMetricsQueryParams';
1515
import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy';
16-
import {isVisualize} from 'sentry/views/explore/queryParams/visualize';
16+
import {
17+
isVisualize,
18+
isVisualizeFunction,
19+
} from 'sentry/views/explore/queryParams/visualize';
1720

1821
const METRICS_DATASET = 'metrics';
1922

@@ -70,7 +73,7 @@ export function useSaveMetricsMultiQuery() {
7073
...groupBys,
7174
...(yAxes.length > 0 ? [{yAxes, chartType}] : []),
7275
],
73-
metric: metricQuery.metric,
76+
...(isVisualizeFunction(visualize) ? {metric: metricQuery.metric} : {}),
7477
fields: metricQuery.queryParams.fields,
7578
orderby: metricQuery.queryParams.sortBys[0]
7679
? encodeSort(metricQuery.queryParams.sortBys[0])

0 commit comments

Comments
 (0)