Skip to content

[ML] Fixes anomaly charts for rare detectors when model plot is enabled #66075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 12, 2020
Merged

[ML] Fixes anomaly charts for rare detectors when model plot is enabled #66075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x-pack/plugins/ml/common/util/job_utils.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export function mlFunctionToESAggregation(functionName: string): string | null;

export function isModelPlotEnabled(job: Job, detectorIndex: number, entityFields: any[]): boolean;

export function isModelPlotChartableForDetector(job: Job, detectorIndex: number): boolean;

export function getSafeAggregationName(fieldName: string, index: number): string;

export function getLatestDataOrBucketTimestamp(
Expand Down
22 changes: 12 additions & 10 deletions x-pack/plugins/ml/common/util/job_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,20 @@ export function isModelPlotChartableForDetector(job, detectorIndex) {
const dtr = dtrs[detectorIndex];
const functionName = dtr.function;

// Model plot can be charted for any of the functions which map to ES aggregations,
// Model plot can be charted for any of the functions which map to ES aggregations
// (except rare, for which no model plot results are generated),
// plus varp and info_content functions.
isModelPlotChartable =
mlFunctionToESAggregation(functionName) !== null ||
[
'varp',
'high_varp',
'low_varp',
'info_content',
'high_info_content',
'low_info_content',
].includes(functionName) === true;
functionName !== 'rare' &&
(mlFunctionToESAggregation(functionName) !== null ||
[
'varp',
'high_varp',
'low_varp',
'info_content',
'high_info_content',
'low_info_content',
].includes(functionName) === true);
Copy link
Member

@jgowdyelastic jgowdyelastic May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be converted to use the constants from kibana/x-pack/plugins/ml/common/constants/aggregation_types.ts

Copy link
Contributor Author

@peteharverson peteharverson May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, although will save this for a follow-up as I wanted to keep the code churn in this fix to a minimum. I suspect there are a lot of places in the code where we use hard-coded function names which could be switched to the constants from kibana/x-pack/plugins/ml/common/constants/aggregation_types.ts.

}

return isModelPlotChartable;
Expand Down
13 changes: 12 additions & 1 deletion x-pack/plugins/ml/common/util/job_utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,14 @@ describe('ML - job utils', () => {

const job2 = {
analysis_config: {
detectors: [{ function: 'count' }, { function: 'info_content' }],
detectors: [
{ function: 'count' },
{ function: 'info_content' },
{
function: 'rare',
by_field_name: 'mlcategory',
},
],
},
model_plot_config: {
enabled: true,
Expand All @@ -325,6 +332,10 @@ describe('ML - job utils', () => {
test('returns true for info_content detector when model plot is enabled', () => {
expect(isModelPlotChartableForDetector(job2, 1)).toBe(true);
});

test('returns false for rare by mlcategory when model plot is enabled', () => {
expect(isModelPlotChartableForDetector(job2, 2)).toBe(false);
});
});

describe('getPartitioningFieldNames', () => {
Expand Down
3 changes: 2 additions & 1 deletion ...ugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { chartLimits, getChartType } from '../../util/chart_utils';
import { getEntityFieldList } from '../../../../common/util/anomaly_utils';
import {
isSourceDataChartableForDetector,
isModelPlotChartableForDetector,
isModelPlotEnabled,
} from '../../../../common/util/job_utils';
import { mlResultsService } from '../../services/results_service';
Expand Down Expand Up @@ -420,7 +421,7 @@ function processRecordsForDisplay(anomalyRecords) {
// is chartable, and if model plot is enabled for the job.
const job = mlJobService.getJob(record.job_id);
let isChartable = isSourceDataChartableForDetector(job, record.detector_index);
if (isChartable === false) {
if (isChartable === false && isModelPlotChartableForDetector(job, record.detector_index)) {
// Check if model plot is enabled for this job.
// Need to check the entity fields for the record in case the model plot config has a terms list.
const entityFields = getEntityFieldList(record);
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/ml/public/application/explorer/explorer_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { getEntityFieldList } from '../../../common/util/anomaly_utils';
import {
isSourceDataChartableForDetector,
isModelPlotChartableForDetector,
isModelPlotEnabled,
} from '../../../common/util/job_utils';
import { parseInterval } from '../../../common/util/parse_interval';
Expand Down Expand Up @@ -636,7 +637,10 @@ export async function loadAnomaliesTableData(
// TODO - when job_service is moved server_side, move this to server endpoint.
const job = mlJobService.getJob(jobId);
let isChartable = isSourceDataChartableForDetector(job, anomaly.detectorIndex);
if (isChartable === false) {
if (
isChartable === false &&
isModelPlotChartableForDetector(job, anomaly.detectorIndex)
) {
// Check if model plot is enabled for this job.
// Need to check the entity fields for the record in case the model plot config has a terms list.
// If terms is specified, model plot is only stored if both the partition and by fields appear in the list.
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/ml/public/application/timeseriesexplorer/timeseries_search_service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import _ from 'lodash';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { ml } from '../services/ml_api_service';
import { isModelPlotEnabled } from '../../../common/util/job_utils';
import {
isModelPlotChartableForDetector,
isModelPlotEnabled,
} from '../../../common/util/job_utils';
// @ts-ignore
import { buildConfigFromDetector } from '../util/chart_config_builder';
import { mlResultsService } from '../services/results_service';
Expand All @@ -24,7 +27,10 @@ function getMetricData(
latestMs: number,
interval: string
): Observable<ModelPlotOutput> {
if (isModelPlotEnabled(job, detectorIndex, entityFields)) {
if (
isModelPlotChartableForDetector(job, detectorIndex) &&
isModelPlotEnabled(job, detectorIndex, entityFields)
) {
// Extract the partition, by, over fields on which to filter.
const criteriaFields = [];
const detector = job.analysis_config.detectors[detectorIndex];
Expand Down
9 changes: 4 additions & 5 deletions x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { ResizeChecker } from '../../../../../../src/plugins/kibana_utils/public
import { ANOMALIES_TABLE_DEFAULT_QUERY_SIZE } from '../../../common/constants/search';
import {
isModelPlotEnabled,
isModelPlotChartableForDetector,
isSourceDataChartableForDetector,
isTimeSeriesViewDetector,
mlFunctionToESAggregation,
Expand Down Expand Up @@ -506,11 +507,9 @@ export class TimeSeriesExplorer extends React.Component {
contextForecastData: undefined,
focusChartData: undefined,
focusForecastData: undefined,
modelPlotEnabled: isModelPlotEnabled(
currentSelectedJob,
selectedDetectorIndex,
entityControls
),
modelPlotEnabled:
isModelPlotChartableForDetector(currentSelectedJob, selectedDetectorIndex) &&
isModelPlotEnabled(currentSelectedJob, selectedDetectorIndex, entityControls),
hasResults: false,
dataNotChartable: false,
}
Expand Down