maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[ML] Fixes population chart marker positions, swimlane race condition. (#26716)

Browse source 
* [ML] Fixes the skipping rule for metricData.
* [ML] Adds a test to check against correct filtering of null/0.
* [ML] Gets rid of _.has() and fixed a rare error with field values containing a trailing dot.
* [ML] Fixes rare/event_distribution chart filtering.
* [ML] Fixes race condition related to view by swimlane initialization.
* [ML] Updated comment to clarify metricData filtering.
This commit is contained in:
Walter Rafelsberger 2018-12-06 16:33:36 +01:00 committed by GitHub
parent 5df466e4b7
commit 9721db5673
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 187 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
x-pack/plugins/ml/public/explorer/explorer.html
View file

@ -84,14 +84,14 @@
<div
class="ml-explorer-swimlane euiText"
ng-if="showViewBySwimlane()"
ng-if="showViewBySwimlane"
ng-mouseenter="setSwimlaneSelectActive(true)"
ng-mouseleave="setSwimlaneSelectActive(false)"
>
<ml-explorer-swimlane swimlane-type="viewBy" />
</div>
<div ng-if="!showViewBySwimlane()" class="text-center visError euiText">
<div ng-if="!showViewBySwimlane" class="text-center visError euiText">
<div class="item top"></div>
<div class="item">
<h4 class="euiTitle euiTitle--small">No {{swimlaneViewByFieldName}} influencers found</h4>

37
x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.js
View file

@ -209,7 +209,16 @@ export function explorerChartsContainerServiceFactory(
const filterField = records[0].by_field_value || records[0].over_field_value;
chartData = eventDistribution.filter(d => (d.entity !== filterField));
_.map(metricData, (value, time) => {
if (value > 0) {
// The filtering for rare/event_distribution charts needs to be handled
// differently because of how the source data is structured.
// For rare chart values we are only interested wether a value is either `0` or not,
// `0` acts like a flag in the chart whether to display the dot/marker.
// All other charts (single metric, population) are metric based and with
// those a value of `null` acts as the flag to hide a datapoint.
if (
(chartType === CHART_TYPE.EVENT_DISTRIBUTION && value > 0) ||
(chartType !== CHART_TYPE.EVENT_DISTRIBUTION && value !== null)
) {
chartData.push({
date: +time,
value: value,
@ -246,7 +255,7 @@ export function explorerChartsContainerServiceFactory(
if (chartPoint !== undefined) {
chartPoint.anomalyScore = record.record_score;
if (_.has(record, 'actual')) {
if (record.actual !== undefined) {
chartPoint.actual = record.actual;
chartPoint.typical = record.typical;
} else {
@ -263,7 +272,7 @@ export function explorerChartsContainerServiceFactory(
}
}
if (_.has(record, 'multi_bucket_impact')) {
if (record.multi_bucket_impact !== undefined) {
chartPoint.multiBucketImpact = record.multi_bucket_impact;
}
}
@ -388,13 +397,13 @@ export function explorerChartsContainerServiceFactory(
return;
}
const jobId = record.job_id;
if (!_.has(aggregatedData, jobId)) {
if (aggregatedData[jobId] === undefined) {
aggregatedData[jobId] = {};
}
const detectorsForJob = aggregatedData[jobId];
const detectorIndex = record.detector_index;
if (!_.has(detectorsForJob, detectorIndex)) {
if (detectorsForJob[detectorIndex] === undefined) {
detectorsForJob[detectorIndex] = {};
}
@ -404,11 +413,11 @@ export function explorerChartsContainerServiceFactory(
if (firstFieldName !== undefined) {
const groupsForDetector = detectorsForJob[detectorIndex];
if (!_.has(groupsForDetector, firstFieldName)) {
if (groupsForDetector[firstFieldName] === undefined) {
groupsForDetector[firstFieldName] = {};
}
const valuesForGroup = groupsForDetector[firstFieldName];
if (!_.has(valuesForGroup, firstFieldValue)) {
if (valuesForGroup[firstFieldValue] === undefined) {
valuesForGroup[firstFieldValue] = {};
}
@ -423,7 +432,7 @@ export function explorerChartsContainerServiceFactory(
}
if (isSecondSplit === false) {
if (!_.has(dataForGroupValue, 'maxScoreRecord')) {
if (dataForGroupValue.maxScoreRecord === undefined) {
dataForGroupValue.maxScore = record.record_score;
dataForGroupValue.maxScoreRecord = record;
} else {
@ -437,17 +446,17 @@ export function explorerChartsContainerServiceFactory(
const secondFieldName = record.over_field_name || record.by_field_name;
const secondFieldValue = record.over_field_value || record.by_field_value;
if (!_.has(dataForGroupValue, secondFieldName)) {
if (dataForGroupValue[secondFieldName] === undefined) {
dataForGroupValue[secondFieldName] = {};
}
const splitsForGroup = dataForGroupValue[secondFieldName];
if (!_.has(splitsForGroup, secondFieldValue)) {
if (splitsForGroup[secondFieldValue] === undefined) {
splitsForGroup[secondFieldValue] = {};
}
const dataForSplitValue = splitsForGroup[secondFieldValue];
if (!_.has(dataForSplitValue, 'maxScoreRecord')) {
if (dataForSplitValue.maxScoreRecord === undefined) {
dataForSplitValue.maxScore = record.record_score;
dataForSplitValue.maxScoreRecord = record;
} else {
@ -460,7 +469,7 @@ export function explorerChartsContainerServiceFactory(
} else {
// Detector with no partition or by field.
const dataForDetector = detectorsForJob[detectorIndex];
if (!_.has(dataForDetector, 'maxScoreRecord')) {
if (dataForDetector.maxScoreRecord === undefined) {
dataForDetector.maxScore = record.record_score;
dataForDetector.maxScoreRecord = record;
} else {
@ -478,13 +487,13 @@ export function explorerChartsContainerServiceFactory(
// Convert to an array of the records with the highest record_score per unique series.
_.each(aggregatedData, (detectorsForJob) => {
_.each(detectorsForJob, (groupsForDetector) => {
if (_.has(groupsForDetector, 'maxScoreRecord')) {
if (groupsForDetector.maxScoreRecord !== undefined) {
// Detector with no partition / by field.
recordsForSeries.push(groupsForDetector.maxScoreRecord);
} else {
_.each(groupsForDetector, (valuesForGroup) => {
_.each(valuesForGroup, (dataForGroupValue) => {
if (_.has(dataForGroupValue, 'maxScoreRecord')) {
if (dataForGroupValue.maxScoreRecord !== undefined) {
recordsForSeries.push(dataForGroupValue.maxScoreRecord);
} else {
// Second level of aggregation for partition and by/over.

135
x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.test.js
View file

@ -4,22 +4,57 @@
* you may not use this file except in compliance with the Elastic License.
*/
import _ from 'lodash';
import mockAnomalyChartRecords from './__mocks__/mock_anomaly_chart_records.json';
import mockDetectorsByJob from './__mocks__/mock_detectors_by_job.json';
import mockJobConfig from './__mocks__/mock_job_config.json';
import mockSeriesPromisesResponse from './__mocks__/mock_series_promises_response.json';
// Some notes on the tests and mocks:
//
// 'call anomalyChangeListener with actual series config'
// This test uses the standard mocks and uses the data as is provided via the mock files.
// The mocked services check for values in the data (e.g. 'mock-job-id', 'farequore-2017')
// and return the mock data from the files.
//
// 'filtering should skip values of null'
// This is is used to verify that values of `null` get filtered out but `0` is kept.
// The test clones mock data from files and adjusts job_id and indices to trigger
// suitable responses from the mocked services. The mocked services check against the
// provided alternative values and return specific modified mock responses for the test case.
const mockJobConfigClone = _.cloneDeep(mockJobConfig);
// adjust mock data to tests against null/0 values
const mockMetricClone = _.cloneDeep(mockSeriesPromisesResponse[0][0]);
mockMetricClone.results['1486712700000'] = null;
mockMetricClone.results['1486713600000'] = 0;
jest.mock('../../services/job_service', () => ({
mlJobService: {
getJob() { return mockJobConfig; },
getJob(jobId) {
// this is for 'call anomalyChangeListener with actual series config'
if (jobId === 'mock-job-id') {
return mockJobConfig;
}
// this is for 'filtering should skip values of null'
mockJobConfigClone.datafeed_config.indices = [`farequote-2017-${jobId}`];
return mockJobConfigClone;
},
detectorsByJob: mockDetectorsByJob
}
}));
jest.mock('../../services/results_service', () => ({
mlResultsService: {
getMetricData() {
return Promise.resolve(mockSeriesPromisesResponse[0][0]);
getMetricData(indices) {
// this is for 'call anomalyChangeListener with actual series config'
if (indices[0] === 'farequote-2017') {
return Promise.resolve(mockSeriesPromisesResponse[0][0]);
}
// this is for 'filtering should skip values of null'
return Promise.resolve(mockMetricClone);
},
getRecordsForCriteria() {
return Promise.resolve(mockSeriesPromisesResponse[0][1]);
@ -27,8 +62,17 @@ jest.mock('../../services/results_service', () => ({
getScheduledEventsByBucket() {
return Promise.resolve(mockSeriesPromisesResponse[0][2]);
},
getEventDistributionData() {
return Promise.resolve([]);
getEventDistributionData(indices) {
// this is for 'call anomalyChangeListener with actual series config'
if (indices[0] === 'farequote-2017') {
return Promise.resolve([]);
}
// this is for 'filtering should skip values of null' and
// resolves with a dummy object to trigger the processing
// of the event distribution chartdata filtering
return Promise.resolve([{
entity: 'mock'
}]);
}
}
}));
@ -112,7 +156,7 @@ describe('explorerChartsContainerService', () => {
});
test('call anomalyChangeListener with actual series config', (done) => {
let testCount = 0;
let callbackCount = 0;
const expectedTestCount = 3;
const anomalyDataChangeListener = explorerChartsContainerServiceFactory(
@ -128,9 +172,84 @@ describe('explorerChartsContainerService', () => {
);
function callback(data) {
testCount++;
callbackCount++;
expect(data).toMatchSnapshot();
if (testCount === expectedTestCount) {
if (callbackCount === expectedTestCount) {
done();
}
}
});
test('filtering should skip values of null', (done) => {
let callbackCount = 0;
const expectedTestCount = 3;
const anomalyDataChangeListener = explorerChartsContainerServiceFactory(
mockMlSelectSeverityService,
callback,
mockChartContainer
);
const mockAnomalyChartRecordsClone = _.cloneDeep(mockAnomalyChartRecords).map((d) => {
d.job_id = 'mock-job-id-distribution';
return d;
});
anomalyDataChangeListener(
mockAnomalyChartRecordsClone,
1486656000000,
1486670399999
);
function callback(data) {
callbackCount++;
if (callbackCount === 1) {
expect(data.seriesToPlot).toHaveLength(0);
}
if (callbackCount === 3) {
expect(data.seriesToPlot).toHaveLength(1);
// the mock source dataset has a length of 115. one data point has a value of `null`,
// and another one `0`. the received dataset should have a length of 114,
// it should remove the datapoint with `null` and keep the one with `0`.
const chartData = data.seriesToPlot[0].chartData;
expect(chartData).toHaveLength(114);
expect(chartData.filter(d => d.value === 0)).toHaveLength(1);
expect(chartData.filter(d => d.value === null)).toHaveLength(0);
}
if (callbackCount === expectedTestCount) {
done();
}
}
});
test('field value with trailing dot should not throw an error', (done) => {
let callbackCount = 0;
const expectedTestCount = 3;
const anomalyDataChangeListener = explorerChartsContainerServiceFactory(
mockMlSelectSeverityService,
callback,
mockChartContainer
);
const mockAnomalyChartRecordsClone = _.cloneDeep(mockAnomalyChartRecords);
mockAnomalyChartRecordsClone[1].partition_field_value = 'AAL.';
expect(() => {
anomalyDataChangeListener(
mockAnomalyChartRecordsClone,
1486656000000,
1486670399999
);
}).not.toThrow();
function callback() {
callbackCount++;
if (callbackCount === expectedTestCount) {
done();
}
}

64
x-pack/plugins/ml/public/explorer/explorer_controller.js
View file

@ -398,9 +398,13 @@ module.controller('MlExplorerController', function (
$scope.initializeVis();
$scope.showViewBySwimlane = function () {
return $scope.viewBySwimlaneData !== null && $scope.viewBySwimlaneData.laneLabels && $scope.viewBySwimlaneData.laneLabels.length > 0;
};
function setShowViewBySwimlane() {
$scope.showViewBySwimlane = (
$scope.viewBySwimlaneData !== null &&
$scope.viewBySwimlaneData.laneLabels &&
$scope.viewBySwimlaneData.laneLabels.length > 0
);
}
function getSelectionTimeRange(cellData) {
// Returns the time range of the cell(s) currently selected in the swimlane.
@ -635,7 +639,7 @@ module.controller('MlExplorerController', function (
});
}
function loadViewBySwimlaneOptions() {
function setViewBySwimlaneOptions() {
// Obtain the list of 'View by' fields per job.
$scope.swimlaneViewByFieldName = null;
let viewByOptions = []; // Unique influencers for the selected job(s).
@ -733,9 +737,6 @@ module.controller('MlExplorerController', function (
$scope.appState.mlExplorerSwimlane.viewBy = $scope.swimlaneViewByFieldName;
$scope.appState.save();
}
loadViewBySwimlane([]);
}
function loadOverallData() {
@ -782,7 +783,7 @@ module.controller('MlExplorerController', function (
// Trigger loading of the 'view by' swimlane -
// only load once the overall swimlane so that we can match the time span.
loadViewBySwimlaneOptions();
setViewBySwimlaneOptions();
} else {
$scope.hasResults = false;
}
@ -791,8 +792,8 @@ module.controller('MlExplorerController', function (
// Tell the result components directives to render.
// Need to use $timeout to ensure the broadcast happens after the child scope is updated with the new data.
$timeout(() => {
$scope.$broadcast('render');
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.OVERALL));
loadViewBySwimlane([]);
}, 0);
});
@ -852,6 +853,7 @@ module.controller('MlExplorerController', function (
}
// 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.
setShowViewBySwimlane();
$timeout(() => {
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.VIEW_BY));
}, 0);
@ -1006,28 +1008,32 @@ module.controller('MlExplorerController', function (
// would fail because the Explorer Charts Container's directive wasn't linked yet and not being subscribed
// to the anomalyDataChange listener used in loadDataForCharts().
function finish() {
if ($scope.overallSwimlaneData !== undefined) {
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.OVERALL));
}
if ($scope.viewBySwimlaneData !== undefined) {
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.VIEW_BY));
}
mlExplorerDashboardService.anomalyDataChange.changed($scope.anomalyChartRecords || [], timerange.earliestMs, timerange.latestMs);
setShowViewBySwimlane();
if (cellData !== undefined && 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');
}
$timeout(() => {
if ($scope.overallSwimlaneData !== undefined) {
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.OVERALL));
}
if ($scope.viewBySwimlaneData !== undefined) {
mlExplorerDashboardService.swimlaneDataChange.changed(mapScopeToSwimlaneProps(SWIMLANE_TYPE.VIEW_BY));
}
mlExplorerDashboardService.anomalyDataChange.changed($scope.anomalyChartRecords || [], timerange.earliestMs, timerange.latestMs);
if (influencers.length === 0) {
loadTopInfluencers(jobIds, timerange.earliestMs, timerange.latestMs);
loadDataForCharts(jobIds, timerange.earliestMs, timerange.latestMs);
} else {
loadDataForCharts(jobIds, timerange.earliestMs, timerange.latestMs, influencers);
}
loadAnomaliesTableData();
if (cellData !== undefined && 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');
}
if (influencers.length === 0) {
loadTopInfluencers(jobIds, timerange.earliestMs, timerange.latestMs);
loadDataForCharts(jobIds, timerange.earliestMs, timerange.latestMs);
} else {
loadDataForCharts(jobIds, timerange.earliestMs, timerange.latestMs, influencers);
}
loadAnomaliesTableData();
}, 0);
}
if (isChartsContainerInitialized) {