⚠ Change webhook request received / response written logs to log level 4 #2334
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
/assign @alvaroaleman @fabriziopandini |
k8s-ci-robot
added
approved
| @@ -136,11 +136,11 @@ func (wh *Webhook) writeAdmissionResponse(w io.Writer, ar v1.AdmissionReview) { | |||
| } | |||
| } else { | |||
| res := ar.Response | |||
| if log := wh.getLogger(nil); log.V(1).Enabled() { | |||
| if log := wh.getLogger(nil); log.V(4).Enabled() { | |||
There was a problem hiding this comment.
It is technically a breaking change in that the logs may disappear when running on v1, right?
There was a problem hiding this comment.
Yup. Not sure if we consider logs as part of our API. As far as I'm aware k/k doesn't.
If it's just about changing the PR title, no objections against changing to breaking
There was a problem hiding this comment.
I don't have full context and I defer the decision to the team, but as far as I remember in previous versions of controller runtime those logs lines were not generated, so it could also be considered a fix...
There was a problem hiding this comment.
Oh okay, is this new relative to 0.14?
There was a problem hiding this comment.
Just checked. The behavior was the same with 0.14. We just didn't notice it before as we are running our controllers per default with log level 0. (we just use a higher log level for dev)
The recommended production log level is 2 though IIRC. I couldn't find a quote for that though.
But based on the guidance I think 1 is not appropriate (https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md).
sbueringer
changed the title
|
Thanks for this PR, those logs are really noisy and they are making it harder to read controller logs. |
sbueringer
changed the title
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, sbueringer 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 |
Tested a bit with the CR beta and the logs are very verbose.
I"m not sure if that was also the case with v0.14 but 4 seems more reasonable.
xref: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md