[Infrastructure UI] Hosts table sorting and pagination #143834

jennypavlova · 2022-10-24T08:42:06Z

Summary

This PR adds pagination and column sorting to the hosts' table. I use default pagination settings (10 per page with the options to show 25 and 50). In order to avoid a change in the snapshot API for now I implemented it the same way as the inventory table view. If we see that the loading is slow and we need to optimize it we can change the snapshot API as well

Testing:
I opened the inventory page in table view and compared the way pagination and sorting work there with the hosts' table using different sorting types ( ascending/descending order for each column )

Something I found out with the sorting while testing: (using EuiInMemoryTable ) When we have a text value (like the host.name) and we sort it (alphabetically for example) we will show first all the hosts starting with a capital letter (sorted alphabetically) and then the other hosts (sorted alphabetically) so the order will be for example Hosta, Hostb, ahost, bhost. It works the same way on the inventory page so I guess the sorting is implemented to be case-sensitive on purpose.

Old PR: neptunian#3

…ge values

…tion

elasticmachine · 2022-10-24T09:27:34Z

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

jennypavlova · 2022-10-24T09:27:41Z

@crespocarlos Based on our discussion on the previous PR I am opening this one against main ( I got a warning that the comments may be lost if I change the old one so I will keep it to track the comments there until we merge this one)

…tion

crespocarlos

Cool! Paging and sorting works. Perhaps we could try to look into the type changes and see if we can try to simplify it.

crespocarlos · 2022-10-25T08:29:55Z

x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table.tsx

-      items={items}
-      columns={HostsTableColumns}
-    />
+    <EuiInMemoryTable pagination={true} sorting={true} items={items} columns={HostsTableColumns} />


nit: a shorthand for the boolean props, when we pass true, could be

Suggested change

<EuiInMemoryTable pagination={true} sorting={true} items={items} columns={HostsTableColumns} />

<EuiInMemoryTable pagination sorting items={items} columns={HostsTableColumns} />

crespocarlos · 2022-10-25T08:53:33Z

x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table_columns.tsx

+export interface HostNodeRow extends HostMetics {
  os?: string | null | undefined;
  servicesOnHost?: number | null | undefined;
+  name: string;


Maybe this could be simplified, since we're already declaring the attributes as ?, which means they can be undefined, we could remove undefined from the union type. wdyt?

Suggested change

export interface HostNodeRow extends HostMetics {

os?: string | null | undefined;

servicesOnHost?: number | null | undefined;

name: string;

export interface HostNodeRow extends HostMetics {

os?: string | null;

servicesOnHost?: number | null;

name?: string | null;

crespocarlos · 2022-10-25T09:01:25Z

x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table_columns.tsx

  os?: string | null | undefined;
  servicesOnHost?: number | null | undefined;
+  name: string;
 }

 export interface HostMetics {


I think we could achieve the same results, without changing the types here. I played with it and EuiInMemoryTable complains that item type is not compatible because name is now declared as a mandatory field. If we make it optional, it should work without these changes here and on HostsTableColumns - correct me if I'm wrong or missing something.

I added it as mandatory because we should always have a hostname and it won't be a case to have a hostname equal to null, right 🤔 ? Yes in general we can have it as optional similar to os to set - if we have a host without a name but do we have this case?

crespocarlos · 2022-10-25T09:02:04Z

x-pack/plugins/infra/public/pages/metrics/hosts/components/hosts_table_columns.tsx

@@ -41,59 +42,63 @@ export const HostsTableColumns: Array<EuiBasicTableColumn<HostNodeRow>> = [
      defaultMessage: 'Operating System',
    }),
    field: 'os',
+    sortable: true,
    render: (os: string) => <EuiText size="s">{os ?? '-'}</EuiText>,


Maybe we don't need this os ?? '-' anymore because we're handling null values in use_host_tables?

yes, true, forgot it after the change. Removed 👍

crespocarlos · 2022-10-25T09:03:03Z

x-pack/plugins/infra/public/pages/metrics/hosts/hooks/use_host_table.ts

-    return nodes.map(({ metrics, path }) => ({
-      ...last(path),
+  const items: HostNodeRow[] = useMemo(() => {
+    const valuesMapping: Record<keyof HostMetics, 'value' | 'avg' | 'max'> = {


This is a smart solution :). If we don't need to make the changes on hosts_table_column we may not need this anymore - which would be even simpler

I changed the columns because for the sorting we need the fields to have the value as a number or string - the value we want to use for sorting the table - so for example memory: {avg: 1, value: 2, max: 3} should be memory: 1 because we show the avg in the table and for sorting the column the average value (1) will be used ( similar to all the columns ). That's why I changed the structure there.

crespocarlos · 2022-10-25T09:05:42Z

x-pack/plugins/infra/public/pages/metrics/hosts/hooks/use_host_table.ts

+    };
+    return nodes.map(({ metrics, path, name }) => ({
+      name,
+      os: last(path)?.os ?? '-',


If you prefer, you can get rid of lodash by doing the following:

const lastPath = { ...path.at(-1), os: path.at(-1)?.os ?? '-' };

new TS cool feature

Nice idea, thank you!

crespocarlos · 2022-10-25T09:06:26Z

x-pack/plugins/infra/public/pages/metrics/hosts/hooks/use_host_table.ts

      ...metrics.reduce((data, metric) => {
-        data[metric.name as keyof HostMetics] = metric;
+        const metricName = metric.name as keyof HostMetics;
+        data[metricName] = metric[valuesMapping[metricName]];


Should we handle null values here too? In case these any of these metrics are null, should we set the default value to 0?

The NumberCell component will handle the null case and will display N/A in that case - I think it's more accurate when the metric is not present instead of showing 0 - what do you think?

…tion

jennypavlova · 2022-10-25T17:43:49Z

@crespocarlos As discussed I changed the mapping so we can have the same metric values format as we have now and changed the field names inside HostsTableColumns. Also changed label to name and removed not used values from the path object.

…tion

kibana-ci · 2022-10-26T09:38:18Z

💚 Build Succeeded

Buildkite Build
Commit: 0bebe90

Metrics [docs]

Async chunks

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

id	before	after	diff
`infra`	1009.5KB	1009.6KB	+158.0B

History

💛 Build #82597 was flaky 896599c
💚 Build #82399 succeeded 40f2ca1
💚 Build #82248 succeeded acf6ae2
💚 Build #82051 succeeded 2c55c8b

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

cc @jennypavlova

crespocarlos

LGTM! Thanks for taking the time to discuss the possibilities to improve the code

neptunian and others added 30 commits October 6, 2022 10:59

remove lens, add snapshot api

f309f35

merge

3b99bb5

add no data

fc545f6

Add cpu type

26710d6

[WIP] Add eui basic table and columns

5a212ca

[WIP] Add cpu cores and os

4d47a82

[WIP] Mapping data to the eui basic table format

20719b5

Add Memory Total

392e035

Refactor host mapping and add types

d1b432d

Scale up memory usage percentage

72f9d8c

Make os optional in the model

a20a278

Text cells formatting

5309f1f

Change os.type to os.name

b5e6432

Move snapshot nodes mapping to a hook and test it

b54de0e

Add translation

4153f3b

Add fixed range and cleanup

5ae30be

Merge branch 'main' into example-snapshot-table

13f8677

Fix diskLatency field name

9ab9f8a

Merge branch 'main' into example-snapshot-table

191a3d0

Remove not existing showQueryBar prop from SearchBar and set time ran…

263ee4d

…ge values

Merge branch 'main' into example-snapshot-table

4ecda79

Use last path

f68a313

Test last path change

0d07ca1

Type imports

e1f58f1

Change the way lastPath.os is set

662c7ae

Type import

a4bd9a5

Merge branch 'main' into example-snapshot-table

d493a03

Merge branch 'main' into example-snapshot-table

36a30da

Sorting columns

6fb75fc

Use EuiInMemoryTable pagination and sorting and change mapping

bd1c4c0

jennypavlova added the release_note:skip Skip the PR/issue when compiling release notes label Oct 24, 2022

jennypavlova added 3 commits October 24, 2022 10:46

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

6f3ffa3

…tion

Add mapping for the metric values used in hosts table

0f49d15

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

2c55c8b

…tion

jennypavlova marked this pull request as ready for review October 24, 2022 09:27

jennypavlova requested a review from a team as a code owner October 24, 2022 09:27

jennypavlova self-assigned this Oct 24, 2022

jennypavlova added 2 commits October 24, 2022 18:53

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

acf6ae2

…tion

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

40f2ca1

…tion

crespocarlos reviewed Oct 25, 2022

View reviewed changes

jennypavlova added 5 commits October 25, 2022 17:20

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

9a38423

…tion

Shorthand props

2c203aa

Os from path - replace lodash and remove not needed null handling

1ec5a10

Fix types

e280dbd

Remove type export

9d384f2

jennypavlova force-pushed the 143510-trying-hosts-table-sorting-and-pagination branch from 99358e8 to c5c5a12 Compare October 25, 2022 17:39

Change metrics mapping

d49f071

jennypavlova force-pushed the 143510-trying-hosts-table-sorting-and-pagination branch from c5c5a12 to d49f071 Compare October 25, 2022 17:41

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

896599c

…tion

jennypavlova requested a review from crespocarlos October 25, 2022 17:45

Merge branch 'main' into 143510-trying-hosts-table-sorting-and-pagina…

0bebe90

…tion

crespocarlos approved these changes Oct 26, 2022

View reviewed changes

jennypavlova merged commit 2de4f15 into elastic:main Oct 26, 2022

kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Oct 26, 2022

neptunian added the Epic: Host Observability label Nov 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Infrastructure UI] Hosts table sorting and pagination #143834

[Infrastructure UI] Hosts table sorting and pagination #143834

jennypavlova commented Oct 24, 2022 •

edited

Loading

elasticmachine commented Oct 24, 2022

jennypavlova commented Oct 24, 2022

crespocarlos left a comment

crespocarlos Oct 25, 2022

crespocarlos Oct 25, 2022

crespocarlos Oct 25, 2022

jennypavlova Oct 25, 2022

crespocarlos Oct 25, 2022

jennypavlova Oct 25, 2022

crespocarlos Oct 25, 2022

jennypavlova Oct 25, 2022

crespocarlos Oct 25, 2022

jennypavlova Oct 25, 2022

crespocarlos Oct 25, 2022 •

edited

Loading

jennypavlova Oct 25, 2022

jennypavlova commented Oct 25, 2022

kibana-ci commented Oct 26, 2022

crespocarlos left a comment

	<EuiInMemoryTable pagination={true} sorting={true} items={items} columns={HostsTableColumns} />
	<EuiInMemoryTable pagination sorting items={items} columns={HostsTableColumns} />

[Infrastructure UI] Hosts table sorting and pagination #143834

[Infrastructure UI] Hosts table sorting and pagination #143834

Conversation

jennypavlova commented Oct 24, 2022 • edited Loading

Summary

elasticmachine commented Oct 24, 2022

jennypavlova commented Oct 24, 2022

crespocarlos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennypavlova commented Oct 25, 2022

kibana-ci commented Oct 26, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

crespocarlos left a comment

Choose a reason for hiding this comment

jennypavlova commented Oct 24, 2022 •

edited

Loading

crespocarlos Oct 25, 2022 •

edited

Loading