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] AIOps: Use package instead of context for using field stats flyout #194517

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Oct 1, 2024

Summary

Part of #187772.
Follow up to #193657.

The previous PR #193657 moved FieldStatsFlyout to a package, the aiops plugin didn't make full use of that refactor by still passing in the flyout into the app context.

Checklist

@walterra walterra self-assigned this Oct 1, 2024
@walterra walterra force-pushed the ml-aiops-field-stats-refactor branch from ebec56f to a978bae Compare October 1, 2024 07:56
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 582 611 +29

Async chunks

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

id before after diff
aiops 548.4KB 551.9KB +3.5KB
ml 4.6MB 4.6MB -6.9KB
total -3.4KB

Page load bundle

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

id before after diff
aiops 7.9KB 8.1KB +175.0B
Unknown metric groups

API count

id before after diff
aiops 74 73 -1

async chunk count

id before after diff
aiops 30 32 +2

History

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

cc @walterra

@walterra walterra marked this pull request as ready for review October 1, 2024 09:47
@walterra walterra requested a review from a team as a code owner October 1, 2024 09:47
@walterra walterra added :ml v9.0.0 Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.16.0 labels Oct 1, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra added backport:skip This commit does not require backporting backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes and removed backport:skip This commit does not require backporting labels Oct 1, 2024
Copy link
Contributor

@rbrtj rbrtj left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

@walterra walterra merged commit eebfba4 into elastic:main Oct 2, 2024
30 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11144656170

@walterra walterra deleted the ml-aiops-field-stats-refactor branch October 2, 2024 13:23
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 2, 2024
…ut (elastic#194517)

## Summary

Part of elastic#187772.
Follow up to elastic#193657.

The previous PR elastic#193657 moved `FieldStatsFlyout` to a package, the
`aiops` plugin didn't make full use of that refactor by still passing in
the flyout into the app context.

### Checklist

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit eebfba4)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 2, 2024
…s flyout (#194517) (#194710)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] AIOps: Use package instead of context for using field stats
flyout (#194517)](#194517)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Walter
Rafelsberger","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-02T13:23:23Z","message":"[ML]
AIOps: Use package instead of context for using field stats flyout
(#194517)\n\n## Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/187772.\r\nFollow up to
#193657.\r\n\r\nThe previous PR #193657 moved `FieldStatsFlyout` to a
package, the\r\n`aiops` plugin didn't make full use of that refactor by
still passing in\r\nthe flyout into the app context.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"eebfba4f9bfe2592ce92d35aed9cf3c0c8540130","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":[":ml","release_note:skip","v9.0.0","Feature:ML/AIOps","v8.16.0","backport:version"],"title":"[ML]
AIOps: Use package instead of context for using field stats
flyout","number":194517,"url":"https://github.com/elastic/kibana/pull/194517","mergeCommit":{"message":"[ML]
AIOps: Use package instead of context for using field stats flyout
(#194517)\n\n## Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/187772.\r\nFollow up to
#193657.\r\n\r\nThe previous PR #193657 moved `FieldStatsFlyout` to a
package, the\r\n`aiops` plugin didn't make full use of that refactor by
still passing in\r\nthe flyout into the app context.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"eebfba4f9bfe2592ce92d35aed9cf3c0c8540130"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194517","number":194517,"mergeCommit":{"message":"[ML]
AIOps: Use package instead of context for using field stats flyout
(#194517)\n\n## Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/187772.\r\nFollow up to
#193657.\r\n\r\nThe previous PR #193657 moved `FieldStatsFlyout` to a
package, the\r\n`aiops` plugin didn't make full use of that refactor by
still passing in\r\nthe flyout into the app context.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"eebfba4f9bfe2592ce92d35aed9cf3c0c8540130"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Walter Rafelsberger <[email protected]>
walterra added a commit to walterra/kibana that referenced this pull request Oct 11, 2024
walterra added a commit that referenced this pull request Oct 11, 2024
…ts flyout (#194517) (#195860)

## Summary

This reverts #194517 ([ML] AIOps: Use package instead of context for
using field stats flyout) committed in
eebfba4.

The PR refactored how `FieldStatsFlyout` is passed in as a dependency to
change point detection. Unfortunately the refactor caused the dropdown
to select the split field to break when used in the flyout for the
options of the change point detection embeddable.

This revert is done to restore the original behavior. In a follow up I
will revisit the refactor to work properly.

### Checklist

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 11, 2024
…ts flyout (elastic#194517) (elastic#195860)

## Summary

This reverts elastic#194517 ([ML] AIOps: Use package instead of context for
using field stats flyout) committed in
eebfba4.

The PR refactored how `FieldStatsFlyout` is passed in as a dependency to
change point detection. Unfortunately the refactor caused the dropdown
to select the split field to break when used in the flyout for the
options of the change point detection embeddable.

This revert is done to restore the original behavior. In a follow up I
will revisit the refactor to work properly.

### Checklist

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit 37a420b)
kibanamachine added a commit that referenced this pull request Oct 11, 2024
…ld stats flyout (#194517) (#195860) (#195930)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Revert [ML] AIOps: Use package instead of context for using field
stats flyout (#194517)
(#195860)](#195860)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Walter
Rafelsberger","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-11T14:15:53Z","message":"Revert
[ML] AIOps: Use package instead of context for using field stats flyout
(#194517) (#195860)\n\n## Summary\r\n\r\nThis reverts #194517 ([ML]
AIOps: Use package instead of context for\r\nusing field stats flyout)
committed in\r\neebfba4f9bfe2592ce92d35aed9cf3c0c8540130.\r\n\r\nThe PR
refactored how `FieldStatsFlyout` is passed in as a dependency
to\r\nchange point detection. Unfortunately the refactor caused the
dropdown\r\nto select the split field to break when used in the flyout
for the\r\noptions of the change point detection embeddable.\r\n\r\nThis
revert is done to restore the original behavior. In a follow up
I\r\nwill revisit the refactor to work properly.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"37a420b950aa7ced58360283ec4a4e63d5d410f8","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug",":ml","release_note:skip","v9.0.0","Feature:ML/AIOps","v8.16.0","backport:version"],"title":"Revert
[ML] AIOps: Use package instead of context for using field stats flyout
(#194517)","number":195860,"url":"https://github.com/elastic/kibana/pull/195860","mergeCommit":{"message":"Revert
[ML] AIOps: Use package instead of context for using field stats flyout
(#194517) (#195860)\n\n## Summary\r\n\r\nThis reverts #194517 ([ML]
AIOps: Use package instead of context for\r\nusing field stats flyout)
committed in\r\neebfba4f9bfe2592ce92d35aed9cf3c0c8540130.\r\n\r\nThe PR
refactored how `FieldStatsFlyout` is passed in as a dependency
to\r\nchange point detection. Unfortunately the refactor caused the
dropdown\r\nto select the split field to break when used in the flyout
for the\r\noptions of the change point detection embeddable.\r\n\r\nThis
revert is done to restore the original behavior. In a follow up
I\r\nwill revisit the refactor to work properly.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"37a420b950aa7ced58360283ec4a4e63d5d410f8"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195860","number":195860,"mergeCommit":{"message":"Revert
[ML] AIOps: Use package instead of context for using field stats flyout
(#194517) (#195860)\n\n## Summary\r\n\r\nThis reverts #194517 ([ML]
AIOps: Use package instead of context for\r\nusing field stats flyout)
committed in\r\neebfba4f9bfe2592ce92d35aed9cf3c0c8540130.\r\n\r\nThe PR
refactored how `FieldStatsFlyout` is passed in as a dependency
to\r\nchange point detection. Unfortunately the refactor caused the
dropdown\r\nto select the split field to break when used in the flyout
for the\r\noptions of the change point detection embeddable.\r\n\r\nThis
revert is done to restore the original behavior. In a follow up
I\r\nwill revisit the refactor to work properly.\r\n\r\n###
Checklist\r\n\r\n- [x] This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)","sha":"37a420b950aa7ced58360283ec4a4e63d5d410f8"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Walter Rafelsberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :ml release_note:skip Skip the PR/issue when compiling release notes v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants