Add support for gzip content-encoding

[ivan-valkov]

Resolves #775

If an Accept-Encoding:gzip header is present, will compress the output and set the correct Content-Encoding header in the response. Added tests for both the wsgi and asgi implementations.

[csmarchbanks]

Thanks! Generally this looks reasonable though I have not fully tested it yet.

One thing to note, the golang client allows disabling compression as part of the options passed to the handler. I am wondering if we should do something similar here? The reason being, in some cases the cpu usage required to compress a large response can slow everything down too much.

[ivan-valkov]

One thing to note, the golang client allows disabling compression as part of the options passed to the handler. I am wondering if we should do something similar here? The reason being, in some cases the cpu usage required to compress a large response can slow everything down too much.

Yeah, definitely, this makes sense. What would be the preferred semantic? GZIP compression enabled or disabled by default? I am conscious that if we enable it by default, some existing users of the client might see unexpected CPU spikes when using the new version of the client, so maybe we should require enabling GZIP compression explicitly?

[csmarchbanks]

I would say let's default to negotiating gzip as most use cases will not see much of a CPU spike, and advanced users can turn it off. This also aligns us with client_golang (enabled by default) and client_java (always enabled).

[csmarchbanks]

Instead of an environment variable what would you think of passing an additional argument to make_(w|a)sgi_app? My thought is that most users should have control over creating the apps so an env var is not necessary. We try to only use env vars in places where users might not have access to the code, such as registering metrics.

[ivan-valkov]

Yeah, makes sense. I suppose we need to expose this option in start_http_server as well?

[csmarchbanks]

I think that would be nice, though not completely required as start_http_server doesn't need to support all configuration for advanced user.

[ivan-valkov]

awesome, yeah, this makes sense. I have updated the PR now and added documentation to the README. PTAL when you get some time. I don't think there is a release notes file we need to update but maybe worth mentioning this change when releasing the next version of the client (:

[csmarchbanks]

Thanks! I plan to review this early next week. I will definitely mention it in the release notes for the next version!

[csmarchbanks]

This looks great, thanks a lot!