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.

Sign up for GitHub

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

Jump to bottom

Hardcode namespace for prometheus metrics #12400

Merged
merged 1 commit into from
Jun 3, 2019
Merged

Hardcode namespace for prometheus metrics #12400

merged 1 commit into from
Jun 3, 2019

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Jun 3, 2019

All prometheus metrics should be under prometheus.metrics, fix it so
the metricset can be reused on light modules (#12270).

@jsoriano
Hardcode namespace for prometheus metrics
5cea601 
All prometheus metrics should be under `prometheus.metrics`, fix it so
the metricset can be reused on light modules (elastic#12270).
@jsoriano jsoriano requested a review from a team as a code owner June 3, 2019 18:11
@jsoriano jsoriano self-assigned this Jun 3, 2019
kaiyan-sheng
kaiyan-sheng reviewed Jun 3, 2019
View reviewed changes
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Could you update data.json under _meta/testdata please 😬

@jsoriano
Copy link
Member Author

jsoriano commented Jun 3, 2019

Could you update data.json under _meta/testdata please grimacing

It shouldn't change anything in current metricset, so keeping the files as they are is intentional, because they shouldn't change after this PR 🙂

This change is for future light modules that will use this metricset but their module name wil be different. In these cases we still want the metrics to be writen under prometheus.metrics and not under the module namespace.

@kaiyan-sheng
Copy link
Contributor

Could you update data.json under _meta/testdata please grimacing

It shouldn't change anything in current metricset, so keeping the files as they are is intentional, because they shouldn't change after this PR 🙂

Oh yes thank you! I didn't realize old code already have mb.Event{ModuleFields: e} which module name here is prometheus.

@jsoriano jsoriano merged commit 3ab727b into elastic:master Jun 3, 2019
@jsoriano jsoriano deleted the prometheus-collector-fix-namespace branch June 3, 2019 21:35
andrewvc pushed a commit to andrewvc/beats that referenced this pull request Jun 12, 2019
@jsoriano @andrewvc
Hardcode namespace for prometheus metrics (elastic#12400)
381e161 
All prometheus metrics should be under `prometheus.metrics`, fix it so
the metricset can be reused on light modules (elastic#12270).
@jsoriano jsoriano mentioned this pull request Sep 2, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaiyan-sheng kaiyan-sheng kaiyan-sheng approved these changes

@exekias exekias exekias approved these changes

Assignees

@jsoriano jsoriano

Labels
Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsoriano @kaiyan-sheng @exekias @andresrc