Skip to content

[ML] AIOps: Adds/edits change point charts embeddable from the Dashboard app - #163694

Merged
darnautov merged 20 commits into
elastic:mainfrom
darnautov:ml-161248-embeddable-from-dashboard
Aug 15, 2023
Merged

[ML] AIOps: Adds/edits change point charts embeddable from the Dashboard app#163694
darnautov merged 20 commits into
elastic:mainfrom
darnautov:ml-161248-embeddable-from-dashboard

Conversation

@darnautov

@darnautov darnautov commented Aug 11, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Part of #161248

  • Add embeddable from the dashboard app
image
  • Add/edit embeddable input
image
  • Adds a UI actions for editing existing embeddable panels

Checklist

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

darnautov commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@darnautov
darnautov marked this pull request as ready for review August 11, 2023 16:43
@darnautov
darnautov requested a review from a team as a code owner August 11, 2023 16:43
@elasticmachine

elasticmachine commented Aug 11, 2023

Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

@darnautov darnautov mentioned this pull request Aug 11, 2023
Closed
alvarezmelissa87
alvarezmelissa87 reviewed Aug 14, 2023
View reviewed changes
Comment thread x-pack/plugins/aiops/public/components/change_point_detection/split_field_selector.tsx Outdated
onChange={onChangeCallback}
isClearable
data-test-subj="aiopsChangePointSplitField"
// @ts-ignore

@alvarezmelissa87 alvarezmelissa87 Aug 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: would be good to know what the ignore is for

@darnautov darnautov Aug 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed in fdbbbba

@alvarezmelissa87

alvarezmelissa87 commented Aug 14, 2023

Copy link
Copy Markdown
Contributor

Looks like the options for partitions aren't updated when the split field is changed. The selected partition field should also be removed if it no longer corresponds to the split field.

chartsEmbeddableBug.mp4

alvarezmelissa87
alvarezmelissa87 reviewed Aug 14, 2023
View reviewed changes
Comment thread x-pack/plugins/aiops/public/components/change_point_detection/partitions_selector.tsx
const { runRequest, cancelRequest } = useCancellableSearch();

const fetchResults = useCallback(
async (searchValue: string) => {

@alvarezmelissa87 alvarezmelissa87 Aug 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be missing something but when does the searchValue get updated for this?

@darnautov darnautov Aug 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for spotting this @alvarezmelissa87! fixed in 837b991

@jgowdyelastic

jgowdyelastic commented Aug 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Clicking about in the configuration form, changing selections causes error toasts to appear quite frequently.
For example, use the kibana sample data logs, select agent.keyword as the split field and choose any partition. Then change the split field to clientip

image

other errors I saw:

image

image

@darnautov

darnautov commented Aug 15, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing @jgowdyelastic. I completely forgot about the ip type. Added support for it in c8a7fdf.
Filter agg isn't working for the ip field type, in that case the user can type an exact value.

alvarezmelissa87
alvarezmelissa87 approved these changes Aug 15, 2023
View reviewed changes

@alvarezmelissa87 alvarezmelissa87 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Latest changes LGTM ⚡

@kibana-ci

kibana-ci commented Aug 15, 2023

Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 456 474 +18

Async chunks

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

id before after diff
aiops 568.8KB 595.3KB +26.5KB
ml 3.5MB 3.5MB +25.0B
transform 404.7KB 404.7KB +25.0B
total +26.6KB

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.6KB 7.9KB +297.0B
Unknown metric groups

async chunk count

id before after diff
aiops 15 18 +3

References to deprecated APIs

id before after diff
aiops 21 19 -2

History

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

cc @darnautov

jgowdyelastic
jgowdyelastic approved these changes Aug 15, 2023
View reviewed changes

@jgowdyelastic jgowdyelastic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added a single nit pick, but otherwise LGTM

Comment thread x-pack/plugins/aiops/public/components/change_point_detection/max_series_control.tsx
onValidationChange(maxSeriesValidator(newValue));
}
}}
min={1}

@jgowdyelastic jgowdyelastic Aug 15, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be a constant, it's used on line 39 too.

@darnautov
darnautov merged commit fb6ac2e into elastic:main Aug 15, 2023
@darnautov
darnautov deleted the ml-161248-embeddable-from-dashboard branch August 15, 2023 17:10
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Aug 15, 2023
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Aug 16, 2023
@darnautov @hop-dev
[ML] AIOps: Add/edit change point charts embeddable from the Dashboar…
a4a4d62 
…d app (elastic#163694)
@szabosteve szabosteve mentioned this pull request Aug 21, 2023
Merged
1 task done
@szabosteve szabosteve changed the title [ML] AIOps: Add/edit change point charts embeddable from the Dashboard app [ML] AIOps: Adds/edits change point charts embeddable from the Dashboard app Aug 22, 2023
bryce-b pushed a commit that referenced this pull request Aug 22, 2023
@darnautov Bryce Buchanan
[ML] AIOps: Add/edit change point charts embeddable from the Dashboar…
ba68f49 
…d app (#163694)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes
@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

Assignees

@darnautov darnautov

Labels

backport:skip This PR 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) t// v8.10.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@darnautov @elasticmachine @alvarezmelissa87 @jgowdyelastic @kibana-ci @kibanamachine