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

Add edits for prometheus module docs #3520

Merged
merged 2 commits into from
Feb 8, 2017

Conversation

dedemorton
Copy link
Contributor

A few minor editorial fixes.

@ruflin The fields.yml file for the collector metricset is empty: https://github.com/elastic/beats/blob/master/metricbeat/module/prometheus/collector/_meta/fields.yml. Can you update the fields.yml file (or let me know the fields/descriptions, and I can add them)?

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

The fields.yml for collector will stay empty as we don't know the field in advance of the exporter. That is the reason it is a "dyanmic" metricset.


All events with the same labels are grouped together as one event.

//REVIEWERS: Should we say something about which Prometheus exporters are supported? Users might follow the link and think that all the exporters listed on the page are supported, but I'm not sure if that's true.
Copy link
Contributor

Choose a reason for hiding this comment

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

All the exporters are supported as it is all the same format.

@dedemorton
Copy link
Contributor Author

@ruflin I think users might be confused if they try look at the "Exported Fields" topic to figure out more about the fields returned by the collector metricset. Could we add some basic content in the fields.yml file so that we end up with a section that says something like the following? At least they'll understand why the field they're looking for is missing from the doc.

collector Fields
Stats about the Prometheus exporter. The fields exported by this metricset vary depending on the exporter that you're using.

@ruflin
Copy link
Contributor

ruflin commented Feb 6, 2017

@dedemorton We should not use the fields.yml here as it is also use to generate the template and other things. But we should add this to the docs of the metricset itself.

@dedemorton
Copy link
Contributor Author

@ruflin Updated based on your feedback.

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

@dedemorton LGTM. Needs a rebase because of the other PR's that were merged ...

@dedemorton dedemorton force-pushed the edit_prometheus_module branch from 4e42145 to 9017a26 Compare February 7, 2017 18:46
@dedemorton
Copy link
Contributor Author

@ruflin Rebased. Should be ready to merge as soon as the checks have run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants