cloudendpoints / esp

Extensible Service Proxy
https://cloud.google.com/endpoints/
BSD 2-Clause "Simplified" License
281 stars 73 forks source link

GZIP Support #345

Open bbassingthwaite opened 6 years ago

bbassingthwaite commented 6 years ago

I can't find any docs around enabling this or support for this? Am I missing a hidden feature or does endpoints just not do this? It would be a huge improvement if transcoded JSON responses were compressed. Thanks!

qiwzhang commented 6 years ago

This is mainly for gRPC transcoding to gzip compress the JSON response. is that right?

Hi @PiotrSikora @lizan Does Nginx support GZIP compression in Response? If no, how easy to add it?

PiotrSikora commented 6 years ago

@qiwzhang it does (gzip on;).

lizan commented 6 years ago

If you are using custom nginx.conf adding gzip on; should do compression. http://nginx.org/en/docs/http/ngx_http_gzip_module.html

bbassingthwaite commented 6 years ago

I've tried many variations of this and have not been able to get this to work.

One attempt:

gzip             on;
gzip_types       application/json;

I expect this to work, am I missing something?

qiwzhang commented 6 years ago

We never try it. Hi @mangchiandjjoe Could you try it?

mangchiandjjoe commented 6 years ago

The default value of gzip_types is "text/html" Nginx only compress for the response content type is text/html. With this configuration, it works for me.

gzip            on;
gzip_types      text/plain application/xml application/json;

$ curl -v -H "Accept-Encoding: gzip" "http://127.0.0.1:8090/shelves"

*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8090 (#0)
> GET /shelves HTTP/1.1
> Host: 127.0.0.1:8090
> User-Agent: curl/7.55.0
> Accept: */*
> Accept-Encoding: gzip
> 
< HTTP/1.1 200 OK
< Server: nginx/1.13.4
< Date: Sat, 03 Feb 2018 00:59:35 GMT
< Content-Type: application/json; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
< X-Powered-By: Express
< ETag: W/"5b-j+/3rV/oFOnatLQ4z3XKYtlDXRc"
< Content-Encoding: gzip
< 
Warning: Binary output can mess up your terminal. Use "--output -" to tell 
Warning: curl to output it to your terminal anyway, or consider "--output 
Warning: <FILE>" to save to a file.
* Failed writing body (0 != 77)
* Failed writing data
* stopped the pause stream!
* Closing connection 0
mangchiandjjoe commented 6 years ago

Could you please check your backend is returning "Content-Type" header?

bbassingthwaite commented 6 years ago

The backed is grpc so I'm not setting any content to type.

mangchiandjjoe commented 6 years ago

nginx requires content-type header to enable gzip compression. the original response can be already compressed by the backend or content type doesnot requires compression(jpg, png,..)

https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_gzip_filter_module.c#L257

    if (!conf->enable
        || (r->headers_out.status != NGX_HTTP_OK
            && r->headers_out.status != NGX_HTTP_FORBIDDEN
            && r->headers_out.status != NGX_HTTP_NOT_FOUND)
        || (r->headers_out.content_encoding
            && r->headers_out.content_encoding->value.len)
        || (r->headers_out.content_length_n != -1
            && r->headers_out.content_length_n < conf->min_length)
        || ngx_http_test_content_type(r, &conf->types) == NULL // disabled by this line 
        || r->header_only)
    {
        return ngx_http_next_header_filter(r);
    }
bbassingthwaite commented 6 years ago

What does this mean for gRPC? From my end, i've added the gzip headers but the gRPC transcoding to JSON is never gzipped by nginx.

lizan commented 6 years ago

The content type for transcoding is set to application/json. I don’t think we set content-length but use chunked encoding, I think setting length for transcoding might work.

It will be only for unary calls, does that solves your problem?

bbassingthwaite commented 6 years ago

That does solve my problem.

ghost commented 6 years ago

This would greatly help me as well, any thoughts on when this might be available?

@lizan Is there any chance gzip could work for streaming calls as well or is there a technical reason that can't be done?

ashi009 commented 5 years ago

@lizan Is it possible to add gzip support in start_esp now? I just learned that GCP LB won't handle gzip compression. So ESP as the application frontend should do this.

qiwzhang commented 5 years ago

At current state, just adding "gzip on" only works for HTTP backend.

ashi009 commented 5 years ago

A small gRPC response gets bloated for over 20x in size due to JSON and the lacking of compression.

Could you evaluate the efforts needed for adding such support? We can help that out if this won't stand in the way of the esp dev roadmap.

craigmulligan commented 5 years ago

Would you be open to PR adding a startup gzip option that adds gzip to the nginx config? I'd prefer to use the esp options rather than manage my own nginx config.

qiwzhang commented 5 years ago

Yes, we will appreciate your contribution. Please go ahead.

craigmulligan commented 5 years ago

Cool, as a quick fix I just made the origin server add to do the gzipping. Will circle back to this when we have time. If anyone else wants to pick it up in the meantime :+1:

burk commented 5 years ago

The content type for transcoding is set to application/json. I don’t think we set content-length but use chunked encoding, I think setting length for transcoding might work.

It will be only for unary calls, does that solves your problem?

@lizan What does "setting length for transcoding" mean specifically? Is that a nginx option?

qiwzhang commented 5 years ago

No, it is in the ESP proto to json response transcoding code. It currently doesn't set it. It is not easy, data flow wise, it writes response headers first, then do body translation, so at the time of writing headers, content-length is not known.