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

Conversation

qn895
Copy link
Member

@qn895 qn895 commented Jun 29, 2020

Summary

This PR brings several enhancements from meta issue #69538

UI

  • Collapse the annotation panel by default in both Single Metric Viewer and Anomaly Explorer (with count for how many annotations)

Screen Shot 2020-06-29 at 8 52 54 AM

Screen Shot 2020-06-29 at 8 54 38 AM

  • Put anomalies content (charts and table) inside a EuiPanel

Screen Shot 2020-06-29 at 8 50 54 AM

  • Show only user annotations by default

Screen Shot 2020-06-30 at 12 58 41 PM

If there's no user annotations, it will show user (0) selected

Screen Shot 2020-06-30 at 12 59 46 PM

  • Add a control to allow the user to also show the system generated annotations

Screen Shot 2020-06-29 at 8 50 10 AM

  • Increase the width for Event so that it can show the full message

  • Add search bar

  • Add ability to filter annotations by partitioning field name/values

  • Filter the annotations table in the Single Metric Viewer to show annotations for the selected partitioning fields only
    2020-06-30 at 1 03 PM

  • Add checkbox in the creation fly-out to indicate whether or not the annotation applies to only that partition or the whole job. This will record the following properties if they exist in the chartProperties:

    • detector_index
    • partition_field_value
    • over_field_value
    • by_field_value

Screen Shot 2020-06-30 at 1 06 13 PM

Service

Checklist

Delete any items that are not applicable to this PR.

@qn895 qn895 added enhancement New value added to drive a business result :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 v7.9.0 labels Jun 29, 2020
@qn895 qn895 self-assigned this Jun 29, 2020
[ML] Fix detector column name
…ions-enhancements

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@qn895 qn895 added release_note:enhancement and removed enhancement New value added to drive a business result labels Jun 30, 2020
@qn895 qn895 marked this pull request as ready for review June 30, 2020 18:09
@qn895 qn895 requested a review from a team as a code owner June 30, 2020 18:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Great progress overall! I added some questions.

One more suggestion here since it applies to the PR overall:
aggregations seemed to me like a very generic name for what we're now returning as part of the annotations results, it's quite a generic name and internal Elasticsearch concept. I wonder if we could rename it to somewhat more specific that reflects the type of data we're returning (types of events in this case so far)

@peteharverson
Copy link
Contributor

Might not be directly related to the changes in this PR, but the message here should be altered to say something like No annotations for the selected time range. There are annotations for this job, just not for the time period displayed in the main focus chart area (indication outside time range is covered by #54025).

image

@qn895 qn895 changed the title [ML] WIP Anomaly Detection: Annotations enhancements [ML] Anomaly Detection: Annotations enhancements Jul 1, 2020
[ML] Refactor ANNOTATION_EVENT_USER & minor fixes
@peteharverson
Copy link
Contributor

Not needed for this PR, but a nice follow-up would be to allow filtering from the partitioning field cells in the annotations table in the Anomaly Explorer. Here I wanted to quickly add a filter for the 193.106.172.144 mentioned in the annotation:

image

qn895 added 3 commits July 8, 2020 14:40
…ions-enhancements

# Conflicts:
#	x-pack/plugins/ml/public/application/explorer/actions/load_explorer_data.ts
#	x-pack/plugins/ml/public/application/explorer/explorer.js
#	x-pack/plugins/ml/public/application/explorer/explorer_utils.js
#	x-pack/plugins/ml/public/application/explorer/reducers/explorer_reducer/reducer.ts
#	x-pack/plugins/ml/public/application/explorer/reducers/explorer_reducer/state.ts
- Add partitioning fields & detector to annotation flyout
- Fix annotation layout overlay in AE
- Add direct link to SMV with partitioning fields in detector
- Update width for partitioning columns
- Show all partitioning columns for both SMV and AE
- Fix bug with annotation not editable by finding the original annotation
- When updating system generated annotation, persist the `event` field
@qn895
Copy link
Member Author

qn895 commented Jul 13, 2020

@elasticmachine merge upstream

@qn895
Copy link
Member Author

qn895 commented Jul 13, 2020

@elasticmachine merge upstream

…ions-enhancements

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
…ions-enhancements

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
…ions-enhancements

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest changes and LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

…ions-enhancements

# Conflicts:
#	x-pack/plugins/ml/server/models/results_service/get_partition_fields_values.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
ml 390 +1 389

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@qn895 qn895 merged commit c24f180 into elastic:master Jul 14, 2020
@qn895 qn895 deleted the transform-annotations-enhancements branch July 14, 2020 17:36
qn895 added a commit to qn895/kibana that referenced this pull request Jul 14, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
qn895 added a commit that referenced this pull request Jul 14, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants