Skip to content

Commit bf052e4

Browse files
authored
ref(multi-query): Use shared chart visualization in multi query mode (#96666)
This refactors the multi query mode to use the same shared chart visualization as other parts of explore.
1 parent 99ca862 commit bf052e4

File tree

4 files changed

+32
-185
lines changed

4 files changed

+32
-185
lines changed

static/app/views/explore/multiQueryMode/hooks/useMultiQueryTimeseries.tsx

Lines changed: 3 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import {useCallback, useMemo} from 'react';
2-
import isEqual from 'lodash/isEqual';
32

43
import {DiscoverDatasets} from 'sentry/utils/discover/types';
54
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
6-
import usePrevious from 'sentry/utils/usePrevious';
75
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
86
import {formatSort} from 'sentry/views/explore/contexts/pageParamsContext/sortBys';
97
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
108
import {
119
type SpansRPCQueryExtras,
1210
useProgressiveQuery,
1311
} from 'sentry/views/explore/hooks/useProgressiveQuery';
12+
import {TOP_EVENTS_LIMIT} from 'sentry/views/explore/hooks/useTopEvents';
1413
import {
1514
getQueryMode,
1615
useReadQueriesFromLocation,
@@ -24,12 +23,9 @@ interface UseMultiQueryTimeseriesOptions {
2423
}
2524

2625
interface UseMultiQueryTimeseriesResults {
27-
canUsePreviousResults: boolean;
2826
result: ReturnType<typeof useSortedTimeSeries>;
2927
}
3028

31-
export const DEFAULT_TOP_EVENTS = 5;
32-
3329
export function useMultiQueryTimeseries({
3430
enabled,
3531
index,
@@ -102,56 +98,17 @@ function useMultiQueryTimeseriesImpl({
10298
fields,
10399
orderby,
104100
interval,
105-
topEvents: mode === Mode.SAMPLES ? undefined : DEFAULT_TOP_EVENTS,
101+
topEvents: mode === Mode.SAMPLES ? undefined : TOP_EVENTS_LIMIT,
106102
enabled,
107103
...queryExtras,
108104
};
109105
}, [query, yAxes, fields, orderby, interval, mode, enabled, queryExtras]);
110106

111-
const previousQuery = usePrevious(query);
112-
const previousOptions = usePrevious(options);
113-
const canUsePreviousResults = useMemo(() => {
114-
if (!isEqual(query, previousQuery)) {
115-
return false;
116-
}
117-
118-
if (!isEqual(options.interval, previousOptions.interval)) {
119-
return false;
120-
}
121-
122-
if (!isEqual(options.fields, previousOptions.fields)) {
123-
return false;
124-
}
125-
126-
if (!isEqual(options.orderby, previousOptions.orderby)) {
127-
return false;
128-
}
129-
130-
if (!isEqual(options.topEvents, previousOptions.topEvents)) {
131-
return false;
132-
}
133-
134-
// The query we're using has remained the same except for the y axis.
135-
// This means we can re-use the previous results to prevent a loading state.
136-
return true;
137-
}, [
138-
query,
139-
previousQuery,
140-
options.interval,
141-
options.fields,
142-
options.orderby,
143-
options.topEvents,
144-
previousOptions.interval,
145-
previousOptions.fields,
146-
previousOptions.orderby,
147-
previousOptions.topEvents,
148-
]);
149-
150107
const timeseriesResult = useSortedTimeSeries(
151108
options,
152109
'api.explorer.stats',
153110
DiscoverDatasets.SPANS
154111
);
155112

156-
return {result: timeseriesResult, canUsePreviousResults};
113+
return {result: timeseriesResult};
157114
}

static/app/views/explore/multiQueryMode/queryRow.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export function QueryRow({query: queryParts, index, totalQueryRows}: Props) {
4949
enabled: mode === Mode.SAMPLES,
5050
});
5151

52-
const {result: timeseriesResult, canUsePreviousResults} = useMultiQueryTimeseries({
52+
const {result: timeseriesResult} = useMultiQueryTimeseries({
5353
index,
5454
enabled: true,
5555
});
@@ -84,7 +84,6 @@ export function QueryRow({query: queryParts, index, totalQueryRows}: Props) {
8484
mode={mode}
8585
query={queryParts}
8686
timeseriesResult={timeseriesResult}
87-
canUsePreviousResults={canUsePreviousResults}
8887
/>
8988
<MultiQueryTable
9089
confidences={[]}

static/app/views/explore/multiQueryMode/queryVisualizations/chart.tsx

Lines changed: 28 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import {Fragment, useCallback, useMemo} from 'react';
2-
import {useTheme} from '@emotion/react';
1+
import {Fragment, useMemo} from 'react';
32
import styled from '@emotion/styled';
43

54
import Feature from 'sentry/components/acl/feature';
@@ -13,29 +12,22 @@ import {t} from 'sentry/locale';
1312
import {defined} from 'sentry/utils';
1413
import {trackAnalytics} from 'sentry/utils/analytics';
1514
import {parseFunction, prettifyParsedFunction} from 'sentry/utils/discover/fields';
16-
import {isTimeSeriesOther} from 'sentry/utils/timeSeries/isTimeSeriesOther';
17-
import {markDelayedData} from 'sentry/utils/timeSeries/markDelayedData';
1815
import useOrganization from 'sentry/utils/useOrganization';
1916
import usePageFilters from 'sentry/utils/usePageFilters';
20-
import usePrevious from 'sentry/utils/usePrevious';
2117
import useProjects from 'sentry/utils/useProjects';
2218
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
2319
import {determineSeriesSampleCountAndIsSampled} from 'sentry/views/alerts/rules/metric/utils/determineSeriesSampleCount';
24-
import {Area} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/area';
25-
import {Bars} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/bars';
26-
import {Line} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/line';
27-
import {TimeSeriesWidgetVisualization} from 'sentry/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization';
2820
import {Widget} from 'sentry/views/dashboards/widgets/widget/widget';
21+
import {ChartVisualization} from 'sentry/views/explore/components/chart/chartVisualization';
22+
import type {ChartInfo} from 'sentry/views/explore/components/chart/types';
2923
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
3024
import {determineDefaultChartType} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
3125
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
3226
import {useAddCompareQueryToDashboard} from 'sentry/views/explore/multiQueryMode/hooks/useAddCompareQueryToDashboard';
33-
import {DEFAULT_TOP_EVENTS} from 'sentry/views/explore/multiQueryMode/hooks/useMultiQueryTimeseries';
3427
import {
3528
type ReadableExploreQueryParts,
3629
useUpdateQueryAtIndex,
3730
} from 'sentry/views/explore/multiQueryMode/locationUtils';
38-
import {INGESTION_DELAY} from 'sentry/views/explore/settings';
3931
import {EXPLORE_CHART_TYPE_OPTIONS} from 'sentry/views/explore/spans/charts';
4032
import {ConfidenceFooter} from 'sentry/views/explore/spans/charts/confidenceFooter';
4133
import {combineConfidenceForSeries} from 'sentry/views/explore/utils';
@@ -45,7 +37,6 @@ import {getAlertsUrl} from 'sentry/views/insights/common/utils/getAlertsUrl';
4537

4638
const CHART_HEIGHT = 260;
4739
interface MultiQueryChartProps {
48-
canUsePreviousResults: boolean;
4940
index: number;
5041
mode: Mode;
5142
query: ReadableExploreQueryParts;
@@ -57,21 +48,10 @@ export function MultiQueryModeChart({
5748
query: queryParts,
5849
mode,
5950
timeseriesResult,
60-
canUsePreviousResults,
6151
}: MultiQueryChartProps) {
62-
const theme = useTheme();
63-
6452
const yAxes = queryParts.yAxes;
6553
const isTopN = mode === Mode.AGGREGATE;
6654

67-
const [confidence, numSeries] = useMemo(() => {
68-
const series = yAxes.flatMap(yAxis => timeseriesResult.data[yAxis]).filter(defined);
69-
return [
70-
combineConfidenceForSeries(series),
71-
Math.min(series.length, DEFAULT_TOP_EVENTS),
72-
];
73-
}, [timeseriesResult.data, yAxes]);
74-
7555
const [interval, setInterval, intervalOptions] = useChartInterval();
7656

7757
const formattedYAxes = yAxes.map(yaxis => {
@@ -81,74 +61,27 @@ export function MultiQueryModeChart({
8161

8262
const updateChartType = useUpdateQueryAtIndex(index);
8363

84-
const previousTimeseriesResult = usePrevious(timeseriesResult);
85-
86-
const getSeries = useCallback(() => {
87-
const shouldUsePreviousResults =
88-
timeseriesResult.isPending &&
89-
canUsePreviousResults &&
90-
yAxes.every(yAxis => previousTimeseriesResult.data.hasOwnProperty(yAxis));
91-
92-
const data = yAxes.flatMap((yAxis, i) => {
93-
const series = shouldUsePreviousResults
94-
? previousTimeseriesResult.data[yAxis]
95-
: timeseriesResult.data[yAxis];
96-
return (series ?? []).map(s => {
97-
// We replace the series name with the formatted series name here
98-
// when possible as it's cleaner to read.
99-
//
100-
// We can't do this in top N mode as the series name uses the row
101-
// values instead of the aggregate function.
102-
if (s.yAxis === yAxis) {
103-
return {
104-
...s,
105-
seriesName: formattedYAxes[i] ?? yAxis,
106-
};
107-
}
108-
return s;
109-
});
110-
});
111-
return {
112-
data,
113-
error: shouldUsePreviousResults
114-
? previousTimeseriesResult.error
115-
: timeseriesResult.error,
116-
loading: shouldUsePreviousResults
117-
? previousTimeseriesResult.isPending
118-
: timeseriesResult.isPending,
119-
};
120-
}, [
121-
timeseriesResult.isPending,
122-
timeseriesResult.error,
123-
timeseriesResult.data,
124-
canUsePreviousResults,
125-
yAxes,
126-
previousTimeseriesResult.error,
127-
previousTimeseriesResult.isPending,
128-
previousTimeseriesResult.data,
129-
formattedYAxes,
130-
]);
131-
132-
const {data, error, loading} = getSeries();
133-
const {sampleCount, isSampled, dataScanned} = determineSeriesSampleCountAndIsSampled(
134-
data,
135-
isTopN
136-
);
137-
13864
const chartType = queryParts.chartType ?? determineDefaultChartType(yAxes);
13965

14066
const visualizationType =
14167
chartType === ChartType.LINE ? 'line' : chartType === ChartType.AREA ? 'area' : 'bar';
14268

143-
const chartInfo = {
144-
chartIcon: <IconGraph type={visualizationType} />,
145-
chartType,
146-
yAxes,
147-
formattedYAxes,
148-
data,
149-
error,
150-
loading,
151-
};
69+
const chartInfo: ChartInfo = useMemo(() => {
70+
const series = yAxes.flatMap(yAxis => timeseriesResult.data[yAxis] ?? []);
71+
const samplingMeta = determineSeriesSampleCountAndIsSampled(series, isTopN);
72+
73+
return {
74+
chartType,
75+
confidence: combineConfidenceForSeries(series),
76+
series,
77+
timeseriesResult,
78+
yAxis: yAxes[0]!,
79+
dataScanned: samplingMeta.dataScanned,
80+
isSampled: samplingMeta.isSampled,
81+
sampleCount: samplingMeta.sampleCount,
82+
samplingMode: undefined,
83+
};
84+
}, [chartType, isTopN, timeseriesResult, yAxes]);
15285

15386
const organization = useOrganization();
15487
const {addToDashboard} = useAddCompareQueryToDashboard(queryParts);
@@ -161,29 +94,6 @@ export function MultiQueryModeChart({
16194
</Fragment>
16295
);
16396

164-
if (chartInfo.loading) {
165-
return (
166-
<Widget
167-
key={index}
168-
height={CHART_HEIGHT}
169-
Title={Title}
170-
Visualization={<TimeSeriesWidgetVisualization.LoadingPlaceholder />}
171-
revealActions="always"
172-
/>
173-
);
174-
}
175-
if (chartInfo.error) {
176-
return (
177-
<Widget
178-
key={index}
179-
height={CHART_HEIGHT}
180-
Title={Title}
181-
Visualization={<Widget.WidgetError error={chartInfo.error} />}
182-
revealActions="always"
183-
/>
184-
);
185-
}
186-
18797
const items: MenuItemProps[] = [];
18898

18999
const project =
@@ -242,13 +152,6 @@ export function MultiQueryModeChart({
242152
},
243153
});
244154

245-
const DataPlottableConstructor =
246-
chartInfo.chartType === ChartType.LINE
247-
? Line
248-
: chartInfo.chartType === ChartType.AREA
249-
? Area
250-
: Bars;
251-
252155
return (
253156
<Widget
254157
key={index}
@@ -261,12 +164,12 @@ export function MultiQueryModeChart({
261164
>
262165
<CompactSelect
263166
triggerProps={{
264-
icon: chartInfo.chartIcon,
167+
icon: <IconGraph type={visualizationType} />,
265168
borderless: true,
266169
showChevron: false,
267170
size: 'xs',
268171
}}
269-
value={chartInfo.chartType}
172+
value={chartType}
270173
menuTitle={t('Type')}
271174
options={EXPLORE_CHART_TYPE_OPTIONS}
272175
onChange={option => {
@@ -306,26 +209,15 @@ export function MultiQueryModeChart({
306209
),
307210
]}
308211
revealActions="always"
309-
Visualization={
310-
<TimeSeriesWidgetVisualization
311-
plottables={chartInfo.data.map(timeSeries => {
312-
return new DataPlottableConstructor(
313-
markDelayedData(timeSeries, INGESTION_DELAY),
314-
{
315-
color: isTimeSeriesOther(timeSeries) ? theme.chartOther : undefined,
316-
stack: 'all',
317-
}
318-
);
319-
})}
320-
/>
321-
}
212+
Visualization={<ChartVisualization chartInfo={chartInfo} />}
322213
Footer={
323214
<ConfidenceFooter
324-
sampleCount={sampleCount}
325-
isSampled={isSampled}
326-
confidence={confidence}
327-
topEvents={isTopN ? numSeries : undefined}
328-
dataScanned={dataScanned}
215+
sampleCount={chartInfo.sampleCount}
216+
isLoading={chartInfo.timeseriesResult?.isPending || false}
217+
isSampled={chartInfo.isSampled}
218+
confidence={chartInfo.confidence}
219+
topEvents={isTopN ? chartInfo.series.length : undefined}
220+
dataScanned={chartInfo.dataScanned}
329221
/>
330222
}
331223
/>

static/app/views/explore/settings.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {t} from 'sentry/locale';
22

3-
export const INGESTION_DELAY = 600;
43
export const CHART_HEIGHT = 260;
54
export const FALLBACK_SERIES_NAME = `(${t('empty string')})`;

0 commit comments

Comments
 (0)