Skip to content

Commit acde4a2

Browse files
authored
ref(dashboards): Move unit scaling into Plottable (#86116)
This is a refactor of the `Plottable` interface. It clarifies and separates the type definitions, and moves some ugly mutation logic away out of the React component and into the constructor classes. This is an important step on the road to multi-type charts, where lines and bars can coexist. It'll become a lot clearer when the public API for `TimeSeriesWidgetVisualization` starts accepting plottables directly. ## Changes The first change is a clarification of the types, and their relationships. - `Plottable` is a generic interface. A `Plottable` is an object that can be put into ECharts. Right now, only one _kind_ of plottable is implemented: `PlottableData`, but others are coming (e.g., Releases, and later other marker types). Right now, `PlottableData` is only used under-the-hood in `TimeSeriesDataVisualization`, but soon this will be the public API. Charts will be created by passing various plottable items to the component - `Area`, `Line`, and `Bars` all implement the `PlottableData` interface. They do this by extending the `AggreateTimeSeries` plottable. `AggregateTimeSeries` is there for sharing code. It specifies the constructor, and some simple scaling logic. Soon, this class will have more interesting properties that will make coordinating data between different plottables easier - A plottable has a _configuration_ and _plotting options_. The configuration has to do with the data. How much is that data delayed? Does it have a specific color? The _plotting options_ have to do with putting the plottable into ECharts. This answers questions like "what is the fallback color?" and "what is the Y axis unit"? These things are computed after taking all plottables into consideration The second change is smaller, and more subtle. It moves data scaling into the plottables. This creates _just a bit_ of code duplication, but is much nicer because this logic is now hidden from the chart (so I can improve it later).
1 parent 3a1cb64 commit acde4a2

File tree

7 files changed

+170
-68
lines changed

7 files changed

+170
-68
lines changed

static/app/views/dashboards/widgets/common/types.tsx

+27
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import type {SeriesOption} from 'echarts';
2+
13
import type {AccuracyStats, Confidence} from 'sentry/types/organization';
4+
import type {DurationUnit, RateUnit, SizeUnit} from 'sentry/utils/discover/fields';
25

36
import type {ThresholdsConfig} from '../../widgetBuilder/buildSteps/thresholdsStep/thresholdsStep';
47

@@ -45,3 +48,27 @@ export type Release = {
4548
export type Aliases = Record<string, string>;
4649

4750
export type LegendSelection = {[key: string]: boolean};
51+
52+
/**
53+
* A `Plottable` is any object that can be converted to an ECharts `Series` and therefore plotted on an ECharts chart. This could be a data series, releases, samples, and other kinds of markers. `TimeSeriesWidgetVisualization` uses `Plottable` objects under the hood, to convert data coming into the component via props into ECharts series.
54+
*/
55+
interface Plottable {
56+
/**
57+
*
58+
* @param plottingOptions Plotting options depend on the specific implementation of the interface.
59+
*/
60+
toSeries(plottingOptions: unknown): SeriesOption[];
61+
}
62+
63+
/**
64+
* `PlottableData` is any plottable that represents a data time series. The points in the series are a pair of a timestamp and a numeric value. This could be a continuous time series of aggregated data, or data samples.
65+
*/
66+
export interface PlottableData extends Plottable {
67+
/**
68+
* @param options Plotting options. The resulting series will have the color `color`, and will be scaled to `unit` if needed.
69+
*/
70+
toSeries(plottingOptions: {
71+
color: string;
72+
unit: DurationUnit | SizeUnit | RateUnit | null;
73+
}): SeriesOption[];
74+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import type {SeriesOption} from 'echarts';
2+
3+
import type {DurationUnit, RateUnit, SizeUnit} from 'sentry/utils/discover/fields';
4+
5+
import type {PlottableData, TimeSeries} from '../../common/types';
6+
import {scaleTimeSeriesData} from '../scaleTimeSeriesData';
7+
8+
type BasicDataPlottableConfig = {
9+
color?: string;
10+
};
11+
12+
export type AggregateTimePlottingOptions = {
13+
/**
14+
* Final plottable color. If no color is specified in configuration, a fallback must be provided while attempting to plot
15+
*/
16+
color: string;
17+
/**
18+
* Final plottable unit. This might be different from the original unit of the data, because we scale all time series to a single common unit.
19+
*/
20+
unit: DurationUnit | SizeUnit | RateUnit | null;
21+
};
22+
23+
/**
24+
* `AggregateTimeSeries` is a plottable that represents a continuous data time series. This is used for tasks like plotting a changing duration over time, for example. This is distinct from plotting items like sample series, threshold lines, etc. This ABC is inherited by specific plottable time series like `Line`, `Area`, and `Bars` to enforce the interface and share functionality.
25+
*/
26+
export abstract class AggregateTimeSeries<TConfig extends BasicDataPlottableConfig>
27+
implements PlottableData
28+
{
29+
// Ideally both the `timeSeries` and `config` would be protected properties.
30+
timeSeries: Readonly<TimeSeries>;
31+
config?: Readonly<TConfig>;
32+
33+
constructor(timeSeries: TimeSeries, config?: TConfig) {
34+
this.timeSeries = timeSeries;
35+
this.config = config;
36+
}
37+
38+
scaleToUnit(destinationUnit: DurationUnit | SizeUnit | RateUnit | null): TimeSeries {
39+
return scaleTimeSeriesData(this.timeSeries, destinationUnit);
40+
}
41+
42+
abstract toSeries(plottingOptions: AggregateTimePlottingOptions): SeriesOption[];
43+
}

static/app/views/dashboards/widgets/timeSeriesWidget/plottables/area.tsx

+27-12
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,45 @@ import type {LineSeriesOption} from 'echarts';
22

33
import LineSeries from 'sentry/components/charts/series/lineSeries';
44

5-
import type {TimeSeries} from '../../common/types';
5+
import type {PlottableData} from '../../common/types';
66
import {splitSeriesIntoCompleteAndIncomplete} from '../splitSeriesIntoCompleteAndIncomplete';
77
import {timeSeriesItemToEChartsDataPoint} from '../timeSeriesItemToEChartsDataPoint';
88

9-
import {Plottable} from './plottable';
9+
import {
10+
type AggregateTimePlottingOptions,
11+
AggregateTimeSeries,
12+
} from './aggregateTimeSeries';
1013

11-
interface AreaOptions {
14+
interface AreaConfig {
15+
/**
16+
* Optional color. If not provided, a backfill from a common palette will be provided to `toSeries`
17+
*/
1218
color?: string;
13-
dataCompletenessDelay?: number;
19+
/**
20+
* Data delay, in seconds. Data older than N seconds will be visually deemphasized.
21+
*/
22+
delay?: number;
1423
}
1524

16-
export class Area extends Plottable<AreaOptions> {
17-
toSeries(timeSeries: TimeSeries, options: AreaOptions) {
25+
/**
26+
* See documentation for `PlottableData` for an explanation.
27+
*/
28+
export class Area extends AggregateTimeSeries<AreaConfig> implements PlottableData {
29+
toSeries(plottingOptions: AggregateTimePlottingOptions): LineSeriesOption[] {
30+
const {timeSeries, config = {}} = this;
31+
32+
const {color, unit} = plottingOptions;
33+
34+
const scaledSeries = this.scaleToUnit(unit);
35+
1836
const [completeTimeSeries, incompleteTimeSeries] =
19-
splitSeriesIntoCompleteAndIncomplete(
20-
timeSeries,
21-
options.dataCompletenessDelay ?? 0
22-
);
37+
splitSeriesIntoCompleteAndIncomplete(scaledSeries, config.delay ?? 0);
2338

2439
const plottableSeries: LineSeriesOption[] = [];
2540

2641
const commonOptions = {
2742
name: timeSeries.field,
28-
color: options.color,
43+
color,
2944
animation: false,
3045
};
3146

@@ -53,7 +68,7 @@ export class Area extends Plottable<AreaOptions> {
5368
type: 'dotted',
5469
},
5570
areaStyle: {
56-
color: options.color,
71+
color,
5772
opacity: 0.8,
5873
},
5974
silent: true,

static/app/views/dashboards/widgets/timeSeriesWidget/plottables/bars.tsx

+35-13
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,58 @@
11
import Color from 'color';
2+
import type {BarSeriesOption, LineSeriesOption} from 'echarts';
23

34
import BarSeries from 'sentry/components/charts/series/barSeries';
45

5-
import type {TimeSeries} from '../../common/types';
6+
import type {PlottableData} from '../../common/types';
67
import {markDelayedData} from '../markDelayedData';
78
import {timeSeriesItemToEChartsDataPoint} from '../timeSeriesItemToEChartsDataPoint';
89

9-
import {Plottable} from './plottable';
10+
import {
11+
type AggregateTimePlottingOptions,
12+
AggregateTimeSeries,
13+
} from './aggregateTimeSeries';
1014

11-
interface BarsOptions {
15+
interface BarsConfig {
16+
/**
17+
* Optional color. If not provided, a backfill from a common palette will be provided to `toSeries`
18+
*/
1219
color?: string;
13-
dataCompletenessDelay?: number;
20+
/**
21+
* Data delay, in seconds. Data older than N seconds will be visually deemphasized.
22+
*/
23+
delay?: number;
24+
/**
25+
* Stack name. If provided, bar plottables with the same stack will be stacked visually.
26+
*/
1427
stack?: string;
1528
}
1629

17-
export class Bars extends Plottable<BarsOptions> {
18-
toSeries(timeSeries: TimeSeries, options: BarsOptions) {
19-
const markedSeries = markDelayedData(timeSeries, options.dataCompletenessDelay ?? 0);
30+
/**
31+
* See documentation for `PlottableData` for an explanation.
32+
*/
33+
export class Bars extends AggregateTimeSeries<BarsConfig> implements PlottableData {
34+
toSeries(
35+
plottingOptions: AggregateTimePlottingOptions
36+
): Array<BarSeriesOption | LineSeriesOption> {
37+
const {timeSeries, config = {}} = this;
38+
39+
const {color, unit} = plottingOptions;
40+
41+
const scaledTimeSeries = this.scaleToUnit(unit);
42+
43+
const markedSeries = markDelayedData(scaledTimeSeries, config.delay ?? 0);
2044

2145
return [
2246
BarSeries({
23-
name: markedSeries.field,
24-
color: markedSeries.color,
25-
stack: options.stack ?? GLOBAL_STACK_NAME,
47+
name: timeSeries.field,
48+
color: timeSeries.color,
49+
stack: config.stack ?? GLOBAL_STACK_NAME,
2650
animation: false,
2751
itemStyle: {
2852
color: params => {
2953
const datum = markedSeries.data[params.dataIndex]!;
3054

31-
return datum.delayed
32-
? Color(options.color).lighten(0.5).string()
33-
: options.color!;
55+
return datum.delayed ? Color(color).lighten(0.5).string() : color!;
3456
},
3557
opacity: 1.0,
3658
},

static/app/views/dashboards/widgets/timeSeriesWidget/plottables/line.tsx

+26-11
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,45 @@ import type {LineSeriesOption} from 'echarts';
22

33
import LineSeries from 'sentry/components/charts/series/lineSeries';
44

5-
import type {TimeSeries} from '../../common/types';
5+
import type {PlottableData} from '../../common/types';
66
import {splitSeriesIntoCompleteAndIncomplete} from '../splitSeriesIntoCompleteAndIncomplete';
77
import {timeSeriesItemToEChartsDataPoint} from '../timeSeriesItemToEChartsDataPoint';
88

9-
import {Plottable} from './plottable';
9+
import {
10+
type AggregateTimePlottingOptions,
11+
AggregateTimeSeries,
12+
} from './aggregateTimeSeries';
1013

11-
interface LineOptions {
14+
interface LineConfig {
15+
/**
16+
* Optional color. If not provided, a backfill from a common palette will be provided to `toSeries`
17+
*/
1218
color?: string;
13-
dataCompletenessDelay?: number;
19+
/**
20+
* Data delay, in seconds. Data older than N seconds will be visually deemphasized.
21+
*/
22+
delay?: number;
1423
}
1524

16-
export class Line extends Plottable<LineOptions> {
17-
toSeries(timeSeries: TimeSeries, options: LineOptions) {
25+
/**
26+
* See documentation for `PlottableData` for an explanation.
27+
*/
28+
export class Line extends AggregateTimeSeries<LineConfig> implements PlottableData {
29+
toSeries(plottingOptions: AggregateTimePlottingOptions) {
30+
const {timeSeries, config = {}} = this;
31+
32+
const {color, unit} = plottingOptions;
33+
34+
const scaledSeries = this.scaleToUnit(unit);
35+
1836
const [completeTimeSeries, incompleteTimeSeries] =
19-
splitSeriesIntoCompleteAndIncomplete(
20-
timeSeries,
21-
options.dataCompletenessDelay ?? 0
22-
);
37+
splitSeriesIntoCompleteAndIncomplete(scaledSeries, config.delay ?? 0);
2338

2439
const plottableSeries: LineSeriesOption[] = [];
2540

2641
const commonOptions = {
2742
name: timeSeries.field,
28-
color: options.color,
43+
color,
2944
animation: false,
3045
};
3146

static/app/views/dashboards/widgets/timeSeriesWidget/plottables/plottable.tsx

-13
This file was deleted.

static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx

+12-19
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import {formatXAxisTimestamp} from './formatXAxisTimestamp';
4141
import {formatYAxisValue} from './formatYAxisValue';
4242
import {isTimeSeriesOther} from './isTimeSeriesOther';
4343
import {ReleaseSeries} from './releaseSeries';
44-
import {scaleTimeSeriesData} from './scaleTimeSeriesData';
4544
import {FALLBACK_TYPE, FALLBACK_UNIT_FOR_FIELD_TYPE} from './settings';
4645

4746
type VisualizationType = 'area' | 'line' | 'bar';
@@ -178,11 +177,6 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
178177
yAxisUnit = FALLBACK_UNIT_FOR_FIELD_TYPE[yAxisFieldType];
179178
}
180179

181-
// Apply unit scaling to all series
182-
const scaledSeries = props.timeSeries.map(timeSeries => {
183-
return scaleTimeSeriesData(timeSeries, yAxisUnit);
184-
});
185-
186180
// Construct plottable items
187181
const numberOfSeriesNeedingColor = props.timeSeries.filter(needsColor).length;
188182

@@ -192,7 +186,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
192186
: [];
193187

194188
let seriesColorIndex = 0;
195-
const plottables = scaledSeries.map(timeSeries => {
189+
const plottables = props.timeSeries.map(timeSeries => {
196190
let color: string;
197191

198192
if (timeSeries.color) {
@@ -209,21 +203,21 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
209203

210204
if (props.visualizationType === 'area') {
211205
return new Area(timeSeries, {
212-
dataCompletenessDelay,
206+
delay: dataCompletenessDelay,
213207
color,
214208
});
215209
}
216210

217211
if (props.visualizationType === 'bar') {
218212
return new Bars(timeSeries, {
219-
dataCompletenessDelay,
213+
delay: dataCompletenessDelay,
220214
color,
221215
stack: props.stacked ? GLOBAL_STACK_NAME : undefined,
222216
});
223217
}
224218

225219
return new Line(timeSeries, {
226-
dataCompletenessDelay,
220+
delay: dataCompletenessDelay,
227221
color,
228222
});
229223
});
@@ -273,13 +267,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
273267
return formatTooltipValue(value, FALLBACK_TYPE);
274268
}
275269

276-
const timeSeries = scaledSeries.find(t => t.field === field);
277-
278-
return formatTooltipValue(
279-
value,
280-
timeSeries?.meta?.fields?.[field] ?? FALLBACK_TYPE,
281-
timeSeries?.meta?.units?.[field] ?? undefined
282-
);
270+
return formatTooltipValue(value, yAxisFieldType, yAxisUnit ?? undefined);
283271
},
284272
nameFormatter: seriesName => {
285273
return props.aliases?.[seriesName] ?? formatSeriesName(seriesName);
@@ -289,14 +277,19 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
289277
})(deDupedParams, asyncTicket);
290278
};
291279

292-
let visibleSeriesCount = scaledSeries.length;
280+
let visibleSeriesCount = plottables.length;
293281
if (releaseSeries) {
294282
visibleSeriesCount += 1;
295283
}
296284

297285
const showLegend = visibleSeriesCount > 1;
298286

299-
const dataSeries = plottables.flatMap(plottable => plottable.series);
287+
const dataSeries = plottables.flatMap(plottable =>
288+
plottable.toSeries({
289+
color: plottable.config?.color!, // Colors are assigned from palette, above, so they're guaranteed to be there
290+
unit: yAxisUnit,
291+
})
292+
);
300293

301294
return (
302295
<BaseChart

0 commit comments

Comments
 (0)