diff --git a/app/benchmark_internal_test.go b/app/benchmark_internal_test.go index 7a14f5ee9..499ce1ed8 100644 --- a/app/benchmark_internal_test.go +++ b/app/benchmark_internal_test.go @@ -34,7 +34,7 @@ func readReportFiles(b *testing.B, path string) []report.Report { if err != nil { return err } - reports = append(reports, rpt) + reports = append(reports, *rpt) return nil }); err != nil { b.Fatal(err) diff --git a/app/collector.go b/app/collector.go index 5d9deea9b..fbd21f7cd 100644 --- a/app/collector.go +++ b/app/collector.go @@ -282,7 +282,7 @@ func NewFileCollector(path string, window time.Duration) (Collector, error) { if err != nil { return err } - reports = append(reports, rpt) + reports = append(reports, *rpt) return nil }); err != nil { return nil, err diff --git a/app/multitenant/memcache_client.go b/app/multitenant/memcache_client.go index 5137e2d90..86a35c8c5 100644 --- a/app/multitenant/memcache_client.go +++ b/app/multitenant/memcache_client.go @@ -184,14 +184,13 @@ func (c *MemcacheClient) FetchReports(ctx context.Context, keys []string) (map[s continue } go func(key string) { - rep := report.MakeReport() - err := rep.ReadBinary(ctx, bytes.NewBuffer(item.Value), true, true) + rep, err := report.MakeFromBinary(ctx, bytes.NewBuffer(item.Value), true, true) if err != nil { log.Warningf("Corrupt report in memcache %v: %v", key, err) ch <- result{key: key} return } - ch <- result{key: key, report: &rep} + ch <- result{key: key, report: rep} }(key) } diff --git a/app/multitenant/s3_client.go b/app/multitenant/s3_client.go index 8f87109cd..250c7136e 100644 --- a/app/multitenant/s3_client.go +++ b/app/multitenant/s3_client.go @@ -87,7 +87,7 @@ func (store *S3Store) fetchReport(ctx context.Context, key string) (*report.Repo return nil, err } defer resp.Body.Close() - return report.MakeFromBinary(ctx, resp.Body) + return report.MakeFromBinary(ctx, resp.Body, true, true) } // StoreReportBytes stores a report. diff --git a/app/router.go b/app/router.go index fb4956949..fc507419c 100644 --- a/app/router.go +++ b/app/router.go @@ -116,7 +116,6 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { post := router.Methods("POST").Subrouter() 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) ) @@ -138,7 +137,8 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { return } - if err := rpt.ReadBinary(ctx, reader, gzipped, isMsgpack); err != nil { + rpt, err := report.MakeFromBinary(ctx, reader, gzipped, isMsgpack) + if err != nil { respondWith(w, http.StatusBadRequest, err) return } @@ -148,7 +148,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) { buf, _ = rpt.WriteBinary() } - if err := a.Add(ctx, rpt, buf.Bytes()); err != nil { + if err := a.Add(ctx, *rpt, buf.Bytes()); err != nil { log.Errorf("Error Adding report: %v", err) respondWith(w, http.StatusInternalServerError, err) return diff --git a/report/marshal.go b/report/marshal.go index b67993d5f..38d98253d 100644 --- a/report/marshal.go +++ b/report/marshal.go @@ -72,11 +72,11 @@ var gzipWriterPool = &sync.Pool{ New: func() interface{} { w, _ := gzip.NewWriterLevel(nil, gzip.DefaultCompression); return w }, } -// ReadBinary reads bytes into a Report. +// MakeFromBinary constructs a Report from binary data. // // Will decompress the binary if gzipped is true, and decode as // msgpack if true, otherwise JSON -func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, msgpack bool) error { +func MakeFromBinary(ctx context.Context, r io.Reader, gzipped bool, msgpack bool) (*Report, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "report.ReadBinary") defer span.Finish() var err error @@ -91,7 +91,7 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms if gzipped { r, err = gzip.NewReader(r) if err != nil { - return err + return nil, err } } // Read everything into memory before decoding: it's faster @@ -100,10 +100,11 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms defer bufferPool.Put(buf) uncompressedSize, err := buf.ReadFrom(r) if err != nil { - return err + return nil, err } + rep := MakeReport() if err := codec.NewDecoderBytes(buf.Bytes(), codecHandle(msgpack)).Decode(&rep); err != nil { - return err + return nil, err } log.Debugf( "Received report sizes: compressed %d bytes, uncompressed %d bytes (%.2f%%)", @@ -112,22 +113,13 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms float32(compressedSize)/float32(uncompressedSize)*100, ) span.LogFields(otlog.Uint64("compressedSize", compressedSize), otlog.Int64("uncompressedSize", uncompressedSize)) - return nil -} - -// MakeFromBinary constructs a Report from a gzipped msgpack. -func MakeFromBinary(ctx context.Context, r io.Reader) (*Report, error) { - rep := MakeReport() - if err := rep.ReadBinary(ctx, r, true, true); err != nil { - return nil, err - } return &rep, nil } // MakeFromFile construct a Report from a file, with the encoding // determined by the extension (".msgpack" or ".json", with an // optional ".gz"). -func MakeFromFile(ctx context.Context, path string) (rpt Report, _ error) { +func MakeFromFile(ctx context.Context, path string) (rpt *Report, _ error) { f, err := os.Open(path) if err != nil { return rpt, err @@ -139,8 +131,7 @@ func MakeFromFile(ctx context.Context, path string) (rpt Report, _ error) { return rpt, err } - err = rpt.ReadBinary(ctx, f, gzipped, msgpack) - return rpt, err + return MakeFromBinary(ctx, f, gzipped, msgpack) } // WriteToFile writes a Report to a file. The encoding is determined diff --git a/report/marshal_test.go b/report/marshal_test.go index 227f8e23d..2926043b3 100644 --- a/report/marshal_test.go +++ b/report/marshal_test.go @@ -1,7 +1,6 @@ package report_test import ( - "bytes" "context" "reflect" "testing" @@ -15,22 +14,13 @@ import ( func TestRoundtrip(t *testing.T) { r1 := report.MakeReport() buf, _ := r1.WriteBinary() - original := append([]byte{}, buf.Bytes()...) // copy the contents for later - r2, err := report.MakeFromBinary(context.Background(), buf) + r2, err := report.MakeFromBinary(context.Background(), buf, true, true) if err != nil { t.Error(err) } if !reflect.DeepEqual(r1, *r2) { t.Errorf("%v != %v", r1, *r2) } - r3 := report.MakeReport() - err = r3.ReadBinary(context.Background(), bytes.NewBuffer(original), true, true) - if err != nil { - t.Error(err) - } - if !reflect.DeepEqual(r1, r3) { - t.Errorf("%v != %v", r1, r3) - } } // Create a Report for test purposes that contains about one of @@ -74,7 +64,7 @@ func makeTestReport() report.Report { func TestBiggerRoundtrip(t *testing.T) { r1 := makeTestReport() buf, _ := r1.WriteBinary() - r2, err := report.MakeFromBinary(context.Background(), buf) + r2, err := report.MakeFromBinary(context.Background(), buf, true, true) if err != nil { t.Error(err) }