Skip to content

Commit

Permalink
ref(ddm): Remove normalization logic (#66132)
Browse files Browse the repository at this point in the history
Remove normalization logic from FE.

- closes #65765
  • Loading branch information
ArthurKnaus committed Mar 5, 2024
1 parent 1c5a430 commit 59ee6b3
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 275 deletions.
66 changes: 0 additions & 66 deletions static/app/utils/metrics/normalizeMetricValue.spec.tsx

This file was deleted.

109 changes: 0 additions & 109 deletions static/app/utils/metrics/normalizeMetricValue.tsx

This file was deleted.

27 changes: 9 additions & 18 deletions static/app/views/ddm/chart/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {isChartHovered} from 'sentry/components/charts/utils';
import {t} from 'sentry/locale';
import type {ReactEchartsRef} from 'sentry/types/echarts';
import mergeRefs from 'sentry/utils/mergeRefs';
import {formatMetricsUsingUnitAndOp} from 'sentry/utils/metrics/formatters';
import {formatMetricUsingUnit} from 'sentry/utils/metrics/formatters';
import {MetricDisplayType} from 'sentry/utils/metrics/types';
import type {CombinedMetricChartProps, Series} from 'sentry/views/ddm/chart/types';
import type {UseFocusAreaResult} from 'sentry/views/ddm/chart/useFocusArea';
Expand Down Expand Up @@ -70,9 +70,7 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(
({series, displayType, height, group, samples, focusArea}, forwardedRef) => {
const chartRef = useRef<ReactEchartsRef>(null);

const firstUnit = series.find(s => !s.hidden)?.unit || series[0]?.unit || 'none';
const firstOperation =
series.find(s => !s.hidden)?.operation || series[0]?.operation || '';
const firstUnit = series.find(s => !s.hidden)?.unit || 'none';

useEffect(() => {
if (!group) {
Expand Down Expand Up @@ -112,23 +110,18 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(

const chartProps = useMemo(() => {
const hasMultipleUnits = new Set(seriesToShow.map(s => s.unit)).size > 1;
const seriesMeta = seriesToShow.reduce(
const seriesUnits = seriesToShow.reduce(
(acc, s) => {
acc[s.seriesName] = {
unit: s.unit,
operation: s.operation,
};
acc[s.seriesName] = s.unit;
return acc;
},
{} as Record<string, {operation: string; unit: string}>
{} as Record<string, string>
);

const timeseriesFormatters = {
valueFormatter: (value: number, seriesName?: string) => {
const meta = seriesName
? seriesMeta[seriesName]
: {unit: firstUnit, operation: undefined};
return formatMetricsUsingUnitAndOp(value, meta.unit, meta.operation);
const unit = (seriesName && seriesUnits[seriesName]) ?? 'none';
return formatMetricUsingUnit(value, unit);
},
isGroupedByDate: true,
bucketSize,
Expand Down Expand Up @@ -223,10 +216,9 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(
id: MAIN_Y_AXIS_ID,
axisLabel: {
formatter: (value: number) => {
return formatMetricsUsingUnitAndOp(
return formatMetricUsingUnit(
value,
hasMultipleUnits ? 'none' : firstUnit,
firstOperation
hasMultipleUnits ? 'none' : firstUnit
);
},
},
Expand Down Expand Up @@ -262,7 +254,6 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(
samples,
focusArea,
firstUnit,
firstOperation,
]);

return (
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/ddm/chart/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ export type Series = {
color: string;
data: {name: number; value: number}[];
id: string;
operation: string;
seriesName: string;
unit: string;
groupBy?: Record<string, string>;
hidden?: boolean;
paddingIndices?: Set<number>;
release?: string;
scalingFactor?: number;
transaction?: string;
};

Expand Down
36 changes: 14 additions & 22 deletions static/app/views/ddm/chart/useFocusArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {space} from 'sentry/styles/space';
import type {DateString} from 'sentry/types';
import type {EChartBrushEndHandler, ReactEchartsRef} from 'sentry/types/echarts';
import mergeRefs from 'sentry/utils/mergeRefs';
import {getMetricConversionFunction} from 'sentry/utils/metrics/normalizeMetricValue';
import {MAIN_X_AXIS_ID, MAIN_Y_AXIS_ID} from 'sentry/views/ddm/chart/chart';
import type {ValueRect} from 'sentry/views/ddm/chart/chartUtils';
import {getValueRect} from 'sentry/views/ddm/chart/chartUtils';
Expand Down Expand Up @@ -42,6 +41,7 @@ interface UseFocusAreaOptions {
export interface UseFocusAreaProps extends FocusAreaProps {
chartRef: RefObject<ReactEchartsRef>;
opts: UseFocusAreaOptions;
scalingFactor: number;
chartUnit?: string;
onZoom?: (range: DateTimeObject) => void;
sampleUnit?: string;
Expand All @@ -52,8 +52,7 @@ type BrushEndResult = Parameters<EChartBrushEndHandler>[0];
export function useFocusArea({
selection: selection,
opts: {widgetIndex, isDisabled, useFullYAxis},
sampleUnit = 'none',
chartUnit = 'none',
scalingFactor,
onAdd,
onDraw,
onRemove,
Expand Down Expand Up @@ -111,13 +110,11 @@ export function useFocusArea({
return;
}

const valueConverter = getMetricConversionFunction(chartUnit, sampleUnit);

const range = getSelectionRange(
brushEnd,
!!useFullYAxis,
getValueRect(chartRef),
valueConverter
scalingFactor
);
onAdd?.({
widgetIndex,
Expand All @@ -134,7 +131,7 @@ export function useFocusArea({
});
isDrawingRef.current = false;
},
[isDisabled, sampleUnit, chartUnit, useFullYAxis, chartRef, onAdd, widgetIndex]
[isDisabled, useFullYAxis, scalingFactor, onAdd, widgetIndex]
);

const handleRemove = useCallback(() => {
Expand Down Expand Up @@ -210,18 +207,16 @@ export function useFocusArea({
onZoom={handleZoomIn}
chartRef={chartRef}
useFullYAxis={!!useFullYAxis}
sampleUnit={sampleUnit}
chartUnit={chartUnit}
scalingFactor={scalingFactor}
/>
) : null,
}),
[
applyChartProps,
chartUnit,
handleRemove,
handleZoomIn,
hasFocusArea,
sampleUnit,
scalingFactor,
selection,
useFullYAxis,
]
Expand All @@ -232,11 +227,10 @@ export type UseFocusAreaResult = ReturnType<typeof useFocusArea>;

type FocusAreaOverlayProps = {
chartRef: RefObject<ReactEchartsRef>;
chartUnit: string;
onRemove: () => void;
onZoom: () => void;
rect: FocusAreaSelection | null;
sampleUnit: string;
scalingFactor: number;
useFullYAxis: boolean;
};

Expand All @@ -246,8 +240,7 @@ function FocusAreaOverlay({
onRemove,
useFullYAxis,
chartRef,
sampleUnit,
chartUnit,
scalingFactor,
}: FocusAreaOverlayProps) {
const [position, setPosition] = useState<AbsolutePosition | null>(null);
const wrapperRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -275,9 +268,8 @@ function FocusAreaOverlay({
}
const finder = {xAxisId: MAIN_X_AXIS_ID, yAxisId: MAIN_Y_AXIS_ID};

const valueConverter = getMetricConversionFunction(sampleUnit, chartUnit);
const max = valueConverter(rect.range.max);
const min = valueConverter(rect.range.min);
const max = rect.range.max * scalingFactor;
const min = rect.range.min * scalingFactor;

const topLeft = chartInstance.convertToPixel(finder, [
getTimestamp(rect.range.start),
Expand Down Expand Up @@ -314,7 +306,7 @@ function FocusAreaOverlay({
if (!isEqual(newPosition, position)) {
setPosition(newPosition);
}
}, [chartRef, rect, sampleUnit, chartUnit, useFullYAxis, position]);
}, [chartRef, rect, scalingFactor, useFullYAxis, position]);

useEffect(() => {
updatePosition();
Expand Down Expand Up @@ -352,7 +344,7 @@ const getSelectionRange = (
params: BrushEndResult,
useFullYAxis: boolean,
boundingRect: ValueRect,
valueConverter: (value: number) => number
scalingFactor: number
): SelectionRange => {
const rect = params.areas[0];

Expand All @@ -362,8 +354,8 @@ const getSelectionRange = (
const startDate = getDateString(Math.max(startTimestamp, boundingRect.xMin));
const endDate = getDateString(Math.min(endTimestamp, boundingRect.xMax));

const min = useFullYAxis ? NaN : valueConverter(Math.min(...rect.coordRange[1]));
const max = useFullYAxis ? NaN : valueConverter(Math.max(...rect.coordRange[1]));
const min = useFullYAxis ? NaN : Math.min(...rect.coordRange[1]) / scalingFactor;
const max = useFullYAxis ? NaN : Math.max(...rect.coordRange[1]) / scalingFactor;

return {
start: startDate,
Expand Down
Loading

0 comments on commit 59ee6b3

Please sign in to comment.