From 335f4de960fff78db2c668fe5868eda0d88f64f6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 19 Mar 2024 11:28:11 -0700 Subject: [PATCH] Add global log package (#5085) * Add the log/global package * Implement the stubbed features * Add ConcurrentSafe tests * Restructure with internal implementation * Add internal global state * Use internal state in log/global * Add TestDelegation * Fix lint * Clean log_test.go * Clean up * Add changelog entry * Simplify TestMultipleGlobalLoggerProvider * Shorten log.go * Fix comment text wrapping * Shorten state_test.go * Don't pollute output in TestSetLoggerProvider --- CHANGELOG.md | 3 + log/global/log.go | 49 +++++++++ log/global/log_test.go | 24 +++++ log/internal/global/log.go | 96 ++++++++++++++++++ log/internal/global/log_test.go | 160 ++++++++++++++++++++++++++++++ log/internal/global/state.go | 53 ++++++++++ log/internal/global/state_test.go | 75 ++++++++++++++ 7 files changed, 460 insertions(+) create mode 100644 log/global/log.go create mode 100644 log/global/log_test.go create mode 100644 log/internal/global/log.go create mode 100644 log/internal/global/log_test.go create mode 100644 log/internal/global/state.go create mode 100644 log/internal/global/state_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d6d0bef9..58e61d055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `SeverityUndefined` `const` to `go.opentelemetry.io/otel/log`. This value represents an unset severity level. (#5072) - Add `Empty` function in `go.opentelemetry.io/otel/log` to return a `KeyValue` for an empty value. (#5076) +- Add `go.opentelemetry.io/otel/log/global` to manage the global `LoggerProvider`. + This package is provided with the anticipation that all functionality will be migrate to `go.opentelemetry.io/otel` when `go.opentelemetry.io/otel/log` stabilizes. + At which point, users will be required to migrage their code, and this package will be deprecated then removed. (#5085) ### Changed diff --git a/log/global/log.go b/log/global/log.go new file mode 100644 index 000000000..71ec57798 --- /dev/null +++ b/log/global/log.go @@ -0,0 +1,49 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +/* +Package global provides access to a global implementation of the OpenTelemetry +Logs Bridge API. + +This package is experimental. It will be deprecated and removed when the [log] +package becomes stable. Its functionality will be migrated to +go.opentelemetry.io/otel. +*/ +package global // import "go.opentelemetry.io/otel/log/global" + +import ( + "go.opentelemetry.io/otel/log" + "go.opentelemetry.io/otel/log/internal/global" +) + +// Logger returns a [log.Logger] configured with the provided name and options +// from the globally configured [log.LoggerProvider]. +// +// If this is called before a global LoggerProvider is configured, the returned +// Logger will be a No-Op implementation of a Logger. When a global +// LoggerProvider is registered for the first time, the returned Logger is +// updated in-place to report to this new LoggerProvider. There is no need to +// call this function again for an updated instance. +// +// This is a convenience function. It is equivalent to: +// +// GetLoggerProvider().Logger(name, options...) +func Logger(name string, options ...log.LoggerOption) log.Logger { + return GetLoggerProvider().Logger(name, options...) +} + +// GetLoggerProvider returns the globally configured [log.LoggerProvider]. +// +// If a global LoggerProvider has not been configured with [SetLoggerProvider], +// the returned Logger will be a No-Op implementation of a LoggerProvider. When +// a global LoggerProvider is registered for the first time, the returned +// LoggerProvider and all of its created Loggers are updated in-place. There is +// no need to call this function again for an updated instance. +func GetLoggerProvider() log.LoggerProvider { + return global.GetLoggerProvider() +} + +// SetLoggerProvider configures provider as the global [log.LoggerProvider]. +func SetLoggerProvider(provider log.LoggerProvider) { + global.SetLoggerProvider(provider) +} diff --git a/log/global/log_test.go b/log/global/log_test.go new file mode 100644 index 000000000..de6f15a72 --- /dev/null +++ b/log/global/log_test.go @@ -0,0 +1,24 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package global // import "go.opentelemetry.io/otel/log/global" + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/log" + "go.opentelemetry.io/otel/log/noop" +) + +func TestMultipleGlobalLoggerProvider(t *testing.T) { + type provider struct{ log.LoggerProvider } + + p1, p2 := provider{}, noop.NewLoggerProvider() + + SetLoggerProvider(&p1) + SetLoggerProvider(p2) + + assert.Equal(t, p2, GetLoggerProvider()) +} diff --git a/log/internal/global/log.go b/log/internal/global/log.go new file mode 100644 index 000000000..ffa5db996 --- /dev/null +++ b/log/internal/global/log.go @@ -0,0 +1,96 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package global // import "go.opentelemetry.io/otel/log/internal/global" + +import ( + "context" + "sync" + "sync/atomic" + + "go.opentelemetry.io/otel/log" + "go.opentelemetry.io/otel/log/embedded" +) + +// instLib defines the instrumentation library a logger is created for. +// +// Do not use sdk/instrumentation (API cannot depend on the SDK). +type instLib struct{ name, version string } + +type loggerProvider struct { + embedded.LoggerProvider + + mu sync.Mutex + loggers map[instLib]*logger + delegate log.LoggerProvider +} + +// Compile-time guarantee loggerProvider implements LoggerProvider. +var _ log.LoggerProvider = (*loggerProvider)(nil) + +func (p *loggerProvider) Logger(name string, options ...log.LoggerOption) log.Logger { + p.mu.Lock() + defer p.mu.Unlock() + + if p.delegate != nil { + return p.delegate.Logger(name, options...) + } + + cfg := log.NewLoggerConfig(options...) + key := instLib{name, cfg.InstrumentationVersion()} + + if p.loggers == nil { + l := &logger{name: name, options: options} + p.loggers = map[instLib]*logger{key: l} + return l + } + + if l, ok := p.loggers[key]; ok { + return l + } + + l := &logger{name: name, options: options} + p.loggers[key] = l + return l +} + +func (p *loggerProvider) setDelegate(provider log.LoggerProvider) { + p.mu.Lock() + defer p.mu.Unlock() + + p.delegate = provider + for _, l := range p.loggers { + l.setDelegate(provider) + } + p.loggers = nil // Only set logger delegates once. +} + +type logger struct { + embedded.Logger + + name string + options []log.LoggerOption + + delegate atomic.Value // log.Logger +} + +// Compile-time guarantee logger implements Logger. +var _ log.Logger = (*logger)(nil) + +func (l *logger) Emit(ctx context.Context, r log.Record) { + if del, ok := l.delegate.Load().(log.Logger); ok { + del.Emit(ctx, r) + } +} + +func (l *logger) Enabled(ctx context.Context, r log.Record) bool { + var enabled bool + if del, ok := l.delegate.Load().(log.Logger); ok { + enabled = del.Enabled(ctx, r) + } + return enabled +} + +func (l *logger) setDelegate(provider log.LoggerProvider) { + l.delegate.Store(provider.Logger(l.name, l.options...)) +} diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go new file mode 100644 index 000000000..d7bfb990c --- /dev/null +++ b/log/internal/global/log_test.go @@ -0,0 +1,160 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package global + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/log" + "go.opentelemetry.io/otel/log/embedded" + "go.opentelemetry.io/otel/log/noop" +) + +func TestLoggerProviderConcurrentSafe(t *testing.T) { + p := &loggerProvider{} + + done := make(chan struct{}) + stop := make(chan struct{}) + + go func() { + defer close(done) + var logger log.Logger + for i := 0; ; i++ { + logger = p.Logger(fmt.Sprintf("a%d", i)) + select { + case <-stop: + _ = logger + return + default: + } + } + }() + + p.setDelegate(noop.NewLoggerProvider()) + close(stop) + <-done +} + +func TestLoggerConcurrentSafe(t *testing.T) { + l := &logger{} + + done := make(chan struct{}) + stop := make(chan struct{}) + + go func() { + defer close(done) + + ctx := context.Background() + var r log.Record + + var enabled bool + for { + l.Emit(ctx, r) + enabled = l.Enabled(ctx, r) + + select { + case <-stop: + _ = enabled + return + default: + } + } + }() + + l.setDelegate(noop.NewLoggerProvider()) + close(stop) + <-done +} + +type testLoggerProvider struct { + embedded.LoggerProvider + + loggers map[string]*testLogger + loggerN int +} + +func (p *testLoggerProvider) Logger(name string, _ ...log.LoggerOption) log.Logger { + if p.loggers == nil { + l := &testLogger{} + p.loggers = map[string]*testLogger{name: l} + p.loggerN++ + return l + } + + if l, ok := p.loggers[name]; ok { + return l + } + + p.loggerN++ + l := &testLogger{} + p.loggers[name] = l + return l +} + +type testLogger struct { + embedded.Logger + + emitN, enabledN int +} + +func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } +func (l *testLogger) Enabled(context.Context, log.Record) bool { + l.enabledN++ + return true +} + +func emitRecord(l log.Logger) { + ctx := context.Background() + var r log.Record + + _ = l.Enabled(ctx, r) + l.Emit(ctx, r) +} + +func TestDelegation(t *testing.T) { + provider := &loggerProvider{} + + const preName = "pre" + pre0, pre1 := provider.Logger(preName), provider.Logger(preName) + assert.Same(t, pre0, pre1, "same logger instance not returned") + + alt := provider.Logger("alt") + assert.NotSame(t, pre0, alt) + + delegate := &testLoggerProvider{} + provider.setDelegate(delegate) + + want := 2 // (pre0/pre1) and (alt) + if !assert.Equal(t, want, delegate.loggerN, "previous Loggers not delegated") { + want = delegate.loggerN + } + + pre2 := provider.Logger(preName) + if !assert.Equal(t, want, delegate.loggerN, "previous Logger recreated") { + want = delegate.loggerN + } + + post := provider.Logger("test") + want++ + assert.Equal(t, want, delegate.loggerN, "new Logger not delegated") + + emitRecord(pre0) + emitRecord(pre2) + + if assert.IsType(t, &testLogger{}, pre2, "wrong pre-delegation Logger type") { + assert.Equal(t, 2, pre2.(*testLogger).emitN, "Emit not delegated") + assert.Equal(t, 2, pre2.(*testLogger).enabledN, "Enabled not delegated") + } + + emitRecord(post) + + if assert.IsType(t, &testLogger{}, post, "wrong post-delegation Logger type") { + assert.Equal(t, 1, post.(*testLogger).emitN, "Emit not delegated") + assert.Equal(t, 1, post.(*testLogger).enabledN, "Enabled not delegated") + } +} diff --git a/log/internal/global/state.go b/log/internal/global/state.go new file mode 100644 index 000000000..dbe1c2fbf --- /dev/null +++ b/log/internal/global/state.go @@ -0,0 +1,53 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package global // import "go.opentelemetry.io/otel/log/internal/global" + +import ( + "errors" + "sync" + "sync/atomic" + + "go.opentelemetry.io/otel/internal/global" + "go.opentelemetry.io/otel/log" +) + +var ( + globalLoggerProvider = defaultLoggerProvider() + + delegateLoggerOnce sync.Once +) + +func defaultLoggerProvider() *atomic.Value { + v := &atomic.Value{} + v.Store(loggerProviderHolder{provider: &loggerProvider{}}) + return v +} + +type loggerProviderHolder struct { + provider log.LoggerProvider +} + +// GetLoggerProvider returns the global LoggerProvider. +func GetLoggerProvider() log.LoggerProvider { + return globalLoggerProvider.Load().(loggerProviderHolder).provider +} + +// SetLoggerProvider sets the global LoggerProvider. +func SetLoggerProvider(provider log.LoggerProvider) { + current := GetLoggerProvider() + if _, cOk := current.(*loggerProvider); cOk { + if _, mpOk := provider.(*loggerProvider); mpOk && current == provider { + err := errors.New("invalid delegation: LoggerProvider self-delegation") + global.Error(err, "No delegate will be configured") + return + } + } + + delegateLoggerOnce.Do(func() { + if def, ok := current.(*loggerProvider); ok { + def.setDelegate(provider) + } + }) + globalLoggerProvider.Store(loggerProviderHolder{provider: provider}) +} diff --git a/log/internal/global/state_test.go b/log/internal/global/state_test.go new file mode 100644 index 000000000..971c958d8 --- /dev/null +++ b/log/internal/global/state_test.go @@ -0,0 +1,75 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package global + +import ( + "sync" + "testing" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/testr" + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/internal/global" + "go.opentelemetry.io/otel/log" + "go.opentelemetry.io/otel/log/noop" +) + +func TestSetLoggerProvider(t *testing.T) { + reset := func() { + globalLoggerProvider = defaultLoggerProvider() + delegateLoggerOnce = sync.Once{} + } + + t.Run("Set With default is a noop", func(t *testing.T) { + t.Cleanup(reset) + + t.Cleanup(func(orig logr.Logger) func() { + global.SetLogger(testr.New(t)) // Don't pollute output. + return func() { global.SetLogger(orig) } + }(global.GetLogger())) + SetLoggerProvider(GetLoggerProvider()) + + provider, ok := GetLoggerProvider().(*loggerProvider) + if !ok { + t.Fatal("Global GetLoggerProvider should be the default logger provider") + } + if provider.delegate != nil { + t.Fatal("logger provider should not delegate when setting itself") + } + }) + + t.Run("First Set() should replace the delegate", func(t *testing.T) { + t.Cleanup(reset) + + SetLoggerProvider(noop.NewLoggerProvider()) + if _, ok := GetLoggerProvider().(*loggerProvider); ok { + t.Fatal("Global GetLoggerProvider was not changed") + } + }) + + t.Run("Set() should delegate existing Logger Providers", func(t *testing.T) { + t.Cleanup(reset) + + provider := GetLoggerProvider() + SetLoggerProvider(noop.NewLoggerProvider()) + + if del := provider.(*loggerProvider); del.delegate == nil { + t.Fatal("The delegated logger providers should have a delegate") + } + }) + + t.Run("non-comparable types should not panic", func(t *testing.T) { + t.Cleanup(reset) + + type nonComparableLoggerProvider struct { + log.LoggerProvider + noCmp [0]func() //nolint:structcheck,unused // This is indeed used. + } + + provider := nonComparableLoggerProvider{} + SetLoggerProvider(provider) + assert.NotPanics(t, func() { SetLoggerProvider(provider) }) + }) +}