Bug 1903999: Httplog response code is always zero #494
Conversation
|
/assign @sttts |
|
/retest |
p0lyn0mial
changed the title
openshift-ci-robot
added
the
bugzilla/severity-high
|
@p0lyn0mial: This pull request references Bugzilla bug 1903999, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
In response to this:
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. |
openshift-ci-robot
added
the
bugzilla/valid-bug
p0lyn0mial
force-pushed
the
upstream-healthz-httplog-status
branch
2 times, most recently
from
eb3ed69
to
f21cea3
Compare
| klog.V(2).Infof("%s check failed: %s\n%v", strings.Join(failedChecks, ","), name, failedVerboseLogOutput.String()) | ||
| http.Error(httplog.Unlogged(r, w), fmt.Sprintf("%s%s check failed", individualCheckOutput.String(), name), http.StatusInternalServerError) | ||
| http.Error(w, fmt.Sprintf("%s%s check failed", individualCheckOutput.String(), name), http.StatusInternalServerError) |
There was a problem hiding this comment.
to capture the status code and put it in the log file.
httplog.Unlogged returns the original ResponseWriter, or w if it is not our inserted logger.
p0lyn0mial
force-pushed
the
upstream-healthz-httplog-status
branch
from
f21cea3
to
4fea27d
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
… it doesn't log stack trace when HTTP 500 response is returned
p0lyn0mial
force-pushed
the
upstream-healthz-httplog-status
branch
from
4fea27d
to
96dcb7f
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: p0lyn0mial, sttts 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@p0lyn0mial: All pull requests linked via external trackers have merged: Bugzilla bug 1903999 has been moved to the MODIFIED state. In response to this:
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. |
It turned out that httplog was suddenly disabled for
health checks on the paths like /healthz or /readyz.I suspect it was disabled because by default we attach a stack trace for requests with >= 500 HTTP status code which can be verbose (an example below).
I decided to expose a function that allows for disabling putting a stack trace into to log only for
/readyzhandler. Since an HTTP500is considered a normal response to signal a LB to stop sending traffic for that path.A response with a stack trace:
a response without the stack trace:
TODO:
watchendpoint