From d931411f2c921045dd5a40a0cf730eca3c62f5c5 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Mon, 7 Aug 2023 20:54:37 +0000 Subject: [PATCH 1/2] feat(storage): trace span covers life of a Reader Make the tracing span created in NewRangeReader cover the entire lifecycle of the Reader. NewRangeReader returns before the data transfer is complete, so it's more useful to track the entire period until the Reader is closed. NewRangeReader is still tracked with a span at the level of individual transports (HTTP and gRPC). --- storage/reader.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/storage/reader.go b/storage/reader.go index 1bb65ec8107..9772b566354 100644 --- a/storage/reader.go +++ b/storage/reader.go @@ -87,8 +87,9 @@ func (o *ObjectHandle) NewReader(ctx context.Context) (*Reader, error) { // that file will be served back whole, regardless of the requested range as // Google Cloud Storage dictates. func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) (r *Reader, err error) { - ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.NewRangeReader") - defer func() { trace.EndSpan(ctx, err) }() + // This span covers the life of the reader. It is closed via the context + // in Reader.Close. + ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.Reader") if err := o.validate(); err != nil { return nil, err @@ -116,6 +117,7 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) } r, err = o.c.tc.NewRangeReader(ctx, params, opts...) + r.ctx = ctx return r, err } @@ -204,11 +206,14 @@ type Reader struct { gotCRC uint32 // running crc reader io.ReadCloser + ctx context.Context } // Close closes the Reader. It must be called when done reading. func (r *Reader) Close() error { - return r.reader.Close() + err := r.reader.Close() + trace.EndSpan(r.ctx, err) + return err } func (r *Reader) Read(p []byte) (int, error) { From f6a90ec13aa7255bc69a890c3d413759e990c19d Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Wed, 9 Aug 2023 17:35:27 +0000 Subject: [PATCH 2/2] fix nil reader case --- storage/reader.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/storage/reader.go b/storage/reader.go index 9772b566354..580353053ac 100644 --- a/storage/reader.go +++ b/storage/reader.go @@ -117,7 +117,14 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) } r, err = o.c.tc.NewRangeReader(ctx, params, opts...) - r.ctx = ctx + + // Pass the context so that the span can be closed in Reader.Close, or close the + // span now if there is an error. + if err == nil { + r.ctx = ctx + } else { + trace.EndSpan(ctx, err) + } return r, err }