Skip to content

Commit 3df2736

Browse files
ZylphrexZylphrex
authored
feat(tracemetrics): Add metric to aggregate params (#102991)
Frontend for #102965 to add the selected metric into the params of the aggregate function.
1 parent 28e4119 commit 3df2736

File tree

5 files changed

+73
-41
lines changed

5 files changed

+73
-41
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import type {TraceMetric} from 'sentry/views/explore/metrics/metricQuery';
1414
import {useMetricVisualize} from 'sentry/views/explore/metrics/metricsQueryParams';
1515
import {TraceMetricKnownFieldKey} from 'sentry/views/explore/metrics/types';
16+
import {makeMetricsAggregate} from 'sentry/views/explore/metrics/utils';
1617
import {
1718
useQueryParamsAggregateSortBys,
1819
useQueryParamsGroupBys,
@@ -33,7 +34,13 @@ interface MetricAggregatesTableResult {
3334
result: ReturnType<typeof useSpansQuery<any[]>>;
3435
}
3536

36-
const COUNT_AGGREGATE = `count(${TraceMetricKnownFieldKey.METRIC_NAME})`;
37+
function makeCountAggregate(traceMetric: TraceMetric): string {
38+
return makeMetricsAggregate({
39+
aggregate: 'count',
40+
traceMetric,
41+
attribute: TraceMetricKnownFieldKey.METRIC_NAME,
42+
});
43+
}
3744

3845
export function useMetricAggregatesTable({
3946
enabled,
@@ -43,14 +50,15 @@ export function useMetricAggregatesTable({
4350
}: UseMetricAggregatesTableOptions) {
4451
const canTriggerHighAccuracy = useCallback(
4552
(result: ReturnType<typeof useMetricAggregatesTableImp>['result']) => {
53+
const countAggregate = makeCountAggregate(traceMetric);
4654
const canGoToHigherAccuracyTier = result.meta?.dataScanned === 'partial';
4755
const hasData =
4856
defined(result.data) &&
4957
(result.data.length > 1 ||
50-
(result.data.length === 1 && Boolean(result.data[0][COUNT_AGGREGATE])));
58+
(result.data.length === 1 && Boolean(result.data[0][countAggregate])));
5159
return !hasData && canGoToHigherAccuracyTier;
5260
},
53-
[]
61+
[traceMetric]
5462
);
5563
return useProgressiveQuery<typeof useMetricAggregatesTableImp>({
5664
queryHookImplementation: useMetricAggregatesTableImp,
@@ -103,15 +111,15 @@ function useMetricAggregatesTableImp({
103111
const discoverQuery: NewQuery = {
104112
id: undefined,
105113
name: 'Explore - Metric Aggregates',
106-
fields: [...fields, COUNT_AGGREGATE],
114+
fields: [...fields, makeCountAggregate(traceMetric)],
107115
orderby: sortBys.map(formatSort),
108116
query,
109117
version: 2,
110118
dataset: DiscoverDatasets.TRACEMETRICS,
111119
};
112120

113121
return EventView.fromNewQueryWithPageFilters(discoverQuery, selection);
114-
}, [fields, query, selection, sortBys]);
122+
}, [fields, query, selection, sortBys, traceMetric]);
115123

116124
const result = useSpansQuery({
117125
enabled: enabled && Boolean(traceMetric.name) && fields.length > 0,

‎static/app/views/explore/metrics/metricToolbar/aggregateDropdown.tsx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
useMetricVisualize,
77
useSetMetricVisualize,
88
} from 'sentry/views/explore/metrics/metricsQueryParams';
9+
import {updateVisualizeYAxis} from 'sentry/views/explore/metrics/utils';
910

1011
export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) {
1112
const visualize = useMetricVisualize();
@@ -19,12 +20,7 @@ export function AggregateDropdown({traceMetric}: {traceMetric: TraceMetric}) {
1920
options={OPTIONS_BY_TYPE[traceMetric.type] ?? []}
2021
value={visualize.parsedFunction?.name ?? ''}
2122
onChange={option => {
22-
setVisualize(
23-
visualize.replace({
24-
yAxis: `${option.value}(value)`,
25-
chartType: undefined, // Reset chart type to let determineDefaultChartType decide
26-
})
27-
);
23+
setVisualize(updateVisualizeYAxis(visualize, option.value, traceMetric));
2824
}}
2925
/>
3026
);

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
5353
expect.objectContaining({
5454
metric: {name: 'foo', type: 'counter'},
5555
queryParams: expect.objectContaining({
56-
aggregateFields: [new VisualizeFunction('per_second(value)')],
56+
aggregateFields: [new VisualizeFunction('per_second(value,foo,counter,-)')],
5757
}),
5858
}),
5959
]);
@@ -63,7 +63,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
6363
expect.objectContaining({
6464
metric: {name: 'bar', type: 'gauge'},
6565
queryParams: expect.objectContaining({
66-
aggregateFields: [new VisualizeFunction('per_second(value)')],
66+
aggregateFields: [new VisualizeFunction('per_second(value,bar,gauge,-)')],
6767
}),
6868
}),
6969
]);
@@ -73,7 +73,9 @@ describe('MultiMetricsQueryParamsProvider', () => {
7373
expect.objectContaining({
7474
metric: {name: 'qux', type: 'distribution'},
7575
queryParams: expect.objectContaining({
76-
aggregateFields: [new VisualizeFunction('per_second(value)')],
76+
aggregateFields: [
77+
new VisualizeFunction('per_second(value,qux,distribution,-)'),
78+
],
7779
}),
7880
}),
7981
]);
@@ -88,15 +90,15 @@ describe('MultiMetricsQueryParamsProvider', () => {
8890
act(() =>
8991
result.current[0]!.setQueryParams(
9092
result.current[0]!.queryParams.replace({
91-
aggregateFields: [new VisualizeFunction('sum(value)')],
93+
aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')],
9294
})
9395
)
9496
);
9597
expect(result.current).toEqual([
9698
expect.objectContaining({
9799
metric: {name: 'foo', type: 'counter'},
98100
queryParams: expect.objectContaining({
99-
aggregateFields: [new VisualizeFunction('sum(value)')],
101+
aggregateFields: [new VisualizeFunction('sum(value,foo,counter,-)')],
100102
}),
101103
}),
102104
]);
@@ -106,23 +108,23 @@ describe('MultiMetricsQueryParamsProvider', () => {
106108
expect.objectContaining({
107109
metric: {name: 'qux', type: 'gauge'},
108110
queryParams: expect.objectContaining({
109-
aggregateFields: [new VisualizeFunction('avg(value)')],
111+
aggregateFields: [new VisualizeFunction('avg(value,qux,gauge,-)')],
110112
}),
111113
}),
112114
]);
113115

114116
act(() =>
115117
result.current[0]!.setQueryParams(
116118
result.current[0]!.queryParams.replace({
117-
aggregateFields: [new VisualizeFunction('last(value)')],
119+
aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')],
118120
})
119121
)
120122
);
121123
expect(result.current).toEqual([
122124
expect.objectContaining({
123125
metric: {name: 'qux', type: 'gauge'},
124126
queryParams: expect.objectContaining({
125-
aggregateFields: [new VisualizeFunction('last(value)')],
127+
aggregateFields: [new VisualizeFunction('last(value,qux,gauge,-)')],
126128
}),
127129
}),
128130
]);
@@ -132,23 +134,23 @@ describe('MultiMetricsQueryParamsProvider', () => {
132134
expect.objectContaining({
133135
metric: {name: 'bar', type: 'distribution'},
134136
queryParams: expect.objectContaining({
135-
aggregateFields: [new VisualizeFunction('p75(value)')],
137+
aggregateFields: [new VisualizeFunction('p75(value,bar,distribution,-)')],
136138
}),
137139
}),
138140
]);
139141

140142
act(() =>
141143
result.current[0]!.setQueryParams(
142144
result.current[0]!.queryParams.replace({
143-
aggregateFields: [new VisualizeFunction('p99(value)')],
145+
aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')],
144146
})
145147
)
146148
);
147149
expect(result.current).toEqual([
148150
expect.objectContaining({
149151
metric: {name: 'bar', type: 'distribution'},
150152
queryParams: expect.objectContaining({
151-
aggregateFields: [new VisualizeFunction('p99(value)')],
153+
aggregateFields: [new VisualizeFunction('p99(value,bar,distribution,-)')],
152154
}),
153155
}),
154156
]);
@@ -158,7 +160,7 @@ describe('MultiMetricsQueryParamsProvider', () => {
158160
expect.objectContaining({
159161
metric: {name: 'foo', type: 'counter'},
160162
queryParams: expect.objectContaining({
161-
aggregateFields: [new VisualizeFunction('per_second(value)')],
163+
aggregateFields: [new VisualizeFunction('per_second(value,foo,counter,-)')],
162164
}),
163165
}),
164166
]);

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

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@ import {
1818
type MetricQuery,
1919
type TraceMetric,
2020
} from 'sentry/views/explore/metrics/metricQuery';
21+
import {updateVisualizeYAxis} from 'sentry/views/explore/metrics/utils';
2122
import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy';
2223
import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
23-
import {
24-
isVisualizeFunction,
25-
VisualizeFunction,
26-
} from 'sentry/views/explore/queryParams/visualize';
24+
import {isVisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
2725

2826
interface MultiMetricsQueryParamsContextValue {
2927
metricQueries: MetricQuery[];
@@ -93,25 +91,20 @@ export function MultiMetricsQueryParamsProvider({
9391
const allowedAggregations = OPTIONS_BY_TYPE[newTraceMetric.type];
9492

9593
if (
96-
!allowedAggregations?.find(option => option.value === selectedAggregation)
94+
selectedAggregation &&
95+
allowedAggregations?.find(option => option.value === selectedAggregation)
9796
) {
98-
// the currently selected aggregation isn't supported on the new metric
99-
const defaultAggregation =
100-
DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'per_second';
97+
// the currently selected aggregation changed types
10198
aggregateFields = [
102-
new VisualizeFunction(`${defaultAggregation}(value)`),
99+
updateVisualizeYAxis(visualize, selectedAggregation, newTraceMetric),
103100
...metricQuery.queryParams.aggregateFields.filter(isGroupBy),
104101
];
105-
} else if (
106-
selectedAggregation === 'per_second' ||
107-
selectedAggregation === 'per_minute'
108-
) {
109-
// TODO: this else if branch can go away once the metric type is lifted
110-
// to the top level
111-
112-
// the currently selected aggregation changed types
102+
} else {
103+
// the currently selected aggregation isn't supported on the new metric
104+
const defaultAggregation =
105+
DEFAULT_YAXIS_BY_TYPE[newTraceMetric.type] || 'per_second';
113106
aggregateFields = [
114-
new VisualizeFunction(`${selectedAggregation}(value)`),
107+
updateVisualizeYAxis(visualize, defaultAggregation, newTraceMetric),
115108
...metricQuery.queryParams.aggregateFields.filter(isGroupBy),
116109
];
117110
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
type SampleTableColumnKey,
2828
} from 'sentry/views/explore/metrics/types';
2929
import {isGroupBy, type GroupBy} from 'sentry/views/explore/queryParams/groupBy';
30+
import type {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
3031
import {Visualize} from 'sentry/views/explore/queryParams/visualize';
3132
import type {PickableDays} from 'sentry/views/explore/utils';
3233

@@ -199,3 +200,35 @@ export function getMetricTableColumnType(
199200
}
200201
return 'value';
201202
}
203+
204+
export function makeMetricsAggregate({
205+
aggregate,
206+
traceMetric,
207+
attribute,
208+
}: {
209+
aggregate: string;
210+
traceMetric: TraceMetric;
211+
attribute?: string;
212+
}) {
213+
const args = [
214+
attribute ?? 'value', // hard coded to `value` for now, but can be other attributes
215+
traceMetric.name,
216+
traceMetric.type,
217+
'-', // hard coded to `-` for now, but can be other units`
218+
];
219+
return `${aggregate}(${args.join(',')})`;
220+
}
221+
222+
export function updateVisualizeYAxis(
223+
visualize: VisualizeFunction,
224+
aggregate: string,
225+
traceMetric: TraceMetric
226+
): VisualizeFunction {
227+
return visualize.replace({
228+
yAxis: makeMetricsAggregate({
229+
aggregate,
230+
traceMetric,
231+
}),
232+
chartType: undefined,
233+
});
234+
}

0 commit comments

Comments
 (0)