Replace minio-go with aws-sdk-go for s3-compatible log backend #670
Conversation
| logrus.WithFields(logrus.Fields{"bucketName": s.bucket, "key": objectName}).Debug("Downloading log") | ||
|
|
||
| // stream the logs to an in-memory buffer | ||
| target := &aws.WriteAtBuffer{} |
There was a problem hiding this comment.
have used https://docs.aws.amazon.com/sdk-for-go/api/service/s3/#S3.GetObject w/ some success in the past to avoid having to copy into a buffer here, am less familiar with the s3Manager api.
There was a problem hiding this comment.
happy to punt on optimizing here
There was a problem hiding this comment.
I think that should work as well. Will look at making the change tomorrow.
There was a problem hiding this comment.
@rdallman I've looked into replacing this with s3.GetObject and the challenge there is that we wouldn't be able to simply return the Body of s3.GetObjectOutput as the io.Reader result of GetLog. The reason is that the result body needs to be closed in order to not leak connections (see aws/aws-sdk-go#408), so unfortunately we wouldn't be able to avoid copying into an intermediate buffer.
There was a problem hiding this comment.
gotcha. we could change LogStore.GetLog to return an io.ReadCloser ?
There was a problem hiding this comment.
thanks for digging in, looks good.
fwiw aws is releasing a sdk 2.0 for go https://aws.amazon.com/blogs/developer/aws-sdk-for-go-2-0-developer-preview/ that is supposed to be less nasty, we could probably migrate pretty easily since the surface area is pretty small.
Minio-go has a couple of deficiencies that are resolved by switching to aws-sdk-go for our s3 client driver.
io.Readerinterface, since the size of the uploaded object must be computed prior to making a request. This would preclude any kind of streaming readers in the future.Last-Modifiedheader in an HTTP response. This makes it incompatible with some major cloud vendor S3/Object Storage implementations, for example Oracle's.Also fixes #646