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.

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

Add logging options to jaeger.sampler #2566

Merged
merged 25 commits into from Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
056fced
add logger option
tttoad Jul 9, 2022
6c88603
replace logr.Logger with NullLogger
tttoad Jul 13, 2022
cd67037
Fix import format error
tttoad Jul 13, 2022
51203cf
fix go.mod and go.sum
tttoad Jul 13, 2022
9fb176d
add changelog
tttoad Jul 15, 2022
c46c09a
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Jul 21, 2022
e116740
use logr.Logger
tttoad Jul 23, 2022
9b41b31
go mod tidy
tttoad Jul 23, 2022
a1084fa
fix go.sum
tttoad Jul 23, 2022
28d3d49
change option name
tttoad Jul 23, 2022
0d7b251
fix changelog
tttoad Jul 25, 2022
9d90370
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Jul 25, 2022
eb0a994
use logr.Discard
tttoad Jul 26, 2022
7820104
change info to error
tttoad Jul 26, 2022
f43a860
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Jul 30, 2022
8346701
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Aug 5, 2022
15dda3d
Merge branch 'main' into sampler/log
tttoad Aug 6, 2022
ab6449a
fix changelog
tttoad Aug 6, 2022
e726816
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Aug 6, 2022
1be3ce2
Merge branch 'sampler/log' of github.com:tttoad/opentelemetry-go-cont…
tttoad Aug 6, 2022
a533b8a
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Aug 20, 2022
42eeac4
Merge branch 'main' of github.com:tttoad/opentelemetry-go-contrib int…
tttoad Nov 4, 2022
f65b24a
docs: remove useless text
tttoad Nov 4, 2022
59d859a
fix: dependabot config
tttoad Nov 5, 2022
5abca4b
Merge branch 'main' into sampler/log
tttoad Nov 7, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Expand Up @@ -8,7 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Changed
### Added

- The `WithLogger` option to `go.opentelemetry.io/contrib/samplers/jaegerremote` to allow users to pass a `logr.Logger` and have operations logged. (#2566)

### Changed

- Upgraded all `semconv` package use to `v1.12.0`. (#2589)

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.17

require (
github.com/go-logr/logr v1.2.3
github.com/gogo/protobuf v1.3.2
github.com/stretchr/testify v1.8.0
go.opentelemetry.io/otel v1.8.0
go.opentelemetry.io/otel/sdk v1.8.0
go.opentelemetry.io/otel/trace v1.8.0
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.8.0 // indirect
golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
40 changes: 40 additions & 0 deletions samplers/jaegerremote/logger.go
@@ -0,0 +1,40 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package jaegerremote // import "go.opentelemetry.io/contrib/samplers/jaegerremote"

import "github.com/go-logr/logr"

// NullLogger is implementation of the Logger interface that is no-op
var NullLoggger = &nullLogger{}
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

type nullLogger struct{}

func (n *nullLogger) Init(info logr.RuntimeInfo) {}

func (n *nullLogger) Enabled(level int) bool {
return false
}

func (n *nullLogger) Info(level int, msg string, keysAndValues ...interface{}) {}

func (n *nullLogger) Error(err error, msg string, keysAndValues ...interface{}) {}

func (n *nullLogger) WithValues(keysAndValues ...interface{}) logr.LogSink {
return n
}

func (n *nullLogger) WithName(name string) logr.LogSink {
return n
}
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")
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
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.New(NullLoggger),
}
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