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

Location zone and resolver metric support #64

Merged
merged 1 commit into from
Oct 15, 2019

Conversation

ampant
Copy link
Contributor

@ampant ampant commented Sep 19, 2019

Proposed changes

  • I have read the CONTRIBUTING guide
  • I have proven my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have ensured the README is up to date
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch on my own fork

collector/nginx_plus.go Outdated Show resolved Hide resolved
collector/nginx_plus.go Show resolved Hide resolved
collector/nginx_plus.go Outdated Show resolved Hide resolved
@Dean-Coakley Dean-Coakley added the enhancement Pull requests for new features/feature enhancements label Sep 19, 2019
@ampant ampant changed the title [WIP] Location zone and resolver metric support Location zone and resolver metric support Sep 30, 2019
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

@ampant please see my suggestions.

Additinally, please update the main readme to include the newly added metrics -- https://github.com/nginxinc/nginx-prometheus-exporter#exported-metrics

Note that currently the code doesn't work for the resolver metrics:

./nginx-prometheus-exporter -nginx.plus --nginx.scrape-uri http://demo.nginx.com/api
2019/10/01 13:43:31 Starting NGINX Prometheus Exporter Version=0.4.2 GitCommit=ac03689
panic: descriptor Desc{fqName: "nginxplus_resolver_unknown", help: "The total number of requests completed with an unknown error.", constLabels: {}, variableLabels: [resolver]} is invalid: duplicate label names

goroutine 1 [running]:
. . .

collector/nginx_plus.go Outdated Show resolved Hide resolved
collector/nginx_plus.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Dean-Coakley Dean-Coakley left a comment

Choose a reason for hiding this comment

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

Some feedback from @pleshakov was missed. Please take note.

collector/nginx_plus.go Outdated Show resolved Hide resolved
collector/nginx_plus.go Show resolved Hide resolved
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@ampant ampant merged commit 330108c into master Oct 15, 2019
@ampant ampant deleted the location/resolver-metrics branch October 15, 2019 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants