diff --git a/static/app/utils/array/isNonEmptyArray.ts b/static/app/utils/array/isNonEmptyArray.ts new file mode 100644 index 00000000000000..8c4c3f935ab3d7 --- /dev/null +++ b/static/app/utils/array/isNonEmptyArray.ts @@ -0,0 +1,11 @@ +type NonEmptyArray = [T, ...T[]]; + +/** + * Allows TypeScript to infer that arrays have at least one item so that + * expressions like `array[0]` typecheck naturally without needed non-null + * assertions. Unfortunately heavily limited to only work with `[0]` and not + * `.at(0)` or any other index. + */ +export function isNonEmptyArray(array: T[]): array is NonEmptyArray { + return array.length > 0; +} diff --git a/static/app/utils/duration/millisecondsToInterval.spec.tsx b/static/app/utils/duration/closestIntervalToDuration.spec.tsx similarity index 70% rename from static/app/utils/duration/millisecondsToInterval.spec.tsx rename to static/app/utils/duration/closestIntervalToDuration.spec.tsx index 1eed695b8731f5..1bb744f9c16cbe 100644 --- a/static/app/utils/duration/millisecondsToInterval.spec.tsx +++ b/static/app/utils/duration/closestIntervalToDuration.spec.tsx @@ -1,22 +1,6 @@ -import {millisecondsToClosestInterval} from 'sentry/utils/duration/millisecondsToInterval'; +import {closestIntervalToDuration} from 'sentry/utils/duration/closestIntervalToDuration'; -const TEST_INTERVALS = [ - '1m', - '2m', - '5m', - '10m', - '15m', - '30m', - '1h', - '2h', - '3h', - '4h', - '6h', - '12h', - '1d', -]; - -describe('millisecondsToClosestInterval()', () => { +describe('closestIntervalToDuration()', () => { it.each([ [60_000, '1m'], [2 * 60_000, '2m'], @@ -28,7 +12,7 @@ describe('millisecondsToClosestInterval()', () => { [6 * 3600_000, '6h'], [24 * 3600_000, '1d'], ])('returns an exact string for valid granularity (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, TEST_INTERVALS)).toBe(expected); + expect(closestIntervalToDuration(ms, TEST_INTERVALS)).toBe(expected); }); it.each([ @@ -47,7 +31,7 @@ describe('millisecondsToClosestInterval()', () => { ])( 'rounds to the nearest interval when between two valid granularities (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, TEST_INTERVALS)).toBe(expected); + expect(closestIntervalToDuration(ms, TEST_INTERVALS)).toBe(expected); } ); @@ -57,7 +41,7 @@ describe('millisecondsToClosestInterval()', () => { ])( 'clamps to the smallest valid interval for values below the minimum (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, TEST_INTERVALS)).toBe(expected); + expect(closestIntervalToDuration(ms, TEST_INTERVALS)).toBe(expected); } ); @@ -67,16 +51,16 @@ describe('millisecondsToClosestInterval()', () => { ])( 'clamps to the largest valid interval for values above the maximum (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, TEST_INTERVALS)).toBe(expected); + expect(closestIntervalToDuration(ms, TEST_INTERVALS)).toBe(expected); } ); it.each([ - [0, undefined], - [-60_000, undefined], - [Infinity, undefined], - ])('returns undefined for invalid inputs (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, TEST_INTERVALS)).toBe(expected); + [0, '1m'], + [-60_000, '1m'], + [Infinity, '1d'], + ])('returns an interval for out-of-range inputs (%s)', (ms, expected) => { + expect(closestIntervalToDuration(ms, TEST_INTERVALS)).toBe(expected); }); describe('less availableIntervals option', () => { @@ -87,16 +71,16 @@ describe('millisecondsToClosestInterval()', () => { [90_000, '1m'], // 3m is equidistant between 1m and 5m — ties go to larger [3 * 60_000, '5m'], - // exact match still works + // Exact match still works [5 * 60_000, '5m'], ])('restricts selection to the provided available intervals (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, availableIntervals)).toBe(expected); + expect(closestIntervalToDuration(ms, availableIntervals)).toBe(expected); }); it.each([[1_000, '1m']])( 'clamps to the first available interval for values below the minimum (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, availableIntervals)).toBe(expected); + expect(closestIntervalToDuration(ms, availableIntervals)).toBe(expected); } ); @@ -106,8 +90,24 @@ describe('millisecondsToClosestInterval()', () => { ])( 'clamps to the last available interval for values above the maximum (%s)', (ms, expected) => { - expect(millisecondsToClosestInterval(ms, availableIntervals)).toBe(expected); + expect(closestIntervalToDuration(ms, availableIntervals)).toBe(expected); } ); }); }); + +const TEST_INTERVALS = [ + '1m', + '2m', + '5m', + '10m', + '15m', + '30m', + '1h', + '2h', + '3h', + '4h', + '6h', + '12h', + '1d', +]; diff --git a/static/app/utils/duration/millisecondsToInterval.tsx b/static/app/utils/duration/closestIntervalToDuration.tsx similarity index 67% rename from static/app/utils/duration/millisecondsToInterval.tsx rename to static/app/utils/duration/closestIntervalToDuration.tsx index f320d0810ae0b6..eb896e0369ccb1 100644 --- a/static/app/utils/duration/millisecondsToInterval.tsx +++ b/static/app/utils/duration/closestIntervalToDuration.tsx @@ -1,3 +1,4 @@ +import {isNonEmptyArray} from 'sentry/utils/array/isNonEmptyArray'; import {intervalToMilliseconds} from 'sentry/utils/duration/intervalToMilliseconds'; import {RangeMap, type Range} from 'sentry/utils/number/rangeMap'; @@ -5,24 +6,39 @@ import {RangeMap, type Range} from 'sentry/utils/number/rangeMap'; * Converts a millisecond value to the closest valid interval string. * If the milliseconds value is not one of the exact valid interval durations, * it will return the closest valid interval string (based on rounding rules). - * @param ms - The milliseconds value to convert. - * @param availableIntervals - Array of available interval strings (e.g. '1m', '5m', '1h') to choose from. - * @returns The closest valid interval string. */ -export function millisecondsToClosestInterval( - ms: number, +export function closestIntervalToDuration( + duration: number, availableIntervals: string[] -): string | undefined { - if (ms <= 0 || !Number.isFinite(ms)) { - return undefined; +): string | null { + if (!isNonEmptyArray(availableIntervals)) { + return null; } - // sort the intervals in ascending order in case they are not in order already const sortedIntervals = availableIntervals.sort( (a, b) => intervalToMilliseconds(a) - intervalToMilliseconds(b) ); - // calculate the MIDPOINT value ranges to allow the interval to be chosen. + const shortestIntervalDuration = intervalToMilliseconds(sortedIntervals.at(0)!); + if (duration <= shortestIntervalDuration) { + // TypeScript correctly unpacks the tuple syntax here, so it knows that + // `[0]` must be defined. `.at(0)` doesn't have that benefit + return sortedIntervals[0]; + } + + const longestIntervalDuration = intervalToMilliseconds(sortedIntervals.at(-1)!); + if (duration >= longestIntervalDuration) { + // Due to how `noUncheckedIndexedAccess` works, TypeScript here doesn't know + // that the last element _also_ must exist. The non-null assertion is not + // avoidable + return sortedIntervals.at(-1)!; + } + + if (!Number.isFinite(duration)) { + return null; + } + + // Calculate the MIDPOINT value ranges to allow the interval to be chosen. // For example if the available intervals are [1m, 5m, 1h, 4h, 6h, 1d], the valid interval range // boundaries would be the numbers exactly in between the intervals. // so for example: @@ -33,8 +49,10 @@ export function millisecondsToClosestInterval( // - anything from 5h -> 12h would give the 6h interval, // - anything from 12h -> Infinity would give the 1d interval, const intervalRanges: Array> = []; + for (let i = 0; i < sortedIntervals.length; i++) { const range: Range = {min: 0, max: 0, value: sortedIntervals[i]!}; + if (i < sortedIntervals.length - 1) { // min value should cover end of the previous interval (or 0 if there is no previous interval) if (i === 0) { @@ -42,7 +60,7 @@ export function millisecondsToClosestInterval( } else { range.min = intervalRanges[i - 1]!.max; } - // max value should cover up until the value that is considered "closest" to the interval. + // Max value should cover up until the value that is considered "closest" to the interval. // Any value up to halfway between the current and next interval would take the current interval. const halfIntervalDifference = Math.round( Math.abs( @@ -53,7 +71,7 @@ export function millisecondsToClosestInterval( range.max = intervalToMilliseconds(sortedIntervals[i]!) + halfIntervalDifference; intervalRanges.push(range); } else if (sortedIntervals.length > 1) { - // last interval should cover all values close to and greater than the last interval + // Last interval should cover all values close to and greater than the last interval range.min = intervalRanges[i - 1]?.max ?? 0; range.max = Infinity; intervalRanges.push(range); @@ -65,6 +83,7 @@ export function millisecondsToClosestInterval( } const intervalRangeMap = new RangeMap(intervalRanges ?? []); - const closestInterval = intervalRangeMap.get(ms); + const closestInterval = intervalRangeMap.get(duration)!; + return closestInterval; } diff --git a/static/app/views/dashboards/widgets/heatMapWidget/settings.ts b/static/app/views/dashboards/widgets/heatMapWidget/settings.ts index ced6b27a8f9b9e..d6f7eec3908dd8 100644 --- a/static/app/views/dashboards/widgets/heatMapWidget/settings.ts +++ b/static/app/views/dashboards/widgets/heatMapWidget/settings.ts @@ -16,11 +16,11 @@ export const HEATMAP_COLORS = [ ] as const; /** - * Target width, in pixels, of a single heat map X-axis (time) bucket. The - * interval is chosen so that columns are roughly this wide for the rendered - * container width. + * Target size, in pixels, of a single heat map bucket along each axis. Both the + * X-axis (time) interval and the Y-axis bucket count are chosen so that cells + * are roughly this size, keeping them approximately square. */ -export const PIXELS_PER_X_BUCKET = 15; +export const PIXELS_PER_BUCKET = 15; /** * Scale used for the heat map's Z axis (the cell color). A logarithmic scale diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.spec.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.spec.tsx new file mode 100644 index 00000000000000..88e6a63bef53ba --- /dev/null +++ b/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.spec.tsx @@ -0,0 +1,386 @@ +import {PageFiltersFixture} from 'sentry-fixture/pageFilters'; + +import {PIXELS_PER_BUCKET} from 'sentry/views/dashboards/widgets/heatMapWidget/settings'; +import {calculateHeatMapBucketDimensions} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions'; + +const AVAILABLE_INTERVALS = [ + '1m', + '5m', + '15m', + '30m', + '1h', + '2h', + '4h', + '6h', + '12h', + '1d', +]; + +function makeSelection(period: string) { + return PageFiltersFixture({ + datetime: {period, start: null, end: null, utc: null}, + }); +} + +describe('calculateHeatMapBucketDimensions()', () => { + describe('null guard rails', () => { + it('returns null when width is 0', () => { + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 0, height: 300}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('returns null when height is 0', () => { + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 0}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('returns null when width is negative', () => { + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: -100, height: 300}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('returns null when height is negative', () => { + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: -50}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('returns null when availableIntervals is empty', () => { + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + [] + ) + ).toBeNull(); + }); + + it('returns null when the chosen interval is sub-pixel', () => { + // A 1m interval over a 90d range on a narrow chart rounds to 0px wide, + // which would cause a division-by-zero when computing yBuckets. + expect( + calculateHeatMapBucketDimensions( + makeSelection('90d'), + {width: 100, height: 300}, + ['1m'] + ) + ).toBeNull(); + }); + }); + + describe('return shape', () => { + it('returns an object with `interval` and `yBuckets`', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + expect(result).toEqual( + expect.objectContaining({ + interval: expect.any(String), + yBuckets: expect.any(Number), + }) + ); + }); + + it('always returns an interval from the provided list', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + expect(AVAILABLE_INTERVALS).toContain(result?.interval); + }); + + it('returns a positive yBuckets count', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + expect(result?.yBuckets).toBeGreaterThan(0); + }); + }); + + describe('interval selection', () => { + it('picks a finer interval for a wider container', () => { + const wide = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 1200, height: 300}, + AVAILABLE_INTERVALS + ); + const narrow = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 300, height: 300}, + AVAILABLE_INTERVALS + ); + + // A wider container means more columns, so each bucket spans less time. + // The interval index in the sorted list should be <= for the wider case. + const wideIdx = AVAILABLE_INTERVALS.indexOf(wide!.interval); + const narrowIdx = AVAILABLE_INTERVALS.indexOf(narrow!.interval); + expect(wideIdx).toBeLessThanOrEqual(narrowIdx); + }); + + it('picks a coarser interval for a longer time range at the same width', () => { + const short = calculateHeatMapBucketDimensions( + makeSelection('1h'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + const long = calculateHeatMapBucketDimensions( + makeSelection('7d'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + const shortIdx = AVAILABLE_INTERVALS.indexOf(short!.interval); + const longIdx = AVAILABLE_INTERVALS.indexOf(long!.interval); + expect(longIdx).toBeGreaterThanOrEqual(shortIdx); + }); + + it('only returns intervals from the restricted set', () => { + const restricted = ['5m', '1h', '1d']; + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + restricted + ); + + expect(restricted).toContain(result?.interval); + }); + + it('returns the single available interval when only one is provided', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 300}, + ['1h'] + ); + + expect(result?.interval).toBe('1h'); + }); + }); + + describe('y-axis bucket count', () => { + it('produces more y-axis buckets for a taller container', () => { + const tall = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 600}, + AVAILABLE_INTERVALS + ); + const short = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 200}, + AVAILABLE_INTERVALS + ); + + expect(tall!.yBuckets).toBeGreaterThanOrEqual(short!.yBuckets); + }); + + it('adjusts y-axis buckets based on the chosen x-axis interval pixel size', () => { + // The y-axis bucket count is `height / intervalAsPixels`, not a fixed + // `height / PIXELS_PER_BUCKET`. Two different time ranges at the same + // dimensions can produce different y-axis counts because the chosen + // interval snaps differently. + const a = calculateHeatMapBucketDimensions( + makeSelection('1h'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + const b = calculateHeatMapBucketDimensions( + makeSelection('7d'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + // They _may_ be different — the point is that yBuckets is dynamic, not + // a simple division by PIXELS_PER_BUCKET. + expect(a!.yBuckets).toBeGreaterThan(0); + expect(b!.yBuckets).toBeGreaterThan(0); + }); + }); + + describe('square cell goal', () => { + it('produces roughly square cells for a typical dashboard layout', () => { + // For a 24h window in a 720×360 container, the function should aim to + // make cells where the pixel width ≈ pixel height. + const width = 720; + const height = 360; + + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width, height}, + AVAILABLE_INTERVALS + )!; + + // Reconstruct the pixel dimensions of a single cell + const timeRangeMs = 24 * 60 * 60 * 1000; + const intervalMs = parseIntervalToMs(result.interval); + const cellPixelWidth = (intervalMs / timeRangeMs) * width; + const cellPixelHeight = height / result.yBuckets; + + // The cells should be approximately square. Allow a generous ratio + // since we're snapping to discrete intervals. + const aspectRatio = cellPixelWidth / cellPixelHeight; + expect(aspectRatio).toBeGreaterThan(0.2); + expect(aspectRatio).toBeLessThan(5); + }); + + it('targets PIXELS_PER_BUCKET as the ideal cell size', () => { + // The ideal (pre-snap) X-axis bucket width in pixels is PIXELS_PER_BUCKET. + // After snapping to a real interval, the actual pixel width may differ, + // but the y-axis bucket count is derived from that snapped size. + const width = 900; + const height = 300; + + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width, height}, + AVAILABLE_INTERVALS + )!; + + const timeRangeMs = 24 * 60 * 60 * 1000; + const intervalMs = parseIntervalToMs(result.interval); + const actualPixelWidth = (intervalMs / timeRangeMs) * width; + + // The snapped interval pixel size should be in the same order of + // magnitude as PIXELS_PER_BUCKET. + expect(actualPixelWidth).toBeGreaterThan(PIXELS_PER_BUCKET * 0.25); + expect(actualPixelWidth).toBeLessThan(PIXELS_PER_BUCKET * 10); + }); + }); + + describe('edge cases', () => { + it('handles a very narrow container gracefully', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 1, height: 300}, + AVAILABLE_INTERVALS + ); + + // Should still return a valid result (likely the coarsest interval) + expect(result).not.toBeNull(); + expect(result!.yBuckets).toBeGreaterThan(0); + }); + + it('returns null for a very short container', () => { + // A 1px tall container produces yBuckets = 0, which is not a + // meaningful heat map. + expect( + calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 1}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('produces at least 1 y-axis bucket when there is reasonable height', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('24h'), + {width: 800, height: 30}, + AVAILABLE_INTERVALS + ); + + expect(result).not.toBeNull(); + expect(result!.yBuckets).toBeGreaterThanOrEqual(1); + }); + + it('returns null for a very short time period where no grid is viable', () => { + // With a 1m window the finest interval (1m) spans the full width, + // so yBuckets rounds to 0 — no meaningful grid can be formed. + expect( + calculateHeatMapBucketDimensions( + makeSelection('1m'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ) + ).toBeNull(); + }); + + it('returns null when the interval spans the full chart width', () => { + // 1m period + 1m interval → intervalAsPixels equals the full width. + // height / width rounds to 0, so no sensible grid can be formed. + expect( + calculateHeatMapBucketDimensions(makeSelection('1m'), {width: 800, height: 300}, [ + '1m', + ]) + ).toBeNull(); + }); + + it('handles a very long time period (90d)', () => { + const result = calculateHeatMapBucketDimensions( + makeSelection('90d'), + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + // The coarsest interval should be selected for such a long window. + expect(result).not.toBeNull(); + expect(result!.interval).toBe('1d'); + }); + + it('handles absolute date ranges', () => { + const selection = PageFiltersFixture({ + datetime: { + period: null, + start: '2024-01-01T00:00:00', + end: '2024-01-02T00:00:00', + utc: true, + }, + }); + + const result = calculateHeatMapBucketDimensions( + selection, + {width: 800, height: 300}, + AVAILABLE_INTERVALS + ); + + expect(result).not.toBeNull(); + expect(AVAILABLE_INTERVALS).toContain(result?.interval); + expect(result?.yBuckets).toBeGreaterThan(0); + }); + }); +}); + +/** + * Helper: Convert an interval string like '5m' or '1h' to milliseconds for + * assertions. This mirrors `intervalToMilliseconds` but is duplicated here to + * keep the test self-contained. + */ +function parseIntervalToMs(interval: string): number { + const match = /^(\d+)([mhdw])$/.exec(interval); + if (!match) { + throw new Error(`Unparseable interval: ${interval}`); + } + const value = parseInt(match[1]!, 10); + const multipliers: Record = { + m: 60 * 1000, + h: 60 * 60 * 1000, + d: 24 * 60 * 60 * 1000, + w: 7 * 24 * 60 * 60 * 1000, + }; + return value * multipliers[match[2]!]!; +} diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.tsx new file mode 100644 index 00000000000000..415ecbbd34ef10 --- /dev/null +++ b/static/app/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions.tsx @@ -0,0 +1,76 @@ +import {getDiffInMinutes} from 'sentry/components/charts/utils'; +import type {PageFilters} from 'sentry/types/core'; +import {defined} from 'sentry/utils/defined'; +import {closestIntervalToDuration} from 'sentry/utils/duration/closestIntervalToDuration'; +import {intervalToMilliseconds} from 'sentry/utils/duration/intervalToMilliseconds'; +import {PIXELS_PER_BUCKET} from 'sentry/views/dashboards/widgets/heatMapWidget/settings'; + +/** + * Calculates the bucket dimensions for a given heat map. The bucket selection + * depends on the dimensions of the chart, the selected time range, and the + * available intervals. The dimensions of the chart depend on the layout, the + * available intervals depend on the current UI (e.g., Explore might have + * different available intervals from another UI), and the current selected time + * range. We aim to make square heat map cells. We first calculate which of the + * available intervals is most closely matches the desired pixel width. Then we + * calculate the exact number of Y-axis buckets that will make square cells. The + * output can be fed directly to the heat map API. + */ +export function calculateHeatMapBucketDimensions( + selection: PageFilters, + dimensions: CartesianDimensions, + availableIntervals: string[] +): BucketDimensions | null { + if ( + dimensions.width <= 0 || + dimensions.height <= 0 || + availableIntervals.length === 0 + ) { + return null; + } + + const timeRangeAsMilliseconds = getDiffInMinutes(selection.datetime) * 60 * 1000; + + const bucketWidthAsMilliseconds = Math.round( + timeRangeAsMilliseconds / (dimensions.width / PIXELS_PER_BUCKET) + ); + + const interval = closestIntervalToDuration( + bucketWidthAsMilliseconds, + availableIntervals + ); + + if (!defined(interval)) { + return null; + } + + const intervalAsMilliseconds = intervalToMilliseconds(interval); + const intervalAsPixels = Math.round( + (intervalAsMilliseconds / timeRangeAsMilliseconds) * dimensions.width + ); + + if (intervalAsPixels <= 0) { + return null; + } + + const yBuckets = Math.round(dimensions.height / intervalAsPixels); + + if (yBuckets <= 0) { + return null; + } + + return { + interval, + yBuckets, + }; +} + +type BucketDimensions = { + interval: string; + yBuckets: number; +}; + +type CartesianDimensions = { + height: number; + width: number; +}; diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.spec.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.spec.tsx deleted file mode 100644 index 7237634b137512..00000000000000 --- a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.spec.tsx +++ /dev/null @@ -1,53 +0,0 @@ -import {PageFiltersFixture} from 'sentry-fixture/pageFilters'; - -import {getHeatmapXAxisBucketInterval} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval'; - -function makeSelection(period: string) { - return PageFiltersFixture({datetime: {period, start: null, end: null, utc: null}}); -} - -const INTERVAL_OPTIONS = [ - {value: '1m', label: '1 minute'}, - {value: '5m', label: '5 minutes'}, - {value: '1h', label: '1 hour'}, - {value: '12h', label: '12 hours'}, - {value: '1d', label: '1 day'}, -]; - -describe('getHeatmapXAxisBucketInterval()', () => { - it('snaps to a wider interval as the container gets narrower', () => { - const values = INTERVAL_OPTIONS.map(option => option.value); - const wide = getHeatmapXAxisBucketInterval( - makeSelection('24h'), - '1h', - 1200, - INTERVAL_OPTIONS - ); - const narrow = getHeatmapXAxisBucketInterval( - makeSelection('24h'), - '1h', - 200, - INTERVAL_OPTIONS - ); - // A narrower container fits fewer columns, so each bucket spans more time. - expect(values.indexOf(narrow)).toBeGreaterThanOrEqual(values.indexOf(wide)); - }); - - it('only returns one of the provided interval options', () => { - const result = getHeatmapXAxisBucketInterval( - makeSelection('24h'), - '1h', - 724, - INTERVAL_OPTIONS - ); - expect(INTERVAL_OPTIONS.map(option => option.value)).toContain(result); - }); - - it('falls back to the given interval before the container has been measured', () => { - // Width 0 yields no finite bucket size, so there is nothing to snap to and - // the currently selected interval is kept. - expect( - getHeatmapXAxisBucketInterval(makeSelection('24h'), '1h', 0, INTERVAL_OPTIONS) - ).toBe('1h'); - }); -}); diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.tsx deleted file mode 100644 index 1083fc94b47697..00000000000000 --- a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import {getDiffInMinutes} from 'sentry/components/charts/utils'; -import type {PageFilters} from 'sentry/types/core'; -import {millisecondsToClosestInterval} from 'sentry/utils/duration/millisecondsToInterval'; -import {PIXELS_PER_X_BUCKET} from 'sentry/views/dashboards/widgets/heatMapWidget/settings'; - -/** - * Computes the X-axis bucket interval for the heatmap API. - * The X-axis bucket interval is derived from the container width and the number of - * pixels per X bucket. - */ -export function getHeatmapXAxisBucketInterval( - selection: PageFilters, - interval: string, - chartContainerWidth: number, - intervalOptions: Array<{label: string; value: string}> -): string { - const timeRangeInMs = getDiffInMinutes(selection.datetime) * 60 * 1000; - const msPerXBucket = Math.round( - timeRangeInMs / (chartContainerWidth / PIXELS_PER_X_BUCKET) - ); - const xBucketInterval = millisecondsToClosestInterval( - msPerXBucket, - intervalOptions.map(option => option.value) - ); - return xBucketInterval || interval; -} diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.spec.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.spec.tsx deleted file mode 100644 index 20383577b5c890..00000000000000 --- a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.spec.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import {PageFiltersFixture} from 'sentry-fixture/pageFilters'; - -import {getHeatmapYAxisBucketCount} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount'; - -function makeSelection(period: string) { - return PageFiltersFixture({datetime: {period, start: null, end: null, utc: null}}); -} - -describe('getHeatmapYAxisBucketCount()', () => { - it('returns 0 before the container has been measured', () => { - expect(getHeatmapYAxisBucketCount(makeSelection('24h'), '1h', 0)).toBe(0); - }); - - it('returns 0 for a non-positive interval', () => { - expect(getHeatmapYAxisBucketCount(makeSelection('24h'), '0', 800)).toBe(0); - }); - - it('returns a positive bucket count once the container is measured', () => { - expect( - getHeatmapYAxisBucketCount(makeSelection('24h'), '1h', 800) - ).toBeGreaterThanOrEqual(1); - }); - - it('fits fewer Y buckets into a wider container', () => { - const narrow = getHeatmapYAxisBucketCount(makeSelection('24h'), '1h', 400); - const wide = getHeatmapYAxisBucketCount(makeSelection('24h'), '1h', 1600); - expect(wide).toBeLessThanOrEqual(narrow); - }); -}); diff --git a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.tsx b/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.tsx deleted file mode 100644 index 72f21e7e51872a..00000000000000 --- a/static/app/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import {getDiffInMinutes} from 'sentry/components/charts/utils'; -import type {PageFilters} from 'sentry/types/core'; -import {intervalToMilliseconds} from 'sentry/utils/duration/intervalToMilliseconds'; -import {STACKED_GRAPH_HEIGHT} from 'sentry/views/explore/metrics/settings'; - -/** - * Computes the number of Y-axis buckets for the heatmap API so that cells - * are roughly square. The X-axis bucket count comes from the time range - * divided by the selected interval. We derive Y buckets by scaling - * xBuckets by the container's height/width aspect ratio. - */ -export function getHeatmapYAxisBucketCount( - selection: PageFilters, - interval: string, - chartContainerWidth: number -): number { - const timeRangeInMs = getDiffInMinutes(selection.datetime) * 60 * 1000; - const intervalInMs = intervalToMilliseconds(interval); - if (intervalInMs <= 0 || chartContainerWidth <= 0) { - return 0; - } - const xBuckets = Math.round(timeRangeInMs / intervalInMs); - if (xBuckets <= 0) { - return 0; - } - - return Math.max(1, Math.round(xBuckets * (STACKED_GRAPH_HEIGHT / chartContainerWidth))); -} diff --git a/static/app/views/explore/metrics/hooks/metricHeatmapApiOptions.tsx b/static/app/views/explore/metrics/hooks/metricHeatmapApiOptions.tsx index 52e70b3b088fe4..6d6e3fe9ac6c85 100644 --- a/static/app/views/explore/metrics/hooks/metricHeatmapApiOptions.tsx +++ b/static/app/views/explore/metrics/hooks/metricHeatmapApiOptions.tsx @@ -13,34 +13,37 @@ import {createTraceMetricEventsFilter} from 'sentry/views/explore/metrics/utils' interface MetricHeatmapApiOptions { enabled: boolean; - interval: string; organization: Organization; query: string; selection: PageFilters; traceMetric: TraceMetric; - yBuckets: number; + interval?: string | null; + yBuckets?: number | null; } export function metricHeatmapApiOptions({ - traceMetric, - enabled, organization, selection, + traceMetric, query, + enabled, interval, yBuckets, }: MetricHeatmapApiOptions) { const traceMetricFilter = createTraceMetricEventsFilter([traceMetric]); const combinedQuery = query ? `${traceMetricFilter} (${query})` : traceMetricFilter; - const intervalInMilliseconds = intervalToMilliseconds(interval); + const intervalInMilliseconds = defined(interval) ? intervalToMilliseconds(interval) : 0; + const valid = + defined(interval) && defined(yBuckets) && yBuckets > 0 && intervalInMilliseconds > 0; + const {start, end, statsPeriod} = normalizeDateTimeParams(selection.datetime); const usesRelativeDateRange = !defined(start) && !defined(end) && defined(statsPeriod); return apiOptions.as()( '/organizations/$organizationIdOrSlug/events-heatmap/', { - path: enabled ? {organizationIdOrSlug: organization.slug} : skipToken, + path: !enabled || !valid ? skipToken : {organizationIdOrSlug: organization.slug}, query: { dataset: DiscoverDatasets.TRACEMETRICS, xAxis: 'time', diff --git a/static/app/views/explore/metrics/metricPanel/index.tsx b/static/app/views/explore/metrics/metricPanel/index.tsx index 7db1286be8a73e..8fd3f49cf457c8 100644 --- a/static/app/views/explore/metrics/metricPanel/index.tsx +++ b/static/app/views/explore/metrics/metricPanel/index.tsx @@ -20,8 +20,7 @@ import { } from 'sentry/utils/useChartInterval'; import {useDimensions} from 'sentry/utils/useDimensions'; import {useOrganization} from 'sentry/utils/useOrganization'; -import {getHeatmapXAxisBucketInterval} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/getHeatmapXAxisBucketInterval'; -import {getHeatmapYAxisBucketCount} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/getHeatmapYAxisBucketCount'; +import {calculateHeatMapBucketDimensions} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/calculateHeatMapBucketDimensions'; import {mergeMetricUnit} from 'sentry/views/dashboards/widgets/heatMapWidget/utils/mergeMetricUnit'; import {EXPLORE_FIVE_MIN_STALE_TIME} from 'sentry/views/explore/constants'; import {useMetricsPanelAnalytics} from 'sentry/views/explore/hooks/useAnalytics'; @@ -52,6 +51,7 @@ import { useSetMetricVisualizes, } from 'sentry/views/explore/metrics/metricsQueryParams'; import {MetricToolbar} from 'sentry/views/explore/metrics/metricToolbar'; +import {STACKED_GRAPH_HEIGHT} from 'sentry/views/explore/metrics/settings'; import {updateVisualizeYAxis} from 'sentry/views/explore/metrics/utils'; import { useQueryParamsAggregateSortBys, @@ -122,11 +122,8 @@ export function MetricPanel({ const isHeatmap = visualize.chartType === ChartType.HEATMAP; - // use the biggest interval for the heat map as this produces better patterns const [interval, setInterval, intervalOptions] = useChartInterval({ - unspecifiedStrategy: isHeatmap - ? ChartIntervalUnspecifiedStrategy.USE_BIGGEST - : ChartIntervalUnspecifiedStrategy.USE_SMALLEST, + unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SMALLEST, }); const [title, setTitle] = useState(() => { @@ -158,39 +155,39 @@ export function MetricPanel({ staleTime: Infinity, }); - const hasHeatMap = canUseMetricsHeatMap(organization); + const areHeatMapsEnabled = canUseMetricsHeatMap(organization); const {result: timeseriesResult} = useMetricTimeseries({ traceMetric, enabled: - !(hasHeatMap && isHeatmap) && + !(areHeatMapsEnabled && isHeatmap) && (!isMetricOptionsEmpty || (isVisualizeEquation(visualize) && Boolean(visualize.expression.text))), }); + const contentHeightRef = useRef(null); const chartContainerRef = useRef(null); const {width: chartContainerWidth} = useDimensions({elementRef: chartContainerRef}); - const xBucketInterval = getHeatmapXAxisBucketInterval( - selection, - interval, - chartContainerWidth, - intervalOptions - ); - const yBuckets = getHeatmapYAxisBucketCount( + + const heatMapBucketDimensions = calculateHeatMapBucketDimensions( selection, - xBucketInterval, - chartContainerWidth + { + width: chartContainerWidth, + height: STACKED_GRAPH_HEIGHT, + }, + intervalOptions.map(intervalOption => intervalOption.value) ); const heatmapApiOptions = metricHeatmapApiOptions({ - traceMetric, - enabled: hasHeatMap && isHeatmap && !isMetricOptionsEmpty && yBuckets > 0, organization, selection, + traceMetric, query: userQuery, - interval: xBucketInterval, - yBuckets, + interval: heatMapBucketDimensions?.interval, + yBuckets: heatMapBucketDimensions?.yBuckets, + enabled: areHeatMapsEnabled && isHeatmap && !isMetricOptionsEmpty, }); + const heatmapResult = useQuery({ ...heatmapApiOptions, select: data => { @@ -263,7 +260,7 @@ export function MetricPanel({ onChange={option => handleChartTypeChange(option.value)} /> setInterval(value)} trigger={triggerProps => ( @@ -284,8 +281,6 @@ export function MetricPanel({ ); - const contentHeightRef = useRef(null); - return ( @@ -320,7 +315,7 @@ export function MetricPanel({ > - {hasHeatMap && isHeatmap ? ( + {areHeatMapsEnabled && isHeatmap ? (