From 6f8ba3528d5f9007b9abe246b3d9375c9358e8f6 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 8 Aug 2022 16:03:46 +0200 Subject: [PATCH] [ML] Explain Log Rate Spikes: Fix stale brush props. --- .../src/dual_brush/dual_brush.tsx | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx b/x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx index b4eb2e0809f4..c593da55ffde 100644 --- a/x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx +++ b/x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx @@ -5,6 +5,7 @@ * 2.0. */ +import { isEqual } from 'lodash'; import React, { useEffect, useRef } from 'react'; import * as d3Brush from 'd3-brush'; @@ -74,7 +75,14 @@ export function DualBrush({ }: DualBrushProps) { const d3BrushContainer = useRef(null); const brushes = useRef([]); + + // We need to pass props to refs here because the d3-brush code doesn't consider + // native React prop changes. The brush code does its own check whether these props changed then. + // The initialized brushes might otherwise act on stale data. const widthRef = useRef(width); + const minRef = useRef(min); + const maxRef = useRef(max); + const snapTimestampsRef = useRef(snapTimestamps); const { baselineMin, baselineMax, deviationMin, deviationMax } = windowParameters; @@ -93,11 +101,14 @@ export function DualBrush({ function brushend(this: d3Selection.BaseType) { const currentWidth = widthRef.current; - const x = d3.scaleLinear().domain([min, max]).rangeRound([0, currentWidth]); + const x = d3 + .scaleLinear() + .domain([minRef.current, maxRef.current]) + .rangeRound([0, currentWidth]); const px2ts = (px: number) => Math.round(x.invert(px)); - const xMin = x(min) ?? 0; - const xMax = x(max) ?? 0; + const xMin = x(minRef.current) ?? 0; + const xMax = x(maxRef.current) ?? 0; const minExtentPx = Math.round((xMax - xMin) / 100); const baselineBrush = d3.select('#aiops-brush-baseline'); @@ -157,8 +168,8 @@ export function DualBrush({ newWindowParameters.baselineMax = px2ts(newBaselineMax); } - const snappedWindowParameters = snapTimestamps - ? getSnappedWindowParameters(newWindowParameters, snapTimestamps) + const snappedWindowParameters = snapTimestampsRef.current + ? getSnappedWindowParameters(newWindowParameters, snapTimestampsRef.current) : newWindowParameters; const newBrushPx = { @@ -235,7 +246,7 @@ export function DualBrush({ mlBrushSelection .attr('class', 'brush') .selectAll('.overlay') - .attr('width', width) + .attr('width', widthRef.current) .style('pointer-events', 'none'); mlBrushSelection @@ -252,10 +263,13 @@ export function DualBrush({ .data(brushes.current, (d) => (d as DualBrush).id); mlBrushSelection.each(function (brushObject, i, n) { - const x = d3.scaleLinear().domain([min, max]).rangeRound([0, widthRef.current]); + const x = d3 + .scaleLinear() + .domain([minRef.current, maxRef.current]) + .rangeRound([0, widthRef.current]); brushObject.brush.extent([ [0, BRUSH_MARGIN], - [width, BRUSH_HEIGHT - BRUSH_MARGIN], + [widthRef.current, BRUSH_HEIGHT - BRUSH_MARGIN], ]); brushObject.brush(d3.select(n[i] as SVGGElement)); const xStart = x(brushObject.start) ?? 0; @@ -268,11 +282,17 @@ export function DualBrush({ widthRef.current = width; newBrush('baseline', baselineMin, baselineMax); newBrush('deviation', deviationMin, deviationMax); - } else { - if (widthRef.current !== width) { - widthRef.current = width; - updateBrushes(); - } + } else if ( + widthRef.current !== width || + minRef.current !== min || + maxRef.current !== max || + !isEqual(snapTimestampsRef.current, snapTimestamps) + ) { + widthRef.current = width; + minRef.current = min; + maxRef.current = max; + snapTimestampsRef.current = snapTimestamps; + updateBrushes(); } drawBrushes();