Skip to content
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.

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

[ML] Anomaly Detection: Fix validation error when no data in index. #86114

Merged
merged 11 commits into from
Dec 17, 2020
32 changes: 32 additions & 0 deletions x-pack/plugins/ml/common/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,38 @@ export const getMessages = once(() => {
url:
'https://www.elastic.co/guide/en/machine-learning/{{version}}/ml-configuring-aggregation.html',
},
cardinality_no_results: {
status: VALIDATION_STATUS.WARNING,
heading: i18n.translate(
'xpack.ml.models.jobValidation.messages.cardinalityNoResultsHeading',
{
defaultMessage: 'Field cardinality',
}
),
text: i18n.translate('xpack.ml.models.jobValidation.messages.cardinalityNoResultsMessage', {
defaultMessage: `Cardinality checks could not be run. The query to validate fields didn't return any documents.`,
}),
},
cardinality_field_not_exists: {
status: VALIDATION_STATUS.WARNING,
heading: i18n.translate(
'xpack.ml.models.jobValidation.messages.cardinalityFieldNotExistsHeading',
{
defaultMessage: 'Field cardinality',
}
),
text: i18n.translate(
'xpack.ml.models.jobValidation.messages.cardinalityFieldNotExistsMessage',
{
defaultMessage: `Cardinality checks could not be run for field {fieldName}. The query to validate the field didn't return any documents.`,
values: {
fieldName: '"{{fieldName}}"',
},
}
),
url:
'https://www.elastic.co/guide/en/machine-learning/{{version}}/ml-configuring-aggregation.html',
},
cardinality_by_field: {
status: VALIDATION_STATUS.WARNING,
text: i18n.translate('xpack.ml.models.jobValidation.messages.cardinalityByFieldMessage', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ export class ValidateJobUI extends Component {
const duration = typeof getDuration === 'function' ? getDuration() : undefined;
const fields = this.props.fields;

if (typeof job === 'object') {
// Run job validation only if a job config has been passed on and the duration makes sense to run it.
// Otherwise we skip the call and display a generic warning, but let the user move on to the next wizard step.
if (typeof job === 'object' && duration.start !== null && duration.end !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

nit, this whole condition could be wrapped in an outer if (typeof job === 'object') {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b366241. Just became aware that this is still plain JavaScript so I added another check against typeof duration === 'object') too.

let shouldShowLoadingIndicator = true;

this.props.ml
Expand Down Expand Up @@ -262,6 +264,33 @@ export class ValidateJobUI extends Component {
});
}
}, delay);
} else if (typeof job === 'object' && duration.start === null && duration.end === null) {
this.setState({
...this.state,
ui: {
...this.state.ui,
iconType: statusToEuiIconType(VALIDATION_STATUS.WARNING),
isLoading: false,
isModalVisible: true,
},
data: {
messages: [
{
id: 'job_validation_skipped',
text: i18n.translate('xpack.ml.validateJob.jobValidationSkippedText', {
defaultMessage:
'Job validation could not be run because of insufficient sample data.',
}),
status: VALIDATION_STATUS.WARNING,
},
],
success: true,
},
title: job.job_id,
});
if (typeof this.props.setIsValid === 'function') {
this.props.setIsValid(true);
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { MlClient } from '../../lib/ml_client';

const callAs = {
fieldCaps: () => Promise.resolve({ body: { fields: [] } }),
search: () => Promise.resolve({ body: { hits: { total: { value: 0, relation: 'eq' } } } }),
search: () => Promise.resolve({ body: { hits: { total: { value: 1, relation: 'eq' } } } }),
};

const mlClusterClient = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,38 @@ describe('ML - validateCardinality', () => {
);
});

it('cardinality no results', () => {
const job = getJobConfig('partition_field_name');

const mockCardinality = cloneDeep(mockResponses);
mockCardinality.search.hits.total.value = 0;
mockCardinality.search.aggregations.airline_count.doc_count = 0;

return validateCardinality(
mlClusterClientFactory(mockCardinality),
(job as unknown) as CombinedJob
).then((messages) => {
const ids = messages.map((m) => m.id);
expect(ids).toStrictEqual(['cardinality_no_results']);
});
});

it('cardinality field not exists', () => {
const job = getJobConfig('partition_field_name');

const mockCardinality = cloneDeep(mockResponses);
mockCardinality.search.hits.total.value = 1;
mockCardinality.search.aggregations.airline_count.doc_count = 0;

return validateCardinality(
mlClusterClientFactory(mockCardinality),
(job as unknown) as CombinedJob
).then((messages) => {
const ids = messages.map((m) => m.id);
expect(ids).toStrictEqual(['cardinality_field_not_exists']);
});
});

it('fields not aggregatable', () => {
const job = getJobConfig('partition_field_name');
job.analysis_config.detectors.push({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,35 +132,54 @@ const validateFactory = (client: IScopedClusterClient, job: CombinedJob): Valida
datafeedConfig
);

uniqueFieldNames.forEach((uniqueFieldName) => {
const field = stats.aggregatableExistsFields.find(
(fieldData) => fieldData.fieldName === uniqueFieldName
);
if (field !== undefined && typeof field === 'object' && field.stats) {
modelPlotCardinality +=
modelPlotConfigFieldCount > 0 ? modelPlotConfigFieldCount : field.stats.cardinality!;

if (isInvalid(field.stats.cardinality!)) {
messages.push({
id: messageId || (`cardinality_${type}_field` as MessageId),
fieldName: uniqueFieldName,
});
}
} else {
// only report uniqueFieldName as not aggregatable if it's not part
// of a valid categorization configuration and if it's not a scripted field or runtime mapping.
if (
!isValidCategorizationConfig(job, uniqueFieldName) &&
!isScriptField(job, uniqueFieldName) &&
!isRuntimeMapping(job, uniqueFieldName)
) {
if (stats.totalCount === 0) {
messages.push({
id: 'cardinality_no_results',
});
} else {
uniqueFieldNames.forEach((uniqueFieldName) => {
const aggregatableNotExistsField = stats.aggregatableNotExistsFields.find(
(fieldData) => fieldData.fieldName === uniqueFieldName
);

if (aggregatableNotExistsField !== undefined) {
messages.push({
id: 'field_not_aggregatable',
id: 'cardinality_field_not_exists',
fieldName: uniqueFieldName,
});
} else {
const field = stats.aggregatableExistsFields.find(
(fieldData) => fieldData.fieldName === uniqueFieldName
);
if (field !== undefined && typeof field === 'object' && field.stats) {
modelPlotCardinality +=
modelPlotConfigFieldCount > 0
? modelPlotConfigFieldCount
: field.stats.cardinality!;

if (isInvalid(field.stats.cardinality!)) {
messages.push({
id: messageId || (`cardinality_${type}_field` as MessageId),
fieldName: uniqueFieldName,
});
}
} else {
// only report uniqueFieldName as not aggregatable if it's not part
// of a valid categorization configuration and if it's not a scripted field or runtime mapping.
if (
!isValidCategorizationConfig(job, uniqueFieldName) &&
!isScriptField(job, uniqueFieldName) &&
!isRuntimeMapping(job, uniqueFieldName)
) {
messages.push({
id: 'field_not_aggregatable',
fieldName: uniqueFieldName,
});
}
}
}
}
});
});
}
} catch (e) {
// checkAggregatableFieldsExist may return an error if 'fielddata' is
// disabled for text fields (which is the default). If there was only
Expand Down