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

fix (daemon) : Add logging to provide additional information for non-200 status codes (#3766) #4467

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rohanKanojia
Copy link
Contributor

Fixes: Issue #3766

Relates to: Issue #3766

Solution/Idea

Add a custom response writer to capture the response body and status code so that we can conditionally log the response body in case of failure.

Proposed changes

  1. Add CustomResponseWriter object as a wrapper around http.ResponseWriter and override methods to capture response status code and body.
  2. Add interceptResponseBodyMiddleware that would inject the abovementioned response writer into HTTP Handlers.
  3. Use interceptResponseBodyMiddleware in daemon endpoint declarations and add handler for conditionally logging response body when response is not successful

Testing

What is the bottom-line functionality that needs testing? Describe in pseudo-code or in English. Use verifiable statements that tie your changes to existing functionality.

  1. Run crc daemon to start CRC daemon
  2. Make some requests to the daemon API endpoints, so that they fail. You would notice that there is some additional logging whenever response code is not 200.

For example if we make requests like these

# For failed requests, there would be additional logging of response body
curl -i --unix-socket ~/.crc/crc-http.sock http://crc/api/stop
curl -i --unix-socket ~/.crc/crc-http.sock http://crc/api/start
curl -i --unix-socket ~/.crc/crc-http.sock http://crc/api/delete

# For successful requests, it won't log
curl -i --unix-socket ~/.crc/crc-http.sock http://crc/api/version

We see these logs (notice there is no logging in case of successful request done in the end) :

ERRO [19/Nov/2024:22:11:06 +0530] "GET /api/stop" Response Body: Instance is already stopped
 
@ - - [19/Nov/2024:22:11:06 +0530] "GET /api/stop HTTP/1.1" 500 28

ERRO [19/Nov/2024:22:11:00 +0530] "GET /api/start" Response Body: lstat /home/rokumar/.crc/bin/crc: no such file or directory
 
@ - - [19/Nov/2024:22:11:00 +0530] "GET /api/start HTTP/1.1" 500 60

ERRO [19/Nov/2024:22:08:44 +0530] "GET /api/delete" Response Body: Cannot load machine: no such libmachine vm: crc
 
@ - - [19/Nov/2024:22:08:44 +0530] "GET /api/delete HTTP/1.1" 500 48

@ - - [19/Nov/2024:22:12:33 +0530] "GET /api/version HTTP/1.1" 200 101

…200 status codes (crc-org#3766)

+ Add a custom log writer in order to intercept response body and status
  code for additional logging
+ Log response body when response code is not 200

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Copy link

openshift-ci bot commented Nov 19, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gbraad for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

openshift-ci bot commented Nov 19, 2024

Hi @rohanKanojia. Thanks for your PR.

I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant