Skip to content

Commit 28c4cfc

Browse files
ZylphrexZylphrex
authored
feat(explore): Allow separate sort bys in samples and aggregates (#94577)
The sort bys are not transferable from samples to aggregates mode. So make them 2 separate controls.
1 parent 101b3d6 commit 28c4cfc

9 files changed

Lines changed: 333 additions & 140 deletions

File tree

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import type {Location} from 'history';
2+
3+
import {defined} from 'sentry/utils';
4+
import type {Sort} from 'sentry/utils/discover/fields';
5+
import {decodeSorts} from 'sentry/utils/queryString';
6+
7+
import type {Visualize} from './visualizes';
8+
9+
export function defaultAggregateSortBys(yAxes: string[]): Sort[] {
10+
if (yAxes[0]) {
11+
return [
12+
{
13+
field: yAxes[0],
14+
kind: 'desc' as const,
15+
},
16+
];
17+
}
18+
19+
return [];
20+
}
21+
22+
export function getAggregateSortBysFromLocation(
23+
location: Location,
24+
groupBys: string[],
25+
visualizes: Visualize[]
26+
) {
27+
const sortBys = decodeSorts(location.query.aggregateSort);
28+
29+
if (sortBys.length > 0 && validateSorts(sortBys, groupBys, visualizes)) {
30+
return sortBys;
31+
}
32+
33+
// we want to check the `sort` query param for backwards compatibility
34+
// when the both modes shared a single query param for the sort
35+
const sortBysFallback = decodeSorts(location.query.sort);
36+
37+
if (
38+
sortBysFallback.length > 0 &&
39+
validateSorts(sortBysFallback, groupBys, visualizes)
40+
) {
41+
return sortBysFallback;
42+
}
43+
44+
return defaultAggregateSortBys(visualizes.map(visualize => visualize.yAxis));
45+
}
46+
47+
function validateSorts(
48+
sortBys: Sort[],
49+
groupBys: string[],
50+
visualizes: Visualize[]
51+
): boolean {
52+
return sortBys.every(
53+
sortBy =>
54+
groupBys.includes(sortBy.field) ||
55+
visualizes.some(visualize => visualize.yAxis === sortBy.field)
56+
);
57+
}
58+
59+
export function updateLocationWithAggregateSortBys(
60+
location: Location,
61+
sortBys: Sort[] | null | undefined
62+
) {
63+
if (defined(sortBys)) {
64+
location.query.aggregateSort = sortBys.map(sortBy =>
65+
sortBy.kind === 'desc' ? `-${sortBy.field}` : sortBy.field
66+
);
67+
68+
// make sure to clear the cursor every time the query is updated
69+
delete location.query.cursor;
70+
} else if (sortBys === null) {
71+
delete location.query.aggregateSort;
72+
}
73+
}

‎static/app/views/explore/contexts/pageParamsContext/index.spec.tsx‎

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ describe('PageParamsProvider', function () {
7878
fields: ['id', 'timestamp'],
7979
mode: Mode.AGGREGATE,
8080
query: '',
81-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
81+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
82+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
8283
aggregateFields: [
8384
{groupBy: 'span.op'},
8485
{
@@ -111,7 +112,8 @@ describe('PageParamsProvider', function () {
111112
],
112113
mode: Mode.SAMPLES,
113114
query: '',
114-
sortBys: [{field: 'timestamp', kind: 'desc'}],
115+
sampleSortBys: [{field: 'timestamp', kind: 'desc'}],
116+
aggregateSortBys: [{field: 'count(span.duration)', kind: 'desc'}],
115117
aggregateFields: [{groupBy: ''}, new Visualize('count(span.duration)')],
116118
})
117119
);
@@ -128,7 +130,8 @@ describe('PageParamsProvider', function () {
128130
fields: ['id', 'span.op', 'timestamp'],
129131
mode: Mode.AGGREGATE,
130132
query: '',
131-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
133+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
134+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
132135
aggregateFields: [
133136
{groupBy: 'span.op'},
134137
new Visualize('count(span.self_time)', {
@@ -150,7 +153,8 @@ describe('PageParamsProvider', function () {
150153
fields: ['id', 'timestamp', 'span.self_time'],
151154
mode: Mode.AGGREGATE,
152155
query: '',
153-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
156+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
157+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
154158
aggregateFields: [
155159
{groupBy: 'browser.name'},
156160
new Visualize('count(span.self_time)', {
@@ -173,7 +177,8 @@ describe('PageParamsProvider', function () {
173177
fields: ['id', 'timestamp', 'span.self_time'],
174178
mode: Mode.AGGREGATE,
175179
query: '',
176-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
180+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
181+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
177182
aggregateFields: [
178183
new Visualize('count(span.self_time)', {
179184
chartType: ChartType.AREA,
@@ -195,7 +200,8 @@ describe('PageParamsProvider', function () {
195200
fields: ['id', 'timestamp', 'span.self_time'],
196201
mode: Mode.AGGREGATE,
197202
query: '',
198-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
203+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
204+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
199205
aggregateFields: [
200206
{groupBy: ''},
201207
new Visualize('count(span.self_time)', {
@@ -217,7 +223,8 @@ describe('PageParamsProvider', function () {
217223
fields: ['id', 'timestamp', 'span.self_time'],
218224
mode: Mode.AGGREGATE,
219225
query: '',
220-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
226+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
227+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
221228
aggregateFields: [
222229
{groupBy: 'span.op'},
223230
new Visualize('count(span.self_time)', {
@@ -238,7 +245,8 @@ describe('PageParamsProvider', function () {
238245
yAxes: ['count(span.self_time)'],
239246
},
240247
],
241-
sortBys: null,
248+
sampleSortBys: null,
249+
aggregateSortBys: null,
242250
});
243251

244252
act(() => setMode(Mode.SAMPLES));
@@ -249,7 +257,8 @@ describe('PageParamsProvider', function () {
249257
fields: ['id', 'timestamp', 'span.self_time'],
250258
mode: Mode.SAMPLES,
251259
query: '',
252-
sortBys: [{field: 'timestamp', kind: 'desc'}],
260+
sampleSortBys: [{field: 'timestamp', kind: 'desc'}],
261+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'desc'}],
253262
aggregateFields: [
254263
{groupBy: ''},
255264
new Visualize('count(span.self_time)', {
@@ -263,7 +272,8 @@ describe('PageParamsProvider', function () {
263272
it('correctly updates mode from aggregates to sample with group bys', function () {
264273
renderTestComponent({
265274
mode: Mode.AGGREGATE,
266-
sortBys: null,
275+
sampleSortBys: null,
276+
aggregateSortBys: null,
267277
fields: ['id', 'sdk.name', 'sdk.version', 'timestamp'],
268278
aggregateFields: [
269279
{groupBy: 'sdk.name'},
@@ -292,7 +302,8 @@ describe('PageParamsProvider', function () {
292302
],
293303
mode: Mode.SAMPLES,
294304
query: '',
295-
sortBys: [{field: 'timestamp', kind: 'desc'}],
305+
sampleSortBys: [{field: 'timestamp', kind: 'desc'}],
306+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'desc'}],
296307
aggregateFields: [
297308
{groupBy: 'sdk.name'},
298309
{groupBy: 'sdk.version'},
@@ -317,7 +328,8 @@ describe('PageParamsProvider', function () {
317328
fields: ['id', 'timestamp', 'span.self_time'],
318329
mode: Mode.AGGREGATE,
319330
query: 'foo:bar',
320-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
331+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
332+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
321333
aggregateFields: [
322334
{groupBy: 'span.op'},
323335
new Visualize('count(span.self_time)', {
@@ -339,7 +351,8 @@ describe('PageParamsProvider', function () {
339351
fields: ['id', 'timestamp', 'span.self_time'],
340352
mode: Mode.SAMPLES,
341353
query: '',
342-
sortBys: [{field: 'id', kind: 'desc'}],
354+
sampleSortBys: [{field: 'id', kind: 'desc'}],
355+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
343356
aggregateFields: [
344357
{groupBy: 'span.op'},
345358
new Visualize('count(span.self_time)', {
@@ -361,7 +374,8 @@ describe('PageParamsProvider', function () {
361374
fields: ['id', 'timestamp', 'span.self_time'],
362375
mode: Mode.SAMPLES,
363376
query: '',
364-
sortBys: [{field: 'timestamp', kind: 'desc'}],
377+
sampleSortBys: [{field: 'timestamp', kind: 'desc'}],
378+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
365379
aggregateFields: [
366380
{groupBy: 'span.op'},
367381
new Visualize('count(span.self_time)', {
@@ -392,7 +406,8 @@ describe('PageParamsProvider', function () {
392406
fields: ['id', 'timestamp', 'span.self_time'],
393407
mode: Mode.AGGREGATE,
394408
query: '',
395-
sortBys: [{field: 'max(span.duration)', kind: 'desc'}],
409+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
410+
aggregateSortBys: [{field: 'max(span.duration)', kind: 'desc'}],
396411
aggregateFields: [
397412
{groupBy: 'span.op'},
398413
new Visualize('min(span.self_time)', {
@@ -426,7 +441,8 @@ describe('PageParamsProvider', function () {
426441
fields: ['id', 'timestamp', 'span.self_time'],
427442
mode: Mode.AGGREGATE,
428443
query: '',
429-
sortBys: [{field: 'min(span.self_time)', kind: 'desc'}],
444+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
445+
aggregateSortBys: [{field: 'min(span.self_time)', kind: 'desc'}],
430446
aggregateFields: [
431447
{groupBy: 'span.op'},
432448
new Visualize('min(span.self_time)', {
@@ -460,7 +476,8 @@ describe('PageParamsProvider', function () {
460476
fields: ['id', 'timestamp', 'span.self_time'],
461477
mode: Mode.AGGREGATE,
462478
query: '',
463-
sortBys: [{field: 'sdk.name', kind: 'desc'}],
479+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
480+
aggregateSortBys: [{field: 'sdk.name', kind: 'desc'}],
464481
aggregateFields: [
465482
{groupBy: 'sdk.name'},
466483
new Visualize('count(span.self_time)', {
@@ -491,7 +508,8 @@ describe('PageParamsProvider', function () {
491508
fields: ['id', 'timestamp', 'span.self_time'],
492509
mode: Mode.AGGREGATE,
493510
query: '',
494-
sortBys: [{field: 'count(span.self_time)', kind: 'desc'}],
511+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
512+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'desc'}],
495513
aggregateFields: [
496514
{groupBy: 'sdk.name'},
497515
new Visualize('count(span.self_time)', {
@@ -513,7 +531,8 @@ describe('PageParamsProvider', function () {
513531
fields: ['id', 'timestamp'],
514532
mode: Mode.AGGREGATE,
515533
query: '',
516-
sortBys: [{field: 'count(span.duration)', kind: 'desc'}],
534+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
535+
aggregateSortBys: [{field: 'count(span.duration)', kind: 'desc'}],
517536
aggregateFields: [{groupBy: 'span.op'}, new Visualize('count(span.duration)')],
518537
})
519538
);
@@ -541,7 +560,8 @@ describe('PageParamsProvider', function () {
541560
fields: ['id', 'timestamp', 'span.self_time', 'span.duration'],
542561
mode: Mode.AGGREGATE,
543562
query: '',
544-
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
563+
sampleSortBys: [{field: 'timestamp', kind: 'asc'}],
564+
aggregateSortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
545565
aggregateFields: [
546566
{groupBy: 'span.op'},
547567
new Visualize('count(span.self_time)', {

0 commit comments

Comments
 (0)