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
✨ Cleanup webhook logging #2326
✨ Cleanup webhook logging #2326
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
d03a406
to
f36694d
Compare
f36694d
to
0f47d28
Compare
@@ -69,7 +69,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
// verify the content type is accurate | |||
if contentType := r.Header.Get("Content-Type"); contentType != "application/json" { | |||
err = fmt.Errorf("contentType=%s, expected application/json", contentType) | |||
wh.getLogger(nil).Error(err, "unable to process a request with an unknown content type", "content type", contentType) | |||
wh.getLogger(nil).Error(err, "unable to process a request with unknown content type") |
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.
content type is already contained in the error + we don't need it as separate k/v pair for log filtering
@@ -93,7 +93,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
wh.writeResponse(w, reviewResponse) | |||
return | |||
} | |||
wh.getLogger(nil).V(1).Info("received request", "UID", req.UID, "kind", req.Kind, "resource", req.Resource) | |||
wh.getLogger(&req).V(1).Info("received request") |
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.
k/v pairs are added by getLogger, kind is not needed as we already have webhookKind
@FillZpp should be removed /assign @FillZpp @alvaroaleman |
/lgtm |
This PR changes UID consistently to requestID + some smaller cleanups.
Result logs look like this: