Add input groups support #685
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
|
@kaiyan-sheng is this PR ready for review? Asking because it's in draft status so I wasn't sure if you wanted us to start reviewing it or if it's more of a WIP at this point. Thanks! |
|
Thanks for the instructions on running this with the AWS package changes! For the search endpoint I'm not sure whether |
|
Same thinking as @jen-huang on this. Maybe there is an alternative option here. One thing that bugs me is that we try to munch to different concepts into a single API endpoint. Even with the change, a package stays a package but it contains integrations / input groups. Maybe there should be 2 different API endpoints? One for packages, one for integrations. The format returned would be similar but not identical. Like this we can keep the package endpoint as is and add a new additional one with a new format. Maybe Kibana would then make 2 API calls instead of one to get the full overview? Or the integrations endpoint would also have an option to show packages but not the other way around? Note: Replace |
Right, we need to decide what each item in the response from the Given that this is a package registry API, I'm slightly in favor of having each item in the API response stand for an individual package. If that package defines multiple policy templates, those should be nested under that package ("17 policy templates as a property of the single AWS package result"). This would preserve the relationship between packages and policy templates, and it would be up to the UI to decide on the best presentation of this relationship: either show the hierarchy, like @ruflin once suggested using Google search results and sub-results as an example, or flatten it so each tile in the search results UI could be a package or a policy template, like @sorantis had shown in the mockups for this feature. Basically, we would be separating what information is to be presented (API response) from how that information should be presented (UI). IOW, the API wouldn't need to decide or have any influence on how it's data should be presented. Since this would be an additive change in the API response, older versions of Kibana would continue to render search results the way they do today (i.e. ignoring policy templates) even with the new response structure, while newer versions of Kibana would know how to render the additional information returned in the response. |
+1, will make the change and update API response in PR description soon. Thank you all for the input!! |
|
I took a closer look at the response of the package info endpoint
|
|
Thanks @jen-huang for your review!!
We have a separate field for
What do you mean by top-level here? My understanding is |
I think this is a valid concern. We need a way to reflect this property in API:
We agreed with @ruflin and @ycombinator to kick off with predefined input groups: "logs" and "metrics". We're just cautious and don't want to give the end-user too much control. In the feature we may extend this set. |
Ahh yes sorry I missed these. Will add it in next commit! Thank you!! |
@mtojek What do you mean by "predefined" input groups? Does this mean we expect Kibana to automatically know about "logs" and "metrics" groups and automatically populate pretty titles in the UI like |
|
Thanks for the changes to return Edit: the empty paths are present on the package info endpoint too, for both icons and screenshots "policy_templates": [
{
"name": "billing",
"title": "AWS Billing",
"description": "Collect AWS billing metrics",
"icons": [
{
"src": "/img/logo_billing.svg",
"path": "",
"title": "AWS Billing logo",
"size": "32x32",
"type": "image/svg+xml"
}
]
},
{
"name": "cloudtrail",
"title": "AWS Cloudtrail",
"description": "Collect logs from AWS Cloudtrail",
"icons": [
{
"src": "/img/logo_cloudtrail.svg",
"path": "",
"title": "AWS Cloudtrail logo",
"size": "32x32",
"type": "image/svg+xml"
}
]
},
...
} |
|
Hi, me once again 😅 After implementing per-integration tiles in Kibana, I realized that now the category count looks off in Kibana compared to the number of tiles. Kibana forwards category count results from package registry's
Thoughts?
|
|
My understanding is that the list of categories with counts on the left is a form of faceted navigation for the search results (tiles) on the right. If so, personally I think the best approach is to have the data on the left be derived from the data on the right, i.e. option 1. I suspect until we get into 10s of 1000s of search results we're not going to see a performance issue. Is there a reason we didn't do this to begin with and instead have a |
|
Hi, another set of questions this morning Once at the policy_template > input level:
And again at the data_stream > streams level for
Is there any reason to have input-level vars anymore with this new structure? Can all vars be at the package- or stream- level now? |
@jen-huang Good catch :) This is actually a mistake on the |
I just checked the package spec and it looks like there is no hard requirement on the input groups being defined as either That said, input groups also require a field called Just a side note: when looking at the package spec for input groups specs, I got a bit confused because we call them policy groups sometimes and input groups sometimes. I'm going to make a PR now to make things consistent and call all references input groups. |
|
Actually, I just noticed that there is, in fact a required |
|
@ycombinator Thank you for pointing that out. Somehow I managed to miss input groups in this PR specifically for adding input groups support... Now it's added. I added input groups as a part of the |
|
I'm a bit confused now, because I remember we talked about predefining groups like metrics or logs. Now I see that they should be defined in manifests. I understand the concern @jen-huang raised regarding rendering descriptions. I'm fine with both approaches, just better to be on the same page :) |
|
I agree with the confusion, @mtojek. If we allow each package to define input groups via the One solution could be to treat input groups like we do categories. We truly predefine them via an Personally, I'd prefer to start with this stricter |
|
I also prefer to start with the strict predefined groups and only open it later to be more flexible. |
|
Thanks for the Without param: With |
|
@jen-huang Ha sorry I was fixing it and forgot to comment :) You beat me to it! I think now with the new commit, it should look better. |
|
Based on the discussion and consensus above about predefining input groups, I've created elastic/package-spec#158 for restricting the input groups to @kaiyan-sheng @jen-huang please note that this will most likely impact this PR here and corresponding changes in Kibana so you might want to wait till elastic/package-spec#158 is reviewed and merged. |
|
@kaiyan-sheng @jen-huang I've just merged elastic/package-spec#158. Please take a look and make the necessary adjustments in this PR and the related Kibana PR as well. I think the main change is that each policy template will now just return the name of its input group (no title or description). Kibana will need to generate the title/description. |
|
@ycombinator Done, thank you! |
|
👋 @jen-huang @ycombinator Thank you for all the comments and help! I know the work on Kibana side is still on going but I'm planning to merge this PR for now and create more smaller PRs later once you see more missing pieces on the registry side. |
|
Great work! |
This PR is to add support of input groups to package registry:
/searchendpoint/packageendpoint/packageendpoint/packageendpoint/packageendpointinclude_policy_templatesquery parameter for/categoriesendpointHow to test it
I copied
awsintegration from elastic/integrations#767 intoelastic/pacakge-registry/build/package-storage/packages/aws/0.5.2and started package registry withgo run ..Sample Results
Here are results from some endpoints.
http://localhost:8080/search?package=awsoutput:Expand
http://localhost:8080/package/aws/0.5.2/output:Expand
http://localhost:8080/categories?include_policy_templates=trueendpoint:Expand
``` [ { "id": "aws", "title": "AWS", "count": 20 }, { "id": "azure", "title": "Azure", "count": 2 }, { "id": "cloud", "title": "Cloud", "count": 21 }, { "id": "compute", "title": "compute", "count": 1 }, { "id": "containers", "title": "Containers", "count": 1 }, { "id": "crm", "title": "CRM", "count": 1 }, { "id": "custom", "title": "Custom", "count": 13 }, { "id": "datastore", "title": "Datastore", "count": 4 }, { "id": "message_queue", "title": "Message Queue", "count": 1 }, { "id": "monitoring", "title": "Monitoring", "count": 2 }, { "id": "network", "title": "Network", "count": 21 }, { "id": "os_system", "title": "OS \u0026 System", "count": 2 }, { "id": "productivity", "title": "Productivity", "count": 1 }, { "id": "security", "title": "Security", "count": 24 }, { "id": "web", "title": "Web", "count": 4 } ] ```