Skip to content

Commit

Permalink
[ML] Update datafeedAggregations check
Browse files Browse the repository at this point in the history
  • Loading branch information
qn895 committed Nov 5, 2020
1 parent 9769d94 commit f54c8e8
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 12 deletions.
22 changes: 22 additions & 0 deletions x-pack/plugins/ml/common/util/datafeed_utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* 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.
*/

import { Aggregation, Datafeed } from '../types/anomaly_detection_jobs';

export const getDatafeedAggregations = (
datafeedConfig: Partial<Datafeed> | undefined
): Aggregation | undefined => {
if (datafeedConfig?.aggregations !== undefined) return datafeedConfig.aggregations;
if (datafeedConfig?.aggs !== undefined) return datafeedConfig.aggs;
return undefined;
};

export const getAggregationBucketsName = (aggregations: any): string | undefined => {
if (typeof aggregations === 'object') {
const keys = Object.keys(aggregations);
return keys.length > 0 ? keys[0] : undefined;
}
};
6 changes: 4 additions & 2 deletions x-pack/plugins/ml/common/util/job_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { MlServerLimits } from '../types/ml_server_info';
import { JobValidationMessage, JobValidationMessageId } from '../constants/messages';
import { ES_AGGREGATION, ML_JOB_AGGREGATION } from '../constants/aggregation_types';
import { MLCATEGORY } from '../constants/field_types';
import { getDatafeedAggregations } from './datafeed_utils';

export interface ValidationResults {
valid: boolean;
Expand Down Expand Up @@ -564,8 +565,9 @@ export function basicJobAndDatafeedValidation(job: Job, datafeed: Datafeed): Val
let valid = true;

if (datafeed && job) {
const datafeedAggConfig = datafeed.aggregations ?? datafeed?.aggs;
if (datafeedAggConfig !== undefined && !job.analysis_config?.summary_count_field_name) {
const datafeedAggregations = getDatafeedAggregations(datafeed);

if (datafeedAggregations !== undefined && !job.analysis_config?.summary_count_field_name) {
valid = false;
messages.push({ id: 'missing_summary_count_field_name' });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '../../lib/query_utils';
import { AggCardinality } from '../../../common/types/fields';
import { DatafeedOverride } from '../../../common/types/modules';
import { getDatafeedAggregations } from '../../../common/util/datafeed_utils';

const SAMPLER_TOP_TERMS_THRESHOLD = 100000;
const SAMPLER_TOP_TERMS_SHARD_SIZE = 5000;
Expand Down Expand Up @@ -599,11 +600,11 @@ export class DataVisualizer {
const index = indexPatternTitle;
const size = 0;
const filterCriteria = buildBaseFilterCriteria(timeFieldName, earliestMs, latestMs, query);
const datafeedAggConfig = datafeedConfig?.aggregations ?? datafeedConfig?.aggs;
const datafeedAggregations = getDatafeedAggregations(datafeedConfig);

// Value count aggregation faster way of checking if field exists than using
// filter aggregation with exists query.
const aggs: Aggs = datafeedAggConfig !== undefined ? { ...datafeedAggConfig } : {};
const aggs: Aggs = datafeedAggregations !== undefined ? { ...datafeedAggregations } : {};

aggregatableFields.forEach((field, i) => {
const safeFieldName = getSafeAggregationName(field, i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { initCardinalityFieldsCache } from './fields_aggs_cache';
import { DatafeedOverride } from '../../../common/types/modules';
import { AggCardinality } from '../../../common/types/fields';
import { isValidAggregationField } from '../../../common/util/validation_utils';
import { getDatafeedAggregations } from '../../../common/util/datafeed_utils';

/**
* Service for carrying out queries to obtain data
Expand Down Expand Up @@ -46,7 +47,8 @@ export function fieldsServiceProvider({ asCurrentUser }: IScopedClusterClient) {
fields: fieldNames,
});
const aggregatableFields: string[] = [];
const datafeedAggConfig = datafeedConfig?.aggregations ?? datafeedConfig?.aggs;
const datafeedAggregations = getDatafeedAggregations(datafeedConfig);

fieldNames.forEach((fieldName) => {
if (
typeof datafeedConfig?.script_fields === 'object' &&
Expand All @@ -55,8 +57,8 @@ export function fieldsServiceProvider({ asCurrentUser }: IScopedClusterClient) {
aggregatableFields.push(fieldName);
}
if (
datafeedAggConfig !== undefined &&
isValidAggregationField(datafeedAggConfig, fieldName)
datafeedAggregations !== undefined &&
isValidAggregationField(datafeedAggregations, fieldName)
) {
aggregatableFields.push(fieldName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { validateTimeRange, isValidTimeField } from './validate_time_range';
import { validateJobSchema } from '../../routes/schemas/job_validation_schema';
import { CombinedJob } from '../../../common/types/anomaly_detection_jobs';
import type { MlClient } from '../../lib/ml_client';
import { getDatafeedAggregations } from '../../../common/util/datafeed_utils';

export type ValidateJobPayload = TypeOf<typeof validateJobSchema>;

Expand Down Expand Up @@ -102,8 +103,8 @@ export async function validateJob(
}

// if datafeed has aggregation, require job config to include a valid summary_doc_field_name
const datafeedAggConfig = job.datafeed_config?.aggregations ?? job.datafeed_config?.aggs;
if (datafeedAggConfig !== undefined && !job.analysis_config?.summary_count_field_name) {
const datafeedAggregations = getDatafeedAggregations(job.datafeed_config);
if (datafeedAggregations !== undefined && !job.analysis_config?.summary_count_field_name) {
validationMessages.push({ id: 'missing_summary_count_field_name' });
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { CombinedJob } from '../../../common/types/anomaly_detection_jobs';
import { Detector } from '../../../common/types/anomaly_detection_jobs';
import { MessageId, JobValidationMessage } from '../../../common/constants/messages';
import { isValidAggregationField } from '../../../common/util/validation_utils';
import { getDatafeedAggregations } from '../../../common/util/datafeed_utils';

function isValidCategorizationConfig(job: CombinedJob, fieldName: string): boolean {
return (
Expand Down Expand Up @@ -80,7 +81,7 @@ const validateFactory = (client: IScopedClusterClient, job: CombinedJob): Valida
index: job.datafeed_config.indices.join(','),
fields: uniqueFieldNames,
});
const datafeedAggConfig = datafeedConfig?.aggregations ?? datafeedConfig?.aggs;
const datafeedAggregations = getDatafeedAggregations(datafeedConfig);

let aggregatableFieldNames: string[] = [];
// parse fieldCaps to return an array of just the fields which are aggregatable
Expand All @@ -94,8 +95,8 @@ const validateFactory = (client: IScopedClusterClient, job: CombinedJob): Valida
}
// if datafeed has aggregation fields, check recursively if field exist
if (
datafeedAggConfig !== undefined &&
isValidAggregationField(datafeedAggConfig, fieldName)
datafeedAggregations !== undefined &&
isValidAggregationField(datafeedAggregations, fieldName)
) {
return true;
}
Expand Down

0 comments on commit f54c8e8

Please sign in to comment.