feat: add compression handler to server handler #5433
Conversation
burnerlee
changed the title
burnerlee
force-pushed
the
handler-compression
branch
from
50cb4f1
to
fb970c6
Compare
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Signed-off-by: burnerlee <avi.aviral140@gmail.com>
Signed-off-by: burnerlee <avi.aviral140@gmail.com>
burnerlee
force-pushed
the
handler-compression
branch
from
cd9f2f0
to
898f461
Compare
| reader := bytes.NewReader([]byte(act)) | ||
| gzreader, _ := gzip.NewReader(reader) | ||
| output, _ := io.ReadAll(gzreader) | ||
| act = string(output) |
There was a problem hiding this comment.
This adjusts the test, but if you're running opa with
opa run --server --log-level debug
will it spill gzip binary data into your logs?
There was a problem hiding this comment.
yes if the client sends the request with Accept-Encoding header set to gzip, then it would do so. But isn't it the intended behaviour?
There was a problem hiding this comment.
😅 spewing binary data into the logs is not the intended behaviour. For the metrics endpoint, we'll log "binary payload" instead -- this is what the test was about. For the data endpoints, I think we should include the decompressed data in the debug logs.
There was a problem hiding this comment.
I suppose decompressing what we've previously compressed to log it is also kind of weird, and an indicator that logging should be wired differently. 🤔
| @@ -71,6 +71,7 @@ require ( | |||
| github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect | |||
| github.com/golang/snappy v0.0.4 // indirect | |||
| github.com/google/flatbuffers v1.12.1 // indirect | |||
| github.com/gorilla/handlers v1.5.1 // indirect | |||
There was a problem hiding this comment.
Given that these were recently archived, I'm wondering what other options would be.
There was a problem hiding this comment.
how about just include the handlers internally I see the implementation of gorilla/handlers is not dependent on another gorilla package https://github.com/gorilla/handlers/blob/v1.5.1/compress.go#L63
There was a problem hiding this comment.
Yeah that's a possibility. Sorry there are so many things in flight here right now, please note #5483.
|
🧹 cleaning out open PRs. Feel free to re-open if you happen to get back to this. 😃 |
|
I think this should wait until gorilla/mux has been replaced, #5483 |
Signed-off-by: burnerlee avi.aviral140@gmail.com
resolves #5310
Feature addition
This PR adds
CompressionHandlerto the server initialised in server.go file.Also, it updates the test - TestRequestLogging to enable checking for gzip compression. If
gzipis specified asAccept-Encodingheader value, then a gzip compressed payload is returned and the response hasContent-Encodingheader set togzip. We verify this, and verify the compressed payload by decompressing it manually and matching it to the original string.