Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Commit

Permalink
Incorporated PR comment. Replaced os.Stat() with a CountingReader to …
Browse files Browse the repository at this point in the history
…report number of bytes read.
  • Loading branch information
rickkw committed Jul 2, 2018
1 parent 5b21bd0 commit 0089b75
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions uploader/s3.go
Expand Up @@ -24,6 +24,21 @@ const (
defaultS3PartSize = 64 * 1024 * 1024 // 64MB per part
)

// CountingReader is a wrapper of io.Reader to count number of bytes read
type CountingReader struct {
reader io.Reader
BytesRead int
}

// Read aggregates number of bytes read
func (r * CountingReader) Read(p []byte) (n int, err error) {
n, err = r.reader.Read(p)
if err == nil {
r.BytesRead += n
}
return
}

// S3Uploader uploads logs to S3
type S3Uploader struct {
log logrus.FieldLogger
Expand Down Expand Up @@ -90,11 +105,6 @@ func (u *S3Uploader) Upload(ctx context.Context, local string, remote string, ct
}
}()

// Record number of bytes being uploaded. At this point, file must exist.
if stat, err := os.Stat(local); err == nil {
u.metrics.Counter("titus.executor.S3Uploader.uploadMB", int(stat.Size()/1024/1024), nil)
}

return u.uploadFile(ctx, f, remote, contentType)
}

Expand All @@ -105,17 +115,21 @@ func (u *S3Uploader) uploadFile(ctx context.Context, local io.Reader, remote str
contentType = defaultS3ContentType
}

// wrap input io.Reader with a counting reader
reader := &CountingReader{local, 0}

result, err := u.s3Uploader.UploadWithContext(ctx, &s3manager.UploadInput{
ACL: aws.String(defaultS3ACL),
ContentType: aws.String(contentType),
Bucket: aws.String(u.bucketName),
Key: aws.String(remote),
Body: local,
Body: io.Reader(reader),
})
if err != nil {
return err
}

u.metrics.Counter("titus.executor.S3Uploader.uploadMB", int(reader.BytesRead/1024/1024), nil)

This comment has been minimized.

Copy link
@sargun

sargun Jul 2, 2018

Contributor

Maybe successfullyUploadedBytes and report bytes?

This comment has been minimized.

Copy link
@rickkw

rickkw Jul 2, 2018

Author Contributor

titus.executor.S3Uploader.successfullyUploadedBytes?

This comment has been minimized.

Copy link
@sargun

sargun Jul 2, 2018

Contributor

Yep.

This comment has been minimized.

Copy link
@sargun

sargun Jul 2, 2018

Contributor

I would say bytes rather than MB, because technically, dividing by 1024 / 1024 is MiB -- no reason not to report in Bytes

u.log.Printf("Successfully uploaded file from: %s to: %s", local, result.Location)

return nil
Expand Down

0 comments on commit 0089b75

Please sign in to comment.