Skip to content
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

kube-apiserver: httplog: don't print stack traces when proxies return 304s #32747

Closed
lavalamp opened this issue Sep 15, 2016 · 0 comments · Fixed by #33286
Closed

kube-apiserver: httplog: don't print stack traces when proxies return 304s #32747

lavalamp opened this issue Sep 15, 2016 · 0 comments · Fixed by #33286
Labels
area/apiserver help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@lavalamp
Copy link
Member

I noticed that the code that prints stack traces when we return an unexpected status code runs even if we're just proxying that code from a service or pod. We should probably not do that.

@lavalamp lavalamp added priority/backlog Higher priority than priority/awaiting-more-evidence. area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Sep 15, 2016
k8s-github-robot pushed a commit that referenced this issue Sep 16, 2016
Automatic merge from submit-queue

prevent printing of stack trace when proxying 304 requests in api gateway

partially addresses #32747.
PhilibertDugas added a commit to PhilibertDugas/kubernetes that referenced this issue Sep 22, 2016
Attempt at closing kubernetes#32747,

With the `RequestInfoResolver` struct, it's possible to inspect the
request and get the `Verb`. In this case, the `proxy` value is what I
was looking for to avoid logging stacktraces.

I'm wrapping the `.Log()` call with an `if` statement to remove all
stacktrace logging when we proxied through the apiserver

Another approach would have been to add another kind of
`StacktracePred` in the `httplog` package. I found this path to be
trickier to code as it's currently only accepting int values.
k8s-github-robot pushed a commit that referenced this issue Sep 22, 2016
…g-from-apiserver

Automatic merge from submit-queue

Apiserver don't log stacktrace when proxying

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, read our contributor guidelines https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md and developer guide https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md
2. If you want *faster* PR reviews, read how: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md
3. Follow the instructions for writing a release note: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes
-->

**What this PR does / why we need it**:

When we are proxying unexpected status from a service or a pod, we print the stack traces (which is not the wanted behaviour). This is an attempt at fixing the issue #32747,

With the `RequestInfoResolver` struct, it's possible to inspect the request and get the `Verb`. In this case, the `proxy` value is what I was looking for to avoid logging stack traces.

I'm wrapping the `.Log()` call with an `if` statement to remove all stack traces logging when the call is a proxy from a service or a pod

Another approach would have been to add another kind of `StacktracePred` in the `httplog` package. I found this path to be trickier to code as it's currently only accepting int values.

**Which issue this PR fixes** : fixes #32747

**Special notes for your reviewer**: N/A

**Release note**:
<!--  Steps to write your release note:
1. Use the release-note-* labels to set the release note state (if you have access) 
2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. 
-->
```release-note
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apiserver help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant