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: Annotations enhancements #70198

Merged
merged 34 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ff3f180
[ML] Improve Annotations
qn895 Jun 29, 2020
9ffddf3
[ML] Clean up logs & update snapshot
qn895 Jun 29, 2020
e517350
[ML] Add unique ID
qn895 Jun 29, 2020
20ce417
[ML] Add user(0) if no bucket for user found
qn895 Jun 29, 2020
2501b5f
[ML] Add dynamic columns to annotation table
qn895 Jun 29, 2020
e0ce77f
[ML] Refactor unique aggregations to bundle with getAnnotations
qn895 Jun 30, 2020
f96e042
[ML] Fix type issue & aria obj warnings
qn895 Jun 30, 2020
4877939
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jun 30, 2020
ffd474a
[ML] Update snapshot for annotation_table
qn895 Jun 30, 2020
2ee31d4
[ML] Add EUI panel around Annotations in Anomaly Explorer
qn895 Jul 1, 2020
36a5a44
[ML] Refactor ANNOTATION_EVENT_USER & minor fixes
qn895 Jul 1, 2020
def94c3
[ML] Fix annotations_table queryText
qn895 Jul 2, 2020
bbdbec6
[ML] Fix types of annotation fields & add Total badge
qn895 Jul 2, 2020
55aa5c8
[ML] Add internationalization to series
qn895 Jul 2, 2020
a56fa66
[ML] Fix partition field type
qn895 Jul 2, 2020
07e396d
[ML] Remove Annotations panel if no focus annotations data
qn895 Jul 2, 2020
002eaf3
[ML] Remove annotations in SMV if no annotations
qn895 Jul 2, 2020
5b6249d
[ML] Refactor to make the toggle button visibly active
qn895 Jul 2, 2020
010c096
[ML] Refactor to make the toggle button visibly active
qn895 Jul 2, 2020
0d945fa
[ML] Update snapshot for annotations_table
qn895 Jul 2, 2020
b69be14
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jul 8, 2020
1f2684d
[ML] Pete's feedback
qn895 Jul 8, 2020
e57889f
[ML] Update snapshot, fix i18n, persist event field when editing
qn895 Jul 8, 2020
dfe3950
[ML] Remove partitions if apply to series is unchecked
qn895 Jul 9, 2020
aa98817
[ML] Dynamic partition columns for SMV & default query to include del…
qn895 Jul 9, 2020
02024e5
[ML] Fix annotation table in job list not having terms
qn895 Jul 9, 2020
0e24cb3
[ML] Change SMV annotations to only show series + non-partitioned
qn895 Jul 9, 2020
687fed9
Merge branch 'master' into transform-annotations-enhancements
elasticmachine Jul 13, 2020
de5e991
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jul 13, 2020
fe58c7d
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jul 13, 2020
c06b83b
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jul 13, 2020
4871783
[ML] Fix issue with EUI upgrade and current_series table
qn895 Jul 14, 2020
ad7a947
[ML] Update snapshot for annotations_table
qn895 Jul 14, 2020
00b6c88
Merge remote-tracking branch 'upstream/master' into transform-annotat…
qn895 Jul 14, 2020
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
33 changes: 32 additions & 1 deletion x-pack/plugins/ml/common/types/annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,15 @@ export interface Annotation {
annotation: string;
job_id: string;
type: ANNOTATION_TYPE.ANNOTATION | ANNOTATION_TYPE.COMMENT;
event?: string;
walterra marked this conversation as resolved.
Show resolved Hide resolved
detector_index?: number;
partition_field_name?: string;
partition_field_value?: string;
over_field_name?: string;
over_field_value?: string;
by_field_name?: string;
by_field_value?: string;
}

export function isAnnotation(arg: any): arg is Annotation {
return (
arg.timestamp !== undefined &&
Expand All @@ -93,3 +100,27 @@ export function isAnnotations(arg: any): arg is Annotations {
}
return arg.every((d: Annotation) => isAnnotation(d));
}

export interface FieldToBucket {
Copy link
Contributor

Choose a reason for hiding this comment

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

The interfaces defined here will be applicable for other types than annotations I'm sure, but till we hit that use case, probably makes sense to leave them defined in here. I had a quick look to see if I could find something similar in our existing types, and the Kibana data plugin, but couldn't find anything.

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;
annotations: Record<string, Annotations>;
success: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import React, { Component, Fragment, FC, ReactNode } from 'react';
import useObservable from 'react-use/lib/useObservable';
import * as Rx from 'rxjs';
import { cloneDeep } from 'lodash';

import {
EuiButton,
Expand All @@ -21,12 +22,12 @@ import {
EuiSpacer,
EuiTextArea,
EuiTitle,
EuiCheckbox,
} from '@elastic/eui';

import { CommonProps } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';

import { ANNOTATION_MAX_LENGTH_CHARS } from '../../../../../common/constants/annotations';
import {
annotation$,
Expand All @@ -35,21 +36,32 @@ import {
} from '../../../services/annotations_service';
import { AnnotationDescriptionList } from '../annotation_description_list';
import { DeleteAnnotationModal } from '../delete_annotation_modal';

import { ml } from '../../../services/ml_api_service';
import { getToastNotifications } from '../../../util/dependency_cache';
interface Entity {
fieldName: string;
fieldType: string;
fieldValue: string;
}

interface Props {
annotation: AnnotationState;
chartDetails: {
entityData: { entities: Entity[] };
functionLabel: string;
};
detectorIndex: number;
}

interface State {
isDeleteModalVisible: boolean;
applyAnnotationToSeries: boolean;
}

class AnnotationFlyoutUI extends Component<CommonProps & Props> {
public state: State = {
isDeleteModalVisible: false,
applyAnnotationToSeries: false,
};

public annotationSub: Rx.Subscription | null = null;
Expand Down Expand Up @@ -150,11 +162,24 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
};

public saveOrUpdateAnnotation = () => {
const { annotation } = this.props;

if (annotation === null) {
const { annotation: originalAnnotation, chartDetails, detectorIndex } = this.props;
if (originalAnnotation === null) {
return;
}
const annotation = cloneDeep(originalAnnotation);

if (this.state.applyAnnotationToSeries && chartDetails?.entityData?.entities) {
chartDetails.entityData.entities.forEach((entity: Entity) => {
const { fieldName, fieldType, fieldValue } = entity;
// @ts-ignore
annotation[`${fieldType}_name`] = fieldName;
// @ts-ignore
annotation[`${fieldType}_value`] = fieldValue;
walterra marked this conversation as resolved.
Show resolved Hide resolved
});
annotation.detector_index = detectorIndex;
}
// Mark the annotation created by `user` instead of automatically generated
annotation.event = 'user';
walterra marked this conversation as resolved.
Show resolved Hide resolved

annotation$.next(null);

Expand Down Expand Up @@ -286,6 +311,23 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
value={annotation.annotation}
/>
</EuiFormRow>
<EuiFormRow>
peteharverson marked this conversation as resolved.
Show resolved Hide resolved
<EuiCheckbox
id={'xpack.ml.timeSeriesExplorer.annotationFlyout.applyToPartition'}
walterra marked this conversation as resolved.
Show resolved Hide resolved
label={
<FormattedMessage
id="xpack.ml.timeSeriesExplorer.annotationFlyout.applyToPartitionTextLabel"
walterra marked this conversation as resolved.
Show resolved Hide resolved
defaultMessage="Apply annotation to this series"
/>
}
checked={this.state.applyAnnotationToSeries}
onChange={() =>
this.setState({
applyAnnotationToSeries: !this.state.applyAnnotationToSeries,
Copy link
Contributor

Choose a reason for hiding this comment

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

When editing a series level annotation, this checkbox should be initialized with the checked state. For me, it is normally opening unchecked.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now checked on by default here 1f2684d

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, if editing an existing series level annotation, if the user then unticks the 'apply to series' checkbox, we should clear the partitioning and detector index fields of the annotation.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

@qn895 , I'm not sure if this screenshot is still current. But I'm noticing the footer actions. The 'Delete' action should be next to the 'Update' button instead of even spacing between all buttons. You also do not need the cross next to 'Cancel'

For example... (checkmark not needed in your case)
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Tested latest edits, and the partitioning and detector fields are now removed as expected if I untick the 'apply to series' checkbox when editing an annotation.

})
}
/>
</EuiFormRow>
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading