10000 add cache metrics for NGINX plus by sheharyaar · Pull Request #540 · nginx/nginx-prometheus-exporter · GitHub
[go: up one dir, main page]

Skip to content

Conversation

sheharyaar
Copy link
Contributor

Proposed changes

Fixes #522 and exposes cache metrics from NGINX Plus API.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • 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 8000 my changes
  • I have ensured the README is up to date
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch on my own fork

@sheharyaar sheharyaar requested a review from a team as a code owner November 3, 2023 09:57
@github-actions github-actions bot added the documentation Pull requests/issues for documentation label Nov 3, 2023
@brianehlert brianehlert added the enhancement Pull requests for new features/feature enhancements label Nov 3, 2023
@shaun-nx shaun-nx self-requested a review November 10, 2023 09:19
@sheharyaar
Copy link
Contributor Author

The lint check has failed, do I need to perform gofumt in my branch or is it fine ?

@jjngx
Copy link
Contributor
jjngx commented Nov 13, 2023

The lint check has failed, do I need to perform gofumt in my branch or is it fine ?

Yes, please do run gofumpt.

Signed-off-by: Mohammad Shehar Yaar Tausif <sheharyaar48@gmail.com>
@github-actions github-actions bot removed the enhancement Pull requests for new features/feature enhancements label Nov 13, 2023
@sheharyaar
Copy link
Contributor Author

Done, fixed lint issues and rebased

@shaun-nx
Copy link

Hi @sheharyaar, thanks for contributing to this project! We can merge this PR as soon as the lint errors are fixed.

I get these two errors when running make lint locally

collector/nginx_plus.go:1210: File is not `gofumpt`-ed (gofumpt)

collector/nginx_plus.go:1190:97: unnecessary conversion (unconvert)
		ch <- prometheus.MustNewConstMetric(c.cacheZoneMetrics["cold"], prometheus.GaugeValue, float64(cold), name)

For the first error, just run gomt -w collector/nginx_plus.go

For the 2nd error, it seems like the float64(cold) conversion isn't needed.
I would remove this conversation and test if the metric still displays as you expect.

@shaun-nx
Copy link

@sheharyaar ignore my last com 8000 mand, I was late to the trigger 😆

@shaun-nx shaun-nx merged commit d57f771 into nginx:main Nov 13, 2023
@shaun-nx
Copy link

Thank you @sheharyaar ! 🎉

@sheharyaar
Copy link
Contributor Author

Gald to contribute, welcome !

@lucacome lucacome added the enhancement Pull requests for new features/feature enhancements label Nov 14, 2023
@sheharyaar sheharyaar deleted the sheharyaar/issue522 branch November 14, 2023 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose cache metrics through exporter
5 participants
0