-
Notifications
You must be signed in to change notification settings - Fork 8k
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] Explain log rate spikes: Page setup #132121
Conversation
Pinging @elastic/ml-ui (:ml) |
@@ -5,6 +5,16 @@ | |||
* 2.0. | |||
*/ | |||
|
|||
import { PluginSetup, PluginStart } from '@kbn/data-plugin/server'; | |||
|
|||
export interface AiopsPluginSetupDeps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this export, and the one below, contribute to the counts of public APIs which don't have comments?
</LazyWrapper> | ||
); | ||
|
||
export const SingleEndpointStreamingDemo: FC = () => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need a comment as looks like it's in the public API.
x-pack/plugins/ml/public/application/routing/routes/new_job/index_or_search.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally. Overall look good. Just found a few issues with the breadcrumbs.
x-pack/plugins/ml/public/application/routing/routes/aiops/single_endpoint_streaming_demo.tsx
Show resolved
Hide resolved
@peteharverson Fixed some of the public API issues but wasn't able to resolve everything because it hits uncommented interfaces imported from other plugins. |
There was a problem hiding this 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
x-pack/plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ⚡
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @walterra |
Builds out UI/code boilerplate necessary before we start implementing the feature's own UI on a dedicated page. - Updates navigation to bring up data view/saved search selection before moving on to the explain log spike rates page. The bar chart race demo page was moved to the aiops/single_endpoint_streaming_demo url. It is kept in this PR so we have two different pages + API endpoints that use streaming. With this still in place it's easier to update the streaming code to be more generic and reusable. - The url/page aiops/explain_log_rate_spikes has been added with some dummy request that slowly streams a data view's fields to the client. This page will host the actual UI to be brought over from the PoC in follow ups to this PR. - The structure to embed aiops plugin pages in the ml plugin has been simplified. Instead of a lot of custom code to load the components at runtime in the aiops plugin itself, this now uses React lazy loading with Suspense, similar to how we load Vega charts in other places. We no longer initialize the aiops client side code during startup of the plugin itself and augment it, instead we statically import components and pass on props/contexts from the ml plugin. - The code to handle streaming chunks on the client side in stream_fetch.ts/use_stream_fetch_reducer.ts has been improved to make better use of TS generics so for a given API endpoint it's able to return the appropriate coresponding return data type and only allows to use the supported reducer actions for that endpoint. Buffering client side actions has been tweaked to handle state updates more quickly if updates from the server are stalling.
Summary
Part of #136265.
Follow up to #131317.
Resolves #132126.
This is the second (and should be last) PR to build out UI/code boilerplate necessary before we start implementing the feature's own UI on a dedicated page.
aiops/single_endpoint_streaming_demo
url. It is kept in this PR so we have two different pages + API endpoints that use streaming. With this still in place it's easier to update the streaming code to be more generic and reusable.aiops/explain_log_rate_spikes
has been added with some dummy request that slowly streams a data view's fields to the client. This page will host the actual UI to be brought over from the PoC in follow ups to this PR.aiops
plugin pages in theml
plugin has been simplified. Instead of a lot of custom code to load the components at runtime in theaiops
plugin itself, this now uses React lazy loading withSuspense
, similar to how we load Vega charts in other places. We no longer initialize theaiops
client side code during startup of the plugin itself and augment it, instead we statically import components and pass on props/contexts from theml
plugin.stream_fetch.ts/use_stream_fetch_reducer.ts
has been improved to make better use of TS generics so for a given API endpoint it's able to return the appropriate coresponding return data type and only allows to use the supported reducer actions for that endpoint. Buffering client side actions has been tweaked to handle state updates more quickly if updates from the server are stalling.Checklist
Delete any items that are not applicable to this PR.
For maintainers