You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by rob05c <gi...@git.apache.org> on 2017/04/04 04:21:24 UTC

[GitHub] incubator-trafficcontrol pull request #425: Add TM2 HTTP gzip support

GitHub user rob05c opened a pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425

    Add TM2 HTTP gzip support

    Because text compresses well, and most Traffic Monitor endpoints are large amounts of repetitive text, this makes most endpoints go from tens of megabytes to hundreds of kilobytes.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rob05c/incubator-trafficcontrol tm2-gzip

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafficcontrol/pull/425.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #425
    
----
commit af98c6257decec09ac18b6b6e51ea6075510ecab
Author: Robert Butts <ro...@gmail.com>
Date:   2017-04-04T04:13:33Z

    Add TM2 HTTP gzip support

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111808124
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    +		encodings := strings.Split(encodingHeader, ",")
    +		for _, encoding := range encodings {
    +			if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC
    +				return true
    +			}
    +		}
    +	}
    +	return false
    +}
    +
    +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written.
    +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) {
    +	// TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer.
    +	if len(b) == 0 || !acceptsGzip(r) {
    +		return b, nil
    +	}
    +	w.Header().Set("Content-Encoding", "gzip")
    +
    +	buf := bytes.Buffer{}
    +	zw := gzip.NewWriter(&buf)
    +
    +	if _, err := zw.Write(b); err != nil {
    +		return nil, fmt.Errorf("gzipping bytes: %v")
    +	}
    +
    +	if err := zw.Close(); err != nil {
    +		return nil, fmt.Errorf("closing gzip writer: %v")
    +	}
    +
    +	return buf.Bytes(), nil
    --- End diff --
    
    I just realized... you have both the original buffer and the compressed one in memory here. You might as well only return the shorter one. That will be the compressed one 99% of the time, but since you have the info, you might as well do the best option.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111804555
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    --- End diff --
    
    This is incorrect. These headers have optional linear whitespace included, which can be spaces, tabs, or a CRLF, as long as it is followed by a space or a tab.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #425: Add Traffic Monitor 2.0 HTTP gzip suppo...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/425
  
    This also makes the GUI faster and more resilient with poor connectivity, because browsers transparently accept and decode gzip in the AJAX requests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111799950
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    +		encodings := strings.Split(encodingHeader, ",")
    +		for _, encoding := range encodings {
    +			if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC
    +				return true
    +			}
    +		}
    +	}
    +	return false
    +}
    +
    +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written.
    +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) {
    +	// TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer.
    +	if len(b) == 0 || !acceptsGzip(r) {
    --- End diff --
    
    Since we're not using interfaces for composability, we've got the length. Why bother compressing very small strings? Maybe set the minimum compression length at some appropriate non-zero number?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111796087
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er
     // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
     func WrapBytes(f func() []byte, contentType string) http.HandlerFunc {
     	return func(w http.ResponseWriter, r *http.Request) {
    +		bytes := f()
    +		bytes, err := gzipIfAccepts(r, w, bytes)
    +		if err != nil {
    +			log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err)
    +			code := http.StatusInternalServerError
    +			w.WriteHeader(code)
    +			if _, err := w.Write([]byte(http.StatusText(code))); err != nil {
    --- End diff --
    
    This err shadows the err above. I'd either use a different name or not shadow. [Nitpick]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111809861
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111799324
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er
     // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
     func WrapBytes(f func() []byte, contentType string) http.HandlerFunc {
     	return func(w http.ResponseWriter, r *http.Request) {
    +		bytes := f()
    +		bytes, err := gzipIfAccepts(r, w, bytes)
    +		if err != nil {
    --- End diff --
    
    Shouldn't this error be handled the same as others? I think it should increment errorCount, like all the other routines, right?
    
    And if it's doing that, why wouldn't it just be a wrapper around WrapErr, which contrary to the indication one might glean from it's name is just the same as this, but with errors as well. In fact, this function might could be written:
    
        return WrapErr( errorCode /* that probably needs to be passed in */,
            func() ([]byte, err) { return f(), nil },
            contentType)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #425: Add Traffic Monitor 2.0 HTTP gzip suppo...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/425
  
    As far as the Wrapper comments go: a lot of @alficles objections are difficult to get right, or impossible to implement, with the current Wrap helpers
    
    The Wrap helpers are are now more duplicate than reasonable. Go's type system isn't powerful enough for these wrappers to work well, especially as the handling complexity grows. They're increasingly difficult to write and read.
    
    I've thought for a while now that they should be refactored, probably into stock `http.HandlerFunc`s. Using `HandlerFunc` would have some duplicate logic, but probably not much more than is already there, and would be far easier to understand.
    
    I'd rather wait to do that refactor in its own PR, than here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111800861
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er
     // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
     func WrapBytes(f func() []byte, contentType string) http.HandlerFunc {
     	return func(w http.ResponseWriter, r *http.Request) {
    +		bytes := f()
    +		bytes, err := gzipIfAccepts(r, w, bytes)
    +		if err != nil {
    +			log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err)
    +			code := http.StatusInternalServerError
    +			w.WriteHeader(code)
    +			if _, err := w.Write([]byte(http.StatusText(code))); err != nil {
    +				log.Warnf("received error writing data request %v: %v\n", r.URL.EscapedPath(), err)
    +			}
    +			return
    +		}
    +
     		w.Header().Set("Content-Type", contentType)
    -		log.Write(w, f(), r.URL.EscapedPath())
    +		log.Write(w, bytes, r.URL.EscapedPath())
    --- End diff --
    
    `log.Write` is a terrible name. It misleads one into thinking that the purpose of this line is to write to a log at w, not unlike what standard library functions like [io.WriteString](https://golang.org/pkg/io/#WriteString) do. Fixing it is probably slightly out of scope... but I register my objection regardless. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111807495
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    +		encodings := strings.Split(encodingHeader, ",")
    +		for _, encoding := range encodings {
    +			if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC
    +				return true
    +			}
    +		}
    +	}
    +	return false
    +}
    +
    +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written.
    +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) {
    +	// TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer.
    +	if len(b) == 0 || !acceptsGzip(r) {
    --- End diff --
    
    Because it's simpler to gzip everything, than add special logic. If performance ever matters, we can add the logic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111807742
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er
     // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
     func WrapBytes(f func() []byte, contentType string) http.HandlerFunc {
     	return func(w http.ResponseWriter, r *http.Request) {
    +		bytes := f()
    +		bytes, err := gzipIfAccepts(r, w, bytes)
    +		if err != nil {
    +			log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err)
    +			code := http.StatusInternalServerError
    +			w.WriteHeader(code)
    +			if _, err := w.Write([]byte(http.StatusText(code))); err != nil {
    --- End diff --
    
    IMO idiomatic naming is better in this case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111802570
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -182,28 +202,49 @@ type SrvFunc func(params url.Values, path string) ([]byte, int)
     func WrapParams(f SrvFunc, contentType string) http.HandlerFunc {
    --- End diff --
    
    Should this not also delegate to `WrapErr`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111807542
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er
     // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
     func WrapBytes(f func() []byte, contentType string) http.HandlerFunc {
     	return func(w http.ResponseWriter, r *http.Request) {
    +		bytes := f()
    +		bytes, err := gzipIfAccepts(r, w, bytes)
    +		if err != nil {
    +			log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err)
    +			code := http.StatusInternalServerError
    +			w.WriteHeader(code)
    +			if _, err := w.Write([]byte(http.StatusText(code))); err != nil {
    +				log.Warnf("received error writing data request %v: %v\n", r.URL.EscapedPath(), err)
    +			}
    +			return
    +		}
    +
     		w.Header().Set("Content-Type", contentType)
    -		log.Write(w, f(), r.URL.EscapedPath())
    +		log.Write(w, bytes, r.URL.EscapedPath())
    --- End diff --
    
    Agreed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111809687
  
    --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go ---
    @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri
     	}
     	return dispatchMap
     }
    +
    +func acceptsGzip(r *http.Request) bool {
    +	encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests
    +	for _, encodingHeader := range encodingHeaders {
    +		encodingHeader := strings.Replace(encodingHeader, " ", "", -1)
    +		encodings := strings.Split(encodingHeader, ",")
    +		for _, encoding := range encodings {
    +			if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC
    +				return true
    +			}
    +		}
    +	}
    +	return false
    +}
    +
    +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written.
    +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) {
    +	// TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer.
    +	if len(b) == 0 || !acceptsGzip(r) {
    +		return b, nil
    +	}
    +	w.Header().Set("Content-Encoding", "gzip")
    +
    +	buf := bytes.Buffer{}
    +	zw := gzip.NewWriter(&buf)
    +
    +	if _, err := zw.Write(b); err != nil {
    +		return nil, fmt.Errorf("gzipping bytes: %v")
    +	}
    +
    +	if err := zw.Close(); err != nil {
    +		return nil, fmt.Errorf("closing gzip writer: %v")
    +	}
    +
    +	return buf.Bytes(), nil
    --- End diff --
    
    As you say, the compressed one will be smaller 99% of the time, and when it is, performance almost certainly doesn't matter. IMO the extra code for that logic isn't worth the maintenance cost.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafficcontrol/pull/425


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---