Skip to content

Commit

Permalink
[ML] Explain Log Rate Spikes: Fix stale brush props. (elastic#138272) (
Browse files Browse the repository at this point in the history
…elastic#138530)

Fixes stale props that would cause a broken brush update when the overall time range selection changes.

(cherry picked from commit 3ff5fb2)

Co-authored-by: Walter Rafelsberger <[email protected]>
  • Loading branch information
kibanamachine and walterra authored Aug 10, 2022
1 parent 3d782db commit 46aa2e9
Showing 1 changed file with 33 additions and 13 deletions.
46 changes: 33 additions & 13 deletions x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { isEqual } from 'lodash';
import React, { useEffect, useRef } from 'react';

import * as d3Brush from 'd3-brush';
Expand Down Expand Up @@ -74,7 +75,14 @@ export function DualBrush({
}: DualBrushProps) {
const d3BrushContainer = useRef(null);
const brushes = useRef<DualBrush[]>([]);

// 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;

Expand All @@ -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');
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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
Expand All @@ -252,10 +263,13 @@ export function DualBrush({
.data<DualBrush>(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;
Expand All @@ -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();
Expand Down

0 comments on commit 46aa2e9

Please sign in to comment.