-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add streams as part of Datasource / Inputs #230
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The struct & example changes LGTM. We'll need to add/update some types in Kibana under EPM & Datasource. I'm happy to add those changes if that's helpful.
Thanks these are helpful examples to look at! |
@jfsiii Would be great if you can look into the changes needed on EPM side of things. I'm already actively working on agent config/datasource schemas & related APIs. |
@@ -87,11 +147,84 @@ | |||
"type": "password" | |||
} | |||
], | |||
"description": "Collecting metrics for nginx." | |||
"description": "Collecting metrics for nginx.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approved too fast.. 🙂 During our call earlier we also discussed the need for dataset
property, i.e. this line in agent config spec: https://github.com/elastic/beats/blob/feature-ingest/x-pack/agent/docs/agent_configuration_example.yml#L50
I can't find it in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the dataset in the most recent commit. Unfortunately it is not the full name yet as it depends on #176
I'm still debating on the name here but I assume if we change it from foo to bar, the change on your end should be pretty straightforward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, doesn't matter what the property name is, as long as it's there
The streams are defined in the dataset and reference an input. The input is defined in the datasource. For the UI to not have to combine to two together, the API already provides the datasource with all inputs and streams listed inside. This change also extended the test packages to have better examples for this change
15b46a5
to
590dab8
Compare
The streams are defined in the dataset and reference an input. The input is defined in the datasource. For the UI to not have to combine to two together, the API already provides the datasource with all inputs and streams listed inside.
This change also extended the test packages to have better examples for this change.
Note: This PR gets the job done. Code could be cleaned up later.