Skip to content

Commit 293547d

Browse files
authored
fix(ui): Fix last release bubble bucket (#86956)
The last release bubble has some more recent releases filtered out because the meaning of `maxTime` differs between the time series plots and the release buckets: <img width="886" alt="image" src="https://github.com/user-attachments/assets/d2d7aa24-817b-450e-9621-6a878c7e9a01" /> This PR changes the bucketing logic so it adds releases > `maxTime` to the last bucket. It assumes we fetch the correct releases outside of the buckets. We will have to fix the tooltip to reflect this without changing `maxTime`. ref #85775
1 parent 79c646d commit 293547d

File tree

4 files changed

+76
-17
lines changed

4 files changed

+76
-17
lines changed

static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/types.tsx

+5
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,9 @@ export type Bucket = {
44
end: number;
55
releases: ReleaseMetaBasic[];
66
start: number;
7+
// This is only set on the last bucket item and represents latest timestamp
8+
// for data whereas `end` represents the point on a chart's x-axis (time).
9+
// e.g. the max timestamp we show on the x-axis is 3:30, but data at that
10+
// point represents data from [3:30, now (final)]
11+
final?: number;
712
};

static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/useReleaseBubbles.tsx

+12-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {ReleaseMetaBasic} from 'sentry/types/release';
2424
import {defined} from 'sentry/utils';
2525
import {getFormat} from 'sentry/utils/dates';
2626
import useOrganization from 'sentry/utils/useOrganization';
27+
import usePageFilters from 'sentry/utils/usePageFilters';
2728
import {useUser} from 'sentry/utils/useUser';
2829
import {
2930
BUBBLE_AREA_SERIES_ID,
@@ -76,7 +77,7 @@ function createReleaseBubbleMouseListeners({
7677
() => (
7778
<ReleasesDrawer
7879
startTs={data.start}
79-
endTs={data.end}
80+
endTs={data.final ?? data.end}
8081
releases={data.releases}
8182
buckets={buckets}
8283
chartRenderer={chartRenderer}
@@ -302,7 +303,7 @@ function ReleaseBubbleSeries({
302303
${tn('%s Release', '%s Releases', numberReleases)}
303304
</div>
304305
<div class="tooltip-release-timerange">
305-
${formatBucketTimestamp(bucket.start)} - ${formatBucketTimestamp(bucket.end)}
306+
${formatBucketTimestamp(bucket.start)} - ${formatBucketTimestamp(bucket.final ?? bucket.end)}
306307
</div>
307308
</div>
308309
@@ -346,13 +347,21 @@ export function useReleaseBubbles({
346347
const {openDrawer} = useDrawer();
347348
const theme = useTheme();
348349
const {options} = useUser();
350+
const {selection} = usePageFilters();
351+
// `maxTime` refers to the max time on x-axis for charts.
352+
// There may be the need to include releases that are > maxTime (e.g. in the
353+
// case of relative date selection). This is used for the tooltip to show the
354+
// proper timestamp for releases.
355+
const releasesMaxTime = defined(selection.datetime.end)
356+
? new Date(selection.datetime.end).getTime()
357+
: Date.now();
349358
const hasReleaseBubbles = organization.features.includes('release-bubbles-ui');
350359
const buckets =
351360
(hasReleaseBubbles &&
352361
releases?.length &&
353362
minTime &&
354363
maxTime &&
355-
createReleaseBuckets(minTime, maxTime, releases)) ||
364+
createReleaseBuckets(minTime, maxTime, releasesMaxTime, releases)) ||
356365
[];
357366

358367
if (!releases || !buckets.length) {

static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/utils/createReleaseBuckets.spec.tsx

+28-6
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,23 @@ describe('createReleaseBuckets', () => {
1111
])(
1212
'creates correct # of buckets for timeSeries with [min, max] of [%d, %d] and %d desired buckets',
1313
(minTime, maxTime, desiredBuckets, expectedBuckets) => {
14-
const buckets = createReleaseBuckets(minTime, maxTime, [], desiredBuckets);
14+
const buckets = createReleaseBuckets(
15+
minTime,
16+
maxTime,
17+
Date.now() + 120391, // Shouldn't affect buckets
18+
[],
19+
desiredBuckets
20+
);
1521
expect(buckets).toHaveLength(expectedBuckets);
1622
}
1723
);
1824

1925
it('creates the correct buckets', () => {
2026
const minTime = Date.now();
2127
const maxTime = Date.now() + 12 * 1000 + 2235;
28+
const finalTime = maxTime + 9999;
2229

23-
const buckets = createReleaseBuckets(minTime, maxTime, []);
30+
const buckets = createReleaseBuckets(minTime, maxTime, finalTime, []);
2431
expect(buckets).toEqual([
2532
{start: 1508208080000, end: 1508208081424, releases: []},
2633
{start: 1508208081424, end: 1508208082848, releases: []},
@@ -31,13 +38,14 @@ describe('createReleaseBuckets', () => {
3138
{start: 1508208088544, end: 1508208089968, releases: []},
3239
{start: 1508208089968, end: 1508208091392, releases: []},
3340
{start: 1508208091392, end: 1508208092816, releases: []},
34-
{start: 1508208092816, end: 1508208094235, releases: []},
41+
{start: 1508208092816, end: 1508208094235, final: finalTime, releases: []},
3542
]);
3643
});
3744

3845
it('buckets releases correctly', () => {
3946
const minTime = Date.now();
4047
const maxTime = Date.now() + 12 * 1000 + 2235;
48+
const finalTime = maxTime + 9999;
4149

4250
const releases = [
4351
{
@@ -76,14 +84,25 @@ describe('createReleaseBuckets', () => {
7684
version: '[email protected]',
7785
date: new Date(1508208094235).toISOString(),
7886
},
79-
// Should not be included
87+
// Note this is included even though it is > maxTime
88+
// because `maxTime` is actually the start time of the
89+
// last time series item. We don't necessarily have the
90+
// ending timestamp of that bucket
8091
{
8192
version: '[email protected]',
82-
date: new Date(1508208094236).toISOString(),
93+
date: new Date(maxTime + 1).toISOString(),
94+
},
95+
{
96+
version: '[email protected]',
97+
date: new Date(maxTime + 10000).toISOString(),
98+
},
99+
{
100+
version: '[email protected]',
101+
date: new Date(minTime - 5000).toISOString(),
83102
},
84103
];
85104

86-
const buckets = createReleaseBuckets(minTime, maxTime, releases);
105+
const buckets = createReleaseBuckets(minTime, maxTime, finalTime, releases);
87106

88107
expect(buckets).toEqual([
89108
{
@@ -110,11 +129,14 @@ describe('createReleaseBuckets', () => {
110129
{
111130
start: 1508208092816,
112131
end: 1508208094235,
132+
final: finalTime,
113133
releases: [
114134
{version: '[email protected]', date: new Date(1508208092816).toISOString()},
115135
{version: '[email protected]', date: new Date(1508208094230).toISOString()},
116136
{version: '[email protected]', date: new Date(1508208094235).toISOString()},
117137
{version: '[email protected]', date: new Date(1508208094235).toISOString()},
138+
{version: '[email protected]', date: new Date(1508208094236).toISOString()},
139+
{version: '[email protected]', date: new Date(1508208104235).toISOString()},
118140
],
119141
},
120142
]);

static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/utils/createReleaseBuckets.tsx

+31-8
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type {Bucket} from 'sentry/views/dashboards/widgets/timeSeriesWidget/rele
2222
export function createReleaseBuckets(
2323
minTime: number | undefined,
2424
maxTime: number | undefined,
25+
finalTime: number,
2526
releases: ReleaseMetaBasic[],
2627
desiredBuckets = 10
2728
): Bucket[] {
@@ -46,8 +47,15 @@ export function createReleaseBuckets(
4647
const bucketStartTs = minTime + i * interval;
4748
// Ending timestamp will clump in the remaining bits if it's not
4849
// evenly distributed
49-
const bucketEndTs = i === desiredBuckets - 1 ? maxTime : bucketStartTs + interval;
50-
buckets.push({start: bucketStartTs, end: bucketEndTs, releases: []});
50+
const isLastBucket = i === desiredBuckets - 1;
51+
const bucketEndTs = isLastBucket ? maxTime : bucketStartTs + interval;
52+
const item: Bucket = {start: bucketStartTs, end: bucketEndTs, releases: []};
53+
54+
if (isLastBucket) {
55+
item.final = finalTime;
56+
}
57+
58+
buckets.push(item);
5159
}
5260

5361
// Loop through releases and update its bucket's counters
@@ -56,19 +64,34 @@ export function createReleaseBuckets(
5664
break;
5765
}
5866

59-
// We can determine the releaase's bucket location by using its timestamp
67+
// We can determine the release's bucket location by using its timestamp
6068
// relative to the series starting timestamp.
6169
const releaseTs = new Date(release.date).getTime();
6270
const bucketIndex = Math.floor((releaseTs - minTime) / interval);
6371
const currentBucket = buckets[bucketIndex];
6472
const bucketEndTs = currentBucket?.end;
6573

66-
// Note we need to check that the release ts is within the ending bounds, solely for
67-
// the last bucket, as the last buckets width can be less than the
68-
// others. (i.e. if the total time difference is not perfectly divible by
69-
// `desiredBuckets`, the last bucket will be the remainder)
70-
if (bucketEndTs && releaseTs <= bucketEndTs) {
74+
// Note that `maxTime` represents the position on the xaxis (time) of the
75+
// last data point. That timestamp corresponds to data at "timestamp" +
76+
// "interval", (e.g. if tooltip says 1:00 and interval between points is
77+
// 30 minutes, the data collected is between 1:00 and 1:30).
78+
//
79+
// The release buckets are different because it has a start and end range
80+
// and for its last bucket, it's ending boundary is `maxTime` to
81+
// correspond to the chart. We cannot change `maxTime` for the buckets
82+
// otherwise we will draw bubbles past the chart data and it would look
83+
// broken.
84+
//
85+
// For now we're going to assume that releases will always fall inbounds
86+
// and stick any releases with timestamp > `maxLife` into the last
87+
// bucket. The tooltip will be slightly wrong because it will leave out
88+
// the last interval.
89+
if (bucketEndTs) {
7190
currentBucket.releases.push(release);
91+
} else if (releaseTs > maxTime) {
92+
// If we couldn't find a bucket, add release to latest bucket
93+
const lastBucket = buckets.at(-1);
94+
lastBucket?.releases.push(release);
7295
}
7396
}
7497

0 commit comments

Comments
 (0)