Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Kemal Akkoyun <[email protected]>
  • Loading branch information
kakkoyun committed Jul 12, 2023
1 parent 64d5e43 commit a3234c4
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 39 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
### Fixed
- [#33](https://github.com/thanos-io/objstore/pull/33) Tracing: Add `ContextWithTracer()` to inject the tracer into the context.
- [#34](https://github.com/thanos-io/objstore/pull/34) Fix ignored options when creating shared credential Azure client.
- [#62](https://github.com/thanos-io/objstore/pull/62) S3: Fix ignored context cancellation in `Iter` method.
- [#62](https://github.com/thanos-io/objstore/pull/62) S3: Fix ignored context cancellation in `Iter` method.

### Added
- [#15](https://github.com/thanos-io/objstore/pull/15) Add Oracle Cloud Infrastructure Object Storage Bucket support.
Expand All @@ -26,7 +26,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
- [#59](https://github.com/thanos-io/objstore/pull/59) Adding method `IsCustomerManagedKeyError` on the bucket interface.
- [#61](https://github.com/thanos-io/objstore/pull/61) Add OpenTelemetry TracingBucket.
> This also changes the behaviour of `client.NewBucket`. Now it returns, uninstrumented and untraced bucket.
You can combine `client.InstrumentedBucket` and `tracing/{opentelemetry,opentracing}.TracedBucket` to have old behavior.
You can combine `objstore.WrapWithMetrics` and `tracing/{opentelemetry,opentracing}.WrapWithTraces` to have old behavior.

### Changed
- [#38](https://github.com/thanos-io/objstore/pull/38) *: Upgrade minio-go version to `v7.0.45`.
Expand Down
6 changes: 0 additions & 6 deletions client/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"gopkg.in/yaml.v2"
)

Expand Down Expand Up @@ -93,8 +92,3 @@ func NewBucket(logger log.Logger, confContentYaml []byte, component string) (obj

return objstore.NewPrefixedBucket(bucket, bucketConf.Prefix), nil
}

// InstrumentedBucket wraps the given bucket with metrics.
func InstrumentedBucket(bkt objstore.Bucket, reg prometheus.Registerer) objstore.InstrumentedBucket {
return objstore.WrapWithMetrics(bkt, reg, bkt.Name())
}
31 changes: 2 additions & 29 deletions client/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"io/ioutil"

"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/trace"

"github.com/thanos-io/objstore/tracing/opentelemetry"
Expand Down Expand Up @@ -39,32 +38,6 @@ func ExampleBucket() {
// false
}

func ExampleInstrumentedBucket() {
// Read the configuration file.
confContentYaml, err := ioutil.ReadFile("testconf/filesystem.conf.yml")
if err != nil {
panic(err)
}

// Create a new bucket.
bucket, err := NewBucket(log.NewNopLogger(), confContentYaml, "example")
if err != nil {
panic(err)
}

// Wrap it with instrumentation.
bucket = InstrumentedBucket(bucket, prometheus.NewRegistry())

// Test it.
exists, err := bucket.Exists(context.Background(), "example")
if err != nil {
panic(err)
}
fmt.Println(exists)
// Output:
// false
}

func ExampleTracingBucketUsingOpenTracing() { //nolint:govet
// Read the configuration file.
confContentYaml, err := ioutil.ReadFile("testconf/filesystem.conf.yml")
Expand All @@ -79,7 +52,7 @@ func ExampleTracingBucketUsingOpenTracing() { //nolint:govet
}

// Wrap it with tracing.
bucket = opentracing.TracedBucket(bucket)
bucket = opentracing.WrapWithTraces(bucket)

// Test it.
exists, err := bucket.Exists(context.Background(), "example")
Expand All @@ -105,7 +78,7 @@ func ExampleTracingBucketUsingOpenTelemetry() { //nolint:govet
}

// Wrap it with tracing.
bucket = opentelemetry.TracedBucket(bucket, trace.NewNoopTracerProvider().Tracer("bucket"))
bucket = opentelemetry.WrapWithTraces(bucket, trace.NewNoopTracerProvider().Tracer("bucket"))

// Test it.
exists, err := bucket.Exists(context.Background(), "example")
Expand Down
2 changes: 1 addition & 1 deletion tracing/opentelemetry/opentelemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type TracingBucket struct {
bkt objstore.Bucket
}

func TracedBucket(bkt objstore.Bucket, tracer trace.Tracer) objstore.InstrumentedBucket {
func WrapWithTraces(bkt objstore.Bucket, tracer trace.Tracer) objstore.InstrumentedBucket {
return TracingBucket{tracer: tracer, bkt: bkt}
}

Expand Down
2 changes: 1 addition & 1 deletion tracing/opentracing/opentracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type TracingBucket struct {
bkt objstore.Bucket
}

func TracedBucket(bkt objstore.Bucket) objstore.InstrumentedBucket {
func WrapWithTraces(bkt objstore.Bucket) objstore.InstrumentedBucket {
return TracingBucket{bkt: bkt}
}

Expand Down

0 comments on commit a3234c4

Please sign in to comment.