Skip to content

Commit 4e07aad

Browse files
authored
chore(storage): Instrument existing media ops with storage trace package (googleapis#12879)
Replace existing media ops tracing with storage trace package As aligned in previous discussions, this supports an overarching span on Reader and Writer until closed. This replaces the remaining legacy tracing in copier and composer as well, completely transitioning to using the storage trace package
1 parent bf00b9d commit 4e07aad

File tree

7 files changed

+21
-33
lines changed

7 files changed

+21
-33
lines changed

storage/copy.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import (
1818
"context"
1919
"errors"
2020
"fmt"
21-
22-
"cloud.google.com/go/internal/trace"
2321
)
2422

2523
// CopierFrom creates a Copier that can copy src to dst.
@@ -82,8 +80,8 @@ type Copier struct {
8280

8381
// Run performs the copy.
8482
func (c *Copier) Run(ctx context.Context) (attrs *ObjectAttrs, err error) {
85-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Copier.Run")
86-
defer func() { trace.EndSpan(ctx, err) }()
83+
ctx, _ = startSpan(ctx, "Copier.Run")
84+
defer func() { endSpan(ctx, err) }()
8785

8886
if err := c.src.validate(); err != nil {
8987
return nil, err
@@ -180,8 +178,8 @@ type Composer struct {
180178

181179
// Run performs the compose operation.
182180
func (c *Composer) Run(ctx context.Context) (attrs *ObjectAttrs, err error) {
183-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Composer.Run")
184-
defer func() { trace.EndSpan(ctx, err) }()
181+
ctx, _ = startSpan(ctx, "Composer.Run")
182+
defer func() { endSpan(ctx, err) }()
185183

186184
if err := c.dst.validate(); err != nil {
187185
return nil, err

storage/grpc_client.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"sync"
2727

2828
"cloud.google.com/go/iam/apiv1/iampb"
29-
"cloud.google.com/go/internal/trace"
3029
gapic "cloud.google.com/go/storage/internal/apiv2"
3130
"cloud.google.com/go/storage/internal/apiv2/storagepb"
3231
"github.com/googleapis/gax-go/v2"
@@ -1069,8 +1068,8 @@ func (c *grpcStorageClient) NewMultiRangeDownloader(ctx context.Context, params
10691068
return nil, errors.New("storage: MultiRangeDownloader requires the experimental.WithGRPCBidiReads option")
10701069
}
10711070

1072-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.grpcStorageClient.NewMultiRangeDownloader")
1073-
defer func() { trace.EndSpan(ctx, err) }()
1071+
ctx, _ = startSpan(ctx, "grpcStorageClient.NewMultiRangeDownloader")
1072+
defer func() { endSpan(ctx, err) }()
10741073
s := callSettings(c.settings, opts...)
10751074
// Force the use of the custom codec to enable zero-copy reads.
10761075
s.gax = append(s.gax, gax.WithGRPCOptions(
@@ -1614,8 +1613,8 @@ func (c *grpcStorageClient) NewRangeReader(ctx context.Context, params *newRange
16141613
return c.NewRangeReaderReadObject(ctx, params, opts...)
16151614
}
16161615

1617-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.grpcStorageClient.NewRangeReader")
1618-
defer func() { trace.EndSpan(ctx, err) }()
1616+
ctx, _ = startSpan(ctx, "grpcStorageClient.NewRangeReader")
1617+
defer func() { endSpan(ctx, err) }()
16191618

16201619
s := callSettings(c.settings, opts...)
16211620

storage/grpc_reader.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"hash/crc32"
2323
"io"
2424

25-
"cloud.google.com/go/internal/trace"
2625
"cloud.google.com/go/storage/internal/apiv2/storagepb"
2726
"github.com/googleapis/gax-go/v2"
2827
"google.golang.org/grpc"
@@ -83,8 +82,8 @@ func (bytesCodecReadObject) Name() string {
8382

8483
// NewRangeReaderReadObject is the legacy (non-bidi) implementation of reads.
8584
func (c *grpcStorageClient) NewRangeReaderReadObject(ctx context.Context, params *newRangeReaderParams, opts ...storageOption) (r *Reader, err error) {
86-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.grpcStorageClient.NewRangeReaderReadObject")
87-
defer func() { trace.EndSpan(ctx, err) }()
85+
ctx, _ = startSpan(ctx, "grpcStorageClient.NewRangeReaderReadObject")
86+
defer func() { endSpan(ctx, err) }()
8887

8988
s := callSettings(c.settings, opts...)
9089

storage/http_client.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
"cloud.google.com/go/auth"
3434
"cloud.google.com/go/iam/apiv1/iampb"
3535
"cloud.google.com/go/internal/optional"
36-
"cloud.google.com/go/internal/trace"
3736
"github.com/google/uuid"
3837
"github.com/googleapis/gax-go/v2/callctx"
3938
"google.golang.org/api/googleapi"
@@ -847,8 +846,8 @@ func (c *httpStorageClient) NewMultiRangeDownloader(ctx context.Context, params
847846
}
848847

849848
func (c *httpStorageClient) NewRangeReader(ctx context.Context, params *newRangeReaderParams, opts ...storageOption) (r *Reader, err error) {
850-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.httpStorageClient.NewRangeReader")
851-
defer func() { trace.EndSpan(ctx, err) }()
849+
ctx, _ = startSpan(ctx, "httpStorageClient.NewRangeReader")
850+
defer func() { endSpan(ctx, err) }()
852851

853852
s := callSettings(c.settings, opts...)
854853

storage/reader.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ import (
2323
"strings"
2424
"sync"
2525
"time"
26-
27-
"cloud.google.com/go/internal/trace"
2826
)
2927

3028
var crc32cTable = crc32.MakeTable(crc32.Castagnoli)
@@ -116,7 +114,8 @@ func (o *ObjectHandle) NewReader(ctx context.Context) (*Reader, error) {
116114
func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) (r *Reader, err error) {
117115
// This span covers the life of the reader. It is closed via the context
118116
// in Reader.Close.
119-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.Reader")
117+
ctx, _ = startSpan(ctx, "Object.Reader")
118+
defer func() { endSpan(ctx, err) }()
120119

121120
if err := o.validate(); err != nil {
122121
return nil, err
@@ -150,8 +149,6 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64)
150149
// span now if there is an error.
151150
if err == nil {
152151
r.ctx = ctx
153-
} else {
154-
trace.EndSpan(ctx, err)
155152
}
156153

157154
return r, err
@@ -167,7 +164,8 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64)
167164
func (o *ObjectHandle) NewMultiRangeDownloader(ctx context.Context) (mrd *MultiRangeDownloader, err error) {
168165
// This span covers the life of the reader. It is closed via the context
169166
// in Reader.Close.
170-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.MultiRangeDownloader")
167+
ctx, _ = startSpan(ctx, "Object.MultiRangeDownloader")
168+
defer func() { endSpan(ctx, err) }()
171169

172170
if err := o.validate(); err != nil {
173171
return nil, err
@@ -195,8 +193,6 @@ func (o *ObjectHandle) NewMultiRangeDownloader(ctx context.Context) (mrd *MultiR
195193
// span now if there is an error.
196194
if err == nil {
197195
r.ctx = ctx
198-
} else {
199-
trace.EndSpan(ctx, err)
200196
}
201197

202198
return r, err
@@ -285,7 +281,7 @@ type Reader struct {
285281
// Close closes the Reader. It must be called when done reading.
286282
func (r *Reader) Close() error {
287283
err := r.reader.Close()
288-
trace.EndSpan(r.ctx, err)
284+
endSpan(r.ctx, err)
289285
return err
290286
}
291287

@@ -436,7 +432,7 @@ func (mrd *MultiRangeDownloader) Add(output io.Writer, offset, length int64, cal
436432
// Call [MultiRangeDownloader.Wait] to avoid this error.
437433
func (mrd *MultiRangeDownloader) Close() error {
438434
err := mrd.reader.close()
439-
trace.EndSpan(mrd.ctx, err)
435+
endSpan(mrd.ctx, err)
440436
return err
441437
}
442438

storage/storage.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import (
4040

4141
"cloud.google.com/go/auth"
4242
"cloud.google.com/go/internal/optional"
43-
"cloud.google.com/go/internal/trace"
4443
"cloud.google.com/go/storage/internal"
4544
"cloud.google.com/go/storage/internal/apiv2/storagepb"
4645
"github.com/googleapis/gax-go/v2"
@@ -1248,7 +1247,7 @@ type MoveObjectDestination struct {
12481247
// It is the caller's responsibility to call Close when writing is done. To
12491248
// stop writing without saving the data, cancel the context.
12501249
func (o *ObjectHandle) NewWriter(ctx context.Context) *Writer {
1251-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.Writer")
1250+
ctx, _ = startSpan(ctx, "Object.Writer")
12521251
return &Writer{
12531252
ctx: ctx,
12541253
o: o,
@@ -1284,7 +1283,7 @@ func (o *ObjectHandle) NewWriter(ctx context.Context) *Writer {
12841283
// objects which were created append semantics and not finalized.
12851284
// This feature is in preview and is not yet available for general use.
12861285
func (o *ObjectHandle) NewWriterFromAppendableObject(ctx context.Context, opts *AppendableWriterOpts) (*Writer, int64, error) {
1287-
ctx = trace.StartSpan(ctx, "cloud.google.com/go/storage.Object.Writer")
1286+
ctx, _ = startSpan(ctx, "Object.WriterFromAppendableObject")
12881287
if o.gen < 0 {
12891288
return nil, 0, errors.New("storage: ObjectHandle.Generation must be set to use NewWriterFromAppendableObject")
12901289
}

storage/writer.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import (
2222
"sync"
2323
"time"
2424
"unicode/utf8"
25-
26-
"cloud.google.com/go/internal/trace"
2725
)
2826

2927
// Interface internalWriter wraps low-level implementations which may vary
@@ -261,7 +259,7 @@ func (w *Writer) Close() error {
261259
w.closed = true
262260
w.mu.Lock()
263261
defer w.mu.Unlock()
264-
trace.EndSpan(w.ctx, w.err)
262+
endSpan(w.ctx, w.err)
265263
return w.err
266264
}
267265

0 commit comments

Comments
 (0)