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

Enable metrics and access logging with gzip turned on #469

Closed

Conversation

tino
Copy link
Contributor

@tino tino commented Mar 18, 2018

Fixes #460.

I wasn't able to find a way to fix this transparently so that casting to *httputil.ReverseProxy would still work (as is done on line 200), so fixed it this way.

I tried to add an combined integration test of TestProxyLogOutput and TestProxyGzipHandler but couldn't get it to fail. I suppose that's got something todo with my go-testing skills :D.

Copy link
Contributor

@magiconair magiconair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that will fix it but I think there are two better alternatives:

  1. Wrap the response writer to capture the status code and the content length since these are the only two values the logger needs
  2. wrap the transport in the proxy handler instead of the http_handler.go function. Then there is no need to cast and type check.

@magiconair
Copy link
Contributor

I'll have a quick look

@magiconair
Copy link
Contributor

I've created #470 for option 1 and added a test. I think that's a cleaner way of solving this. Again, thanks for tracking this down. Could you test #470, please?

@tino tino deleted the enable-metrics-access-logging-with-gzip branch December 6, 2018 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Access logging fails in combination with proxy gzipping
2 participants