-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Allow to disable NGINX metrics #3512
Conversation
44160bb
to
8dd7f6a
Compare
67d0efb
to
769874f
Compare
769874f
to
06d33c1
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -16,7 +16,15 @@ limitations under the License. | |||
|
|||
package metric | |||
|
|||
import "k8s.io/ingress-nginx/internal/ingress" | |||
import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
things in this file weren't being used anywhere before right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct
@@ -821,7 +826,9 @@ stream { | |||
|
|||
proxy_pass http://upstream_balancer; | |||
log_by_lua_block { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cloudn't the whole log_by_lua_block be deactivated here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could. But it makes customizing Nginx config template harder. For example in my use case we call custom Lua module in log_by_lua
phase. If the whole log_by_lua_blok
is disabled we would have to change more lines whereas now we just append our custom logic after line 831.
What this PR does / why we need it:
Allow disabling NGINX prometheus metrics