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

[Infrastructure UI] inital hosts page #138173

Merged
merged 9 commits into from
Aug 26, 2022

Conversation

neptunian
Copy link
Contributor

@neptunian neptunian commented Aug 4, 2022

#138111

Basic implementation of Hosts page that includes a Lens table and Unified Search. The plan is to put this behind a Technical Preview until complete. This approach of using Lens instead of a EuiTable and creating a custom endpoint is a bit unusual and I'd be interested in hearing any feedback for/against it. We'd continue adding more Lens components for other visualizations. The biggest upside is not having to manage data fetching (custom api) or creating custom visualizations / tables. One limitation is Lens only returns at the most Top 10k items. I think we could add a switcher at the top so thats more clear and the user can limit how many Hosts they see. We've always questioned how useful or common it is in Inventory to look at that many hosts at once.

You can read more about pros/cons here https://github.com/elastic/kibana/tree/main/x-pack/plugins/lens#use-lens-embeddable

Screen Shot 2022-08-18 at 5 10 39 PM

  • checks that a Data View exists equal to source.configuration.metricAlias, else creates it because the Lens table requires it
  • Embeds a lens table with supported metrics
  • uses the Unified Search component to create/update the table, currently only supporting a query filter and date range
  • metric for services on host has a separate issue
  • metric for rx and tx have a separate issue

@neptunian neptunian changed the title inital empty hosts page inital hosts page Aug 4, 2022
@neptunian neptunian force-pushed the 138111-initial-hosts-page branch from 53d234e to 9f881b5 Compare August 18, 2022 21:08
@neptunian neptunian force-pushed the 138111-initial-hosts-page branch from 9f881b5 to 37f2103 Compare August 23, 2022 13:37
@neptunian neptunian added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:feature Makes this part of the condensed release notes labels Aug 23, 2022
@neptunian neptunian marked this pull request as ready for review August 23, 2022 18:44
@neptunian neptunian requested a review from a team as a code owner August 23, 2022 18:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

query: Query
): TypedLensByValueInput['attributes'] =>
({
visualizationType: 'lnsDatatable',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is cool.

Did you find a nice way to get from a lens viz to this code or was there a lot of formatting/replacement involved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created the table in Lens and then used the "Open in Playground" feature that gives you all the attributes and just basically copied it into here https://github.com/elastic/kibana/tree/main/x-pack/plugins/lens#getting-started. Vice versa would work as well. I didn't have to do any direct editing of this code other than handing it a Data View. Everything was done in the Lens UI. I was telling Milton in another comment (#138173 (comment)) that the idea would be we'd store this data somewhere as models and generate the lens attributes ourselves once we've got most of the data from the Lens Playground. You can also always paste this back into Lens and view your viz and edit it there.

@matschaffer
Copy link
Contributor

I get the impression this is due to the infra plugin field query issue but this loads really slowly for me. Seems to be ~30s from a basically warm tab.

The initial load seemed to create the data view which caused the whole load to be more like ~90s.

Maybe the speed isn't something to fix in this PR though.

Also, can we add refresh and recent ranges to this selector?

Screen Shot 2022-08-24 at 13 52 24

That'd make it match what's on dashboards usually:

Screen Shot 2022-08-24 at 13 55 02

@matschaffer
Copy link
Contributor

matschaffer commented Aug 24, 2022

A few other things I noticed:

The filter +/- doesn't seem to do anything. I would have expected it to add filters under the search bar similar to dashbaord behavior

Screen Shot 2022-08-24 at 13 56 22

Not sure how to share the query contents. Seems like we should either be able to copy the URL or have a "share" button.

Screen Shot 2022-08-24 at 14 01 01

This could be followup issues as well if you're trying to keep this MVP.

services: { dataViews },
} = useKibana<InfraClientStartDeps>();
const loadDataView = useCallback(async () => {
let view = (await dataViews.find(metricAlias, 1))[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

@matschaffer I'm fairly certain this call and the creteAndSave call below try to resolve all the fields in the data view.
I'm running into the same issue when trying to optimize the source loading for the Logs UI.
I'm making a follow up issue to see if we can make field resolution optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@miltonhultgren miltonhultgren left a comment

Choose a reason for hiding this comment

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

I'm anxious about how we'll maintain the Lens attributes JSON blob but the rest looks good to me 👍🏼

x-pack/plugins/infra/public/pages/metrics/hosts/index.tsx Outdated Show resolved Hide resolved
const {
services: { data },
} = useKibana<InfraClientStartDeps>();
const [dateRange, setDateRange] = useState<TimeRange>({ from: 'now-15m', to: 'now' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we look to use the global Kibana timerange service here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm getting the time range from the Unified Search component and storing it as state so I can pass it to the lens component. Would it make sense to use that here? Could you point me to an example?

Comment on lines +64 to +70
<InfraLoadingPanel
height="100vh"
width="auto"
text={i18n.translate('xpack.infra.waffle.loadingDataText', {
defaultMessage: 'Loading data',
})}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can re-use some of the state screens created for the InfrastructureMetricsTable where there is an animation and the text "loading metrics"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, though technically this is loading the Data View. The lens component has its own loader which is a vertical bar on the top of the table (try clicking refresh) that is used for loading the actual metrics data.

import type { DataView } from '@kbn/data-views-plugin/public';
import { InfraClientStartDeps } from '../../../../types';

const getLensHostsTable = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this isn't really to be reviewed because it's generated but I think we'll need to document how to re-generate this when needed.
This is why I'd wish we could use a Lens-as-Saved-Object instead and just edit it in the Lens UI.

Copy link
Contributor Author

@neptunian neptunian Aug 24, 2022

Choose a reason for hiding this comment

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

If we decide to do it this way, I think all these formulas and fields for calculating the metrics will be stored somewhere, like the inventory model metrics. So we would be generating the lens attributes based on these. Similar to how we do with TSVB https://github.com/elastic/kibana/tree/main/x-pack/plugins/infra/common/inventory_models/host/metrics/tsvb. If you want to view it in Lens to edit it there you can take the generated attributes and paste it into the Lens Playground, save it, and start editing it in the Lens UI.
Screen Shot 2022-08-24 at 8 18 24 AM
See https://github.com/elastic/kibana/tree/main/x-pack/plugins/lens#getting-started

The saved object might be the better way to go, but I thought this might be easier then having to ship them with packages or metricbeat and worrying about the user editing them, but I need to check the status on read only Kibana assets. Also being able to store the fields and formulas for possibly reusing. If we need to update a field, it would be easy to replace it in the code in one place. Once we get the metrics right, I don't suppose they'll be changing very often.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, if we can write some (strongly typed) code to generate these attributes and combine them then that would feel a lot better!

Copy link
Contributor

@klacabane klacabane Aug 26, 2022

Choose a reason for hiding this comment

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

What would storing this configuration as a saved object enable ? Sounds like we want this config to be readonly for users, and if we don't have to update it at runtime neither it would be an unnecessary network call

@neptunian
Copy link
Contributor Author

I'm anxious about how we'll maintain the Lens attributes JSON blob but the rest looks good to me 👍🏼

Let me know if this comment answers that #138173 (comment). That was my general thought.

@smith
Copy link
Contributor

smith commented Aug 24, 2022

/oblt-deploy

tenor-251065666

<>
<SearchBar
showQueryBar={true}
showFilterBar={false}
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to be hiding the filter bar?

Copy link
Contributor Author

@neptunian neptunian Aug 24, 2022

Choose a reason for hiding this comment

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

Supporting filtering is a bit more complicated so I just hid it for now to do in a follow up

@matschaffer matschaffer added the ci:cloud-deploy Create or update a Cloud deployment label Aug 25, 2022
@neptunian neptunian changed the title inital hosts page [Infrastructure UI] inital hosts page Aug 26, 2022
@klacabane
Copy link
Contributor

The biggest upside is not having to manage data fetching (custom api)

Sounds like a big win if we can delegate this to reusable components. otoh we would lack customization capabilities and we're subject to breaking changes from the lens API ?

@neptunian
Copy link
Contributor Author

I get the impression this is due to the infra plugin field query issue but this loads really slowly for me. Seems to be ~30s from a basically warm tab.

The initial load seemed to create the data view which caused the whole load to be more like ~90s.

Maybe the speed isn't something to fix in this PR though.

@matschaffer Are you connected to your edge cluster? Loading the source configuration in order to get the fields used for auto complete takes a long time, I think due to the fact that there are remote clusters. What is your index pattern to in Settings? If you're on edge you probably changed it to add the remote_cluster: prefix.

The Data View that gets auto created uses this same index pattern value in Settings. On the edge cluster this takes me about 10 seconds using remote_cluster:metricbeat-*,metricbeat-*,remote_cluster:metrics-*,metrics-* with the Data View containg 6,672 fields. If the Data View is already created it takes a few seconds. Locally, while running metricbeat, it takes about 2 seconds to create a Data View of metrics-*,metricbeat-* containing 5,901 fields.

Part of the bottleneck seems to be connecting to the remote clusters in order to complete these tasks. Milton has opened an issue to see if we can defer having to resolve these fields. #138173 (comment)

Note:
You would see this during the time it takes to load the source fields:

Screen Shot 2022-08-26 at 8 41 49 AM

This loading sign would be while the Data View is being created/fetched (changed it from Loading to Loading Data View):
Screen Shot 2022-08-26 at 9 06 24 AM

@neptunian
Copy link
Contributor Author

neptunian commented Aug 26, 2022

Also, can we add refresh and recent ranges to this selector?

Screen Shot 2022-08-24 at 13 52 24

That'd make it match what's on dashboards usually:

Screen Shot 2022-08-24 at 13 55 02

@matschaffer I don't see any information in the Unified Search storybook on how to do this but looking into the Lens code, they are using a component provided by navigation.ui called TopNavMenu which uses a stateful version of Unified Search AggregateQuerySearchBar. I'm assuming we would need to use this and can do that in a follow up. I think we need to do use this to handle filters anyway.

@neptunian
Copy link
Contributor Author

neptunian commented Aug 26, 2022

A few other things I noticed:

The filter +/- doesn't seem to do anything. I would have expected it to add filters under the search bar similar to dashbaord behavior

@matschaffer
Yea, I had turned off the filters for the Unified Search because it was a bit more complicated and wanted to separate it out into a different issue. Didn't realize that the table had this when hovering over cells. Doesn't seem to be a way to disable the feature for now but asking Lens.

Not sure how to share the query contents. Seems like we should either be able to copy the URL or have a "share" button.

This could be followup issues as well if you're trying to keep this MVP.

I had opened this issue #138518, but will open another issue just to get the basic url state working. Hoping to get this in first so others could pick up tasks and this PR isn't so big.

@neptunian
Copy link
Contributor Author

neptunian commented Aug 26, 2022

The biggest upside is not having to manage data fetching (custom api)

Sounds like a big win if we can delegate this to reusable components. otoh we would lack customization capabilities and we're subject to breaking changes from the lens API ?

@klacabane Lens is pretty customizable but already I've found some things they don't support. They've been pretty responsive and I think if more of Kibana starts using Lens like this they would build the necessary features. There is still a risk of not being able to do something, at least in the short term. One thing they are adding next week for us is the ability to get the data back from the underlying fetch so we could access the data being returned because currently we can't. This arose from this issue #139506 where we need to pass in the list of hosts to get Alerts back until they have better data and capability for filtering. Another limitation is it only returns at the most Top 10k items. I think we could add a switcher at the top so thats more clear and the user can limit how many Hosts they see. We've always questioned how useful or common it is in Inventory to look at that many hosts at once.

Here are some other pros/cons https://github.com/elastic/kibana/tree/main/x-pack/plugins/lens#use-lens-embeddable

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 950 954 +4

Async chunks

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

id before after diff
infra 1016.5KB 1.0MB +7.6KB

Page load bundle

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

id before after diff
infra 84.2KB 84.4KB +172.0B

History

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

@neptunian neptunian merged commit 7e1b60f into elastic:main Aug 26, 2022
@kibanamachine kibanamachine added v8.5.0 backport:skip This commit does not require backporting labels Aug 26, 2022
Mpdreamz pushed a commit to Mpdreamz/kibana that referenced this pull request Sep 6, 2022
* inital empty hosts page and navigation item

* lens table and unified search

* add data view id

* cleanup

* clear searchs session when unmounting

* cleanup

* add some basic error handling for Data View

* change back loading text for now because of breaking test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment Epic: Host Observability release_note:feature Makes this part of the condensed release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants