Skip to content

Commit 5c334f6

Browse files
authored
ref(dashboards): Set aliases per plottable in TimeSeriesWidgetVisualization (#87023)
We allow plottables to have aliases. e.g., ```jsx const timeSeries = { field: 'failure_rate_5()', ... <TimeSeriesWidgetVisualization plottables={[ new Line(timeSeries) ]} aliases={{'failure_rate_5()': '5XX'}} /> ``` That makes it easy to set nice aliases for long series names in charts. This is not a great API for a few reasons: 1. Aliases have to be constructed ahead of time. Sometimes this is easy, sometime it's annoying, if you're calculating aliases as the same time as constructing the plottables 2. This won't work well in the near future. Right now we're passing around objects with properties like `field: "/insights,prod : eps()"` which is impractical. We're going to break that stuff up 3. This works like crap for charts that have different series with the same `field`, like if you're showing different `avg(span.duration)` from different queries, for example This PR changes the API, and now every plottable can specify its alias, which is much more explicit! e.g., ```jsx const timeSeries = { field: 'failure_rate_5()', ... <TimeSeriesWidgetVisualization plottables={[ new Line(timeSeries, {alias: '5XX'}) ]} /> ``` Each plottable has a `label` property that either returns the alias, or computes a default value if needed. It sets the label directly on the output `Series` so we don't have to do any additional processing at tooltip time 👍🏻 N.B. `InsightsTimeSeriesWidget` still accepts `aliases` and then passes them along to the right plottable. That's just for convenience in that case, I'll be gradually thinning that component out.
1 parent cca33a2 commit 5c334f6

File tree

11 files changed

+46
-39
lines changed

11 files changed

+46
-39
lines changed

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

-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,4 @@ export type Release = {
4242
version: string;
4343
};
4444

45-
export type Aliases = Record<string, string>;
46-
4745
export type LegendSelection = {[key: string]: boolean};

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class Area extends ContinuousTimeSeries implements Plottable {
1515
return new Area(this.constrainTimeSeries(boundaryStart, boundaryEnd), this.config);
1616
}
1717
toSeries(plottingOptions: ContinuousTimeSeriesPlottingOptions): LineSeriesOption[] {
18-
const {timeSeries, config = {}} = this;
18+
const {config = {}} = this;
1919

2020
const color = plottingOptions.color ?? config.color ?? undefined;
2121
const scaledSeries = this.scaleToUnit(plottingOptions.unit);
@@ -26,7 +26,7 @@ export class Area extends ContinuousTimeSeries implements Plottable {
2626
const plottableSeries: LineSeriesOption[] = [];
2727

2828
const commonOptions = {
29-
name: timeSeries.field,
29+
name: this.label,
3030
color,
3131
animation: false,
3232
yAxisIndex: plottingOptions.yAxisPosition === 'left' ? 0 : 1,

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class Bars extends ContinuousTimeSeries<BarsConfig> implements Plottable
2626
toSeries(
2727
plottingOptions: ContinuousTimeSeriesPlottingOptions
2828
): Array<BarSeriesOption | LineSeriesOption> {
29-
const {timeSeries, config = {}} = this;
29+
const {config = {}} = this;
3030

3131
const color = plottingOptions.color ?? config.color ?? undefined;
3232
const scaledTimeSeries = this.scaleToUnit(plottingOptions.unit);
@@ -35,7 +35,7 @@ export class Bars extends ContinuousTimeSeries<BarsConfig> implements Plottable
3535

3636
return [
3737
BarSeries({
38-
name: timeSeries.field,
38+
name: this.label,
3939
stack: config.stack,
4040
yAxisIndex: plottingOptions.yAxisPosition === 'left' ? 0 : 1,
4141
color,

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

+9
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ import type {AggregationOutputType, DataUnit} from 'sentry/utils/discover/fields
44
import {scaleTimeSeriesData} from 'sentry/utils/timeSeries/scaleTimeSeriesData';
55

66
import type {TimeSeries} from '../../common/types';
7+
import {formatSeriesName} from '../formatters/formatSeriesName';
78

89
export type ContinuousTimeSeriesConfig = {
10+
/**
11+
* Optional alias. If not provided, the series name from the legend will be computed from the `TimeSeries`.
12+
*/
13+
alias?: string;
914
/**
1015
* Optional color. If not provided, a backfill from a common palette will be provided to `toSeries`
1116
*/
@@ -48,6 +53,10 @@ export abstract class ContinuousTimeSeries<
4853
this.config = config;
4954
}
5055

56+
get label(): string {
57+
return this.config?.alias ?? formatSeriesName(this.timeSeries.field);
58+
}
59+
5160
get isEmpty(): boolean {
5261
return this.timeSeries.data.every(datum => datum.value === null);
5362
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class Line extends ContinuousTimeSeries implements Plottable {
1515
return new Line(this.constrainTimeSeries(boundaryStart, boundaryEnd), this.config);
1616
}
1717
toSeries(plottingOptions: ContinuousTimeSeriesPlottingOptions) {
18-
const {timeSeries, config = {}} = this;
18+
const {config = {}} = this;
1919

2020
const color = plottingOptions.color ?? config.color ?? undefined;
2121
const scaledSeries = this.scaleToUnit(plottingOptions.unit);
@@ -26,7 +26,7 @@ export class Line extends ContinuousTimeSeries implements Plottable {
2626
const plottableSeries: LineSeriesOption[] = [];
2727

2828
const commonOptions = {
29-
name: timeSeries.field,
29+
name: this.label,
3030
color,
3131
animation: false,
3232
yAxisIndex: plottingOptions.yAxisPosition === 'left' ? 0 : 1,

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ export interface Plottable {
3535
* Start timestamp of the plottable, if applicable
3636
*/
3737
start: string | null;
38-
3938
/**
4039
*
4140
* @param plottingOptions Plotting options depend on the specific implementation of the interface.
4241
*/
4342
toSeries(plottingOptions: unknown): SeriesOption[];
43+
/**
44+
* Optional label for this plottable, if it appears in the legend and in tooltips.
45+
*/
46+
label?: string;
4447
}

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

+6-9
Original file line numberDiff line numberDiff line change
@@ -490,21 +490,18 @@ export default storyBook('TimeSeriesWidgetVisualization', (story, APIReference)
490490
the user changes the legend selection by clicking on legend labels.
491491
</p>
492492
<p>
493-
You can also provide aliases for legends to give them a friendlier name. In this
494-
example, verbose names like "p99(span.duration)" are truncated, and the p99
495-
series is hidden by default.
493+
You can also provide aliases for plottables like <code>Line</code> This will
494+
give the legends and tooltips a friendlier name. In this example, verbose names
495+
like "p99(span.duration)" are truncated, and the p99 series is hidden by
496+
default.
496497
</p>
497498

498499
<MediumWidget>
499500
<TimeSeriesWidgetVisualization
500501
plottables={[
501-
new Area(sampleDurationTimeSeries, {}),
502-
new Area(sampleDurationTimeSeries2, {}),
502+
new Area(sampleDurationTimeSeries, {alias: 'p50'}),
503+
new Area(sampleDurationTimeSeries2, {alias: 'p99'}),
503504
]}
504-
aliases={{
505-
'p99(span.duration)': 'p99',
506-
'p50(span.duration)': 'p50',
507-
}}
508505
legendSelection={legendSelection}
509506
onLegendSelectionChange={setLegendSelection}
510507
/>

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

+2-9
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ import {makeReleasesPathname} from 'sentry/views/releases/utils/pathnames';
3131

3232
import {useWidgetSyncContext} from '../../contexts/widgetSyncContext';
3333
import {NO_PLOTTABLE_VALUES, X_GUTTER, Y_GUTTER} from '../common/settings';
34-
import type {Aliases, LegendSelection, Release} from '../common/types';
34+
import type {LegendSelection, Release} from '../common/types';
3535

36-
import {formatSeriesName} from './formatters/formatSeriesName';
3736
import {formatTooltipValue} from './formatters/formatTooltipValue';
3837
import {formatXAxisTimestamp} from './formatters/formatXAxisTimestamp';
3938
import {formatYAxisValue} from './formatters/formatYAxisValue';
@@ -50,9 +49,6 @@ export interface TimeSeriesWidgetVisualizationProps {
5049
*/
5150
plottables: Plottable[];
5251
/**
53-
* A mapping of time series fields to their user-friendly labels, if needed
54-
*/
55-
aliases?: Aliases;
5652
/**
5753
* Disables navigating to release details when clicked
5854
* TODO(billy): temporary until we implement route based nav
@@ -350,9 +346,6 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
350346

351347
return formatTooltipValue(value, fieldType, unitForType[fieldType] ?? undefined);
352348
},
353-
nameFormatter: seriesName => {
354-
return props.aliases?.[seriesName] ?? formatSeriesName(seriesName);
355-
},
356349
truncate: true,
357350
utc: utc ?? false,
358351
})(deDupedParams, asyncTicket);
@@ -459,7 +452,7 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
459452
left: 0,
460453
formatter(seriesName: string) {
461454
return truncationFormatter(
462-
props.aliases?.[seriesName] ?? formatSeriesName(seriesName),
455+
seriesName,
463456
true,
464457
// Escaping the legend string will cause some special
465458
// characters to render as their HTML equivalents.

static/app/views/insights/common/components/insightsTimeSeriesWidget.tsx

+2-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import useOrganization from 'sentry/utils/useOrganization';
99
import usePageFilters from 'sentry/utils/usePageFilters';
1010
import {useReleaseStats} from 'sentry/utils/useReleaseStats';
1111
import {MISSING_DATA_MESSAGE} from 'sentry/views/dashboards/widgets/common/settings';
12-
import type {Aliases} from 'sentry/views/dashboards/widgets/common/types';
1312
import {Area} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/area';
1413
import {Bars} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/bars';
1514
import {Line} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/line';
@@ -37,7 +36,7 @@ export interface InsightsTimeSeriesWidgetProps {
3736
series: DiscoverSeries[];
3837
title: string;
3938
visualizationType: 'line' | 'area' | 'bar';
40-
aliases?: Aliases;
39+
aliases?: Record<string, string>;
4140
description?: React.ReactNode;
4241
stacked?: boolean;
4342
}
@@ -66,9 +65,9 @@ export function InsightsTimeSeriesWidget(props: InsightsTimeSeriesWidgetProps) {
6665
color: serie.color ?? COMMON_COLORS[timeSeries.field],
6766
delay: INGESTION_DELAY,
6867
stack: props.stacked && props.visualizationType === 'bar' ? 'all' : undefined,
68+
alias: props.aliases?.[timeSeries.field],
6969
});
7070
}),
71-
aliases: props.aliases,
7271
};
7372

7473
const Title = <Widget.WidgetTitle title={props.title} />;

static/app/views/insights/pages/backend/laravel/jobsWidget.tsx

+6-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,12 @@ export function JobsWidget({query}: {query?: string}) {
107107

108108
const plottables = useMemo(() => {
109109
return timeSeries.map(
110-
ts => new Bars(convertSeriesToTimeseries(ts), {color: ts.color, stack: 'stack'})
110+
ts =>
111+
new Bars(convertSeriesToTimeseries(ts), {
112+
color: ts.color,
113+
stack: 'stack',
114+
alias: seriesAliases[ts.seriesName as 'ok' | 'internal_error'],
115+
})
111116
);
112117
}, [timeSeries]);
113118

@@ -120,7 +125,6 @@ export function JobsWidget({query}: {query?: string}) {
120125
isEmpty={isEmpty}
121126
VisualizationType={TimeSeriesWidgetVisualization}
122127
visualizationProps={{
123-
aliases: seriesAliases,
124128
plottables,
125129
}}
126130
/>

static/app/views/insights/pages/backend/laravel/queriesWidget.tsx

+11-7
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ export function QueriesWidget({query}: {query?: string}) {
135135

136136
const colorPalette = getChartColorPalette(timeSeries.length - 2);
137137

138+
const aliases = Object.fromEntries(
139+
queriesRequest.data?.data.map(item => [
140+
getSeriesName(item),
141+
item['sentry.normalized_description'],
142+
]) ?? []
143+
);
144+
138145
const visualization = (
139146
<WidgetVisualizationStates
140147
isEmpty={!hasData}
@@ -143,15 +150,12 @@ export function QueriesWidget({query}: {query?: string}) {
143150
emptyMessage={<TimeSpentInDatabaseWidgetEmptyStateWarning />}
144151
VisualizationType={TimeSeriesWidgetVisualization}
145152
visualizationProps={{
146-
aliases: Object.fromEntries(
147-
queriesRequest.data?.data.map(item => [
148-
getSeriesName(item),
149-
item['sentry.normalized_description'],
150-
]) ?? []
151-
),
152153
plottables: timeSeries.map(
153154
(ts, index) =>
154-
new Line(convertSeriesToTimeseries(ts), {color: colorPalette[index]})
155+
new Line(convertSeriesToTimeseries(ts), {
156+
color: colorPalette[index],
157+
alias: aliases[ts.seriesName],
158+
})
155159
),
156160
}}
157161
/>

0 commit comments

Comments
 (0)