Reports: streamline report serialization

Move the creation of the buffer and the choice of compression level
(which never changes) into WriteBinary(), to simplify the code.

app/router.go

@@ -121,13 +121,13 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) {
 	post.HandleFunc("/api/report", requestContextDecorator(func(ctx context.Context, w http.ResponseWriter, r *http.Request) {
 		var (
 			rpt    report.Report
-			buf    bytes.Buffer
-			reader = io.TeeReader(r.Body, &buf)
+			buf    = &bytes.Buffer{}
+			reader = io.TeeReader(r.Body, buf)
 		)
 
 		gzipped := strings.Contains(r.Header.Get("Content-Encoding"), "gzip")
 		if !gzipped {
-			reader = io.TeeReader(r.Body, gzip.NewWriter(&buf))
+			reader = io.TeeReader(r.Body, gzip.NewWriter(buf))
 		}
 
 		contentType := r.Header.Get("Content-Type")
@@ -150,8 +150,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) {
 
 		// a.Add(..., buf) assumes buf is gzip'd msgpack
 		if !isMsgpack {
-			buf = bytes.Buffer{}
-			rpt.WriteBinary(&buf, gzip.DefaultCompression)
+			buf, _ = rpt.WriteBinary()
 		}
 
 		if err := a.Add(ctx, rpt, buf.Bytes()); err != nil {

extras/fixprobe/main.go

@@ -3,7 +3,6 @@ package main
 
 import (
 	"bytes"
-	"compress/gzip"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -62,8 +61,7 @@ func main() {
 		log.Fatal(err)
 	}
 
-	buf := &bytes.Buffer{}
-	err = fixedReport.WriteBinary(buf, gzip.DefaultCompression)
+	buf, err := fixedReport.WriteBinary()
 	if err != nil {
 		log.Fatal(err)
 	}

probe/appclient/app_client_internal_test.go

@@ -105,7 +105,7 @@ func TestAppClientPublish(t *testing.T) {
 
 	// First few reports might be dropped as the client is spinning up.
 	for i := 0; i < 10; i++ {
-		buf, _ := serializeReport(rpt)
+		buf, _ := rpt.WriteBinary()
 		if err := p.Publish(buf, false); err != nil {
 			t.Error(err)
 		}
@@ -210,7 +210,7 @@ func TestStop(t *testing.T) {
 		case <-receivedReport:
 			done = true
 		default:
-			buf, _ := serializeReport(rpt)
+			buf, _ := rpt.WriteBinary()
 			if err := p.Publish(buf, false); err != nil {
 				t.Error(err)
 			}

probe/appclient/multi_client.go

@@ -2,7 +2,6 @@ package appclient
 
 import (
 	"bytes"
-	"compress/gzip"
 	"errors"
 	"fmt"
 	"net/url"
@@ -156,18 +155,12 @@ func (c *multiClient) Stop() {
 // Publish implements Publisher by publishing the reader to all of the
 // underlying publishers sequentially. To do that, it needs to drain the
 // reader, and recreate new readers for each publisher. Note that it will
-func serializeReport(r report.Report) (*bytes.Buffer, error) {
-	buf := &bytes.Buffer{}
-	err := r.WriteBinary(buf, gzip.DefaultCompression)
-	return buf, err
-}
-
 // publish to one endpoint for each unique ID. Failed publishes don't count.
 func (c *multiClient) Publish(r report.Report) error {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 
-	buf, err := serializeReport(r)
+	buf, err := r.WriteBinary()
 	if err != nil {
 		return err
 	}

report/marshal.go

@@ -26,17 +26,18 @@ func (s *dummySelfer) CodecEncodeSelf(encoder *codec.Encoder) {
 	panic("This shouldn't happen: perhaps something has gone wrong in code generation?")
 }
 
-// WriteBinary writes a Report as a gzipped msgpack.
-func (rep Report) WriteBinary(w io.Writer, compressionLevel int) error {
-	gzwriter, err := gzip.NewWriterLevel(w, compressionLevel)
+// WriteBinary writes a Report as a gzipped msgpack into a bytes.Buffer
+func (rep Report) WriteBinary() (*bytes.Buffer, error) {
+	w := &bytes.Buffer{}
+	gzwriter, err := gzip.NewWriterLevel(w, gzip.DefaultCompression)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	if err = codec.NewEncoder(gzwriter, &codec.MsgpackHandle{}).Encode(&rep); err != nil {
-		return err
+		return nil, err
 	}
 	gzwriter.Close() // otherwise the content won't get flushed to the output stream
-	return nil
+	return w, nil
 }
 
 type byteCounter struct {

report/marshal_test.go

@@ -1,8 +1,6 @@
 package report_test
 
 import (
-	"bytes"
-	"compress/gzip"
 	"reflect"
 	"testing"
 	"time"
@@ -13,11 +11,10 @@ import (
 )
 
 func TestRoundtrip(t *testing.T) {
-	var buf bytes.Buffer
 	r1 := report.MakeReport()
-	r1.WriteBinary(&buf, gzip.DefaultCompression)
+	buf, _ := r1.WriteBinary()
 	bytes := append([]byte{}, buf.Bytes()...) // copy the contents for later
-	r2, err := report.MakeFromBinary(&buf)
+	r2, err := report.MakeFromBinary(buf)
 	if err != nil {
 		t.Error(err)
 	}
@@ -74,10 +71,9 @@ func makeTestReport() report.Report {
 }
 
 func TestBiggerRoundtrip(t *testing.T) {
-	var buf bytes.Buffer
 	r1 := makeTestReport()
-	r1.WriteBinary(&buf, gzip.BestCompression)
-	r2, err := report.MakeFromBinary(&buf)
+	buf, _ := r1.WriteBinary()
+	r2, err := report.MakeFromBinary(buf)
 	if err != nil {
 		t.Error(err)
 	}
@@ -85,29 +81,3 @@ func TestBiggerRoundtrip(t *testing.T) {
 		t.Errorf("%v != %v", r1, *r2)
 	}
 }
-
-func TestRoundtripNoCompression(t *testing.T) {
-	// Make sure that we can use our standard routines for decompressing
-	// something with '0' level compression.
-	var buf bytes.Buffer
-	r1 := report.MakeReport()
-	r1.WriteBinary(&buf, 0)
-	r2, err := report.MakeFromBinary(&buf)
-	if err != nil {
-		t.Error(err)
-	}
-	if !reflect.DeepEqual(r1, *r2) {
-		t.Errorf("%v != %v", r1, *r2)
-	}
-}
-
-func TestMoreCompressionMeansSmaller(t *testing.T) {
-	// Make sure that 0 level compression actually does compress less.
-	var buf1, buf2 bytes.Buffer
-	r := report.MakeReport()
-	r.WriteBinary(&buf1, gzip.DefaultCompression)
-	r.WriteBinary(&buf2, 0)
-	if buf1.Len() >= buf2.Len() {
-		t.Errorf("Compression doesn't change size: %v >= %v", buf1.Len(), buf2.Len())
-	}
-}