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

[ML] Fix errors from annotations searches when event mapping is incorrect (#116101)

Browse source 
* [ML] Fix errors from annotations searches when event mapping is incorrect

* [ML] Delete tests on annotation errors due to incorrect mappings

* [ML] Jest test fix and remove unused servuce method

* [ML] type fix

* [ML] Edits following review
This commit is contained in:
Pete Harverson 2021-10-26 14:02:43 +01:00 committed by GitHub
parent f5463ceaeb
commit 9c92ac881a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 37 additions and 274 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
x-pack/plugins/ml/common/types/annotations.ts
View file

@ -118,26 +118,8 @@ export function isAnnotations(arg: any): arg is Annotations {
return arg.every((d: Annotation) => isAnnotation(d));
}
export interface FieldToBucket {
field: string;
missing?: string | number;
}
export interface FieldToBucketResult {
key: string;
doc_count: number;
}
export interface TermAggregationResult {
doc_count_error_upper_bound: number;
sum_other_doc_count: number;
buckets: FieldToBucketResult[];
}
export type EsAggregationResult = Record<string, TermAggregationResult>;
export interface GetAnnotationsResponse {
aggregations?: EsAggregationResult;
totalCount: number;
annotations: Record<string, Annotations>;
error?: string;
success: boolean;
@ -145,6 +127,5 @@ export interface GetAnnotationsResponse {
export interface AnnotationsTable {
annotationsData: Annotations;
aggregations: EsAggregationResult;
error?: string;
}

56
x-pack/plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js
View file

@ -81,7 +81,6 @@ class AnnotationsTableUI extends Component {
super(props);
this.state = {
annotations: [],
aggregations: null,
isLoading: false,
queryText: `event:(${ANNOTATION_EVENT_USER} or ${ANNOTATION_EVENT_DELAYED_DATA})`,
searchError: undefined,
@ -115,18 +114,11 @@ class AnnotationsTableUI extends Component {
earliestMs: null,
latestMs: null,
maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
fields: [
{
field: 'event',
missing: ANNOTATION_EVENT_USER,
},
],
})
.toPromise()
.then((resp) => {
this.setState((prevState, props) => ({
annotations: resp.annotations[props.jobs[0].job_id] || [],
aggregations: resp.aggregations,
errorMessage: undefined,
isLoading: false,
jobId: props.jobs[0].job_id,
@ -570,41 +562,35 @@ class AnnotationsTableUI extends Component {
onMouseLeave: () => this.onMouseLeaveRow(),
};
};
let filterOptions = [];
const aggregations = this.props.aggregations ?? this.state.aggregations;
if (aggregations) {
const buckets = aggregations.event.buckets;
let foundUser = false;
let foundDelayedData = false;
buckets.forEach((bucket) => {
if (bucket.key === ANNOTATION_EVENT_USER) {
foundUser = true;
}
if (bucket.key === ANNOTATION_EVENT_DELAYED_DATA) {
foundDelayedData = true;
}
});
const adjustedBuckets = [];
if (!foundUser) {
adjustedBuckets.push({ key: ANNOTATION_EVENT_USER, doc_count: 0 });
}
if (!foundDelayedData) {
adjustedBuckets.push({ key: ANNOTATION_EVENT_DELAYED_DATA, doc_count: 0 });
}
// Build the options to show in the Event type filter.
// Do not try and run a search using a terms agg on the event field
// because in 7.9 this field was incorrectly mapped as a text rather than keyword.
// Always display options for user and delayed data types.
const countsByEvent = {
[ANNOTATION_EVENT_USER]: 0,
[ANNOTATION_EVENT_DELAYED_DATA]: 0,
};
annotations.forEach((annotation) => {
// Default to user type for annotations created in early releases which didn't have an event field
const event = annotation.event ?? ANNOTATION_EVENT_USER;
if (countsByEvent[event] === undefined) {
countsByEvent[event] = 0;
}
countsByEvent[event]++;
});
filterOptions = [...adjustedBuckets, ...buckets];
}
const filters = [
{
type: 'field_value_selection',
field: 'event',
name: 'Event',
multiSelect: 'or',
options: filterOptions.map((field) => ({
value: field.key,
name: field.key,
view: `${field.key} (${field.doc_count})`,
options: Object.entries(countsByEvent).map(([key, docCount]) => ({
value: key,
name: key,
view: `${key} (${docCount})`,
})),
'data-test-subj': 'mlAnnotationTableEventFilter',
},

7
x-pack/plugins/ml/public/application/explorer/explorer.js
View file

@ -255,13 +255,9 @@ export class ExplorerUI extends React.Component {
tableData,
swimLaneSeverity,
} = this.props.explorerState;
const { annotationsData, aggregations, error: annotationsError } = annotations;
const { annotationsData, totalCount: allAnnotationsCnt, error: annotationsError } = annotations;
const annotationsCnt = Array.isArray(annotationsData) ? annotationsData.length : 0;
const allAnnotationsCnt = Array.isArray(aggregations?.event?.buckets)
? aggregations.event.buckets.reduce((acc, v) => acc + v.doc_count, 0)
: annotationsCnt;
const badge =
allAnnotationsCnt > annotationsCnt ? (
<EuiBadge color={'hollow'}>
@ -449,7 +445,6 @@ export class ExplorerUI extends React.Component {
<AnnotationsTable
jobIds={selectedJobIds}
annotations={annotationsData}
aggregations={aggregations}
drillDown={true}
numberBadge={false}
/>

18
x-pack/plugins/ml/public/application/explorer/explorer_utils.js
View file

@ -35,7 +35,6 @@ import {
SWIMLANE_TYPE,
VIEW_BY_JOB_LABEL,
} from './explorer_constants';
import { ANNOTATION_EVENT_USER } from '../../../common/constants/annotations';
// create new job objects based on standard job config objects
// new job objects just contain job id, bucket span in seconds and a selected flag.
@ -437,10 +436,7 @@ export function loadOverallAnnotations(selectedJobs, interval, bounds) {
}
export function loadAnnotationsTableData(selectedCells, selectedJobs, interval, bounds) {
const jobIds =
selectedCells !== undefined && selectedCells.viewByFieldName === VIEW_BY_JOB_LABEL
? selectedCells.lanes
: selectedJobs.map((d) => d.id);
const jobIds = getSelectionJobIds(selectedCells, selectedJobs);
const timeRange = getSelectionTimeRange(selectedCells, interval, bounds);
return new Promise((resolve) => {
@ -450,12 +446,6 @@ export function loadAnnotationsTableData(selectedCells, selectedJobs, interval,
earliestMs: timeRange.earliestMs,
latestMs: timeRange.latestMs,
maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
fields: [
{
field: 'event',
missing: ANNOTATION_EVENT_USER,
},
],
})
.toPromise()
.then((resp) => {
@ -463,7 +453,7 @@ export function loadAnnotationsTableData(selectedCells, selectedJobs, interval,
const errorMessage = extractErrorMessage(resp.error);
return resolve({
annotationsData: [],
aggregations: {},
totalCount: 0,
error: errorMessage !== '' ? errorMessage : undefined,
});
}
@ -485,14 +475,14 @@ export function loadAnnotationsTableData(selectedCells, selectedJobs, interval,
d.key = (i + 1).toString();
return d;
}),
aggregations: resp.aggregations,
totalCount: resp.totalCount,
});
})
.catch((resp) => {
const errorMessage = extractErrorMessage(resp);
return resolve({
annotationsData: [],
aggregations: {},
totalCount: 0,
error: errorMessage !== '' ? errorMessage : undefined,
});
});

2
x-pack/plugins/ml/public/application/explorer/reducers/explorer_reducer/state.ts
View file

@ -71,12 +71,10 @@ export function getExplorerDefaultState(): ExplorerState {
overallAnnotations: {
error: undefined,
annotationsData: [],
aggregations: {},
},
annotations: {
error: undefined,
annotationsData: [],
aggregations: {},
},
anomalyChartsDataLoading: true,
chartsData: getDefaultChartsData(),

8
x-pack/plugins/ml/public/application/services/ml_api_service/annotations.ts
View file

@ -5,11 +5,7 @@
* 2.0.
*/
import {
Annotation,
FieldToBucket,
GetAnnotationsResponse,
} from '../../../../common/types/annotations';
import { Annotation, GetAnnotationsResponse } from '../../../../common/types/annotations';
import { http, http$ } from '../http_service';
import { basePath } from './index';
@ -19,7 +15,6 @@ export const annotations = {
earliestMs: number;
latestMs: number;
maxAnnotations: number;
fields?: FieldToBucket[];
detectorIndex?: number;
entities?: any[];
}) {
@ -36,7 +31,6 @@ export const annotations = {
earliestMs: number | null;
latestMs: number | null;
maxAnnotations: number;
fields?: FieldToBucket[];
detectorIndex?: number;
entities?: any[];
}) {

7
x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx
View file

@ -15,7 +15,6 @@ import { extractErrorMessage } from '../../../../../common/util/errors';
import { Annotation } from '../../../../../common/types/annotations';
import { useMlKibana, useNotifications } from '../../../contexts/kibana';
import { getBoundsRoundedToInterval } from '../../../util/time_buckets';
import { ANNOTATION_EVENT_USER } from '../../../../../common/constants/annotations';
import { getControlsForDetector } from '../../get_controls_for_detector';
import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context';
@ -88,12 +87,6 @@ export const TimeSeriesChartWithTooltips: FC<TimeSeriesChartWithTooltipsProps> =
earliestMs: searchBounds.min.valueOf(),
latestMs: searchBounds.max.valueOf(),
maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
fields: [
{
field: 'event',
missing: ANNOTATION_EVENT_USER,
},
],
detectorIndex,
entities: nonBlankEntities,
});

3
x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js
View file

@ -104,7 +104,6 @@ function getTimeseriesexplorerDefaultState() {
entitiesLoading: false,
entityValues: {},
focusAnnotationData: [],
focusAggregations: {},
focusAggregationInterval: {},
focusChartData: undefined,
focusForecastData: undefined,
@ -935,7 +934,6 @@ export class TimeSeriesExplorer extends React.Component {
focusAggregationInterval,
focusAnnotationError,
focusAnnotationData,
focusAggregations,
focusChartData,
focusForecastData,
fullRefresh,
@ -1257,7 +1255,6 @@ export class TimeSeriesExplorer extends React.Component {
detectors={detectors}
jobIds={[this.props.selectedJobId]}
annotations={focusAnnotationData}
aggregations={focusAggregations}
isSingleMetricViewerLinkVisible={false}
isNumberBadgeVisible={true}
/>

13
x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/get_focus_data.ts
View file

@ -26,7 +26,6 @@ import {
import { mlForecastService } from '../../services/forecast_service';
import { mlFunctionToESAggregation } from '../../../../common/util/job_utils';
import { GetAnnotationsResponse } from '../../../../common/types/annotations';
import { ANNOTATION_EVENT_USER } from '../../../../common/constants/annotations';
import { aggregationTypeTransform } from '../../../../common/util/anomaly_utils';
export interface Interval {
@ -42,7 +41,6 @@ export interface FocusData {
focusAnnotationError?: string;
focusAnnotationData?: any[];
focusForecastData?: any;
focusAggregations?: any;
}
export function getFocusData(
@ -98,12 +96,6 @@ export function getFocusData(
earliestMs: searchBounds.min.valueOf(),
latestMs: searchBounds.max.valueOf(),
maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE,
fields: [
{
field: 'event',
missing: ANNOTATION_EVENT_USER,
},
],
detectorIndex,
entities: nonBlankEntities,
})
@ -111,7 +103,7 @@ export function getFocusData(
catchError((resp) =>
of({
annotations: {},
aggregations: {},
totalCount: 0,
error: extractErrorMessage(resp),
success: false,
} as GetAnnotationsResponse)
@ -168,7 +160,6 @@ export function getFocusData(
if (annotations.error !== undefined) {
refreshFocusData.focusAnnotationError = annotations.error;
refreshFocusData.focusAnnotationData = [];
refreshFocusData.focusAggregations = {};
} else {
refreshFocusData.focusAnnotationData = (annotations.annotations[selectedJob.job_id] ?? [])
.sort((a, b) => {
@ -178,8 +169,6 @@ export function getFocusData(
d.key = (i + 1).toString();
return d;
});
refreshFocusData.focusAggregations = annotations.aggregations;
}
}

1
x-pack/plugins/ml/server/models/annotation_service/__mocks__/get_annotations_request.json
View file

@ -1,6 +1,7 @@
{
"index": ".ml-annotations-read",
"size": 500,
"track_total_hits": true,
"body": {
"query": {
"bool": {

38
x-pack/plugins/ml/server/models/annotation_service/annotation.ts
View file

@ -24,7 +24,6 @@ import {
isAnnotations,
getAnnotationFieldName,
getAnnotationFieldValue,
EsAggregationResult,
} from '../../../common/types/annotations';
import { JobId } from '../../../common/types/anomaly_detection_jobs';
@ -35,36 +34,27 @@ interface EsResult {
_id: string;
}
export interface FieldToBucket {
field: string;
missing?: string | number;
}
export interface IndexAnnotationArgs {
jobIds: string[];
earliestMs: number | null;
latestMs: number | null;
maxAnnotations: number;
fields?: FieldToBucket[];
detectorIndex?: number;
entities?: any[];
event?: Annotation['event'];
}
export interface AggTerm {
terms: FieldToBucket;
}
export interface GetParams {
index: string;
size: number;
body: object;
track_total_hits: boolean;
}
export interface GetResponse {
success: true;
annotations: Record<JobId, Annotations>;
aggregations: EsAggregationResult;
totalCount: number;
}
export interface IndexParams {
@ -118,7 +108,6 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
earliestMs,
latestMs,
maxAnnotations,
fields,
detectorIndex,
entities,
event,
@ -126,7 +115,7 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
const obj: GetResponse = {
success: true,
annotations: {},
aggregations: {},
totalCount: 0,
};
const boolCriteria: object[] = [];
@ -215,18 +204,6 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
});
}
// Find unique buckets (e.g. events) from the queried annotations to show in dropdowns
const aggs: Record<string, AggTerm> = {};
if (fields) {
fields.forEach((fieldToBucket) => {
aggs[fieldToBucket.field] = {
terms: {
...fieldToBucket,
},
};
});
}
// Build should clause to further query for annotations in SMV
// we want to show either the exact match with detector index and by/over/partition fields
// OR annotations without any partition fields defined
@ -276,6 +253,7 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
const params: GetParams = {
index: ML_ANNOTATIONS_INDEX_ALIAS_READ,
size: maxAnnotations,
track_total_hits: true,
body: {
query: {
bool: {
@ -295,7 +273,6 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
...(shouldClauses ? { should: shouldClauses, minimum_should_match: 1 } : {}),
},
},
...(fields ? { aggs } : {}),
},
};
@ -308,6 +285,9 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
throw new Error(`Annotations couldn't be retrieved from Elasticsearch.`);
}
// @ts-expect-error incorrect search response type
obj.totalCount = body.hits.total.value;
// @ts-expect-error TODO fix search response types
const docs: Annotations = get(body, ['hits', 'hits'], []).map((d: EsResult) => {
// get the original source document and the document id, we need it
@ -321,10 +301,6 @@ export function annotationProvider({ asInternalUser }: IScopedClusterClient) {
} as Annotation;
});
const aggregations = get(body, ['aggregations'], {}) as EsAggregationResult;
if (fields) {
obj.aggregations = aggregations;
}
if (isAnnotations(docs) === false) {
// No need to translate, this will not be exposed in the UI.
throw new Error(`Annotations didn't pass integrity check.`);

51
x-pack/test/functional/apps/ml/anomaly_detection/annotations.ts
View file

@ -262,56 +262,5 @@ export default function ({ getService }: FtrProviderContext) {
await ml.jobAnnotations.assertAnnotationsRowMissing(annotationId);
});
});
// FAILING ES PROMOTION: https://github.com/elastic/kibana/issues/115849
describe.skip('with errors', function () {
before(async () => {
// Points the read/write aliases of annotations to an index with wrong mappings
// so we can simulate errors when requesting annotations.
await ml.testResources.setupBrokenAnnotationsIndexState(jobId);
});
it('displays error on broken annotation index and recovers after fix', async () => {
await ml.testExecution.logTestStep('loads from job list row link');
await ml.navigation.navigateToMl();
await ml.navigation.navigateToJobManagement();
await ml.jobTable.waitForJobsToLoad();
await ml.jobTable.filterWithSearchString(jobId, 1);
await ml.jobTable.clickOpenJobInSingleMetricViewerButton(jobId);
await ml.commonUI.waitForMlLoadingIndicatorToDisappear();
await ml.testExecution.logTestStep(
'should display the annotations section showing an error'
);
await ml.singleMetricViewer.assertAnnotationsExists('error');
await ml.testExecution.logTestStep('should navigate to anomaly explorer');
await ml.navigation.navigateToAnomalyExplorerViaSingleMetricViewer();
await ml.testExecution.logTestStep(
'should display the annotations section showing an error'
);
await ml.anomalyExplorer.assertAnnotationsPanelExists('error');
await ml.testExecution.logTestStep(
'should display the annotations section without an error'
);
// restores the aliases to point to the original working annotations index
// so we can run tests against successfully loaded annotations sections.
await ml.testResources.restoreAnnotationsIndexState();
await ml.anomalyExplorer.refreshPage();
await ml.anomalyExplorer.assertAnnotationsPanelExists('loaded');
await ml.testExecution.logTestStep('should navigate to single metric viewer');
await ml.navigation.navigateToSingleMetricViewerViaAnomalyExplorer();
await ml.testExecution.logTestStep(
'should display the annotations section without an error'
);
await ml.singleMetricViewer.assertAnnotationsExists('loaded');
});
});
});
}

86
x-pack/test/functional/services/ml/test_resources.ts
View file

@ -24,7 +24,6 @@ export enum SavedObjectType {
export type MlTestResourcesi = ProvidedType<typeof MachineLearningTestResourcesProvider>;
export function MachineLearningTestResourcesProvider({ getService }: FtrProviderContext) {
const es = getService('es');
const kibanaServer = getService('kibanaServer');
const log = getService('log');
const supertest = getService('supertest');
@ -188,91 +187,6 @@ export function MachineLearningTestResourcesProvider({ getService }: FtrProvider
}
},
async setupBrokenAnnotationsIndexState(jobId: string) {
// Creates a temporary annotations index with unsupported mappings.
await es.indices.create({
index: '.ml-annotations-6-wrong-mapping',
body: {
settings: {
number_of_shards: 1,
},
mappings: {
properties: {
field1: { type: 'text' },
},
},
},
});
// Ingests an annotation that will cause dynamic mapping to pick up the wrong field type.
es.create({
id: 'annotation_with_wrong_mapping',
index: '.ml-annotations-6-wrong-mapping',
body: {
annotation: 'Annotation with wrong mapping',
create_time: 1597393915910,
create_username: '_xpack',
timestamp: 1549756800000,
end_timestamp: 1549756800000,
job_id: jobId,
modified_time: 1597393915910,
modified_username: '_xpack',
type: 'annotation',
event: 'user',
detector_index: 0,
},
});
// Points the read/write aliases for annotations to the broken annotations index
// so we can run tests against a state where annotation endpoints return errors.
await es.indices.updateAliases({
body: {
actions: [
{
add: {
index: '.ml-annotations-6-wrong-mapping',
alias: '.ml-annotations-read',
is_hidden: true,
},
},
{ remove: { index: '.ml-annotations-6', alias: '.ml-annotations-read' } },
{
add: {
index: '.ml-annotations-6-wrong-mapping',
alias: '.ml-annotations-write',
is_hidden: true,
},
},
{ remove: { index: '.ml-annotations-6', alias: '.ml-annotations-write' } },
],
},
});
},
async restoreAnnotationsIndexState() {
// restore the original working state of pointing read/write aliases
// to the right annotations index.
await es.indices.updateAliases({
body: {
actions: [
{ add: { index: '.ml-annotations-6', alias: '.ml-annotations-read', is_hidden: true } },
{ remove: { index: '.ml-annotations-6-wrong-mapping', alias: '.ml-annotations-read' } },
{
add: { index: '.ml-annotations-6', alias: '.ml-annotations-write', is_hidden: true },
},
{
remove: { index: '.ml-annotations-6-wrong-mapping', alias: '.ml-annotations-write' },
},
],
},
});
// deletes the temporary annotations index with wrong mappings
await es.indices.delete({
index: '.ml-annotations-6-wrong-mapping',
});
},
async updateSavedSearchRequestBody(body: object, indexPatternTitle: string): Promise<object> {
const indexPatternId = await this.getIndexPatternId(indexPatternTitle);
if (indexPatternId === undefined) {