pkg
pkg copied to clipboard
knative
Consider lowering log level for AdmissionController.Admit failures
/area monitoring /kind question
Expected Behavior
Consider logging validation errors at a lower level (info) to signal that the webhook is working as expected.
Actual Behavior
The AdmissionController.Admit code logs validation error on CRD's with level==error. Validation errors are a normal function of the webhook, so logging them as level==error is perhaps to aggressive and misleading as the webhook is operating normally when validations fail this way.
Additional Info
example of logging statement: https://github.com/knative/pkg/blob/6045ed499615ecca39d92a8277489de1fbe3dcff/webhook/resourcesemantics/validation/validation_admit.go#L177
Hi @eddie4941 if not mistaken other frameworks follow this during validation of any resource. I suspect that for example in a production env these errors should be minimal, you know what you are doing, so I guess an error would be more helpful. When an Info log level is used it means in general that what is happening is normal from the user point of you as well (similar discussion here).
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
/reopen /lifecycle frozen
In @skonto's example it looks like it prints the error when it fails to marshal the response.
I think failing validation shouldn't be an error because it's expected with bad input vs. something unexpected that prevents us from responding to the request.
@dprotaso: Reopened this issue.
In response to this:
/reopen /lifecycle frozen
In @skonto's example it looks like it prints the error when it fails to marshal the response.
I think failing validation shouldn't be an error because it's expected with bad input vs. something unexpected that prevents us from responding to the request.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@julz: This request has been marked as suitable for new contributors.
Please ensure the request meets the requirements listed here.
If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.
In response to this:
/triage accepted /good-first-issue
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
hi, @dprotaso just lower the logging level to INFO and error should be return as before ?
