Add IP Prefix Aggregation-based Visualization #173474

ION28 · 2023-12-16T04:24:44Z

Summary

This PR is for Issue #156121 and adds the ability to perform IP Prefix aggregation-based visualizations within the Kibana UI. Previously this aggregation could only be done in DevTools as a manual query to Elasticsearch and not visualized.

Various Notes

The following two folders & their subfolders had files modified for this
- src/plugins/vis_default_editor/public/components
- src/plugins/data/common/search/aggs
I spent a fair amount of time debating & attempting to build the PrefixLength Input boxes and their interplay with the is_ipv6 toggle button. Originally I tried having only 1 PrefixLength button that the toggle switch would modify the max value / validate the contents of.
- In the end, it seemed much cleaner & straightforward to have two separate input boxes (both prefix_length.tsx components) and just create them with different options. This means that when a user toggles the switch back and forth, they would be seeing/editing two different Prefix Length boxes depending on which way the switch is.
- To make it a little more clear they are different boxes, I put "IPv4" and "IPv6" in the label name for these boxes. Additionally, I think it is helpful this way if you are potentially swapping back and forth between v4 and v6 visualizations.
There is 4 new unit tests, all related to input options, added in the ip_prefix_fn.test.ts file
Note - here is a test CSV file of IPv4 addresses one could import to test locally and see this addition.
alphadataset.csv
- Configure the Override settings in this way if uploading it to Kibana/Elastic

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes

For maintainers

This was checked for breaking API changes and was labeled appropriately

cla-checker-service · 2023-12-16T04:24:50Z

💚 CLA has been signed

ION28 · 2023-12-16T04:34:31Z

CLA has been signed now.

elasticmachine · 2023-12-18T07:18:27Z

Pinging @elastic/kibana-visualizations (Team:Visualizations)

stratoula · 2023-12-18T07:18:35Z

/ci

stratoula

Hey, thanx a lot for your contribution! Some comments from my side:

Clicking on the chart results to an error page. We should fix this by creating either the correct filter or by disabling the creation of this filter for this aggregation. I think the first is the best path forward
Same thing happens when I add the new aggregation on the breakdown and try to filter in/out from the legend

3. There are some failures in the CI. Can you also fix them? 🙇‍♀️

src/plugins/data/common/search/aggs/buckets/ip_prefix_fn.ts

src/plugins/data/common/search/aggs/buckets/create_filter/ip_prefix.ts

src/plugins/vis_default_editor/public/components/agg_params_map.ts

…-fix'

ION28 · 2023-12-19T03:17:46Z

Okay thank you for all of the feedback @stratoula! I'll work on these fixes & let you know when they are ready for review again

box

to pass the prefixLength and isIpv6 toggle to the backend form

for the UI fields

ION28 · 2023-12-26T03:21:21Z

Still working on a couple more items to make this PR ready for review again:

Fixing Range Filter
Adding Tests/Fixing CI issues

ION28 · 2023-12-26T22:54:34Z

@elasticmachine merge upstream

ION28 · 2023-12-26T22:55:18Z

/ci

ION28 · 2023-12-27T00:40:44Z

Guess I can't trigger the CI 😅 Also seems like I can't access any of the Buildkite urls as a non-elastic employee; however, I did run the tests for the data plugin locally and all seems to pass. Will see what the CI says.

Couple notes now that I think this is ready for review again:

I switched to using a single input box for the prefixLength. Because of how that and the isIpv6 switch behavior, I implemented both inputs within the same React Controlled Component. That way, changing the Switch from IPv4 to IPv6 and vice versa can dynamically change the Max value of the prefix Length input box.
Given that setup with the values in the single component, I added a new type "ip_prefix" and the associated code to handle that in src/plugins/data/common/search/expressions
Since last review, I corrected the RangeFilter to work appropriately with the IP Addresses. I expect the most common use of the filter would be to filter out a particular IP Range from the resulting visual.
Since last review, I also added tests for each of the components to match similar the unit tests similar components (e.g. IP Range or the Extended Bounds subcomponent) have

ION28 · 2023-12-27T03:13:44Z

May need to also update this page of the docs https://github.com/elastic/kibana/blob/main/docs%2Fuser%2Fdashboard%2Fcreate-panels-with-editors.asciidoc

ION28 · 2024-01-06T23:15:28Z

@elasticmachine merge upstream

ION28 · 2024-01-06T23:44:27Z

@stratoula thank you for all of your feedback so far on the PR. I believe I have addressed all of your last round of comments. A couple notes:

The explanation for the bug in your screenshot is described in src/plugins/data/common/search/aggs/buckets/create_filter/ip_prefix.ts Given the complexity of when/how that scenario occurs and how I addressed it, I thought it was best to write it out as a block comment for anyone who might review it in the future.
There is one additional case of potential weirdness I discovered during this last round of testing, and I can't think of any way to handle it. Your screenshot above is basically the opposite of this case. For this one, let's say you have an index/data view where one field only contains IPv6 addresses. You do a split series (or X-axis) on that field with a low prefix (say 2), and the is_ipv6 switch is set to "false." The keys in the legend (or the bars themselves) that you can filter on will show IPv4 addresses. However, clicking on them to either filter in or out will have no effect on the resulting chart. Unlike with the former case, I don't know how we would be able to detect that there are IPv6 addresses making up these bars and filter appropriately. This "problem" occurs because Elasticsearch will auto-convert IPv6 addresses to IPv4s in the backend when doing the ip_prefix aggregation, thus IPv6 addresses end up being counted in buckets with an IPv4 address label in the response from ES. I don't believe that Kibana has enough information in this scenario to know to map the IPv4s to IPv6s to let the user the filter the data falling into these ranges out (unless both an IPv4 and an IPv6 filter got created every time the user clicks to apply a single filter). Not sure how you want to handle this behavior.
I also added the small bit to update the documentation to specify support in Kibana for the IP Prefix aggregation.

stratoula · 2024-01-08T08:07:26Z

@ION28 thanx a ton! I am running the CI and I will do another round of review/test in a while.

stratoula · 2024-01-08T08:07:32Z

/ci

…-fix'

stratoula · 2024-01-08T08:34:32Z

There is one additional case of potential weirdness I discovered during this last round of testing, and I can't think of any way to handle it. Your screenshot above is basically the opposite of this case. For this one, let's say you have an index/data view where one field only contains IPv6 addresses. You do a split series (or X-axis) on that field with a low prefix (say 2), and the is_ipv6 switch is set to "false." The keys in the legend (or the bars themselves) that you can filter on will show IPv4 addresses. However, clicking on them to either filter in or out will have no effect on the resulting chart. Unlike with the former case, I don't know how we would be able to detect that there are IPv6 addresses making up these bars and filter appropriately. This "problem" occurs because Elasticsearch will auto-convert IPv6 addresses to IPv4s in the backend when doing the ip_prefix aggregation, thus IPv6 addresses end up being counted in buckets with an IPv4 address label in the response from ES. I don't believe that Kibana has enough information in this scenario to know to map the IPv4s to IPv6s to let the user the filter the data falling into these ranges out (unless both an IPv4 and an IPv6 filter got created every time the user clicks to apply a single filter). Not sure how you want to handle this behavior.

I think it is fine for now, I feel that this is an edge case. We will see how users interact with it and if they fall in this problem often and we will figure out something. I really hope that it is going to be used by users who understand that the ipv6 switch should be set ti true in case of ipv6 addresses.

stratoula

CI complains here

Code seems fine to me and the behavior. Thanx for updating the docs. Let's make CI happy, I will test and review for a final time and I will merge. Thanx for your effort here!

src/plugins/data/common/search/aggs/buckets/create_filter/ip_prefix.ts

ION28 · 2024-01-14T02:43:10Z

@elasticmachine merge upstream

ION28 · 2024-01-14T02:51:37Z

I finally figured out how to run at least some of the CI checks on my local machine since I can't get buildkite to work on my own account. I'm hoping there's no CI problems this time around finally 😅 It passes the jest tests for src/plugins/data and src/plugins/vis_default_editor/ on my box as well as scripts/type_check.js and others in that folder that I ran.

Let me know if there's anything else you'd like me to address @stratoula . Thank you again for all of your feedback thus far!

stratoula · 2024-01-15T07:56:24Z

/ci

kibana-ci · 2024-01-15T09:00:55Z

💚 Build Succeeded

Buildkite Build
Commit: 031a32b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	487	493	+6
`visDefaultEditor`	120	121	+1
total			+7

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`data`	2537	2569	+32

Async chunks

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

id	before	after	diff
`data`	46.9KB	46.9KB	+3.0B
`visDefaultEditor`	140.8KB	142.2KB	+1.4KB
total			+1.5KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`data`	22	23	+1

Page load bundle

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

id	before	after	diff
`data`	410.6KB	414.7KB	+4.2KB

Unknown metric groups

API count

id	before	after	diff
`data`	3188	3220	+32

History

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

stratoula

This looks great 🎉 Thanx @ION28 for all your effort and patience! LGTM

stratoula · 2024-01-15T09:53:42Z

@elasticmachine run elasticsearch-ci/docs

stratoula · 2024-01-15T11:52:40Z

@elasticmachine run elasticsearch-ci/docs

delanni · 2024-01-15T11:53:09Z

@elasticmachine run elasticsearch-ci/docs

delanni · 2024-01-15T12:07:48Z

Admin-merging as per request, it seems we can't trigger the buildkite/docs-build-pr Expected check for this PR.

## Summary Closes elastic#156121 This PR is for Issue elastic#156121 and adds the ability to perform [IP Prefix](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-ipprefix-aggregation.html) aggregation-based visualizations within the Kibana UI. Previously this aggregation could only be done in DevTools as a manual query to Elasticsearch and not visualized. ![image](https://github.com/elastic/kibana/assets/3931697/7e049da9-fd42-41f6-bbaf-99d3d6cb0790) ![image](https://github.com/elastic/kibana/assets/3931697/dcfae012-9d06-4346-9118-3965434ff8b8) ### Various Notes * The following two folders & their subfolders had files modified for this * src/plugins/vis_default_editor/public/components * src/plugins/data/common/search/aggs * I spent a fair amount of time debating & attempting to build the PrefixLength Input boxes and their interplay with the is_ipv6 toggle button. Originally I tried having only 1 PrefixLength button that the toggle switch would modify the max value / validate the contents of. * In the end, it seemed much cleaner & straightforward to have two separate input boxes (both prefix_length.tsx components) and just create them with different options. This means that when a user toggles the switch back and forth, they would be seeing/editing two different Prefix Length boxes depending on which way the switch is. * To make it a little more clear they are different boxes, I put "IPv4" and "IPv6" in the label name for these boxes. Additionally, I think it is helpful this way if you are potentially swapping back and forth between v4 and v6 visualizations. * There is 4 new unit tests, all related to input options, added in the ip_prefix_fn.test.ts file * Note - here is a test CSV file of IPv4 addresses one could import to test locally and see this addition. [alphadataset.csv](https://github.com/elastic/kibana/files/13691358/alphadataset.csv) * Configure the Override settings in this way if uploading it to Kibana/Elastic * ![image](https://github.com/elastic/kibana/assets/3931697/34ce701a-f4d5-4107-8a08-c6195e21c169) ### 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) - [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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [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) ### Risk Matrix Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

ION28 added 3 commits December 15, 2023 00:01

Begin adding IP Prefix as an aggregation-based visualization

2c141f9

Add is_ipv6 toggle to IP Prefix aggregation-based visualization

4966fcb

Add a unit test for IP Prefix aggregation

d46c9c0

ION28 requested review from a team as code owners December 16, 2023 04:24

kibanamachine added the 💝community label Dec 16, 2023

Merge branch 'main' into add-ip-prefix-visualization

5d7fc55

stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Dec 18, 2023

stratoula reviewed Dec 18, 2023

View reviewed changes

stratoula added the release_note:enhancement label Dec 18, 2023

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

2b9f898

…-fix'

ION28 added 3 commits December 19, 2023 21:30

Begin to make UI changes to have only a single prefixLength input

98f28cd

box

Convert to using single prefixLength input box, work on beginning

fe6c73c

to pass the prefixLength and isIpv6 toggle to the backend form

Add IP Prefix type, Single Prefix length box in UI, Add validation

293f70d

for the UI fields

ION28 requested a review from a team as a code owner December 26, 2023 03:15

ION28 added 2 commits December 26, 2023 14:16

Add IP Prefix Range Filtering

37c07ad

Add unit tests for each IP Prefix component added

86aa463

Merge branch 'main' into add-ip-prefix-visualization

4bc8475

ION28 requested a review from stratoula December 27, 2023 00:40

Merge branch 'main' into add-ip-prefix-visualization

826a536

Merge branch 'main' into add-ip-prefix-visualization

0a548f2

ION28 requested a review from stratoula January 6, 2024 23:45

Add two more tests for the create_filter of ip_prefix

5304b64

stratoula changed the title ~~Add IP Prefix Aggregation-based Visualization, Closes Issue #156121~~ Add IP Prefix Aggregation-based Visualization Jan 8, 2024

Merge branch 'main' into add-ip-prefix-visualization

06aa911

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

0bf3986

…-fix'

stratoula reviewed Jan 8, 2024

View reviewed changes

src/plugins/data/common/search/aggs/buckets/create_filter/ip_prefix.ts Outdated Show resolved Hide resolved

Fix a couple type checking errors in the CI

abf0590

Merge branch 'main' into add-ip-prefix-visualization

7e17db9

ION28 requested a review from stratoula January 14, 2024 02:45

ION28 and others added 2 commits January 13, 2024 22:05

Fix a couple small grammatical mistakes in a code comment

2e09727

Merge branch 'main' into add-ip-prefix-visualization

031a32b

stratoula approved these changes Jan 15, 2024

View reviewed changes

delanni merged commit 8f07822 into elastic:main Jan 15, 2024
18 of 19 checks passed

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IP Prefix Aggregation-based Visualization #173474

Add IP Prefix Aggregation-based Visualization #173474

ION28 commented Dec 16, 2023 •

edited by stratoula

cla-checker-service bot commented Dec 16, 2023 •

edited

ION28 commented Dec 16, 2023

elasticmachine commented Dec 18, 2023

stratoula commented Dec 18, 2023

stratoula left a comment •

edited

ION28 commented Dec 19, 2023

ION28 commented Dec 26, 2023 •

edited

ION28 commented Dec 26, 2023

ION28 commented Dec 26, 2023

ION28 commented Dec 27, 2023

ION28 commented Dec 27, 2023

ION28 commented Jan 6, 2024

ION28 commented Jan 6, 2024 •

edited

stratoula commented Jan 8, 2024

stratoula commented Jan 8, 2024

stratoula commented Jan 8, 2024

stratoula left a comment

ION28 commented Jan 14, 2024

ION28 commented Jan 14, 2024

stratoula commented Jan 15, 2024

kibana-ci commented Jan 15, 2024

API count

stratoula left a comment

stratoula commented Jan 15, 2024

stratoula commented Jan 15, 2024

delanni commented Jan 15, 2024

delanni commented Jan 15, 2024

Add IP Prefix Aggregation-based Visualization #173474

Add IP Prefix Aggregation-based Visualization #173474

Conversation

ION28 commented Dec 16, 2023 • edited by stratoula

Summary

Various Notes

Checklist

Risk Matrix

For maintainers

cla-checker-service bot commented Dec 16, 2023 • edited

ION28 commented Dec 16, 2023

elasticmachine commented Dec 18, 2023

stratoula commented Dec 18, 2023

stratoula left a comment • edited

Choose a reason for hiding this comment

ION28 commented Dec 19, 2023

ION28 commented Dec 26, 2023 • edited

ION28 commented Dec 26, 2023

ION28 commented Dec 26, 2023

ION28 commented Dec 27, 2023

ION28 commented Dec 27, 2023

ION28 commented Jan 6, 2024

ION28 commented Jan 6, 2024 • edited

stratoula commented Jan 8, 2024

stratoula commented Jan 8, 2024

stratoula commented Jan 8, 2024

stratoula left a comment

Choose a reason for hiding this comment

ION28 commented Jan 14, 2024

ION28 commented Jan 14, 2024

stratoula commented Jan 15, 2024

kibana-ci commented Jan 15, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

stratoula left a comment

Choose a reason for hiding this comment

stratoula commented Jan 15, 2024

stratoula commented Jan 15, 2024

delanni commented Jan 15, 2024

delanni commented Jan 15, 2024

ION28 commented Dec 16, 2023 •

edited by stratoula

cla-checker-service bot commented Dec 16, 2023 •

edited

stratoula left a comment •

edited

ION28 commented Dec 26, 2023 •

edited

ION28 commented Jan 6, 2024 •

edited