Browse Source

fix(ddm): Sample selection unit conversion (#65415)

The units of the focus area where not converted back to the samples
original unit upon selection.
The selection range was not forwarded to the samples table.
ArthurKnaus 1 year ago
parent
commit
b70cedbc3d

+ 22 - 13
static/app/utils/metrics/normalizeMetricValue.tsx

@@ -63,6 +63,26 @@ const byte2ConversionFactors = {
   exbibytes: 1024 ** 6,
 };
 
+export function getMetricConversionFunction(fromUnit: string, toUnit: string) {
+  let conversionFactors: Record<string, number> | 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 <T extends number | null>(value: T): T => {
+    if (!value || !conversionFactors) {
+      return value;
+    }
+
+    return (value * (conversionFactors[fromUnit] / conversionFactors[toUnit])) as T;
+  };
+}
+
 export function getNormalizedMetricUnit(unit: string) {
   if (!unit) {
     return 'none';
@@ -84,17 +104,6 @@ export function getNormalizedMetricUnit(unit: string) {
 }
 
 export function getMetricValueNormalizer(unit: string) {
-  if (unit in timeConversionFactors) {
-    return (value: number | null) => value && value * timeConversionFactors[unit];
-  }
-
-  if (unit in byte10ConversionFactors) {
-    return (value: number | null) => value && value * byte10ConversionFactors[unit];
-  }
-
-  if (unit in byte2ConversionFactors) {
-    return (value: number | null) => value && value * byte2ConversionFactors[unit];
-  }
-
-  return (value: number | null) => value;
+  const normalizedMetricUnit = getNormalizedMetricUnit(unit);
+  return getMetricConversionFunction(unit, normalizedMetricUnit);
 }

+ 4 - 2
static/app/views/ddm/chart.tsx

@@ -90,8 +90,12 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(
       [router]
     );
 
+    const unit = series.find(s => !s.hidden)?.unit || series[0]?.unit || '';
+
     const focusAreaBrush = useFocusArea({
       ...focusArea,
+      sampleUnit: scatter?.unit,
+      chartUnit: unit,
       chartRef,
       opts: {
         widgetIndex,
@@ -114,8 +118,6 @@ export const MetricChart = forwardRef<ReactEchartsRef, ChartProps>(
     // TODO(ddm): This assumes that all series have the same bucket size
     const bucketSize = series[0]?.data[1]?.name - series[0]?.data[0]?.name;
     const isSubMinuteBucket = bucketSize < 60_000;
-
-    const unit = series.find(s => !s.hidden)?.unit || series[0]?.unit || '';
     const lastBucketTimestamp = series[0]?.data?.[series[0]?.data?.length - 1]?.name;
     const ingestionBuckets = useMemo(
       () => getIngestionDelayBucketCount(bucketSize, lastBucketTimestamp),

+ 31 - 8
static/app/views/ddm/focusArea.tsx

@@ -12,6 +12,7 @@ import {Button} from 'sentry/components/button';
 import {IconClose, IconZoom} from 'sentry/icons';
 import {space} from 'sentry/styles/space';
 import type {EChartBrushEndHandler, ReactEchartsRef} from 'sentry/types/echarts';
+import {getMetricConversionFunction} from 'sentry/utils/metrics/normalizeMetricValue';
 import type {SelectionRange} from 'sentry/utils/metrics/types';
 import type {ValueRect} from 'sentry/views/ddm/chartUtils';
 import {getValueRect} from 'sentry/views/ddm/chartUtils';
@@ -41,7 +42,9 @@ export interface FocusAreaSelection {
 export interface UseFocusAreaProps extends FocusAreaProps {
   chartRef: RefObject<ReactEchartsRef>;
   opts: UseFocusAreaOptions;
+  chartUnit?: string;
   onZoom?: (range: DateTimeObject) => void;
+  sampleUnit?: string;
 }
 
 type BrushEndResult = Parameters<EChartBrushEndHandler>[0];
@@ -50,6 +53,8 @@ export function useFocusArea({
   chartRef,
   selection: selection,
   opts: {widgetIndex, isDisabled, useFullYAxis},
+  sampleUnit = 'none',
+  chartUnit = 'none',
   onAdd,
   onDraw,
   onRemove,
@@ -110,7 +115,14 @@ export function useFocusArea({
         return;
       }
 
-      const range = getSelectionRange(brushEnd, !!useFullYAxis, getValueRect(chartRef));
+      const valueConverter = getMetricConversionFunction(chartUnit, sampleUnit);
+
+      const range = getSelectionRange(
+        brushEnd,
+        !!useFullYAxis,
+        getValueRect(chartRef),
+        valueConverter
+      );
       onAdd?.({
         widgetIndex,
         range,
@@ -126,7 +138,7 @@ export function useFocusArea({
       });
       isDrawingRef.current = false;
     },
-    [chartRef, isDisabled, onAdd, widgetIndex, useFullYAxis]
+    [isDisabled, sampleUnit, chartUnit, useFullYAxis, chartRef, onAdd, widgetIndex]
   );
 
   const handleRemove = useCallback(() => {
@@ -175,6 +187,8 @@ export function useFocusArea({
           onZoom={handleZoomIn}
           chartRef={chartRef}
           useFullYAxis={!!useFullYAxis}
+          sampleUnit={sampleUnit}
+          chartUnit={chartUnit}
         />
       ),
       isDrawingRef,
@@ -191,9 +205,11 @@ export function useFocusArea({
 
 type BrushRectOverlayProps = {
   chartRef: RefObject<ReactEchartsRef>;
+  chartUnit: string;
   onRemove: () => void;
   onZoom: () => void;
   rect: FocusAreaSelection | null;
+  sampleUnit: string;
   useFullYAxis: boolean;
 };
 
@@ -203,6 +219,8 @@ function BrushRectOverlay({
   onRemove,
   useFullYAxis,
   chartRef,
+  sampleUnit,
+  chartUnit,
 }: BrushRectOverlayProps) {
   const [position, setPosition] = useState<AbsolutePosition | null>(null);
   const wrapperRef = useRef<HTMLDivElement>(null);
@@ -223,13 +241,17 @@ function BrushRectOverlay({
     }
     const finder = {xAxisId: 'xAxis', yAxisId: 'yAxis'};
 
+    const valueConverter = getMetricConversionFunction(sampleUnit, chartUnit);
+    const max = valueConverter(rect.range.max ?? null);
+    const min = valueConverter(rect.range.min ?? null);
+
     const topLeft = chartInstance.convertToPixel(finder, [
       getTimestamp(rect.range.start),
-      rect.range.max,
+      max,
     ] as number[]);
     const bottomRight = chartInstance.convertToPixel(finder, [
       getTimestamp(rect.range.end),
-      rect.range.min,
+      min,
     ] as number[]);
 
     if (!topLeft || !bottomRight) {
@@ -258,7 +280,7 @@ function BrushRectOverlay({
     if (!isEqual(newPosition, position)) {
       setPosition(newPosition);
     }
-  }, [chartRef, rect, useFullYAxis, position]);
+  }, [chartRef, rect, sampleUnit, chartUnit, useFullYAxis, position]);
 
   useEffect(() => {
     updatePosition();
@@ -296,7 +318,8 @@ const getTimestamp = date => (date ? moment.utc(date).valueOf() : null);
 const getSelectionRange = (
   params: BrushEndResult,
   useFullYAxis: boolean,
-  boundingRect: ValueRect
+  boundingRect: ValueRect,
+  valueConverter: (value: number) => number
 ): SelectionRange => {
   const rect = params.areas[0];
 
@@ -306,8 +329,8 @@ const getSelectionRange = (
   const startDate = getDate(Math.max(startTimestamp, boundingRect.xMin));
   const endDate = getDate(Math.min(endTimestamp, boundingRect.xMax));
 
-  const min = useFullYAxis ? NaN : Math.min(...rect.coordRange[1]);
-  const max = useFullYAxis ? NaN : Math.max(...rect.coordRange[1]);
+  const min = useFullYAxis ? NaN : valueConverter(Math.min(...rect.coordRange[1]));
+  const max = useFullYAxis ? NaN : valueConverter(Math.max(...rect.coordRange[1]));
 
   return {
     start: startDate,

+ 1 - 0
static/app/views/ddm/widgetDetails.tsx

@@ -108,6 +108,7 @@ export function MetricDetails({widget, onRowHover, focusArea}: MetricDetailsProp
               ) : (
                 <SampleTable
                   mri={widget?.mri}
+                  {...focusArea?.selection?.range}
                   query={queryWithFocusedSeries}
                   onRowHover={onRowHover}
                 />