diff --git a/CHANGELOG.md b/CHANGELOG.md index 4576559b9ad..95d8c15a97c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added +- The `WithLogger` option to `go.opentelemetry.io/contrib/samplers/jaegerremote` to allow users to pass a `logr.Logger` and have operations logged. (#2566) - Add the `messaging.url` & `messaging.system` attributes to all appropriate SQS operations in the `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` package. (#2879) ## [1.11.1/0.36.4/0.5.2] diff --git a/samplers/jaegerremote/go.mod b/samplers/jaegerremote/go.mod index b0d77e8d853..96440823c92 100644 --- a/samplers/jaegerremote/go.mod +++ b/samplers/jaegerremote/go.mod @@ -3,9 +3,9 @@ module go.opentelemetry.io/contrib/samplers/jaegerremote go 1.18 require ( + github.com/go-logr/logr v1.2.3 github.com/gogo/protobuf v1.3.2 github.com/stretchr/testify v1.8.1 - go.opentelemetry.io/otel v1.11.1 go.opentelemetry.io/otel/sdk v1.11.1 go.opentelemetry.io/otel/trace v1.11.1 google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa @@ -13,9 +13,9 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + go.opentelemetry.io/otel v1.11.1 // indirect golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect google.golang.org/protobuf v1.27.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/samplers/jaegerremote/sampler_remote.go b/samplers/jaegerremote/sampler_remote.go index b1138d311e0..c6aed231125 100644 --- a/samplers/jaegerremote/sampler_remote.go +++ b/samplers/jaegerremote/sampler_remote.go @@ -27,7 +27,6 @@ import ( "time" jaeger_api_v2 "go.opentelemetry.io/contrib/samplers/jaegerremote/internal/proto-gen/jaeger-idl/proto/api_v2" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/sdk/trace" ) @@ -105,7 +104,7 @@ func (s *Sampler) ShouldSample(p trace.SamplingParameters) trace.SamplingResult // go-routines it may have started. func (s *Sampler) Close() { if swapped := atomic.CompareAndSwapInt64(&s.closed, 0, 1); !swapped { - otel.Handle(fmt.Errorf("repeated attempt to close the sampler is ignored")) + s.logger.Info("repeated attempt to close the sampler is ignored") return } @@ -151,12 +150,12 @@ func (s *Sampler) setSampler(sampler trace.Sampler) { func (s *Sampler) UpdateSampler() { res, err := s.samplingFetcher.Fetch(s.serviceName) if err != nil { - // log.Printf("failed to fetch sampling strategy: %v", err) + s.logger.Error(err, "failed to fetch sampling strategy") return } strategy, err := s.samplingParser.Parse(res) if err != nil { - // log.Printf("failed to parse sampling strategy response: %v", err) + s.logger.Error(err, "failed to parse sampling strategy response") return } @@ -164,7 +163,7 @@ func (s *Sampler) UpdateSampler() { defer s.Unlock() if err := s.updateSamplerViaUpdaters(strategy); err != nil { - // c.logger.Infof("failed to handle sampling strategy response %+v. Got error: %v", res, err) + s.logger.Error(err, "failed to handle sampling strategy response", "response", res) return } } diff --git a/samplers/jaegerremote/sampler_remote_options.go b/samplers/jaegerremote/sampler_remote_options.go index 2fb293646b3..31f95fb5ebb 100644 --- a/samplers/jaegerremote/sampler_remote_options.go +++ b/samplers/jaegerremote/sampler_remote_options.go @@ -19,6 +19,8 @@ package jaegerremote // import "go.opentelemetry.io/contrib/samplers/jaegerremot import ( "time" + "github.com/go-logr/logr" + "go.opentelemetry.io/otel/sdk/trace" ) @@ -30,6 +32,7 @@ type config struct { samplingParser samplingStrategyParser updaters []samplerUpdater posParams perOperationSamplerParams + logger logr.Logger } // newConfig returns an appropriately configured config. @@ -48,6 +51,7 @@ func newConfig(options ...Option) config { MaxOperations: defaultSamplingMaxOperations, OperationNameLateBinding: defaultSamplingOperationNameLateBinding, }, + logger: logr.Discard(), } for _, option := range options { option.apply(&c) @@ -112,6 +116,13 @@ func WithSamplingRefreshInterval(samplingRefreshInterval time.Duration) Option { }) } +// WithLogger configures the sampler to log operation and debug information with logger. +func WithLogger(logger logr.Logger) Option { + return optionFunc(func(c *config) { + c.logger = logger + }) +} + // samplingStrategyFetcher creates a Option that initializes sampling strategy fetcher. func withSamplingStrategyFetcher(fetcher samplingStrategyFetcher) Option { return optionFunc(func(c *config) { diff --git a/samplers/jaegerremote/sampler_remote_test.go b/samplers/jaegerremote/sampler_remote_test.go index 528b7dad548..7748b751f40 100644 --- a/samplers/jaegerremote/sampler_remote_test.go +++ b/samplers/jaegerremote/sampler_remote_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/go-logr/logr/testr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -112,6 +113,7 @@ func TestRemoteSamplerOptions(t *testing.T) { initSampler := newProbabilisticSampler(0.123) fetcher := new(fakeSamplingFetcher) parser := new(samplingStrategyParserImpl) + logger := testr.New(t) updaters := []samplerUpdater{new(probabilisticSamplerUpdater)} sampler := New( "test", @@ -123,6 +125,7 @@ func TestRemoteSamplerOptions(t *testing.T) { withSamplingStrategyFetcher(fetcher), withSamplingStrategyParser(parser), withUpdaters(updaters...), + WithLogger(logger), ) assert.Equal(t, 42, sampler.posParams.MaxOperations) assert.True(t, sampler.posParams.OperationNameLateBinding) @@ -132,6 +135,7 @@ func TestRemoteSamplerOptions(t *testing.T) { assert.Same(t, fetcher, sampler.samplingFetcher) assert.Same(t, parser, sampler.samplingParser) assert.EqualValues(t, sampler.updaters[0], &perOperationSamplerUpdater{MaxOperations: 42, OperationNameLateBinding: true}) + assert.Equal(t, logger, sampler.logger) } func TestRemoteSamplerOptionsDefaults(t *testing.T) {