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.

Sign up for GitHub

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

Jump to bottom

[Inventory][ECO] Use ControlGroupRenderer to filter by entity types #199174

Merged
merged 27 commits into from
Nov 12, 2024
Merged

[Inventory][ECO] Use ControlGroupRenderer to filter by entity types #199174

merged 27 commits into from
Nov 12, 2024

Conversation

cauemarcondes
Copy link
Contributor

@cauemarcondes cauemarcondes commented Nov 6, 2024 •
edited by kibanamachine
Loading

closes #193397

control.groups.mov
  • Added Entity type control group field on the Inventory page.
  • Added Filters buttons to the Unified Search bar on the Inventory oage
  • Moved common hooks from infra to Obs-shared
  • Refactoring

@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v8.17.0 labels Nov 7, 2024
@cauemarcondes cauemarcondes marked this pull request as ready for review November 7, 2024 14:32
@cauemarcondes cauemarcondes requested review from a team as code owners November 7, 2024 14:32
@cauemarcondes cauemarcondes added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Nov 7, 2024
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 7, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 7, 2024

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

crespocarlos
crespocarlos reviewed Nov 8, 2024
View reviewed changes
Copy link
Contributor

@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 don't think this was introduced by this PR, but do you think we could try to fix the reponsiveness here

image image

x-pack/plugins/observability_solution/inventory/public/components/app_root/index.tsx Show resolved Hide resolved
...ck/plugins/observability_solution/inventory/public/components/search_bar/discover_button.tsx Outdated Show resolved Hide resolved
x-pack/plugins/observability_solution/inventory/public/hooks/use_kibana_query_settings.ts Outdated Show resolved Hide resolved
x-pack/plugins/observability_solution/inventory/public/services/telemetry/telemetry_client.ts
Comment on lines -37 to -42
public reportEntityInventoryEntityTypeFiltered = (
params: EntityInventoryEntityTypeFilteredParams
) => {
this.analytics.reportEvent(TelemetryEventTypes.ENTITY_INVENTORY_ENTITY_TYPE_FILTERED, params);
};

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this telemetry info anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's no longer as straightforward as it was before to get the selected entity types now using control groups. The selected items are in the URL anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. should we align this with Roshan?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc: @roshan-elastic

Copy link

Choose a reason for hiding this comment

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

Hey @cauemarcondes @crespocarlos - will we still be able to populate the telemetry like this?

{
	"timestamp": "2024-11-12T11:20:08.790Z",
	"event_type": "Entity Inventory Search Query Submitted",
	"context": {
	...
	},
	"properties": {
		"kuery_fields": ["entity.name"],
		"entity_types": ["service"],
		"action": "refresh"
	}
}

Not super fussed if we explicitly track the control filter itself but if someone filters by entity.type either via the control filter via a query - we need to track this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we won't be able to. The control group has a complex data structure and it's not possible to easily extract the selected options. It also can select item but exclude them from the query.

x-pack/plugins/observability_solution/inventory/public/hooks/use_unified_search_url.ts Outdated Show resolved Hide resolved
x-pack/plugins/observability_solution/inventory/public/hooks/use_unified_search_url.ts Show resolved Hide resolved
@weltenwort weltenwort requested review from weltenwort and removed request for jennypavlova November 11, 2024 12:51
@cauemarcondes cauemarcondes requested a review from a team as a code owner November 11, 2024 12:56
jennypavlova
jennypavlova approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thanks for the changes!

@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 11, 2024 •
edited
Loading

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1527 1525 -2
inventory 245 276 +31
logsShared 688 687 -1
observabilityShared 214 221 +7
total +35

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
observabilityShared 502 516 +14

Async chunks

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

id before after diff
infra 1.7MB 1.7MB -2.2KB
inventory 233.7KB 236.4KB +2.6KB
investigateApp 483.5KB 483.5KB +18.0B
logsShared 424.3KB 423.9KB -399.0B
observabilityShared 54.9KB 54.9KB +7.0B
total +96.0B

Page load bundle

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

id before after diff
inventory 12.4KB 12.2KB -233.0B
logsShared 175.2KB 175.0KB -134.0B
observabilityShared 75.7KB 93.6KB +17.9KB
total +17.6KB
Unknown metric groups

API count

id before after diff
observabilityShared 508 522 +14

ESLint disabled line counts

id before after diff
inventory 2 1 -1

Total ESLint disabled count

id before after diff
inventory 6 5 -1

History

weltenwort
weltenwort approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Logs UI changes LGTM

crespocarlos
crespocarlos approved these changes Nov 12, 2024
View reviewed changes
Copy link
Contributor

@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.

LGTM! I would just double check with Roshan the removal of the telemetry event. We could also open a follow up PR to add it back, if we need to.

packages/kbn-optimizer/limits.yml
@@ -119,7 +119,7 @@ pageLoadAssetSize:
observabilityAiAssistantManagement: 19279
observabilityLogsExplorer: 46650
observabilityOnboarding: 19573
observabilityShared: 80000
observabilityShared: 111036
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 there is anything we could do to avoid increasing the plugin size here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about this too. But this is out of the scope of this PR. I think this is a good topic for the Devex-ui-wg.

@cauemarcondes cauemarcondes merged commit 4a16e91 into elastic:main Nov 12, 2024
28 checks passed
@cauemarcondes cauemarcondes deleted the inventory-control-group-2 branch November 12, 2024 10:28
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11795377565

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
❌ 8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 199174

Questions ?

Please refer to the Backport tool documentation

cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Nov 12, 2024
@cauemarcondes
[Inventory][ECO] Use ControlGroupRenderer to filter by entity types (e…
fd42650 
…lastic#199174)

closes elastic#193397

https://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e

- Added `Entity type` control group field on the Inventory page.
- Added `Filters` buttons to the Unified Search bar on the Inventory
oage
- Moved common hooks from infra to Obs-shared
- Refactoring

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 4a16e91)
@cauemarcondes cauemarcondes mentioned this pull request Nov 12, 2024
Merged
@cauemarcondes
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Nov 12, 2024
Merged
tkajtoch pushed a commit to tkajtoch/kibana that referenced this pull request Nov 12, 2024
@cauemarcondes @kibanamachine @tkajtoch
[Inventory][ECO] Use ControlGroupRenderer to filter by entity types (e…
c95febe 
…lastic#199174)

closes elastic#193397


https://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e

- Added `Entity type` control group field on the Inventory page. 
- Added `Filters` buttons to the Unified Search bar on the Inventory
oage
- Moved common hooks from infra to Obs-shared
- Refactoring

---------

Co-authored-by: kibanamachine <[email protected]>
cauemarcondes added a commit that referenced this pull request Nov 13, 2024
@cauemarcondes
[8.x] [Inventory][ECO] Use ControlGroupRenderer to filter by entity t…
96d216e 
…ypes (#199174) (#199820)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Inventory][ECO] Use ControlGroupRenderer to filter by entity types
(#199174)](#199174)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Cauê
Marcondes","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-12T10:28:02Z","message":"[Inventory][ECO]
Use ControlGroupRenderer to filter by entity types (#199174)\n\ncloses
https://github.com/elastic/kibana/issues/193397\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e\r\n\r\n-
Added `Entity type` control group field on the Inventory page. \r\n-
Added `Filters` buttons to the Unified Search bar on the
Inventory\r\noage\r\n- Moved common hooks from infra to Obs-shared\r\n-
Refactoring\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"4a16e910e95d62fd4cc52f4a870147d691b1a681","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","v8.17.0"],"number":199174,"url":"https://github.com/elastic/kibana/pull/199174","mergeCommit":{"message":"[Inventory][ECO]
Use ControlGroupRenderer to filter by entity types (#199174)\n\ncloses
https://github.com/elastic/kibana/issues/193397\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e\r\n\r\n-
Added `Entity type` control group field on the Inventory page. \r\n-
Added `Filters` buttons to the Unified Search bar on the
Inventory\r\noage\r\n- Moved common hooks from infra to Obs-shared\r\n-
Refactoring\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"4a16e910e95d62fd4cc52f4a870147d691b1a681"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199174","number":199174,"mergeCommit":{"message":"[Inventory][ECO]
Use ControlGroupRenderer to filter by entity types (#199174)\n\ncloses
https://github.com/elastic/kibana/issues/193397\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e\r\n\r\n-
Added `Entity type` control group field on the Inventory page. \r\n-
Added `Filters` buttons to the Unified Search bar on the
Inventory\r\noage\r\n- Moved common hooks from infra to Obs-shared\r\n-
Refactoring\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"4a16e910e95d62fd4cc52f4a870147d691b1a681"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@cauemarcondes cauemarcondes mentioned this pull request Nov 13, 2024
Closed
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
@cauemarcondes @kibanamachine @CAWilson94
[Inventory][ECO] Use ControlGroupRenderer to filter by entity types (e…
cce79bc 
…lastic#199174)

closes elastic#193397


https://github.com/user-attachments/assets/e78639a8-bc63-4c5a-8676-0ad9b5f0563e

- Added `Entity type` control group field on the Inventory page. 
- Added `Filters` buttons to the Unified Search bar on the Inventory
oage
- Moved common hooks from infra to Obs-shared
- Refactoring

---------

Co-authored-by: kibanamachine <[email protected]>
@iblancof iblancof mentioned this pull request Nov 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roshan-elastic roshan-elastic roshan-elastic left review comments

@jennypavlova jennypavlova jennypavlova approved these changes

@afharo afharo afharo approved these changes

@jbudz jbudz jbudz approved these changes

@crespocarlos crespocarlos crespocarlos approved these changes

@weltenwort weltenwort weltenwort approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Inventory][ECO] Use ControlGroupRenderer to filter by entity types
9 participants
@cauemarcondes @elasticmachine @kibanamachine @weltenwort @crespocarlos @jbudz @afharo @jennypavlova @roshan-elastic