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] Hosts table sorting and pagination #3

Open
wants to merge 95 commits into
base: example-snapshot-table
Choose a base branch
from

Conversation

jennypavlova
Copy link

@jennypavlova jennypavlova commented Oct 19, 2022

Closes elastic#143510

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
image

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.

Copy link

@crespocarlos crespocarlos 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 curious about a couple of things around the usage of the SnapshotNodeMetric and the default sorting attribute. Left a few comments.

I'm assuming that we won't paginate the data on the api side, only on the FE, to prevent the page from growing too much, right?

Probably it's better to open this PR against main, once the table implementation has been merged.

@@ -18,9 +18,17 @@ interface Props {
export const HostsTable: React.FunctionComponent<Props> = ({ nodes }) => {
const items = useHostTable(nodes);

const sortSettings = {

Choose a reason for hiding this comment

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

I wonder whether we need to start the table sorting by cpuCores. Maybe moving forward it'd be nice to define what would be the best attribute here, or maybe leave the default api sorting, which currently is by @timestamp

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure which element should be used by default and set the CPU cores but I can change it. The @timestamp field is not part of the table so maybe it will be a bit confusing to be used for sorting here (as visually there will be no indication inside the table header which column we use for sorting and the assumption is that it's not sorted), I think memory usage will be a good one. So as we are not sure if we need the default set here I will set sorting={true} to be the default and once we decide which column we want we can change it.

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

export interface HostMetics {

Choose a reason for hiding this comment

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

I'm curious here. Why did we change the types here? Doesn't it work with the sortable table?

Copy link
Author

Choose a reason for hiding this comment

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

I changed the structure a bit - so for the sorting functionality we need a structure like field_name: value (string/number) and I was thinking that it's more readable if we have the name mapped separately (instead of using the label from the path)

...metrics.reduce((data, metric) => {
data[metric.name as keyof HostMetics] = metric;
data[metric.name as keyof HostMetics] =
metric.name === 'cpuCores' ? metric?.value : metric?.avg;

Choose a reason for hiding this comment

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

We need to be careful here. If we include a new field or use an attribute other than avg we will have to make sure it reflects here.

Copy link
Author

Choose a reason for hiding this comment

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

That's a good point! Most of the metrics we use currently are using avg and only the CPU cores are using the total number(value). Once we have the use case we can create the mapping separately for the fields we need ( I am not sure how the services on host structure will look like ) - so this mapping it definitely something we want to move separately once we know how the data will be structured.

Copy link
Author

Choose a reason for hiding this comment

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

I added a mapping to the values that should be used for each metric so we can extend it once we add more metrics: 0f49d15

@jennypavlova
Copy link
Author

I'm assuming that we won't paginate the data on the api side, only on the FE, to prevent the page from growing too much, right?

Yes, that's correct. I discussed with Nathan that for now we won't implement the pagination in the snapshot api until we know how we want to do it and if we really need it there (we can also decide in the future to show only the top 100 hosts for example which won't require pagination at all)
So I used the same idea we have in the inventory table view and used the eui in-memory table component to get the pagination out of the box on the FE.

Probably it's better to open this PR against main, once the table implementation has been merged.

Yes, I opened it against the other PR so it's easier to see the diff here and to merge them together. But I agree, it will be better to wait for the other PR to be merged and I will update this one to be against main.

@jennypavlova jennypavlova force-pushed the 143510-trying-hosts-table-sorting-and-pagination branch from ef85fe4 to 505d3a6 Compare October 20, 2022 12:48
@jennypavlova jennypavlova changed the title Hosts table sorting and pagination [Infrastructure UI] Hosts table sorting and pagination Oct 20, 2022
jennypavlova and others added 3 commits October 20, 2022 15:33
…astic#142871)

* remove lens, add snapshot api

* add no data

* Add cpu type

* [WIP] Add eui basic table and columns

* [WIP] Add cpu cores and os

* [WIP] Mapping data to the eui basic table format

* Add Memory Total

* Refactor host mapping and add types

* Scale up memory usage percentage

* Make os optional in the model

* Text cells formatting

* Change os.type to os.name

* Move snapshot nodes mapping to a hook and test it

* Add translation

* Add fixed range and cleanup

* Fix diskLatency field name

* Remove not existing showQueryBar prop from SearchBar and set time range values

* Use last path

* Test last path change

* Type imports

* Change the way lastPath.os is set

* Type import

Co-authored-by: Jenny <[email protected]>
jloleysens and others added 18 commits October 24, 2022 11:00
* added unscoped file client to the files components context

* wip: created some basic files and stories for new filepicker component

* fix some types after require filesclient to be passed in

* added file picker state and some basic tests

* added file picker context

* added missing file client value

* updated file picker stories

* expanded files context;

* remove the size observable and also added a test for file removal and adding of duplicates

* added error content component

* refactor upload component to not take files client as prop

* updated shared file compoennts context value

* added test for adding multiple files at once

* allow passing in string or array of strings

* added some i18n texts

* move the creation of a file kinds registry to a common location

* set file kinds only once

* a bunch of stuff: added title component, using grid, removed responsive on upload controls

* refactor layour components to own component using grid, added new i18n texts for empty state prompt

* minor copy tweak

* added basic story with some files

* added file grid and refactored picker to only exist in modal for now

* get the css grid algo that we want: auto-fill not auto-fit!

* override styling for content area of file

* split stories of files

* delete commented out code

* give the modal a fixed width

* fix upload file state, where we do not want a fixed width modal

* moved styles down to card, and combined margin removal rules

* optimize for filtering files, first pass just filter on names

* include xxl

* moved debounceTime to rxjs land, added test for filtering behaviour

* added story with more images

* big ol wip

* empty prompt when uploading a file

* added pagination

* fixed tests and added some comments

* address lint

* moved copy to i18n and updated size and color of empty error prompt

* remove use of css`

* remove non existant prop

* also reload files

* fileUpload -> files

* update logic for watching if selected

* disambiguate i18n ids

* use abort signal and call sendRequest from file$, filtering done server side now, update tests

* fix a few off by one errors and hook up the new system to the ui

* added test for in flight requests behaviour

* update the files example app

* fix minor card layout styling to make all cards the same size regardless of text or image conten

* added new file picker component

* make file cards a bit wider and text a bit smaller so that it wraps...

* fix issue of throwing abort error and prematurely setting request to completed...

* remove unused import

* replace filter i18n

* a bunch of cool changes

* added export for the file picker component

* updated example app to use multiple file upload

* added some comments and made images load eagerly in file picker for now...

* complete ux for examples

* only files that are "READY" should be in the file picker

* set loading to false if error

* install data-test-subj everywhere!

* added some react component tests

* remove unused import

* fix storybook case

* fix up where the files example plugin is listed, moved it to the developer examples area

* fix potential flashing of loader by debouncing

* do not create new observable on every render

* i18n

* have only filepicker ctx used in filepicker components

* refactor loadImageEagerly -> lazy

* useObservable instead of useEffect

* factor modal footer to own component and remove css util

* use the middle dot luke

* copy update in files example app

* added filter story

Co-authored-by: Kibana Machine <[email protected]>
* [ML] Adding anomaly score explanations

* fixing types

* fixing jest test

* test title

* style improvements

* test data

* removing test data

* better test data

* removing test data

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* using eui colors

* reverting auto changes

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* tiny change

* reverting test data

* tiny change

* removing render functions

* adding tooltip text

* adding comments

* text update

* adding conditional impact score text

* text and scoring changes

* translations

* updating text

* more text changes

Co-authored-by: kibanamachine <[email protected]>
* Remove alerts from cases

* Refactor property actions

* Add tests

* Fix i18n

* Fix tests

* PR feedback

* Do not show property action if empty

Co-authored-by: Kibana Machine <[email protected]>
…astic#143664)

* Changed MAX_TITLE_LENGTH; fixed column width for case title.

* Updated tests with the new case title max length.

* Updated snapshots to fix failing tests.

* Increased the width of the select case modal in alerts.
* Remove deprecated usage of indexPattern.title

* Fix mocks in unit tests

Co-authored-by: Kibana Machine <[email protected]>
* Use brotli compression

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Add integration test for brotli support

* Use import instead of require()

* Suppress build error on importing brok

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* add brok as explicit package dep

* add `server.compression.brotli` config settings

* update documentation

* fix test utils

* fix more test configs

* add tests for endpoints too

* remove against endpoint for now

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: pgayvallet <[email protected]>
… state (elastic#143252)

**Resolves:** [elastic#137149](elastic#137149)

## Summary

Disables rule details enable/disable toggle for ML rules only under basic license.

Before:

https://user-images.githubusercontent.com/3775283/195514871-f0ccb25e-177d-4b4e-83bc-9c26da1718f0.mov

After:

https://user-images.githubusercontent.com/3775283/195513340-95944c6d-da6d-4ab3-9917-4e03f5791d3a.mov


### Checklist

- [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
…43744)

* use loading prop to drive button state

* changed upload complete state

* retry button should be EuiButton

* remove the success icon

* cancel button should use EuiButton

* add the compressed option

* make compressed mode!

* updated upload file story and component to always allow repeated uploads in compressed mode

* added error story

* added has files check

* added test for new #hasFiles
* Add support of React nodes rendering to the embeddable panel
* Add support of React nodes rendering by the error handler
* Update visualizations embeddable to use React for error handling
* ✨ First pass with UI for random sampling

* ✨ Initial working version

* 🔥 Remove unused stuff

* 🔧 Refactor layer settings panel

* 🐛 Fix terms other query and some refactor

* 🏷️ Fix types issues

* 🐛 Fix sampling for other terms agg

* 🐛 Fix issue with count operation

* ✅ Fix jest tests

* 🐛 fix test stability

* 🐛 fix test with newer params

* 💄 Add tech preview label

* ✅ Add new tests for sampling

* ✅ Add more tests

* ✅ Add new test for suggestions

* ✅ Add functional tests for layer actions and random sampling

* Update x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx

Co-authored-by: Michael Marcialis <[email protected]>

* 👌 Integrated design feedback

Co-authored-by: Joe Reuter <[email protected]>
Co-authored-by: Michael Marcialis <[email protected]>
jennypavlova and others added 10 commits October 25, 2022 17:31
elastic#143683)

So far we passed on the abort signal from the client to possibly cancel the analysis, but the signal was not passed on to the ES queries to cancel those. That means the analysis could be cancelled after each step but it did not cancel ES queries that were already running. This PR takes the already existing abort signal and passes it on to all ES queries.

This surfaced an issue with running too many queries in parallel: We didn't have a limit so far when fetching the histogram data. With using the abort signals, Kibana would report a warning if more than 10 queries were run at once. The PR updates fetching histogram data to also do it in chunks of 10 queries like we already do for the p-value aggregation. So the larger bulk of the file diff is the result of wrapping the histogram queries inside a for of to iterate over the chunks of queries.
**Resolves:** [elastic#140579](elastic#140579)

## Summary

It replaces Jira svg icon with a simple one which works correctly in disabled mode.

## Details

The previous icon used an embedded png image which is applied through a `fill` property. It conflicts with the disabled mode which also uses `fill` property to display the icon in grey scale. An icon with embedded png images fixes it.

*Before:*

- Trial license 
<img width="986" alt="Screenshot 2022-10-25 at 09 17 21" src="https://user-images.githubusercontent.com/3775283/197698855-2a77579d-ea3d-45df-a10f-86105f0be300.png">

- Basic license
<img width="983" alt="Screenshot 2022-10-25 at 09 19 25" src="https://user-images.githubusercontent.com/3775283/197698877-0063c363-0bd6-4939-b23b-732828d4572a.png">

*After:*

- Trial license 
<img width="986" alt="Screenshot 2022-10-25 at 09 15 50" src="https://user-images.githubusercontent.com/3775283/197698913-922f921f-a379-4da7-97ce-10492f04ddbb.png">

- Basic license
<img width="987" alt="Screenshot 2022-10-25 at 09 20 24" src="https://user-images.githubusercontent.com/3775283/197698929-f7b6448e-fbf8-49e5-80f3-bc2af07d1847.png">

### Checklist

- [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
…#142780)

* Add buttons with no functionality

* Added basic negate functionality

* Add `NOT` text when negated

* Clean up

* Add jest and functional tests

* Fix merge conflicts

* Rename `negate` to `exclude`

* Fix `unsaved changes` bug

* Move erase button back to beside search

* Clean up

* Add chaining functional tests

* Fix other unsaved changes bug

* Fix mobile view of popover

* Add option to disable exclude/include toggle

* Prevent unsaved changes bug for options list settings

* Add tooltip to run past timeout setting

* Address review comments

* Rename variable

* Set `exclude` to `false` when footer is hidden
…ic#143942)

* Add a warning message displayed for incompatible jobs on EA page
…tic#143583)

* Add link to acceptable formats in advanced setting

* Translation goodness

* Update docs
@jennypavlova jennypavlova force-pushed the 143510-trying-hosts-table-sorting-and-pagination branch from 99358e8 to c5c5a12 Compare October 25, 2022 17:39
@jennypavlova jennypavlova force-pushed the 143510-trying-hosts-table-sorting-and-pagination branch from c5c5a12 to d49f071 Compare October 25, 2022 17:41
jennypavlova and others added 16 commits October 25, 2022 19:43
…elastic#143835)

* added local storage to remember pagination for 5 pages

* addressing PR comments + Failed checks

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* addresing pr comments change pageSizes to pageSize

Co-authored-by: kibanamachine <[email protected]>
* [APM] Refactoring metrics route

* apis

* refactoring memory function

* fixing merge conflicts

* refactoring

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* addin API test

* new settings

* cost settings

* removing cost settings

* refactoring api

* adding test

* Update oss_plugins.json

* refactoring apis

* new page

* fixing build

* metrics ui

* adding filter by serverlessFunctionName

* aws details

* refactoring

* fixing sorting

* tests

* test

* renaming api

* fixing test

* fixing ci

* refactoring

* reverting

* fixing breadcrumb

* addressing pr comments

* sanity check

* fixing merge conflicts

* fixing tests and ci

Co-authored-by: kibanamachine <[email protected]>
* Upgrade to Jest 27

* fix test
…solution/public/timelines (elastic#143824)

* [Security Solution] Fixed flaky Jest config: x-pack/plugins/security_solution/public/timelines

* Improved Ids

* -

* Fixed test files and lazy suricata

* Cleaned up act error for async stuff

* -

* Cleaning up jest errors for timeline components

* -

* Used mount enzyme

* mocked react-beautiful-dnd

* type check

* Fixed snapshot

* mocked one more expensive lib

* Reduced the number of data rows to 1 to simplify rendering

Co-authored-by: Kibana Machine <[email protected]>
* xterm updated to v5. initial work done for max kb warning msg

* max bytes exceeded msg implemented, along with authz gated link to security policies page.

* marker indication on max bytes exceeded fixed

* tests written

* tests written

* fixed a bug where an active search would prevent normal playback/seeking

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* updates per review comments

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* fixed text

* fixed ts error

* fix to scroll hack

Co-authored-by: Karl Godard <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
…nts (elastic#143274)

* added files management privileges definition and locked down metrics and find endpoint to management role

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

Co-authored-by: kibanamachine <[email protected]>
* new layout for panel footer

* remove unused import

* remove footer bg color for darkMode

* replace custom icon styling with prop values

* move inline text style to prop
neptunian pushed a commit that referenced this pull request Aug 27, 2024
## Summary

Resolves elastic#143905. This PR adds support for integration-level outputs.
This means that different integrations within the same agent policy can
now be configured to send data to different locations. This feature is
gated behind `enterprise` level subscription.

For each input, the agent policy will configure sending data to the
following outputs in decreasing order of priority:
1. Output set specifically on the integration policy
2. Output set specifically on the integration's parent agent policy
(including the case where an integration policy belongs to multiple
agent policies)
3. Global default data output set via Fleet Settings

Integration-level outputs will respect the same rules as agent
policy-level outputs:
- Certain integrations are disallowed from using certain output types,
attempting to add them to each other via creation, updating, or
"defaulting", will fail
- `fleet-server`, `synthetics`, and `apm` can only use same-cluster
Elasticsearch output
- When an output is deleted, any integrations that were specifically
using it will "clear" their output configuration and revert back to
either `#2` or `#3` in the above list
- When an output is edited, all agent policies across all spaces that
use it will be bumped to a new revision, this includes:
- Agent policies that have that output specifically set in their
settings (existing behavior)
- Agent policies that contain integrations which specifically has that
output set (new behavior)
- When a proxy is edited, the same new revision bump above will apply
for any outputs using that proxy

The final agent policy YAML that is generated will have:
- `outputs` block that includes:
- Data and monitoring outputs set at the agent policy level (existing
behavior)
- Any additional outputs set at the integration level, if they differ
from the above
- `outputs_permissions` block that includes permissions for each
Elasticsearch output depending on which integrations and/or agent
monitoring are assigned to it

Integration policies table now includes `Output` column. If the output
is defaulting to agent policy-level output, or global setting output, a
tooltip is shown:

<img width="1392" alt="image"
src="https://github.com/user-attachments/assets/5534716b-49b5-402a-aa4a-4ba6533e0ca8">

Configuring an integration-level output is done under Advanced options
in the policy editor. Setting to the blank value will "clear" the output
configuration. The list of available outputs is filtered by what outputs
are available for that integration (see above):

<img width="799" alt="image"
src="https://github.com/user-attachments/assets/617af6f4-e8f8-40b1-b476-848f8ac96e76">

An example of failure: ES output cannot be changed to Kafka while there
is an integration
<img width="1289" alt="image"
src="https://github.com/user-attachments/assets/11847eb5-fd5d-4271-8464-983d7ab39218">


## TODO
- [x] Adjust side effects of editing/deleting output when policies use
it across different spaces
- [x] Add API integration tests
- [x] Update OpenAPI spec
- [x] Create doc issue

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.