Skip to content

Commit 5e2892e

Browse files
feat(dashboards): Add groupBy handling to TimeSeries (#95116)
More work to align `TimeSeries` with the response format of `/events-timeseries` endpoint. This time, `groupBy`! The endpoint returns "group by" information as an array of key-value objects. This PR adds that to the `TimeSeries` type, and updates `TimeSeriesWidgetVisualization` to take `groupBy` into account when creating the legends. Right now, nothing in our code _sets_ `groupBy`, instead it does shenanigans like setting `yAxis` to `"prod"` or `"prod,v1.2"` to get the correct labels. This PR doesn't update any of that usage yet, so the current UI is unaffected. I'll either update that usage in the near future, or we'll get the correct behaviour for free once we switch to using `/events-timeseries`. **e.g.,** In this screenshot, both the `TimeSeries` have a `yAxis` of `"p50(span.duration)"` but different group by, which you can see in the legend. <img width="445" alt="Screenshot 2025-07-09 at 10 23 03 AM" src="https://github.com/user-attachments/assets/3dd224e0-d944-48a9-b3de-47fc6a0a4f02" /> --------- Co-authored-by: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
1 parent 05ca85a commit 5e2892e

File tree

5 files changed

+121
-2
lines changed

5 files changed

+121
-2
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,18 @@ export type TimeSeriesItem = {
4646
incomplete?: boolean;
4747
};
4848

49+
type TimeSeriesGroupBy = {
50+
key: string;
51+
value: string;
52+
};
53+
4954
export type TimeSeries = {
5055
meta: TimeSeriesMeta;
5156
values: TimeSeriesItem[];
5257
yAxis: string;
5358
confidence?: Confidence;
5459
dataScanned?: 'full' | 'partial';
60+
groupBy?: TimeSeriesGroupBy[];
5561
sampleCount?: AccuracyStats<number>;
5662
samplingRate?: AccuracyStats<number | null>;
5763
};

static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.spec.tsx

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,35 @@ describe('formatSeriesName', () => {
8282
expect(formatTimeSeriesName(timeSeries)).toEqual(result);
8383
});
8484
});
85+
86+
describe('groupBy', () => {
87+
it.each([
88+
[
89+
'equation|p75(measurements.cls);76123',
90+
[{key: 'release', value: 'v0.0.2'}],
91+
'v0.0.2',
92+
],
93+
['p95(span.duration)', [{key: 'release', value: 'v0.0.2'}], 'v0.0.2'],
94+
[
95+
'p95(span.duration)',
96+
[
97+
{key: 'release', value: 'v0.0.2'},
98+
{key: 'env', value: 'prod'},
99+
],
100+
'v0.0.2,prod',
101+
],
102+
[
103+
'p95(span.duration)',
104+
[{key: 'release', value: 'frontend@31804d9a5f0b5e4f53055467cd258e1c'}],
105+
'31804d9a5f0b',
106+
],
107+
])('Formats %s with groupBy %s as %s', (name, groupBy, result) => {
108+
const timeSeries = TimeSeriesFixture({
109+
yAxis: name,
110+
groupBy,
111+
});
112+
113+
expect(formatTimeSeriesName(timeSeries)).toEqual(result);
114+
});
115+
});
85116
});

static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,37 @@ import WidgetLegendNameEncoderDecoder from 'sentry/views/dashboards/widgetLegend
99
import type {TimeSeries} from 'sentry/views/dashboards/widgets/common/types';
1010

1111
export function formatTimeSeriesName(timeSeries: TimeSeries): string {
12+
// If the timeSeries has `groupBy` information, the label is made by
13+
// concatenating the values of the groupBy, since there's no point repeating
14+
// the name of the Y axis multiple times in the legend.
15+
if (timeSeries.groupBy?.length && timeSeries.groupBy.length > 0) {
16+
return `${timeSeries.groupBy
17+
?.map(groupBy => {
18+
if (groupBy.key === 'release') {
19+
return formatVersion(groupBy.value);
20+
}
21+
22+
return groupBy.value;
23+
})
24+
.join(',')}`;
25+
}
26+
1227
let {yAxis: seriesName} = timeSeries;
28+
1329
// Decode from series name disambiguation
1430
seriesName = WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(seriesName)!;
1531

16-
// Check if it's a release version
32+
// Attempt to parse the `seriesName` as a version. A correct `TimeSeries`
33+
// would have a `yAxis` like `p50(span.duration)` with a `groupBy` like
34+
// `[{key: "release", value: "proj@1.2.3"}]`. `groupBy` was only introduced
35+
// recently though, so many `TimeSeries` instead mash the group by information
36+
// into the `yAxis` property, e.g., the `yAxis` might have been set to
37+
// `"proj@1.2.3"` just to get the correct rendering in the chart legend. We
38+
// cover these cases by parsing the `yAxis` as a series name. This works badly
39+
// because sometimes it'll interpet email addresses as versions, which causes
40+
// bugs. We should update all usages of `TimeSeriesWidgetVisualization` to
41+
// correctly specify `yAxis` and `groupBy`, and/or to use the time
42+
// `/events-timeseries` endpoint which does this automatically.
1743
seriesName = formatVersion(seriesName);
1844

1945
// Check for special-case measurement formatting

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,21 @@ export abstract class ContinuousTimeSeries<
5858
this.config = config;
5959
}
6060

61+
/**
62+
* Continuous time series names need to be unique to disambiguate them from other series. We use both the `yAxis` and the `groupBy` to create the name. This makes it possible to pass in two different time series with the same `yAxis` as long as they have different `groupBy` information.
63+
*/
6164
get name(): string {
62-
return this.timeSeries.yAxis;
65+
let name = `${this.timeSeries.yAxis}`;
66+
67+
if (this.timeSeries.groupBy?.length) {
68+
name += ` : ${this.timeSeries.groupBy
69+
?.map(groupBy => {
70+
return `${groupBy.key}:${groupBy.value}`;
71+
})
72+
.join(',')}`;
73+
}
74+
75+
return name;
6376
}
6477

6578
get label(): string {

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,15 @@ export default Storybook.story('TimeSeriesWidgetVisualization', (story, APIRefer
923923
default. The legend will always include an entry for every plottable, even if
924924
some plottables have the same alias, as you can see in the second example.
925925
</p>
926+
<p>
927+
By default, <Storybook.JSXNode name="TimeSeriesWidgetVisualization" /> creates
928+
legend labels using all information from the <code>TimeSeries</code> object,
929+
including the <code>yAxis</code> and the <code>groupBy</code>. In the first two
930+
examples, the label uses the <code>yAxis</code> property. In the third example,
931+
each <code>TimeSeries</code> has a <code>groupBy</code> property, so the{' '}
932+
<code>yAxis</code> property is not shown in the label. The best way to override
933+
this is by using the <code>alias</code> of a plottable.
934+
</p>
926935

927936
<code>{JSON.stringify(legendSelection)}</code>
928937

@@ -945,6 +954,40 @@ export default Storybook.story('TimeSeriesWidgetVisualization', (story, APIRefer
945954
]}
946955
/>
947956
</MediumWidget>
957+
<MediumWidget>
958+
<TimeSeriesWidgetVisualization
959+
plottables={[
960+
new Line({
961+
...sampleDurationTimeSeries,
962+
yAxis: 'span.duration()',
963+
groupBy: [
964+
{
965+
key: 'release',
966+
value: 'proj@v0.6.2',
967+
},
968+
{
969+
key: 'env',
970+
value: 'production',
971+
},
972+
],
973+
}),
974+
new Line({
975+
...sampleDurationTimeSeriesP50,
976+
yAxis: 'span.duration()',
977+
groupBy: [
978+
{
979+
key: 'release',
980+
value: 'proj@v0.6.1',
981+
},
982+
{
983+
key: 'env',
984+
value: 'production',
985+
},
986+
],
987+
}),
988+
]}
989+
/>
990+
</MediumWidget>
948991
</Storybook.SideBySide>
949992
</Fragment>
950993
);

0 commit comments

Comments
 (0)