Implement Asset Inventory Bar Chart by albertoblaz · Pull Request #210938 · elastic/kibana

albertoblaz · 2025-02-12T21:45:41Z

Summary

Closes #201711.

Implement "Top 10 Asset Types" bar chart.

The X-axis shows all assets grouped by category (entity.category field), one category per bar
Each bar shows stacked subgroups of assets by source (entity.type field)
The Y-axis shows the counts of assets

Depends on

Integrate Asset Inventory with backend #208417 so that the chart renders data fetched dynamically. When it gets merged, this one will be rebased and will only contain the last commit as changes.

Screenshots

Loading state (animated spinner from Elastic Charts)

Fetching state (animated progress bar)

Chart with fetched data

Chart with filtered, fetched data (by type: "Identity")

Empty chart - no data

Definition of done

Add a bar chart titled "Top 10 Asset Types" to the "Asset Inventory" page.
Use the @elastic/charts library to implement the visualization.
Configure the chart with:
- X-axis: Asset type categories
- Y-axis: Count of assets
- Legend: A color-coded key linking each bar to a specific category.
Ensure the chart is responsive when resizing the screen and adheres to the visual spec.
Integrate the chart so that it updates based on the filters section and the Unified Header component.

How to test

Follow the instructions from this PR to prepare the local env with data.

Alternatively, open the asset_inventory/components/top_assets_bar_chart.tsx file and edit yourself the data prop that we pass into <BarSeries> with mocked data. The data must have the following shape:

[
  { category: 'cloud-compute', source: 'gcp-compute', count: 500, },
  { category: 'cloud-compute', source: 'aws-security', count: 300, },
  { category: 'cloud-storage', source: 'gcp-compute', count: 221, },
  { category: 'cloud-storage', source: 'aws-security', count: 117, },
];

Checklist

This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

No risks whatsoever.

...solutions/security/plugins/security_solution/public/asset_inventory/components/bar_chart.tsx

elasticmachine · 2025-02-18T13:01:12Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

JordanSh

im requesting changes because the aggregation logic should be using es query instead of our own. also, rows could be limited in some way. for example in csp we have limited the amount of items to 500, but we probably want to show the chart for the entire data that we have.

if those assumptions are wrong lmk and so i can remove this block

...solutions/security/plugins/security_solution/public/asset_inventory/components/bar_chart.tsx

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/all_assets.tsx

...solutions/security/plugins/security_solution/public/asset_inventory/components/bar_chart.tsx

opauloh

as per the discussions I suggest we create a new fetcher function, with aggregated data by entity.type

...ions/security/plugins/security_solution/public/asset_inventory/hooks/use_fetch_chart_data.ts

opauloh · 2025-02-24T19:06:39Z

...ions/security/plugins/security_solution/public/asset_inventory/hooks/use_fetch_chart_data.ts

+    notifications: { toasts },
+  } = useKibana().services;
+  return useQuery(
+    ['asset_inventory_top_assets_chart', { params: options }],


nit: Consider extracting the React Query key into a constant to improve maintainability and avoid potential duplication.

I'll tackle this and other minor changes in a refactor PR. But I totally agree with you 💯

...ecurity/plugins/security_solution/public/asset_inventory/components/top_assets_bar_chart.tsx

opauloh · 2025-02-24T19:18:00Z

...ions/security/plugins/security_solution/public/asset_inventory/hooks/use_fetch_chart_data.ts

+import type * as estypes from '@elastic/elasticsearch/lib/api/types';
+import { showErrorToast } from '@kbn/cloud-security-posture';
+import type { IKibanaSearchResponse, IKibanaSearchRequest } from '@kbn/search-types';
+import type { FindingsBaseEsQuery } from '@kbn/cloud-security-posture';


Todo: I think we should rename FindingsBaseEsQuery to not be related to Findings, i.e something like BaseEsQuery (in a separate PR)

(can be done in this PR as well if its just a simple rename)

@opauloh @JordanSh I raised a separate PR, even if minor, since it touches a bunch of unrelated files. Still in draft since it depends on this PR:

Rename FindingsBaseEsQuery interface in CSP package #212427

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/all_assets.tsx

albertoblaz · 2025-02-25T17:47:41Z

/ci

JordanSh · 2025-02-26T12:22:25Z

...ions/security/plugins/security_solution/public/asset_inventory/hooks/use_fetch_chart_data.ts

+  size: 0,
+  index: ASSET_INVENTORY_INDEX_PATTERN,
+  aggs: {
+    '0': {


not sure i follow the 0/1/2 patterns that are repeating here, is this a part of ES? if its just a convenient naming pattern lets find a meaningful name for those

It's part of ES. I used Lens to build a bar chart that splits bars by type and the aggregation payload used these numbers. I tried simplifying the payload and removing the value_count but the aggregation didn't work and returned errors.

lens? im pretty sure we used eui charts and not lens. unless you mean in another PR we also have a lens graph?
regardless, what im asking is can 0 be renamed to something like "entityCategories", 1 to "entityTypes", 2 to "entityIds"

I actually instructed Alberto on how to use Lens to build a chart and retrieve the ES Query, but I forgot to mention that we should rename the aggregations it generates to a more readable value, good call @JordanSh

JordanSh · 2025-02-26T12:34:09Z

...ions/security/plugins/security_solution/public/asset_inventory/hooks/use_fetch_chart_data.ts

+        rawResponse: { aggregations },
+      } = await lastValueFrom(
+        data.search.search<TopAssetsRequest, TopAssetsResponse>({
+          params: getTopAssetsQuery(options) as TopAssetsRequest['params'],


do we actually need type casting here? would be nice if we could add a return type to getTopAssetsQuery

yes, it's needed. Found the same type casting in several files:

packages/kbn-cloud-security-posture/public/src/hooks/use_misconfiguration_findings.ts

packages/kbn-cloud-security-posture/public/src/hooks/use_misconfiguration_preview.ts

packages/kbn-cloud-security-posture/public/src/hooks/use_vulnerabilities_findings.ts

packages/kbn-cloud-security-posture/public/src/hooks/use_vulnerabilities_preview.ts

plugins/cloud_security_posture/public/pages/vulnerabilities/hooks/use_latest_vulnerabilities.tsx

JordanSh · 2025-02-26T12:34:49Z

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/all_assets.tsx


+  const {
+    data: chartData,
+    // error: fetchChartDataError,


remove if not needed

opauloh

LGTM

elasticmachine · 2025-02-26T19:25:32Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 23e7343

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	7094	7097	+3

Async chunks

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

id	before	after	diff
`securitySolution`	8.9MB	8.9MB	+3.1KB

History

💔 Build #279494 failed 12b2d3e
💛 Build #279201 was flaky 03b3186
💔 Build #279186 failed 00682377eacd64a25efc68d766cd81e926d420f3
💚 Build #278875 succeeded 3edb714

cc @albertoblaz

## Summary Closes elastic#201711. Implement "Top 10 Asset Types" bar chart. - The X-axis shows all assets grouped by category (`entity.category` field), one category per bar - Each bar shows stacked subgroups of assets by source (`entity.type` field) - The Y-axis shows the counts of assets ### Depends on - elastic#208417 so that the chart renders data fetched dynamically. When it gets merged, this one will be rebased and will only contain the last commit as changes. ### Screenshots <details><summary>Loading state (animated spinner from <a href="https://eui.elastic.co/#/display/loading#chart" target="_blank">Elastic Charts</a>)</summary> <img width="1378" alt="Screenshot 2025-02-25 at 18 14 39" src="https://github.com/user-attachments/assets/553294e2-aaee-40c0-b1bb-de3e85f64d78" /> </details> <details><summary>Fetching state (animated progress bar)</summary> <img width="1376" alt="Screenshot 2025-02-25 at 18 14 58" src="https://github.com/user-attachments/assets/accdbc0e-40a2-4b30-9f4e-808a466be4d5" /> </details> <details><summary>Chart with fetched data</summary> <img width="1428" alt="Screenshot 2025-02-24 at 13 11 03" src="https://github.com/user-attachments/assets/3c455bc8-5bdd-4ea2-a946-53e138ae081b" /> </details> <details><summary>Chart with filtered, fetched data (by type: "Identity")</summary> <img width="1429" alt="Screenshot 2025-02-24 at 13 11 17" src="https://github.com/user-attachments/assets/a1e75210-757e-42d1-b852-945de5f3f44b" /> </details> <details><summary>Empty chart - no data</summary> <img width="1258" alt="Screenshot 2025-02-13 at 09 47 08" src="https://github.com/user-attachments/assets/c239a5a6-337e-41c9-a9a3-7cdc2c9b1e01" /> </details> ### Definition of done - [x] Add a bar chart titled "Top 10 Asset Types" to the "Asset Inventory" page. - [x] Use the `@elastic/charts` library to implement the visualization. - [x] Configure the chart with: - **X-axis:** Asset type categories - **Y-axis:** Count of assets - **Legend:** A color-coded key linking each bar to a specific category. - [x] Ensure the chart is responsive when resizing the screen and adheres to the [visual spec](https://www.figma.com/design/9zUqAhhglT1EGYG4LOl1X6/Asset-Management?node-id=2946-19648&t=FuD3BEY4FyxAKV38-4). - [x] Integrate the chart so that it updates based on the filters section and the Unified Header component. ### How to test Follow the instructions from [this PR](elastic#208417) to prepare the local env with data. Alternatively, open the `asset_inventory/components/top_assets_bar_chart.tsx` file and edit yourself the `data` prop that we pass into `<BarSeries>` with mocked data. The data must have the following shape: ```js [ { category: 'cloud-compute', source: 'gcp-compute', count: 500, }, { category: 'cloud-compute', source: 'aws-security', count: 300, }, { category: 'cloud-storage', source: 'gcp-compute', count: 221, }, { category: 'cloud-storage', source: 'aws-security', count: 117, }, ]; ``` ### Checklist - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks No risks whatsoever.

## Summary Rename the `FindingsBaseEsQuery` interface exposed by the `@kibana/cloud-security-posture` package as well as all references where it's imported. Separating this renaming into its own PR also lets us tag it with `backport:prev-minor` and avoid potential merge conflicts in the future. ### Depends on - #210938 ### Checklist - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks No risk whatsoever.

Rename the `FindingsBaseEsQuery` interface exposed by the `@kibana/cloud-security-posture` package as well as all references where it's imported. Separating this renaming into its own PR also lets us tag it with `backport:prev-minor` and avoid potential merge conflicts in the future. - elastic#210938 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) No risk whatsoever. (cherry picked from commit aac8413)

## Summary Closes elastic#201711. Implement "Top 10 Asset Types" bar chart. - The X-axis shows all assets grouped by category (`entity.category` field), one category per bar - Each bar shows stacked subgroups of assets by source (`entity.type` field) - The Y-axis shows the counts of assets ### Depends on - elastic#208417 so that the chart renders data fetched dynamically. When it gets merged, this one will be rebased and will only contain the last commit as changes. ### Screenshots <details><summary>Loading state (animated spinner from <a href="https://eui.elastic.co/#/display/loading#chart" target="_blank">Elastic Charts</a>)</summary> <img width="1378" alt="Screenshot 2025-02-25 at 18 14 39" src="https://github.com/user-attachments/assets/553294e2-aaee-40c0-b1bb-de3e85f64d78" /> </details> <details><summary>Fetching state (animated progress bar)</summary> <img width="1376" alt="Screenshot 2025-02-25 at 18 14 58" src="https://github.com/user-attachments/assets/accdbc0e-40a2-4b30-9f4e-808a466be4d5" /> </details> <details><summary>Chart with fetched data</summary> <img width="1428" alt="Screenshot 2025-02-24 at 13 11 03" src="https://github.com/user-attachments/assets/3c455bc8-5bdd-4ea2-a946-53e138ae081b" /> </details> <details><summary>Chart with filtered, fetched data (by type: "Identity")</summary> <img width="1429" alt="Screenshot 2025-02-24 at 13 11 17" src="https://github.com/user-attachments/assets/a1e75210-757e-42d1-b852-945de5f3f44b" /> </details> <details><summary>Empty chart - no data</summary> <img width="1258" alt="Screenshot 2025-02-13 at 09 47 08" src="https://github.com/user-attachments/assets/c239a5a6-337e-41c9-a9a3-7cdc2c9b1e01" /> </details> ### Definition of done - [x] Add a bar chart titled "Top 10 Asset Types" to the "Asset Inventory" page. - [x] Use the `@elastic/charts` library to implement the visualization. - [x] Configure the chart with: - **X-axis:** Asset type categories - **Y-axis:** Count of assets - **Legend:** A color-coded key linking each bar to a specific category. - [x] Ensure the chart is responsive when resizing the screen and adheres to the [visual spec](https://www.figma.com/design/9zUqAhhglT1EGYG4LOl1X6/Asset-Management?node-id=2946-19648&t=FuD3BEY4FyxAKV38-4). - [x] Integrate the chart so that it updates based on the filters section and the Unified Header component. ### How to test Follow the instructions from [this PR](elastic#208417) to prepare the local env with data. Alternatively, open the `asset_inventory/components/top_assets_bar_chart.tsx` file and edit yourself the `data` prop that we pass into `<BarSeries>` with mocked data. The data must have the following shape: ```js [ { category: 'cloud-compute', source: 'gcp-compute', count: 500, }, { category: 'cloud-compute', source: 'aws-security', count: 300, }, { category: 'cloud-storage', source: 'gcp-compute', count: 221, }, { category: 'cloud-storage', source: 'aws-security', count: 117, }, ]; ``` ### Checklist - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks No risks whatsoever.

## Summary Rename the `FindingsBaseEsQuery` interface exposed by the `@kibana/cloud-security-posture` package as well as all references where it's imported. Separating this renaming into its own PR also lets us tag it with `backport:prev-minor` and avoid potential merge conflicts in the future. ### Depends on - elastic#210938 ### Checklist - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks No risk whatsoever.

albertoblaz added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Cloud Security Cloud Security team related labels Feb 12, 2025

albertoblaz self-assigned this Feb 12, 2025

albertoblaz force-pushed the asset-inv-chart branch 2 times, most recently from 2e98414 to 9c697ef Compare February 18, 2025 12:30

albertoblaz commented Feb 18, 2025

View reviewed changes

...solutions/security/plugins/security_solution/public/asset_inventory/components/bar_chart.tsx Outdated Show resolved Hide resolved

albertoblaz marked this pull request as ready for review February 18, 2025 13:01

albertoblaz requested review from a team as code owners February 18, 2025 13:01

JordanSh requested changes Feb 18, 2025

View reviewed changes

albertoblaz force-pushed the asset-inv-chart branch from 9c697ef to 52a7c85 Compare February 18, 2025 15:15

opauloh requested changes Feb 19, 2025

View reviewed changes

albertoblaz force-pushed the asset-inv-chart branch 4 times, most recently from 3e4497c to 913f8e4 Compare February 24, 2025 14:14

albertoblaz requested review from JordanSh and opauloh February 24, 2025 15:23

opauloh requested changes Feb 24, 2025

View reviewed changes

albertoblaz mentioned this pull request Feb 25, 2025

[DO NOT MERGE] Asset Inventory demo with data-grid, charts, filters and adv setting #212415

Closed

albertoblaz force-pushed the asset-inv-chart branch from 3edb714 to 0068237 Compare February 25, 2025 17:09

albertoblaz requested a review from opauloh February 25, 2025 17:13

albertoblaz mentioned this pull request Feb 25, 2025

Rename FindingsBaseEsQuery interface in CSP package #212427

Merged

1 task

albertoblaz force-pushed the asset-inv-chart branch from 0068237 to 7f8a6bc Compare February 25, 2025 17:43

albertoblaz added 3 commits February 25, 2025 18:49

Implement TopAssetsBarChart for Asset Inventory

9133c9d

Fetch chart data via client-side aggregation

8c78d1b

Refactor fetching hooks to reuse helper functions

26c46c1

albertoblaz added 4 commits February 25, 2025 18:49

Implement chart loading state

d77aa10

Implement chart fetching state

51bef18

Fix loading/fetching states in page & filters

2861e94

Omit redundant runtime mapping in chart data hook

03b3186

albertoblaz force-pushed the asset-inv-chart branch from 7f8a6bc to 03b3186 Compare February 25, 2025 17:49

JordanSh reviewed Feb 26, 2025

View reviewed changes

JordanSh approved these changes Feb 26, 2025

View reviewed changes

Improve chart aggregation code

12b2d3e

opauloh approved these changes Feb 26, 2025

View reviewed changes

Fix quick check failing

23e7343

albertoblaz merged commit f0584d9 into elastic:main Feb 27, 2025
9 checks passed

albertoblaz deleted the asset-inv-chart branch February 27, 2025 08:01

kibanamachine added the v9.1.0 label Feb 27, 2025

Conversation

albertoblaz commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Depends on

Screenshots

Definition of done

How to test

Checklist

Identify risks

Uh oh!

Uh oh!

elasticmachine commented Feb 18, 2025

Uh oh!

JordanSh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

opauloh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

albertoblaz commented Feb 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opauloh left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

albertoblaz commented Feb 12, 2025 •

edited

Loading

JordanSh left a comment •

edited

Loading

elasticmachine commented Feb 26, 2025 •

edited

Loading