Skip to content

Commit

Permalink
[ML] Fixes Anomaly Explorer Swimlane race condition, adds tests. (ela…
Browse files Browse the repository at this point in the history
…stic#22814) (elastic#22923)

This PR addresses parts of elastic#22642:
- It gets rid of the use of var that = this;.
- dragSelect's action strings are moved to a constants file.
- Adds jest tests for the ExplorerSwimlane component.

This also fixes the following bugs:
- The way we subscribe listeners to the events of the dragSelect library could result in the same event being triggered multiple times. This in turn could cause race conditions when on each event new data gets fetched but in between angular's scope gets updated and could end up in a non-intended way. The result of this were view-by swimlanes not updating correctly or anomaly charts showing non-related charts. This PR fixes it by filtering out consecutive swimlane click events.
- When the angular based chart container wrapper directive gets destroyed/re-esetup when using the job pick, it missed unmounting the react component, it didn't trigger componentWillUnmount()and didn't unsubscribe from dragSelectListener.
  • Loading branch information
walterra committed Sep 11, 2018
1 parent d86c0b1 commit 704667f
Show file tree
Hide file tree
Showing 8 changed files with 486 additions and 186 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"laneLabels": [
"Overall"
],
"points": [
{
"laneLabel": "Overall",
"time": 1486425600,
"value": 0
},
{
"laneLabel": "Overall",
"time": 1486440000,
"value": 0.01107053
},
{
"laneLabel": "Overall",
"time": 1486454400,
"value": 0.1870243
},
{
"laneLabel": "Overall",
"time": 1486468800,
"value": 0.5947769
},
{
"laneLabel": "Overall",
"time": 1486483200,
"value": 0
}
],
"interval": 14400,
"earliest": 1486425600,
"latest": 1486483200
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ExplorerSwimlane Overall swimlane 1`] = `"<div class=\\"ml-swimlanes\\"><div class=\\"lane\\"><div class=\\"lane-label\\" style=\\"width: 170px;\\">Overall</div><div class=\\"cells-container\\"><div class=\\"sl-cell \\" style=\\"width: 200px;\\" data-lane-label=\\"Overall\\" data-time=\\"1486425600\\"><div class=\\"sl-cell-inner-dragselect\\"></div></div><div class=\\"sl-cell \\" style=\\"width: 200px;\\" data-lane-label=\\"Overall\\" data-time=\\"1486440000\\" data-score=\\"0.01107053\\"><div class=\\"sl-cell-inner\\" style=\\"background-color: rgb(210, 233, 247);\\"></div></div><div class=\\"sl-cell \\" style=\\"width: 200px;\\" data-lane-label=\\"Overall\\" data-time=\\"1486454400\\" data-score=\\"0.1870243\\"><div class=\\"sl-cell-inner\\" style=\\"background-color: rgb(210, 233, 247);\\"></div></div><div class=\\"sl-cell \\" style=\\"width: 200px;\\" data-lane-label=\\"Overall\\" data-time=\\"1486468800\\" data-score=\\"0.5947769\\"><div class=\\"sl-cell-inner\\" style=\\"background-color: rgb(210, 233, 247);\\"></div></div></div></div><div class=\\"time-tick-labels\\"><svg width=\\"800\\" height=\\"25\\"><g class=\\"x axis\\"><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(0,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T00:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(25,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T00:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(50,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T01:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(75,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T01:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(100,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T02:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(125,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T02:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(150,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T03:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(175,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T03:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(200,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T04:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(225,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T04:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(250,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T05:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(275,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T05:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(300,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T06:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(325,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T06:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(350,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T07:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(375,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T07:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(400,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T08:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(425,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T08:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(450,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T09:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(475,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T09:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(500,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T10:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(525,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T10:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(550,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T11:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(575,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T11:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(600,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T12:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(625,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T12:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(650,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T13:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(675,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T13:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(700,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T14:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(725,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T14:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(750,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T15:00:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(775,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T15:30:00Z</text></g><g class=\\"tick\\" style=\\"opacity: 1;\\" transform=\\"translate(800,0)\\"><line y2=\\"6\\" x2=\\"0\\"></line><text dy=\\".71em\\" style=\\"text-anchor: middle;\\" y=\\"9\\" x=\\"0\\">2017-02-07T16:00:00Z</text></g><path class=\\"domain\\" d=\\"M0,6V0H800V6\\"></path></g></svg></div></div>"`;
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@


/*
* Angular controller for the container for the anomaly charts in the
* Service for the container for the anomaly charts in the
* Machine Learning Explorer dashboard.
* The controller processes the data required to draw each of the charts
* The service processes the data required to draw each of the charts
* and manages the layout of the charts in the containing div.
*/

Expand Down
15 changes: 15 additions & 0 deletions x-pack/plugins/ml/public/explorer/explorer_constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

/*
* Contains values for ML anomaly explorer.
*/

export const DRAG_SELECT_ACTION = {
NEW_SELECTION: 'newSelection',
ELEMENT_SELECT: 'elementSelect',
DRAG_START: 'dragStart'
};
110 changes: 95 additions & 15 deletions x-pack/plugins/ml/public/explorer/explorer_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { mlFieldFormatService } from 'plugins/ml/services/field_format_service';
import { JobSelectServiceProvider } from 'plugins/ml/components/job_select_list/job_select_service';
import { isTimeSeriesViewDetector } from 'plugins/ml/../common/util/job_utils';
import { timefilter } from 'ui/timefilter';
import { DRAG_SELECT_ACTION } from './explorer_constants';

uiRoutes
.when('/explorer/?', {
Expand All @@ -55,6 +56,15 @@ uiRoutes
import { uiModules } from 'ui/modules';
const module = uiModules.get('apps/ml');

function getDefaultViewBySwimlaneData() {
return {
fieldName: '',
laneLabels: [],
points: [],
interval: 3600
};
}

module.controller('MlExplorerController', function (
$scope,
$timeout,
Expand Down Expand Up @@ -83,7 +93,10 @@ module.controller('MlExplorerController', function (
const VIEW_BY_JOB_LABEL = 'job ID';

const ALLOW_CELL_RANGE_SELECTION = mlExplorerDashboardService.allowCellRangeSelection;
// make sure dragSelect is only available if the mouse point is actually over a swimlane
let disableDragSelectOnMouseLeave = true;
// skip listening to clicks on swimlanes while they are loading to avoid race conditions
let skipCellClicks = true;
$scope.queryFilters = [];

const dragSelect = new DragSelect({
Expand All @@ -95,7 +108,7 @@ module.controller('MlExplorerController', function (

if (elements.length > 0) {
mlExplorerDashboardService.dragSelect.changed({
action: 'newSelection',
action: DRAG_SELECT_ACTION.NEW_SELECTION,
elements
});
}
Expand All @@ -105,15 +118,15 @@ module.controller('MlExplorerController', function (
onDragStart() {
if (ALLOW_CELL_RANGE_SELECTION) {
mlExplorerDashboardService.dragSelect.changed({
action: 'dragStart'
action: DRAG_SELECT_ACTION.DRAG_START
});
disableDragSelectOnMouseLeave = false;
}
},
onElementSelect() {
if (ALLOW_CELL_RANGE_SELECTION) {
mlExplorerDashboardService.dragSelect.changed({
action: 'elementSelect'
action: DRAG_SELECT_ACTION.ELEMENT_SELECT
});
}
}
Expand All @@ -127,8 +140,7 @@ module.controller('MlExplorerController', function (
};

$scope.viewBySwimlaneOptions = [];
$scope.viewBySwimlaneData = { 'fieldName': '', 'laneLabels': [],
'points': [], 'interval': 3600 };
$scope.viewBySwimlaneData = getDefaultViewBySwimlaneData();

$scope.initializeVis = function () {
// Initialize the AppState in which to store filters.
Expand Down Expand Up @@ -342,34 +354,70 @@ module.controller('MlExplorerController', function (
return influencers;
}

// This queue tracks click events while the swimlanes are loading.
// To avoid race conditions we keep the click events cellData in this queue
// and trigger another event only after the current loading is done.
// The queue is necessary since a click in the overall swimlane triggers
// an update of the viewby swimlanes. If we'd just ignored click events
// during the loading, we could miss programmatically triggered events like
// those coming via AppState when a selection is part of the URL.
const swimlaneCellClickListenerQueue = [];

// swimlaneCellClickListener could trigger multiple times with the same data.
// we track the previous click data here to be able to compare it and filter
// consecutive calls with the same data.
let previousListenerData = null;

// Listener for click events in the swimlane and load corresponding anomaly data.
// Empty cellData is passed on clicking outside a cell with score > 0.
const swimlaneCellClickListener = function (cellData) {
// The reset argument is useful when we intentionally want to reset state comparison
// of click events and want to pass through.
// For example, toggling showCharts isn't considered in the comparison
// and would therefor fail to update properly.
const swimlaneCellClickListener = function (cellData, skipComparison = false) {
if (skipCellClicks === true) {
swimlaneCellClickListenerQueue.push(cellData);
return;
}

if (_.keys(cellData).length === 0) {
// Swimlane deselection - clear anomalies section.
if ($scope.viewByLoadedForTimeFormatted) {
// Reload 'view by' swimlane over full time range.
loadViewBySwimlane([]);
}
clearSelectedAnomalies();
previousListenerData = null;
} else {
const timerange = getSelectionTimeRange(cellData);
$scope.cellData = cellData;

if (cellData.score > 0) {
const jobIds = (cellData.fieldName === VIEW_BY_JOB_LABEL) ?
cellData.laneLabels : $scope.getSelectedJobIds();
const influencers = getSelectionInfluencers(cellData);

const listenerData = {
jobIds,
influencers,
start: timerange.earliestMs,
end: timerange.latestMs,
cellData
};
if (_.isEqual(listenerData, previousListenerData) && skipComparison === false) {
return;
}
previousListenerData = listenerData;

if (cellData.fieldName === undefined) {
// Click is in one of the cells in the Overall swimlane - reload the 'view by' swimlane
// to show the top 'view by' values for the selected time.
loadViewBySwimlaneForSelectedTime(timerange.earliestMs, timerange.latestMs);
$scope.viewByLoadedForTimeFormatted = moment(timerange.earliestMs).format('MMMM Do YYYY, HH:mm');
}

const jobIds = (cellData.fieldName === VIEW_BY_JOB_LABEL) ?
cellData.laneLabels : $scope.getSelectedJobIds();
const influencers = getSelectionInfluencers(cellData);

loadAnomaliesTableData();
loadDataForCharts(jobIds, influencers, timerange.earliestMs, timerange.latestMs);
loadAnomaliesTableData();
} else {
// Multiple cells are selected, all with a score of 0 - clear all anomalies.
$scope.$evalAsync(() => {
Expand Down Expand Up @@ -397,7 +445,8 @@ module.controller('MlExplorerController', function (
const checkboxShowChartsListener = function () {
const showCharts = mlCheckboxShowChartsService.state.get('showCharts');
if (showCharts && $scope.cellData !== undefined) {
swimlaneCellClickListener($scope.cellData);
// passing true as the second argument skips click event filtering
swimlaneCellClickListener($scope.cellData, true);
} else {
const timerange = getSelectionTimeRange($scope.cellData);
mlExplorerDashboardService.anomalyDataChange.changed(
Expand Down Expand Up @@ -451,7 +500,19 @@ module.controller('MlExplorerController', function (
navListener();
});

// track the request to be able to ignore out of date requests
// and avoid race conditions ending up with the wrong charts.
let requestCount = 0;
function loadDataForCharts(jobIds, influencers, earliestMs, latestMs) {
// Just skip doing the request when this function is called without
// the minimum required data.
if ($scope.cellData === undefined && influencers.length === 0) {
return;
}

const newRequestCount = ++requestCount;
requestCount = newRequestCount;

// Loads the data used to populate the anomaly charts and the Top Influencers List.
if (influencers.length === 0) {
getTopInfluencers(jobIds, earliestMs, latestMs);
Expand All @@ -462,6 +523,11 @@ module.controller('MlExplorerController', function (
jobIds, influencers, 0, earliestMs, latestMs, 500
)
.then((resp) => {
// Ignore this response if it's returned by an out of date promise
if (newRequestCount < requestCount) {
return;
}

if ($scope.cellData !== undefined && _.keys($scope.cellData).length > 0) {
$scope.anomalyChartRecords = resp.records;
console.log('Explorer anomaly charts data set:', $scope.anomalyChartRecords);
Expand Down Expand Up @@ -686,6 +752,7 @@ module.controller('MlExplorerController', function (
$timeout(() => {
$scope.$broadcast('render');
mlExplorerDashboardService.swimlaneDataChange.changed('overall');
skipCellClicks = false;
}, 0);
});

Expand All @@ -710,20 +777,33 @@ module.controller('MlExplorerController', function (
}

function loadViewBySwimlane(fieldValues) {
// reset the swimlane data to avoid flickering where the old dataset would briefly show up.
$scope.viewBySwimlaneData = getDefaultViewBySwimlaneData();

skipCellClicks = true;
// finish() function, called after each data set has been loaded and processed.
// The last one to call it will trigger the page render.
function finish() {
console.log('Explorer view by swimlane data set:', $scope.viewBySwimlaneData);
if (swimlaneCellClickListenerQueue.length > 0) {
const cellData = swimlaneCellClickListenerQueue.pop();
swimlaneCellClickListenerQueue.length = 0;
swimlaneCellClickListener(cellData);
return;
}
// Fire event to indicate swimlane data has changed.
// Need to use $timeout to ensure this happens after the child scope is updated with the new data.
$timeout(() => {
skipCellClicks = false;
mlExplorerDashboardService.swimlaneDataChange.changed('viewBy');
}, 0);
}

if ($scope.selectedJobs === undefined ||
$scope.swimlaneViewByFieldName === undefined || $scope.swimlaneViewByFieldName === null) {
$scope.viewBySwimlaneData = { 'fieldName': '', 'laneLabels': [], 'points': [], 'interval': 3600 };
if (
$scope.selectedJobs === undefined ||
$scope.swimlaneViewByFieldName === undefined ||
$scope.swimlaneViewByFieldName === null
) {
finish();
} else {
// Ensure the search bounds align to the bucketing interval used in the swimlane so
Expand Down
Loading

0 comments on commit 704667f

Please sign in to comment.