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.

Sign up for GitHub

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

Jump to bottom

[ML] AIOps: Adds change point detection charts embeddable #162796

Merged
merged 38 commits into from Aug 8, 2023
Merged

[ML] AIOps: Adds change point detection charts embeddable #162796

merged 38 commits into from Aug 8, 2023

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jul 31, 2023 •
edited

Summary

Part of #161248

  • Adds Change point charts as an embeddable component
  • Allows attachment of selected partitions to a new / existing dashboard
    Aug-01-2023 14-46-49
  • Allows attachment of top N change points (with an ascending sorting by p_value)
    Aug-01-2023 14-55-10
  • Allows applying a persistent time range
  • Attachment to a case of a single change point / selected partitions and applied time range
    Aug-01-2023 15-07-33
  • Exposes EmbeddableChangePointChart embeddable component as part of the plugin start contract

Checklist

Delete any items that are not applicable to this PR.

@darnautov darnautov self-assigned this Jul 31, 2023
@darnautov darnautov added :ml release_note:feature Makes this part of the condensed release notes Team:ML Team label for ML (also use :ml) v8.10.0 Feature:Embeddables Relating to the Embeddable system Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis labels Aug 1, 2023
@qn895
Copy link
Member

qn895 commented Aug 7, 2023

Would be good to have a message what's the allowed range (e.g. between 1 and 50) for the max series of change point charts. Right now it's unclear what will give warning and what does not. Should the Attach button also be disabled when the value exceeds?

Screen.Recording.2023-08-07.at.11.46.44.mov

peteharverson
peteharverson reviewed Aug 7, 2023
View reviewed changes
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.

Gave this another test and apart from the disabling of the 'Attach' button when the number of series entered is invalid, all looks good.

@darnautov
Copy link
Contributor Author

Gave this another test and apart from the disabling of the 'Attach' button when the number of series entered is invalid, all looks good.

thanks for reviewing @peteharverson! Max series validation was added in 002e192

@darnautov
Copy link
Contributor Author

@elasticmachine merge upstream

peteharverson
peteharverson approved these changes Aug 8, 2023
View reviewed changes
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.

Other than one minor typo, latest changes LGTM

x-pack/plugins/aiops/public/hooks/use_filters_query.tsx Outdated Show resolved Hide resolved
@darnautov
fix typo, implement destroy method, set context menu id
29572a9
@darnautov
Merge remote-tracking branch 'origin/main' into ml-161248-change-poin…
829bbd7 
…t-embeddable-detail-view

# Conflicts:
#	x-pack/plugins/ml/kibana.jsonc
#	x-pack/plugins/ml/public/application/app.tsx
#	x-pack/plugins/ml/public/application/contexts/kibana/kibana_context.ts
#	x-pack/plugins/ml/public/plugin.ts
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 439 454 +15

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
aiops 0 3 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
aiops 540.3KB 566.7KB +26.4KB
ml 3.4MB 3.4MB +76.0B
total +26.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
aiops 6.2KB 7.8KB +1.6KB
ml 74.0KB 74.1KB +36.0B
total +1.6KB
Unknown metric groups

API count

id before after diff
aiops 60 63 +3

async chunk count

id before after diff
aiops 10 15 +5

ESLint disabled line counts

id before after diff
aiops 23 21 -2

References to deprecated APIs

id before after diff
aiops 13 21 +8

Total ESLint disabled count

id before after diff
aiops 23 21 -2

History

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

cc @darnautov

js-jankisalvi
js-jankisalvi approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@js-jankisalvi js-jankisalvi left a comment

Choose a reason for hiding this comment

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

Verified locally:

  • Add to case actions with different Cases privileges
  • comment stored as persistablestate

Works as expected 👍

@qn895
Copy link
Member

qn895 commented Aug 8, 2023

LGTM 🎉

@darnautov darnautov merged commit b62747e into elastic:main Aug 8, 2023
20 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 8, 2023
@darnautov darnautov deleted the ml-161248-change-point-embeddable-detail-view branch August 8, 2023 14:33
bryce-b pushed a commit to bryce-b/kibana that referenced this pull request Aug 9, 2023
@darnautov @bryce-b
[ML] AIOps: Adds change point detection charts embeddable (elastic#16…
f0bbde2 
…2796)

## Summary

Part of elastic#161248 

- Adds Change point charts as an embeddable component 
- Allows attachment of selected partitions to a new / existing dashboard
![Aug-01-2023
14-46-49](https://github.com/elastic/kibana/assets/5236598/cba12d6e-7e2b-481e-a788-18435a0a9840)
- Allows attachment of top N change points (with an ascending sorting by
`p_value`)
![Aug-01-2023
14-55-10](https://github.com/elastic/kibana/assets/5236598/d99e119d-c428-49d0-bc76-7a33a3ce4ffd)
- Allows applying a persistent time range 
- Attachment to a case of a single change point / selected partitions
and applied time range
![Aug-01-2023
15-07-33](https://github.com/elastic/kibana/assets/5236598/1c4cc131-0897-4fc4-8f45-e8118ab1f941)
- Exposes `EmbeddableChangePointChart` embeddable component as part of
the plugin start contract


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
@szabosteve szabosteve mentioned this pull request Aug 21, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szabosteve szabosteve szabosteve approved these changes

@peteharverson peteharverson peteharverson approved these changes

@qn895 qn895 qn895 approved these changes

@js-jankisalvi js-jankisalvi js-jankisalvi approved these changes

Assignees

@darnautov darnautov

Labels
backport:skip This commit does not require backporting Feature:Embeddables Relating to the Embeddable system Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :ml release_note:feature Makes this part of the condensed release notes Team:ML Team label for ML (also use :ml) v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@darnautov @elasticmachine @qn895 @kibana-ci @peteharverson @szabosteve @js-jankisalvi @kibanamachine