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.

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

Support input groups #137

Merged
merged 17 commits into from
Mar 2, 2021
2 changes: 1 addition & 1 deletion code/go/internal/spec/statik.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions code/go/pkg/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestValidateFile(t *testing.T) {
},
},
"input_template": {},
"policy_groups": {},
}

for pkgName, test := range tests {
Expand Down
1 change: 1 addition & 0 deletions test/packages/policy_groups/_dev/build/docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
README template for integration
1 change: 1 addition & 0 deletions test/packages/policy_groups/_dev/build/docs/dynamodb.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DynamoDB README template file
1 change: 1 addition & 0 deletions test/packages/policy_groups/_dev/build/docs/ec2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
EC2 README template file
5 changes: 5 additions & 0 deletions test/packages/policy_groups/changelog.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- version: 1.0.0
changes:
- description: initial release
type: enhancement
link: https://github.com/elastic/package-spec/pull/0
1 change: 1 addition & 0 deletions test/packages/policy_groups/docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
README for integration
1 change: 1 addition & 0 deletions test/packages/policy_groups/docs/dynamodb.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DynamoDB README file
1 change: 1 addition & 0 deletions test/packages/policy_groups/docs/ec2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
EC2 README file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/packages/policy_groups/img/aws-dynamodb.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/packages/policy_groups/img/aws-ec2.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/packages/policy_groups/img/aws-logo.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
111 changes: 111 additions & 0 deletions test/packages/policy_groups/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
format_version: 1.0.0
name: aws
title: AWS
version: 0.3.17
license: basic
description: AWS Integration
type: integration
categories:
Copy link
Contributor

Choose a reason for hiding this comment

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

@sorantis Could have a group/set its own categories? Is it always a subset of the package categories?

Choose a reason for hiding this comment

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

In some cases it's a bigger set, i.e. DynamoDB is an aws service, a cloud service and a datastore service. datastore here is specific to DynamoDB and will not be applicable to other, for example compute related services.

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is what happens if someone clicks on a category, what will the filter return:

  • All integration sets + package
  • Only package
  • Only integration sets

Based on the above we should know if we need categories inside the integration sets / policy templates.

Copy link
Contributor Author

@mtojek mtojek Feb 23, 2021

Choose a reason for hiding this comment

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

Yeah and extending to what @ruflin said, it's more natural to keep DynamoDB, EC2 and others as separate packages ;)

Copy link

@sorantis sorantis Feb 23, 2021

Choose a reason for hiding this comment

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

If the user clicks on say "Datastore" then they should be able to see all integrations that cover databases, data stores, etc. which also includes cloud data store services. Since the tiles are (will be) generated from policy_templates then I think it's the option 3 in our list.

This creates another question though. What happens if the user searches for AWS only, do we show AWS as a tile? The answer yes if we create a policy template (integration set) that is called AWS.

This should work automatically for other integrations, for example NGINX. When the user searches for Nginx we show the Nginx tile because there's a policy template for it, we don't have to use package level information for it.
Screen Shot 2021-02-23 at 11 25 10

Choose a reason for hiding this comment

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

Speaking of how to express categories in the manifest file, I suggest we keep the top level categories section required, and introduce an optional categories section to a policy template.
This way each policy template will inherit top level categories and also has the option to add more policy template specific categories if needed.
Thoughts? @mtojek @ycombinator @ruflin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words, categories on the root level (in manifest) would be common buckets valid for all policy templates, right?

Choose a reason for hiding this comment

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

yes, with the option to extend this list for each policy template. I.e. for AWS common buckets could be cloud, aws and DynamoDB could optionally extend this list with datastores by defining it at the policy template level.

- aws
- cloud
- network
- security
release: beta
conditions:
kibana.version: '^7.9.0'
icons:
- src: /img/aws-logo.svg
title: logo aws
size: 32x32
type: image/svg+xml
screenshots:
- src: /img/aws-general-dashboard.png
title: General AWS dashboard
size: 1702x1063
type: image/png
vars:
- name: access_key_id
type: text
title: Access Key ID
- name: secret_access_key
type: text
title: Secret Access Key
policy_templates:
Copy link
Contributor

Choose a reason for hiding this comment

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

@mtojek is this the array from which Kibana only shows the first element today? Or is that something different?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe @jen-huang can weigh in on my question since she's already watching this PR. 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, this is what Kibana uses the first element of today

- name: ec2
title: AWS EC2
description: Collect logs and metrics from EC2 service
inputs:
- type: s3
title: Collect logs from EC2 service
description: Collecting EC2 logs using S3 input
policy_group: logs
vars:
- name: visibility_timeout
type: text
title: Visibility Timeout
description: The duration that the received messages are hidden from subsequent retrieve requests after being retrieved by a ReceiveMessage request. The maximum is 12 hours.
- name: api_timeout
type: text
title: API Timeout
description: The maximum duration of AWS API can take. The maximum is half of the visibility timeout value.
- type: aws/metrics
title: Collect metrics from EC2 service
description: Collecting EC2 metrics using AWS CloudWatch
policy_group: metrics
vars:
- name: endpoint
type: text
title: Endpoint
default: "amazonaws.com"
description: URL of the entry point for an AWS web service.
icons:
- src: /img/aws-ec2.svg
title: AWS EC2 logo
size: 32x32
type: image/svg+xml
screenshots:
- src: /img/aws-ec2-overview.png
title: AWS EC2 Overview
size: 1702x1063
type: image/png
vars:
- name: ec2_foobar_property
type: bool
title: EC2 Specific Property
- name: dynamodb
title: AWS DynamoDB
description: Collect logs and metrics from DynamoDB service
categories:
- datastore
inputs:
- type: s3
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
title: Collect logs from the DynamoDB service
mtojek marked this conversation as resolved.
Show resolved Hide resolved
description: Collecting DynamoDB logs using S3 input
policy_group: logs
- type: aws/metrics
title: Collect metrics from DynamoDB service
description: Collecting DynamoDB metrics using AWS CloudWatch
policy_group: metrics
icons:
- src: /img/aws-dynamodb.svg
title: AWS DynamoDB logo
size: 32x32
type: image/svg+xml
screenshots:
- src: /img/aws-dynamodb-overview.png
title: AWS DynamoDB Overview
size: 1702x1063
type: image/png
vars:
- name: dynamodb_foobar_property
type: bool
title: DynamoDB Specific Property
policy_groups:
mtojek marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Ignoring naming on my end for now ;-)

I'm wondering if the groups should be defined as part of each policy_template or global. Having it local would allow to give more descriptive descriptions. At the same time, I don't think we use it today. Global requires much fewer configuration.

An alternative option is to just not have it in the package at first. Only 2-3 groups are supported, required by the spec and hardcoded into Kibana. Like this everyone is forced to use logs/metrics as groups.

Copy link
Contributor Author

@mtojek mtojek Feb 22, 2021

Choose a reason for hiding this comment

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

Actually your both options are related. I agree that global definitions would require less configuration properties and might be easier to setup. The second option is an interesting solution - we can try to kick off the feature with predefined groups and expose only them to users. We need to remember that we won't escape from these tabs in the future :)

Let's collect some other comments on this idea.

Copy link
Contributor

@ycombinator ycombinator Feb 22, 2021

Choose a reason for hiding this comment

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

I'd prefer to start with the most restrictive option we can get away with and then make things less restrictive / more flexible depending on feedback from actual usage. So I'm ++ for hardcoding the group names (logs or metrics) for now.

How far do we take this hardcoding? I assume not just the group names are hardcoded but also the titles — "Logs" or "Metrics". What about the descriptions — should those be somewhat hardcoded as well, e.g. "Logs for $package_name package"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sorantis Any thoughts about hardcoding group names (logs, metrics, later on - checks)?

Choose a reason for hiding this comment

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

responded here #137 (comment)

- name: logs
title: "Logs"
description: "Collect logs from AWS service"
- name: metrics
title: "Metrics"
description: "Collect metrics from AWS service"
owner:
github: elastic/integrations-platforms
7 changes: 6 additions & 1 deletion versions/1/_dev/build/docs/spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@ spec:
type: file
name: "README.md"
contentMediaType: "text/markdown"
required: true
required: true
- description: Other README template files (can be used by policy templates)
type: file
contentMediaType: "text/markdown"
pattern: '^.+.md'
required: false
4 changes: 3 additions & 1 deletion versions/1/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,6 @@
- description: Add specification for package CHANGELOG files.
type: enhancement
link: https://github.com/elastic/package-spec/pull/131

- description: Add policy groups
type: enhancement
link: https://github.com/elastic/package-spec/pull/137
5 changes: 5 additions & 0 deletions versions/1/docs/spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@
contentMediaType: "text/markdown"
name: "README.md"
required: true
- description: Other README files (can be used by policy templates)
type: file
contentMediaType: "text/markdown"
pattern: '^.+.md'
required: false
Loading