Add subcategories definitions #914

jsoriano · 2022-11-24T09:43:45Z

Subcategories support.

It models the categories with the subcategories, loading them from a yaml file. We can consider later to move this yaml file to the spec so it can be also consumed from other places.

New parent_id and parent_name fields are added to subcategories in the API.

Part of elastic/package-spec#451.

elasticmachine · 2022-11-24T09:47:09Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-12-05T16:14:21.992+0000
Duration: 6 min 33 sec

Test stats 🧪

Test	Results
Failed	0
Passed	243
Skipped	0
Total	243

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

jlind23

Should we state somewhere in the code that categories shouldn't have more than 1 nested level for now?

jlind23 · 2022-11-24T10:19:36Z

categories/categories.yml

+    subcategories:
+      app_search:
+        title: "Application Search"
+      connector: # TODO: Do we want three different connector categories?


@mukeshelastic @nimarezainia would you be able to weight in here? Do we want three categories?

yes, let's limit the subcategories to only one level depth.. category -> subcategory. And author can specify only one subcategory

And author can specify only one subcategory

Does it mean that a package can have only a subcategory? We don't have this limitation with categories, and this may complicate things.

categories/categories.go

mrodm · 2022-11-24T10:09:02Z

categories/categories.go

+	// TODO: Check for duplicated categories.
+	return categoriesFile.Categories, nil


I was thinking just about those possible duplicated categories. Could a category be defined as a subcategory of more than category?

This won't be possible with the current approach. We will need to use conventions (like network_security vs. network_monitoring) if we want something to be a subcategory in two different categories.

Another option may be to require the use of subcategories in namespaced names, that would be to require to use subcategories as this for example:

categories: - containers - security.network

This would be a package that belongs to the containers category and to the security's network subcategory.

This has the limitation that we cannot reorganize categories, like may happen if we decide to move current thread_intel to be a security subcategory.

Btw, notice that using another field for subcategories doesn't solve this problem. If for example we have network as subcategory of security and monitoring, we wouldn't know to what category network would be refering in a case like this one:

categories: - security - monitoring subcategories: - network

jsoriano · 2022-11-24T10:46:22Z

Should we state somewhere in the code that categories shouldn't have more than 1 nested level for now?

Current code doesn't support nesting categories. I have added also a comment to the header of the yaml file.

jsoriano · 2022-11-30T09:17:53Z

Updated list of categories adding the ones I missed.

There are still some open questions, they are as TODOs in the yml, but grouping them:

New subcategories that were main categories before. Options:
1. Remove main category, leave the subcategory. I'd prefer this option. To avoid breaking changes this would require the API design with the subcategory_of field, what is also fine for me.
2. Keep main category, add a new subcategory with a different name.
New categories that overlap with old categories (such as datastore vs. database). Options:
1. Keep old categories, dismiss new ones.
2. Keep old categories, but change the title to the new one.
3. Keep both categories. I'd avoid this option as can be confusing.
Duplicated subcategories (such as observability/monitoring vs. infrastructure/monitoring. Options:
1. Keep subcategories with different suffixes. I have already added suffixes to some categories.
2. Use a single category. We would need to decide which one to keep.
3. Decide case by case between the previous options.
4. Allow subcategories to be in multiple main categories. This can be ambiguous as described in Add subcategories definitions #914 (comment)
Too specific product categories. Such as the one for WebSphere, but Kubernetes could be another one. Options:
1. Keep them. This is probably fine, but where to draw the line before adding more.
2. Keep them, but only as subcategories.
3. Decide case by case.

@jlind23 @mukeshelastic @dborodyansky please take a look to these open questions. I can also take the decision, but I may be overlooking something.

amitkanfer · 2022-11-30T09:26:49Z

Maybe we should ask the relevant peers from the solution teams to review and signoff?
Also - i guess it's not a problem to change it later on...? @jsoriano

jlind23 · 2022-11-30T12:10:31Z

@jsoriano my answers below

New subcategories that were main categories before. Options:
1. Remove main category, leave the subcategory. I'd prefer this option. To avoid breaking changes this would require the API design with the subcategory_of field, what is also fine for me.

This one will provide more clarity and avoid duplication problems in the near future.

New categories that overlap with old categories (such as datastore vs. database). Options:
1. Keep old categories, dismiss new ones.
2. Keep old categories, but change the title to the new one.
3. Keep both categories. I'd avoid this option as can be confusing.

Looking at the work done by @dborodyansky I would rather remove the old categories and display the ones. @kpollich could this be a breaking change on the UI end? Don't we have some hard coded featured integration that relies on some categories?

Duplicated subcategories (such as observability/monitoring vs. infrastructure/monitoring. Options:
1. Keep subcategories with different suffixes. I have already added suffixes to some categories.

Keeping subcategories with different suffixes is the best path forward, we might have observability users eager to find monitoring related integrations under the observability umbrella and who won't take a look under the infrastructure umbrella for example.

Too specific product categories. Such as the one for WebSphere, but Kubernetes could be another one. Options:
1. Keep them. This is probably fine, but where to draw the line before adding more.

We should keep them for now. The line about adding more categories was already explained by Akshay in this document:
https://docs.google.com/document/d/1pMsELVC4XbGXeFsR2gvELk5HLrLaNCujztogRzKjbOU/edit

kpollich · 2022-11-30T12:29:45Z

Looking at the work done by @dborodyansky I would rather remove the old categories and display the ones. @kpollich could this be a breaking change on the UI end? Don't we have some hard coded featured integration that relies on some categories?

There is some hardcoding related to categories in the custom integrations plugin here: https://github.com/elastic/kibana/blob/2c774f536e2cd0b01b8829d842684e67df2c3d57/src/plugins/custom_integrations/common/index.ts

We'll need to align this file to the new categories/subcategories concept in general, but if there are any renames we should tackle those first to avoid breakage.

jsoriano · 2022-11-30T12:33:01Z

Thanks @jlind23 for your clarifications, a couple of follow ups:

New subcategories that were main categories before. Options:

Remove main category, leave the subcategory. I'd prefer this option. To avoid breaking changes this would require the API design with the subcategory_of field, what is also fine for me.

This one will provide more clarity and avoid duplication problems in the near future.

👍

New categories that overlap with old categories (such as datastore vs. database). Options:

Keep old categories, dismiss new ones.

Keep old categories, but change the title to the new one.

Keep both categories. I'd avoid this option as can be confusing.

Looking at the work done by @dborodyansky I would rather remove the old categories and display the ones. @kpollich could this be a breaking change on the UI end? Don't we have some hard coded featured integration that relies on some categories?

Removing categories is not an option, at least without breaking changes. The closer option would be option two, keeping old categories, but changing the titles.
There could be a similar fourth option, that would be to keep both old and new categories, but both with the new titles.

Duplicated subcategories (such as observability/monitoring vs. infrastructure/monitoring. Options:

Keep subcategories with different suffixes. I have already added suffixes to some categories.

Keeping subcategories with different suffixes is the best path forward, we might have observability users eager to find monitoring related integrations under the observability umbrella and who won't take a look under the infrastructure umbrella for example.

👍

Regarding the current main monitoring category, should we move it under observability, so we have:

monitoring as subcategory of observability.
monitoring_infrastructure as subcategory of infrastructure.

Or keep the three categories:

monitoring main category as the current one.
monitoring_observability as subcategory of observability.
monitoring_infrastructure as subcategory of infrastructure.

Too specific product categories. Such as the one for WebSphere, but Kubernetes could be another one. Options:

Keep them. This is probably fine, but where to draw the line before adding more.

We should keep them for now. The line about adding more categories was already explained by Akshay in this document: https://docs.google.com/document/d/1pMsELVC4XbGXeFsR2gvELk5HLrLaNCujztogRzKjbOU/edit

👍

jlind23 · 2022-11-30T12:36:29Z

Removing categories is not an option, at least without breaking changes. The closer option would be option two, keeping old categories, but changing the titles.
There could be a similar fourth option, that would be to keep both old and new categories, but both with the new titles.

Sorry I meant option 2 only after checking with @kpollich that this doesn't have any impact on the UI.

jsoriano · 2022-11-30T13:42:54Z

TODOs resolved, and API changed, please review.

CHANGELOG.md

jsoriano · 2022-11-30T13:47:17Z

testdata/generated/storage-indexer/categories.json

-    "count": 14
+    "title": "Web Server",
+    "count": 14,
+    "subcategory_of": "infrastructure"
  }


These will be the changes in the API:

Titles updated for some categories.

subcategory_of added in subcategories.

jsoriano · 2022-11-30T15:48:22Z

testdata/generated/categories-experimental.json

-    "count": 3
+    "title": "Web Server",
+    "count": 3,
+    "subcategory_of": "infrastructure"


@kpollich with this API response, Fleet is not going to be able to get the title of the parent category, should I change this to something like this?

Suggested change

"subcategory_of": "infrastructure"

"parent_id": "infrastructure",

"parent_name": "Infrastructure"

An alternative could be to include the parent category in the response, but this could affect current versions of kibana that would receive categories with count 0.

This also makes me wonder if we want to modify the search APIs, to include packages of subcategories, when searching for a main category. That would for example be to include packages of the web subcategory, when looking for the integration category.

with this API response, Fleet is not going to be able to get the title of the parent category, should I change this to something like this?

It seems like the issue is that integrations can be placed in a subcategory even if there are no integrations placed in the parent category, resulting in a case where the parent category is not included in the response.

So we have a state like this

I think returning the parent_name would be most helpful here, as it allows the UI to render the Infrastructure category in this case even if it does not directly contain any integrations.

PR updated to include the parent_name in the API response, @kpollich please take another look.

testdata/generated/categories.json

Add subcategories

05a09a1

jsoriano requested a review from a team November 24, 2022 09:43

jsoriano self-assigned this Nov 24, 2022

Add changelog

a762eb2

Fix imports

9e72c5d

jsoriano force-pushed the subcategories branch from c379893 to 9e72c5d Compare November 24, 2022 09:50

jsoriano mentioned this pull request Nov 24, 2022

[Change Proposal] Allow for nested category structure elastic/package-spec#451

Closed

jlind23 reviewed Nov 24, 2022

View reviewed changes

mrodm reviewed Nov 24, 2022

View reviewed changes

Add header to categories file

8d5e58f

Unused import

1ff0787

mrodm approved these changes Nov 28, 2022

View reviewed changes

jlind23 approved these changes Nov 29, 2022

View reviewed changes

jsoriano added 2 commits November 30, 2022 09:37

Merge remote-tracking branch 'origin/main' into subcategories

fdf4d63

Sync categories list

0ff2d03

jsoriano added 3 commits November 30, 2022 13:57

Resolve open questions

f30d899

Avoid ambigous definitions of categories

e19b176

Include reference to parent category in API response

c890931

jsoriano requested review from mrodm, mukeshelastic and jlind23 November 30, 2022 13:42

jsoriano requested a review from kpollich November 30, 2022 13:43

jsoriano commented Nov 30, 2022

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

jsoriano commented Nov 30, 2022

View reviewed changes

jlind23 approved these changes Nov 30, 2022

View reviewed changes

jsoriano commented Nov 30, 2022

View reviewed changes

Include parent title for each category

aa973c6

kpollich approved these changes Dec 5, 2022

View reviewed changes

testdata/generated/categories.json Show resolved Hide resolved

Merge branch 'main' into subcategories

fbd1ee0

jsoriano merged commit 50855b4 into elastic:main Dec 5, 2022

jsoriano deleted the subcategories branch December 5, 2022 17:15

jsoriano mentioned this pull request Jan 27, 2023

Prepare version 2.5.0 with new subcategories elastic/package-spec#469

Merged

jsoriano mentioned this pull request Mar 10, 2023

[Change Proposal] Adding categories for Infrastructure Observability elastic/package-spec#488

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subcategories definitions #914

Add subcategories definitions #914

jsoriano commented Nov 24, 2022 •

edited

Loading

elasticmachine commented Nov 24, 2022 •

edited

Loading

Build stats

Test stats 🧪

jlind23 left a comment

jlind23 Nov 24, 2022

mukeshelastic Nov 29, 2022

jsoriano Nov 30, 2022

mrodm Nov 24, 2022

jsoriano Nov 24, 2022 •

edited

Loading

jsoriano Nov 24, 2022

jsoriano commented Nov 24, 2022

jsoriano commented Nov 30, 2022

amitkanfer commented Nov 30, 2022

jlind23 commented Nov 30, 2022

kpollich commented Nov 30, 2022

jsoriano commented Nov 30, 2022

jlind23 commented Nov 30, 2022

jsoriano commented Nov 30, 2022

jsoriano Nov 30, 2022

jsoriano Nov 30, 2022

kpollich Nov 30, 2022

jsoriano Dec 5, 2022

		// TODO: Check for duplicated categories.
		return categoriesFile.Categories, nil

	"subcategory_of": "infrastructure"
	"parent_id": "infrastructure",
	"parent_name": "Infrastructure"

Add subcategories definitions #914

Add subcategories definitions #914

Conversation

jsoriano commented Nov 24, 2022 • edited Loading

elasticmachine commented Nov 24, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

jlind23 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

jsoriano Nov 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoriano commented Nov 24, 2022

jsoriano commented Nov 30, 2022

amitkanfer commented Nov 30, 2022

jlind23 commented Nov 30, 2022

kpollich commented Nov 30, 2022

jsoriano commented Nov 30, 2022

jlind23 commented Nov 30, 2022

jsoriano commented Nov 30, 2022

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

jsoriano commented Nov 24, 2022 •

edited

Loading

elasticmachine commented Nov 24, 2022 •

edited

Loading

jsoriano Nov 24, 2022 •

edited

Loading