Skip to content

Commit 296a2c9

Browse files
authored
ref(profiling) store unit on profile chart (#74152)
Since continuous profiles are in ms, we need to ensure the duration check is correctly computed based off the profile chart units
1 parent fd2c7c7 commit 296a2c9

File tree

5 files changed

+37
-5
lines changed

5 files changed

+37
-5
lines changed

static/app/components/profiling/flamegraph/continuousFlamegraph.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,7 @@ export function ContinuousFlamegraph(): ReactElement {
10801080
batteryChart={
10811081
hasBatteryChart ? (
10821082
<FlamegraphChart
1083+
configViewUnit={flamegraph.profile.unit}
10831084
status={profiles.type}
10841085
chartCanvasRef={batteryChartCanvasRef}
10851086
chartCanvas={batteryChartCanvas}
@@ -1101,6 +1102,7 @@ export function ContinuousFlamegraph(): ReactElement {
11011102
memoryChart={
11021103
hasMemoryChart ? (
11031104
<FlamegraphChart
1105+
configViewUnit={flamegraph.profile.unit}
11041106
status={profiles.type}
11051107
chartCanvasRef={memoryChartCanvasRef}
11061108
chartCanvas={memoryChartCanvas}
@@ -1126,6 +1128,7 @@ export function ContinuousFlamegraph(): ReactElement {
11261128
cpuChart={
11271129
hasCPUChart ? (
11281130
<FlamegraphChart
1131+
configViewUnit={flamegraph.profile.unit}
11291132
status={profiles.type}
11301133
chartCanvasRef={cpuChartCanvasRef}
11311134
chartCanvas={cpuChartCanvas}

static/app/components/profiling/flamegraph/flamegraph.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,7 @@ function Flamegraph(): ReactElement {
13851385
batteryChart={
13861386
hasBatteryChart ? (
13871387
<FlamegraphChart
1388+
configViewUnit={flamegraph.profile.unit}
13881389
status={profiles.type}
13891390
chartCanvasRef={batteryChartCanvasRef}
13901391
chartCanvas={batteryChartCanvas}
@@ -1406,6 +1407,7 @@ function Flamegraph(): ReactElement {
14061407
memoryChart={
14071408
hasMemoryChart ? (
14081409
<FlamegraphChart
1410+
configViewUnit={flamegraph.profile.unit}
14091411
status={profiles.type}
14101412
chartCanvasRef={memoryChartCanvasRef}
14111413
chartCanvas={memoryChartCanvas}
@@ -1431,6 +1433,7 @@ function Flamegraph(): ReactElement {
14311433
cpuChart={
14321434
hasCPUChart ? (
14331435
<FlamegraphChart
1436+
configViewUnit={flamegraph.profile.unit}
14341437
status={profiles.type}
14351438
chartCanvasRef={cpuChartCanvasRef}
14361439
chartCanvas={cpuChartCanvas}

static/app/components/profiling/flamegraph/flamegraphChart.tsx

+12-3
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import {
1616
getPhysicalSpacePositionFromOffset,
1717
transformMatrixBetweenRect,
1818
} from 'sentry/utils/profiling/gl/utils';
19+
import type {Profile} from 'sentry/utils/profiling/profile/profile';
1920
import {FlamegraphChartRenderer} from 'sentry/utils/profiling/renderers/chartRenderer';
2021
import type {Rect} from 'sentry/utils/profiling/speedscope';
22+
import {formatTo} from 'sentry/utils/profiling/units/units';
2123

2224
import {useCanvasScroll} from './interactions/useCanvasScroll';
2325
import {useCanvasZoomOrScroll} from './interactions/useCanvasZoomOrScroll';
@@ -36,6 +38,7 @@ interface FlamegraphChartProps {
3638
chartCanvas: FlamegraphCanvas | null;
3739
chartCanvasRef: HTMLCanvasElement | null;
3840
chartView: CanvasView<FlamegraphChartModel> | null;
41+
configViewUnit: Profile['unit'];
3942
noMeasurementMessage: string | undefined;
4043
setChartCanvasRef: (ref: HTMLCanvasElement | null) => void;
4144
status: RequestState<any>['type'];
@@ -51,9 +54,10 @@ export function FlamegraphChart({
5154
setChartCanvasRef,
5255
canvasBounds,
5356
noMeasurementMessage,
57+
configViewUnit,
5458
}: FlamegraphChartProps) {
55-
const scheduler = useCanvasScheduler(canvasPoolManager);
5659
const theme = useFlamegraphTheme();
60+
const scheduler = useCanvasScheduler(canvasPoolManager);
5761

5862
const [configSpaceCursor, setConfigSpaceCursor] = useState<vec2 | null>(null);
5963
const [startInteractionVector, setStartInteractionVector] = useState<vec2 | null>(null);
@@ -262,15 +266,20 @@ export function FlamegraphChart({
262266
[configSpaceCursor, chartView]
263267
);
264268

269+
const isInsufficientDuration = useMemo(() => {
270+
if (!chart) return false;
271+
return formatTo(chart?.configSpace.width, configViewUnit, 'millisecond') < 200;
272+
}, [chart, configViewUnit]);
273+
265274
let message: string | undefined;
266275

267276
if (chart) {
268277
if (
269-
chart.configSpace.width < 200 * 1e6 &&
278+
isInsufficientDuration &&
270279
(chart.status === 'insufficient data' || chart.status === 'empty metrics')
271280
) {
272281
message = t('Profile duration was too short to collect enough metrics');
273-
} else if (chart.configSpace.width >= 200 && chart.status === 'insufficient data') {
282+
} else if (!isInsufficientDuration && chart.status === 'insufficient data') {
274283
message =
275284
noMeasurementMessage ||
276285
t(

static/app/utils/profiling/flamegraphChart.tsx

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import type {ColorChannels} from 'sentry/utils/profiling/flamegraph/flamegraphTheme';
22
import {Rect} from 'sentry/utils/profiling/speedscope';
3+
import type {ProfilingFormatterUnit} from 'sentry/utils/profiling/units/units';
34

45
import {colorComponentsToRGBA} from './colors/utils';
5-
import {makeFormatter, makeTimelineFormatter} from './units/units';
6+
import {
7+
assertValidProfilingUnit,
8+
makeFormatter,
9+
makeTimelineFormatter,
10+
} from './units/units';
611

712
interface Series {
813
fillColor: string;
@@ -35,6 +40,7 @@ interface ChartOptions {
3540

3641
export class FlamegraphChart {
3742
configSpace: Rect;
43+
unit: ProfilingFormatterUnit;
3844
formatter: ReturnType<typeof makeFormatter>;
3945
tooltipFormatter: ReturnType<typeof makeFormatter>;
4046
timelineFormatter: (value: number) => string;
@@ -63,6 +69,7 @@ export class FlamegraphChart {
6369
if (!measurements || !measurements.length) {
6470
this.formatter = makeFormatter('percent');
6571
this.tooltipFormatter = makeFormatter('percent');
72+
this.unit = 'percent';
6673
this.configSpace = configSpace.clone();
6774
this.status = !measurements ? 'no metrics' : 'empty metrics';
6875
return;
@@ -128,6 +135,9 @@ export class FlamegraphChart {
128135
this.domains.y[1] = this.domains.y[1] + this.domains.y[1] * 0.1;
129136
this.configSpace = configSpace.withHeight(this.domains.y[1] - this.domains.y[0]);
130137

138+
assertValidProfilingUnit(measurements[0].unit);
139+
this.unit = measurements[0].unit;
140+
131141
this.formatter = makeFormatter(
132142
measurements[0].unit,
133143
computeLabelPrecision(this.domains.y[0], this.domains.y[1])

static/app/utils/profiling/units/units.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ const durationMappings: Record<ProfilingFormatterUnit, number> = {
4545
watts: 1,
4646
};
4747

48+
export function assertValidProfilingUnit(
49+
unit: string
50+
): asserts unit is ProfilingFormatterUnit {
51+
if (unit in durationMappings) return;
52+
throw new Error(`Invalid profiling unit: ${unit}`);
53+
}
54+
4855
export function fromNanoJoulesToWatts(nanojoules: number, seconds: number) {
4956
const joules = nanojoules * durationMappings.nanojoules;
5057
return joules / seconds;
@@ -155,7 +162,7 @@ export function makeFormatter(
155162
};
156163
}
157164

158-
return (value: number) => {
165+
return function formatToDuration(value: number): string {
159166
const duration = value * multiplier;
160167

161168
if (duration >= 1) {

0 commit comments

Comments
 (0)