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

Integrate NGINX prometheus exporter and enable metrics server #999

Merged
merged 5 commits into from
Aug 30, 2023

Conversation

ciarams87
Copy link
Member

Proposed changes

Problem: As an operator of NKG in a production environment
I want Prometheus metrics to be exposed for NKG
So that I can observe and monitor the performance of NKG and respond to incidents.

Solution: Enable the controller-runtime metrics server, integrate NGINX prometheus exporter, and add the NGINX metrics to the metrics server

Testing: Unit testing, manual testing for both http and https endpoints, on both exposing the endpoint directly to localhost, and on a prometheus server using the pod annotations

Closes #761

Checklist

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

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@ciarams87 ciarams87 requested a review from a team as a code owner August 24, 2023 14:32
@github-actions github-actions bot added dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request labels Aug 24, 2023
deploy/helm-chart/templates/deployment.yaml Outdated Show resolved Hide resolved
deploy/helm-chart/templates/deployment.yaml Outdated Show resolved Hide resolved
deploy/helm-chart/templates/deployment.yaml Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
internal/mode/static/state/graph/gateway_listener.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
deploy/helm-chart/templates/deployment.yaml Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/mode/static/state/graph/gateway_listener.go Outdated Show resolved Hide resolved
internal/mode/static/state/change_processor.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Show resolved Hide resolved
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

Nice work! I just left a few small suggestions

cmd/gateway/commands.go Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/cli-help.md Outdated Show resolved Hide resolved
docs/monitoring.md Outdated Show resolved Hide resolved
docs/monitoring.md Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/mode/static/metrics/nginx.go Outdated Show resolved Hide resolved
@ciarams87 ciarams87 force-pushed the feat/add-prom-exporter branch from 79d796b to bb0157e Compare August 29, 2023 09:29
@ciarams87 ciarams87 requested a review from kate-osborn August 29, 2023 09:30
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
docs/architecture.md Outdated Show resolved Hide resolved
internal/mode/static/manager.go Show resolved Hide resolved
internal/mode/static/nginx/runtime/manager.go Show resolved Hide resolved
@ciarams87 ciarams87 force-pushed the feat/add-prom-exporter branch from bb0157e to a829f9f Compare August 30, 2023 09:41
@ciarams87 ciarams87 requested a review from sjberman August 30, 2023 14:24
@ciarams87 ciarams87 force-pushed the feat/add-prom-exporter branch from a829f9f to 7cdc73b Compare August 30, 2023 14:44
@ciarams87 ciarams87 dismissed pleshakov’s stale review August 30, 2023 16:54

Dismissing due to PTO, comments have been addressed

@ciarams87 ciarams87 merged commit 37490ef into nginxinc:main Aug 30, 2023
@ciarams87 ciarams87 deleted the feat/add-prom-exporter branch August 30, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Integrate Prometheus Exporter
4 participants