Skip to content

Commit

Permalink
Add logging options to jaeger.sampler (#2566)
Browse files Browse the repository at this point in the history
* add logger option

* replace logr.Logger with NullLogger

* Fix import format error

* fix go.mod and go.sum

* add changelog

* use logr.Logger

* go mod tidy

* fix go.sum

* change option name

* fix changelog
default use NullLogger

* use logr.Discard

* change info to error

* fix changelog

* docs: remove useless text

* fix: dependabot config
  • Loading branch information
tttoad committed Nov 7, 2022
1 parent e97d789 commit d9bccb7
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions samplers/jaegerremote/go.mod
Expand Up @@ -3,19 +3,19 @@ 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
)

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
Expand Down
9 changes: 4 additions & 5 deletions samplers/jaegerremote/sampler_remote.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -151,20 +150,20 @@ 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
}

s.Lock()
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
}
}
Expand Down
11 changes: 11 additions & 0 deletions samplers/jaegerremote/sampler_remote_options.go
Expand Up @@ -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"
)

Expand All @@ -30,6 +32,7 @@ type config struct {
samplingParser samplingStrategyParser
updaters []samplerUpdater
posParams perOperationSamplerParams
logger logr.Logger
}

// newConfig returns an appropriately configured config.
Expand All @@ -48,6 +51,7 @@ func newConfig(options ...Option) config {
MaxOperations: defaultSamplingMaxOperations,
OperationNameLateBinding: defaultSamplingOperationNameLateBinding,
},
logger: logr.Discard(),
}
for _, option := range options {
option.apply(&c)
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions samplers/jaegerremote/sampler_remote_test.go
Expand Up @@ -23,6 +23,7 @@ import (
"testing"
"time"

"github.com/go-logr/logr/testr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand All @@ -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) {
Expand Down

0 comments on commit d9bccb7

Please sign in to comment.