Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hedged-request: Handling in HTTP Client Configuration #119

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Hedged-request: Handling in HTTP Client Configuration #119

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3ca3d25
hedged_request
Vanshikav123 May 19, 2024
e606f9d
http custom transport parameter
Vanshikav123 May 20, 2024
659a3bc
module changes
Vanshikav123 May 20, 2024
1cffe47
applied logic
Vanshikav123 May 21, 2024
540c1e5
added rt
Vanshikav123 May 24, 2024
1b2aa76
readme update
Vanshikav123 May 31, 2024
bd0b7b3
added for other providers
Vanshikav123 Jun 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Generally, you have two ways of using `objstore` module:

First is to import the provider you want e.g. [`github.com/thanos-io/objstore/providers/s3`](providers/s3) and instantiate it with available constructor (e.g. `NewBucket`).

The second option is to use the factory `NewBucket(logger log.Logger, confContentYaml []byte, reg prometheus.Registerer, component string)` that will instantiate the object storage client based on YAML file provided. The YAML file has generally the format like this:
The second option is to use the factory `NewBucket(logger log.Logger, confContentYaml []byte, reg prometheus.Registerer, component string,rt http.RoundTripper)` that will instantiate the object storage client based on YAML file provided. The YAML file has generally the format like this:
GiedriusS marked this conversation as resolved.
Show resolved Hide resolved

```yaml
type: <PROVIDER_TYPE>
Expand All @@ -114,6 +114,8 @@ config:

The exact option depends on provider and are in sections below.

`NewBucket` function now accepts an `http.RoundTripper` parameter allows clients to provide a custom transport for HTTP requests, making the function more flexible , this change facilitates the use of various HTTP client configurations, including hedged HTTP transports.
GiedriusS marked this conversation as resolved.
Show resolved Hide resolved

> NOTE: All code snippets are auto-generated from code and up-to-date.

Check out the [Thanos documentation](https://thanos.io/tip/thanos/storage.md/) to see how Thanos uses this module.
Expand Down
13 changes: 7 additions & 6 deletions client/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package client
import (
"context"
"fmt"
"net/http"
"strings"

"github.com/thanos-io/objstore"
Expand Down Expand Up @@ -49,7 +50,7 @@ type BucketConfig struct {

// NewBucket initializes and returns new object storage clients.
// NOTE: confContentYaml can contain secrets.
func NewBucket(logger log.Logger, confContentYaml []byte, component string) (objstore.Bucket, error) {
func NewBucket(logger log.Logger, confContentYaml []byte, component string, rt http.RoundTripper) (objstore.Bucket, error) {
level.Info(logger).Log("msg", "loading bucket configuration")
bucketConf := &BucketConfig{}
if err := yaml.UnmarshalStrict(confContentYaml, bucketConf); err != nil {
Expand All @@ -64,23 +65,23 @@ func NewBucket(logger log.Logger, confContentYaml []byte, component string) (obj
var bucket objstore.Bucket
switch strings.ToUpper(string(bucketConf.Type)) {
case string(GCS):
bucket, err = gcs.NewBucket(context.Background(), logger, config, component)
bucket, err = gcs.NewBucket(context.Background(), logger, config, component, rt)
case string(S3):
bucket, err = s3.NewBucket(logger, config, component)
bucket, err = s3.NewBucket(logger, config, component, rt)
case string(AZURE):
bucket, err = azure.NewBucket(logger, config, component)
bucket, err = azure.NewBucket(logger, config, component, rt)
case string(SWIFT):
bucket, err = swift.NewContainer(logger, config)
Copy link
Member

@GiedriusS GiedriusS May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not supported by all providers? 🤔 I checked a few like swift/bos and I don't see a reason why this is not implemented there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not supported by all providers? 🤔 I checked a few like swift/bos and I don't see a reason why this is not implemented there.

i added this parameter to the providers which had http.Client , swift does not uses NewBucket but bos/obs can have this parameter i think also i see that obs makes new ObsClient and bos uses NewClient which makes a BOS client...wdyt?

case string(COS):
bucket, err = cos.NewBucket(logger, config, component)
bucket, err = cos.NewBucket(logger, config, component, rt)
case string(ALIYUNOSS):
bucket, err = oss.NewBucket(logger, config, component)
case string(FILESYSTEM):
bucket, err = filesystem.NewBucketFromConfig(config)
case string(BOS):
bucket, err = bos.NewBucket(logger, config, component)
case string(OCI):
bucket, err = oci.NewBucket(logger, config)
bucket, err = oci.NewBucket(logger, config, rt)
case string(OBS):
bucket, err = obs.NewBucket(logger, config)
default:
Expand Down
7 changes: 4 additions & 3 deletions client/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package client
import (
"context"
"fmt"
"net/http"
"os"

"github.com/go-kit/log"
Expand All @@ -23,7 +24,7 @@ func ExampleBucket() {
}

// Create a new bucket.
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example")
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example", http.DefaultTransport)
if err != nil {
panic(err)
}
Expand All @@ -46,7 +47,7 @@ func ExampleTracingBucketUsingOpenTracing() { //nolint:govet
}

// Create a new bucket.
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example")
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example", http.DefaultTransport)
if err != nil {
panic(err)
}
Expand All @@ -72,7 +73,7 @@ func ExampleTracingBucketUsingOpenTelemetry() { //nolint:govet
}

// Create a new bucket.
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example")
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example", http.DefaultTransport)
if err != nil {
panic(err)
}
Expand Down
5 changes: 3 additions & 2 deletions providers/azure/azure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package azure
import (
"context"
"io"
"net/http"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -143,7 +144,7 @@ type Bucket struct {
}

// NewBucket returns a new Bucket using the provided Azure config.
func NewBucket(logger log.Logger, azureConfig []byte, component string) (*Bucket, error) {
func NewBucket(logger log.Logger, azureConfig []byte, component string, rt http.RoundTripper) (*Bucket, error) {
level.Debug(logger).Log("msg", "creating new Azure bucket connection", "component", component)
conf, err := parseConfig(azureConfig)
if err != nil {
Expand Down Expand Up @@ -352,7 +353,7 @@ func NewTestBucket(t testing.TB, component string) (objstore.Bucket, func(), err
if err != nil {
return nil, nil, err
}
bkt, err := NewBucket(log.NewNopLogger(), bc, component)
bkt, err := NewBucket(log.NewNopLogger(), bc, component, http.DefaultTransport)
if err != nil {
t.Errorf("Cannot create Azure storage container:")
return nil, nil, err
Expand Down
6 changes: 3 additions & 3 deletions providers/cos/cos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func parseConfig(conf []byte) (Config, error) {
}

// NewBucket returns a new Bucket using the provided cos configuration.
func NewBucket(logger log.Logger, conf []byte, component string) (*Bucket, error) {
func NewBucket(logger log.Logger, conf []byte, component string, rt http.RoundTripper) (*Bucket, error) {
if logger == nil {
logger = log.NewNopLogger()
}
Expand Down Expand Up @@ -485,7 +485,7 @@ func NewTestBucket(t testing.TB) (objstore.Bucket, func(), error) {
return nil, nil, err
}

b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test")
b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test", http.DefaultTransport)
if err != nil {
return nil, nil, err
}
Expand All @@ -506,7 +506,7 @@ func NewTestBucket(t testing.TB) (objstore.Bucket, func(), error) {
return nil, nil, err
}

b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test")
b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test", http.DefaultTransport)
if err != nil {
return nil, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions providers/gcs/gcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func parseConfig(conf []byte) (Config, error) {
}

// NewBucket returns a new Bucket against the given bucket handle.
func NewBucket(ctx context.Context, logger log.Logger, conf []byte, component string) (*Bucket, error) {
func NewBucket(ctx context.Context, logger log.Logger, conf []byte, component string, rt http.RoundTripper) (*Bucket, error) {
config, err := parseConfig(conf)
if err != nil {
return nil, err
Expand Down Expand Up @@ -302,7 +302,7 @@ func NewTestBucket(t testing.TB, project string) (objstore.Bucket, func(), error
return nil, nil, err
}

b, err := NewBucket(ctx, log.NewNopLogger(), bc, "thanos-e2e-test")
b, err := NewBucket(ctx, log.NewNopLogger(), bc, "thanos-e2e-test", http.DefaultTransport)
if err != nil {
return nil, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions providers/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (b *Bucket) deleteBucket(ctx context.Context) (err error) {
}

// NewBucket returns a new Bucket using the provided oci config values.
func NewBucket(logger log.Logger, ociConfig []byte) (*Bucket, error) {
func NewBucket(logger log.Logger, ociConfig []byte, rt http.RoundTripper) (*Bucket, error) {
GiedriusS marked this conversation as resolved.
Show resolved Hide resolved
level.Debug(logger).Log("msg", "creating new oci bucket connection")
var config = DefaultConfig
var configurationProvider common.ConfigurationProvider
Expand Down Expand Up @@ -375,7 +375,7 @@ func NewTestBucket(t testing.TB) (objstore.Bucket, func(), error) {
return nil, nil, err
}

bkt, err := NewBucket(log.NewNopLogger(), ociConfig)
bkt, err := NewBucket(log.NewNopLogger(), ociConfig, http.DefaultTransport)
if err != nil {
return nil, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions providers/s3/s3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func parseConfig(conf []byte) (Config, error) {
}

// NewBucket returns a new Bucket using the provided s3 config values.
func NewBucket(logger log.Logger, conf []byte, component string) (*Bucket, error) {
func NewBucket(logger log.Logger, conf []byte, component string, rt http.RoundTripper) (*Bucket, error) {
config, err := parseConfig(conf)
if err != nil {
return nil, err
Expand Down Expand Up @@ -605,7 +605,7 @@ func NewTestBucketFromConfig(t testing.TB, location string, c Config, reuseBucke
if err != nil {
return nil, nil, err
}
b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test")
b, err := NewBucket(log.NewNopLogger(), bc, "thanos-e2e-test", http.DefaultTransport)
if err != nil {
return nil, nil, err
}
Expand Down
Loading