diff --git a/static/app/utils/metrics/normalizeMetricValue.spec.tsx b/static/app/utils/metrics/normalizeMetricValue.spec.tsx deleted file mode 100644 index 3c3eacfd20e508..00000000000000 --- a/static/app/utils/metrics/normalizeMetricValue.spec.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import { - getMetricValueNormalizer, - getNormalizedMetricUnit, -} from 'sentry/utils/metrics/normalizeMetricValue'; - -describe('getNormalizedMetricUnit', () => { - it('returns "millisecond" when unit is in timeConversionFactors', () => { - expect(getNormalizedMetricUnit('second')).toBe('millisecond'); - expect(getNormalizedMetricUnit('hour')).toBe('millisecond'); - expect(getNormalizedMetricUnit('hours')).toBe('millisecond'); - expect(getNormalizedMetricUnit('minute')).toBe('millisecond'); - expect(getNormalizedMetricUnit('nanoseconds')).toBe('millisecond'); - }); - - it('returns "byte" when unit is in byte10ConversionFactors', () => { - expect(getNormalizedMetricUnit('kilobyte')).toBe('byte'); - expect(getNormalizedMetricUnit('petabyte')).toBe('byte'); - expect(getNormalizedMetricUnit('petabytes')).toBe('byte'); - }); - - it('returns "byte2" when unit is in byte2ConversionFactors', () => { - expect(getNormalizedMetricUnit('bit')).toBe('byte2'); - expect(getNormalizedMetricUnit('kibibyte')).toBe('byte2'); - expect(getNormalizedMetricUnit('kibibytes')).toBe('byte2'); - }); - - it('returns the unit when it is not in any of the conversion factors', () => { - expect(getNormalizedMetricUnit('foo')).toBe('foo'); - }); - - it('returns none for count operations', () => { - expect(getNormalizedMetricUnit('second', 'count')).toBe('none'); - expect(getNormalizedMetricUnit('seconds', 'count_unique')).toBe('none'); - }); -}); - -describe('getMetricValueNormalizer', () => { - it('returns a function that normalizes the value to milliseconds when the unit is in timeConversionFactors', () => { - expect(getMetricValueNormalizer('second')(1)).toBe(1000); - expect(getMetricValueNormalizer('seconds')(2)).toBe(2000); - - expect(getMetricValueNormalizer('hour')(1)).toBe(3600000); - expect(getMetricValueNormalizer('hours')(2)).toBe(7200000); - }); - - it('returns a function that normalizes the value to bytes when the unit is in byte10ConversionFactors', () => { - expect(getMetricValueNormalizer('byte')(1)).toBe(1); - expect(getMetricValueNormalizer('bytes')(2)).toBe(2); - - expect(getMetricValueNormalizer('terabyte')(1)).toBe(1000 ** 4); - expect(getMetricValueNormalizer('terabytes')(2)).toBe(2 * 1000 ** 4); - }); - - it('returns a function that normalizes the value to bytes when the unit is in byte2ConversionFactors', () => { - expect(getMetricValueNormalizer('bit')(1)).toBe(1 / 8); - expect(getMetricValueNormalizer('bits')(1)).toBe(1 / 8); - - expect(getMetricValueNormalizer('tebibyte')(1)).toBe(1024 ** 4); - expect(getMetricValueNormalizer('tebibytes')(2)).toBe(2 * 1024 ** 4); - }); - - it('skips nomalization for count operations', () => { - expect(getMetricValueNormalizer('second', 'count')(1)).toBe(1); - expect(getMetricValueNormalizer('seconds', 'count_unique')(2)).toBe(2); - }); -}); diff --git a/static/app/utils/metrics/normalizeMetricValue.tsx b/static/app/utils/metrics/normalizeMetricValue.tsx deleted file mode 100644 index 9e6a45373c7792..00000000000000 --- a/static/app/utils/metrics/normalizeMetricValue.tsx +++ /dev/null @@ -1,109 +0,0 @@ -import { - DAY, - HOUR, - MICROSECOND, - MILLISECOND, - MINUTE, - NANOSECOND, - SECOND, - WEEK, -} from 'sentry/utils/formatters'; - -const timeConversionFactors = { - week: WEEK, - weeks: WEEK, - day: DAY, - days: DAY, - hour: HOUR, - hours: HOUR, - minute: MINUTE, - minutes: MINUTE, - second: SECOND, - seconds: SECOND, - millisecond: MILLISECOND, - milliseconds: MILLISECOND, - microsecond: MICROSECOND, - microseconds: MICROSECOND, - nanosecond: NANOSECOND, - nanoseconds: NANOSECOND, -}; - -const byte10ConversionFactors = { - byte: 1, - bytes: 1, - kilobyte: 1000, - kilobytes: 1000, - megabyte: 1000 ** 2, - megabytes: 1000 ** 2, - gigabyte: 1000 ** 3, - gigabytes: 1000 ** 3, - terabyte: 1000 ** 4, - terabytes: 1000 ** 4, - petabyte: 1000 ** 5, - petabytes: 1000 ** 5, - exabyte: 1000 ** 6, - exabytes: 1000 ** 6, -}; - -const byte2ConversionFactors = { - bit: 1 / 8, - bits: 1 / 8, - byte2: 1, - kibibyte: 1024, - kibibytes: 1024, - mebibyte: 1024 ** 2, - mebibytes: 1024 ** 2, - gibibyte: 1024 ** 3, - gibibytes: 1024 ** 3, - tebibyte: 1024 ** 4, - tebibytes: 1024 ** 4, - pebibyte: 1024 ** 5, - pebibytes: 1024 ** 5, - exbibyte: 1024 ** 6, - exbibytes: 1024 ** 6, -}; - -export function getMetricConversionFunction(fromUnit: string, toUnit: string) { - let conversionFactors: Record | null = null; - - if (fromUnit in timeConversionFactors && toUnit in timeConversionFactors) { - conversionFactors = timeConversionFactors; - } else if (fromUnit in byte10ConversionFactors && toUnit in byte10ConversionFactors) { - conversionFactors = byte10ConversionFactors; - } else if (fromUnit in byte2ConversionFactors && toUnit in byte2ConversionFactors) { - conversionFactors = byte2ConversionFactors; - } - - return (value: T): T => { - if (!value || !conversionFactors) { - return value; - } - - return (value * (conversionFactors[fromUnit] / conversionFactors[toUnit])) as T; - }; -} - -export function getNormalizedMetricUnit(unit: string, operation?: string) { - if (!unit || operation === 'count' || operation === 'count_unique') { - return 'none'; - } - - if (unit in timeConversionFactors) { - return 'millisecond'; - } - - if (unit in byte10ConversionFactors) { - return 'byte'; - } - - if (unit in byte2ConversionFactors) { - return 'byte2'; - } - - return unit; -} - -export function getMetricValueNormalizer(unit: string, operation?: string) { - const normalizedMetricUnit = getNormalizedMetricUnit(unit, operation); - return getMetricConversionFunction(unit, normalizedMetricUnit); -} diff --git a/static/app/views/ddm/chart/chart.tsx b/static/app/views/ddm/chart/chart.tsx index e347ea42000ae2..4fb1ce03ca7f29 100644 --- a/static/app/views/ddm/chart/chart.tsx +++ b/static/app/views/ddm/chart/chart.tsx @@ -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'; @@ -70,9 +70,7 @@ export const MetricChart = forwardRef( ({series, displayType, height, group, samples, focusArea}, forwardedRef) => { const chartRef = useRef(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) { @@ -112,23 +110,18 @@ export const MetricChart = forwardRef( 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 + {} as Record ); 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, @@ -223,10 +216,9 @@ export const MetricChart = forwardRef( id: MAIN_Y_AXIS_ID, axisLabel: { formatter: (value: number) => { - return formatMetricsUsingUnitAndOp( + return formatMetricUsingUnit( value, - hasMultipleUnits ? 'none' : firstUnit, - firstOperation + hasMultipleUnits ? 'none' : firstUnit ); }, }, @@ -262,7 +254,6 @@ export const MetricChart = forwardRef( samples, focusArea, firstUnit, - firstOperation, ]); return ( diff --git a/static/app/views/ddm/chart/types.tsx b/static/app/views/ddm/chart/types.tsx index 411dcaa8a98d36..346fb1e6b389df 100644 --- a/static/app/views/ddm/chart/types.tsx +++ b/static/app/views/ddm/chart/types.tsx @@ -6,13 +6,13 @@ export type Series = { color: string; data: {name: number; value: number}[]; id: string; - operation: string; seriesName: string; unit: string; groupBy?: Record; hidden?: boolean; paddingIndices?: Set; release?: string; + scalingFactor?: number; transaction?: string; }; diff --git a/static/app/views/ddm/chart/useFocusArea.tsx b/static/app/views/ddm/chart/useFocusArea.tsx index ae9c63985d0749..f8bae653729589 100644 --- a/static/app/views/ddm/chart/useFocusArea.tsx +++ b/static/app/views/ddm/chart/useFocusArea.tsx @@ -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'; @@ -42,6 +41,7 @@ interface UseFocusAreaOptions { export interface UseFocusAreaProps extends FocusAreaProps { chartRef: RefObject; opts: UseFocusAreaOptions; + scalingFactor: number; chartUnit?: string; onZoom?: (range: DateTimeObject) => void; sampleUnit?: string; @@ -52,8 +52,7 @@ type BrushEndResult = Parameters[0]; export function useFocusArea({ selection: selection, opts: {widgetIndex, isDisabled, useFullYAxis}, - sampleUnit = 'none', - chartUnit = 'none', + scalingFactor, onAdd, onDraw, onRemove, @@ -111,13 +110,11 @@ export function useFocusArea({ return; } - const valueConverter = getMetricConversionFunction(chartUnit, sampleUnit); - const range = getSelectionRange( brushEnd, !!useFullYAxis, getValueRect(chartRef), - valueConverter + scalingFactor ); onAdd?.({ widgetIndex, @@ -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(() => { @@ -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, ] @@ -232,11 +227,10 @@ export type UseFocusAreaResult = ReturnType; type FocusAreaOverlayProps = { chartRef: RefObject; - chartUnit: string; onRemove: () => void; onZoom: () => void; rect: FocusAreaSelection | null; - sampleUnit: string; + scalingFactor: number; useFullYAxis: boolean; }; @@ -246,8 +240,7 @@ function FocusAreaOverlay({ onRemove, useFullYAxis, chartRef, - sampleUnit, - chartUnit, + scalingFactor, }: FocusAreaOverlayProps) { const [position, setPosition] = useState(null); const wrapperRef = useRef(null); @@ -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), @@ -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(); @@ -352,7 +344,7 @@ const getSelectionRange = ( params: BrushEndResult, useFullYAxis: boolean, boundingRect: ValueRect, - valueConverter: (value: number) => number + scalingFactor: number ): SelectionRange => { const rect = params.areas[0]; @@ -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, diff --git a/static/app/views/ddm/chart/useMetricChartSamples.tsx b/static/app/views/ddm/chart/useMetricChartSamples.tsx index 9bb6e068a28709..0272b90dc89ee3 100644 --- a/static/app/views/ddm/chart/useMetricChartSamples.tsx +++ b/static/app/views/ddm/chart/useMetricChartSamples.tsx @@ -8,19 +8,18 @@ import {getFormatter} from 'sentry/components/charts/components/tooltip'; import {isChartHovered} from 'sentry/components/charts/utils'; import type {Field} from 'sentry/components/ddm/metricSamplesTable'; import {t} from 'sentry/locale'; -import type {EChartClickHandler, ReactEchartsRef, Series} from 'sentry/types/echarts'; +import type {EChartClickHandler, ReactEchartsRef} from 'sentry/types/echarts'; import {defined} from 'sentry/utils'; import mergeRefs from 'sentry/utils/mergeRefs'; import {isCumulativeOp} from 'sentry/utils/metrics'; import {formatMetricsUsingUnitAndOp} from 'sentry/utils/metrics/formatters'; -import {getMetricValueNormalizer} from 'sentry/utils/metrics/normalizeMetricValue'; import type {MetricCorrelation, MetricSummary} from 'sentry/utils/metrics/types'; import { getSummaryValueForOp, type MetricsSamplesResults, } from 'sentry/utils/metrics/useMetricsSamples'; import {fitToValueRect, getValueRect} from 'sentry/views/ddm/chart/chartUtils'; -import type {CombinedMetricChartProps} from 'sentry/views/ddm/chart/types'; +import type {CombinedMetricChartProps, Series} from 'sentry/views/ddm/chart/types'; import type {Sample} from 'sentry/views/ddm/widget'; type UseChartSamplesProps = { @@ -60,6 +59,7 @@ export function useMetricChartSamples({ }: UseChartSamplesProps) { const theme = useTheme(); const chartRef = useRef(null); + const scalingFactor = timeseries?.[0]?.scalingFactor ?? 1; const [valueRect, setValueRect] = useState(getValueRect(chartRef)); @@ -144,13 +144,11 @@ export function useMetricChartSamples({ return []; } - const normalizeMetric = getMetricValueNormalizer(unit ?? ''); - return Object.values(samples).map(sample => { const isHighlighted = highlightedSampleId === sample.transactionId; const xValue = moment(sample.timestamp).valueOf(); - const yValue = normalizeMetric(((sample.min ?? 0) + (sample.max ?? 0)) / 2) ?? 0; + const yValue = (((sample.min ?? 0) + (sample.max ?? 0)) / 2) * scalingFactor; const [xPosition, yPosition] = fitToValueRect(xValue, yValue, valueRect); @@ -191,7 +189,14 @@ export function useMetricChartSamples({ z: 10, }; }); - }, [operation, unit, samples, highlightedSampleId, valueRect, theme.purple400]); + }, [ + operation, + samples, + highlightedSampleId, + scalingFactor, + valueRect, + theme.purple400, + ]); const formatterOptions = useMemo(() => { return { @@ -283,6 +288,7 @@ export function useMetricChartSamplesV2({ }: UseMetricChartSamplesV2Options) { const theme = useTheme(); const chartRef = useRef(null); + const timeseriesScalingFactor = timeseries?.[0]?.scalingFactor ?? 1; const [valueRect, setValueRect] = useState(getValueRect(chartRef)); @@ -342,14 +348,12 @@ export function useMetricChartSamplesV2({ return []; } - const normalizeMetric = getMetricValueNormalizer(unit); - return (samples ?? []).map(sample => { const isHighlighted = highlightedSampleId === sample.id; const xValue = moment(sample.timestamp).valueOf(); const value = getSummaryValueForOp(sample.summary, operation); - const yValue = normalizeMetric(value) ?? 0; + const yValue = value * timeseriesScalingFactor; const [xPosition, yPosition] = fitToValueRect(xValue, yValue, valueRect); @@ -385,7 +389,14 @@ export function useMetricChartSamplesV2({ z: 10, }; }); - }, [highlightedSampleId, operation, samples, theme, unit, valueRect]); + }, [ + highlightedSampleId, + operation, + samples, + theme.purple400, + timeseriesScalingFactor, + valueRect, + ]); const formatterOptions = useMemo(() => { return { diff --git a/static/app/views/ddm/summaryTable.tsx b/static/app/views/ddm/summaryTable.tsx index 5e95e7729ee7b4..bb4ae5e3784aa7 100644 --- a/static/app/views/ddm/summaryTable.tsx +++ b/static/app/views/ddm/summaryTable.tsx @@ -13,7 +13,7 @@ import {space} from 'sentry/styles/space'; import {trackAnalytics} from 'sentry/utils/analytics'; import {getUtcDateString} from 'sentry/utils/dates'; import {DEFAULT_SORT_STATE} from 'sentry/utils/metrics/constants'; -import {formatMetricsUsingUnitAndOp} from 'sentry/utils/metrics/formatters'; +import {formatMetricUsingUnit} from 'sentry/utils/metrics/formatters'; import type {FocusedMetricsSeries, SortState} from 'sentry/utils/metrics/types'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; @@ -172,7 +172,6 @@ export const SummaryTable = memo(function SummaryTable({ color, hidden, unit, - operation, transaction, release, avg, @@ -229,18 +228,10 @@ export const SummaryTable = memo(function SummaryTable({ {/* TODO(ddm): Add a tooltip with the full value, don't add on click in case users want to copy the value */} - - {formatMetricsUsingUnitAndOp(avg, unit, operation)} - - - {formatMetricsUsingUnitAndOp(min, unit, operation)} - - - {formatMetricsUsingUnitAndOp(max, unit, operation)} - - - {formatMetricsUsingUnitAndOp(sum, unit, operation)} - + {formatMetricUsingUnit(avg, unit)} + {formatMetricUsingUnit(min, unit)} + {formatMetricUsingUnit(max, unit)} + {formatMetricUsingUnit(sum, unit)} {hasActions && ( diff --git a/static/app/views/ddm/widget.tsx b/static/app/views/ddm/widget.tsx index 39aa8fe8ccfa39..d8bb798462b6d4 100644 --- a/static/app/views/ddm/widget.tsx +++ b/static/app/views/ddm/widget.tsx @@ -31,10 +31,6 @@ import { } from 'sentry/utils/metrics'; import {metricDisplayTypeOptions} from 'sentry/utils/metrics/constants'; import {formatMRIField, MRIToField, parseMRI} from 'sentry/utils/metrics/mri'; -import { - getMetricValueNormalizer, - getNormalizedMetricUnit, -} from 'sentry/utils/metrics/normalizeMetricValue'; import type { FocusedMetricsSeries, MetricCorrelation, @@ -381,14 +377,14 @@ const MetricWidgetBody = memo( [router] ); - const hasCumulativeOp = chartSeries.some(s => isCumulativeOp(s.operation)); - const firstUnit = - chartSeries.find(s => !s.hidden)?.unit || chartSeries[0]?.unit || 'none'; + const hasCumulativeOp = queries.some( + q => !isMetricFormula(q) && isCumulativeOp(q.op) + ); + const firstScalingFactor = chartSeries.find(s => !s.hidden)?.scalingFactor || 1; const focusArea = useFocusArea({ ...focusAreaProps, - sampleUnit: samples?.unit, - chartUnit: firstUnit, + scalingFactor: firstScalingFactor, chartRef, opts: { widgetIndex, @@ -519,35 +515,16 @@ export function getChartTimeseries( const series = data.data.flatMap((group, index) => { const query = filteredQueries[index]; - const metaUnit = data.meta[index]?.[1]?.unit; - + const unit = data.meta[index]?.[1]?.unit; + const scalingFactor = data.meta[index]?.[1]?.scaling_factor ?? 1; + const operation = isMetricFormula(query) ? 'count' : query.op; const isMultiQuery = filteredQueries.length > 1; - let unit = ''; - let operation = ''; - if (!isMetricFormula(query)) { - const parsed = parseMRI(query.mri); - unit = parsed?.unit ?? ''; - operation = query.op ?? ''; - } else { - // Treat formulas as if they were a single query with none as the unit and count as the operation - unit = 'none'; - } - - // TODO(arthur): fully switch to using the meta unit once it's available - if (metaUnit) { - unit = metaUnit; - } - - // We normalize metric units to make related units - // (e.g. seconds & milliseconds) render in the correct ratio - const normalizedUnit = getNormalizedMetricUnit(unit, operation); - const normalizeValue = getMetricValueNormalizer(unit, operation); - return group.map(entry => ({ - unit: normalizedUnit, + unit: unit, operation: operation, - values: entry.series.map(normalizeValue), + values: entry.series, + scalingFactor: scalingFactor, name: getMetricsSeriesName(query, entry.by, isMultiQuery), id: getMetricsSeriesId(query, entry.by), groupBy: entry.by, @@ -563,6 +540,7 @@ export function getChartTimeseries( seriesName: item.name, groupBy: item.groupBy, unit: item.unit, + scalingFactor: item.scalingFactor, operation: item.operation, color: chartPalette[item.id], hidden: focusedSeries && focusedSeries.size > 0 && !focusedSeries.has(item.id),