From 7352c6e088911c3a6d726f1d9752156724a4cde0 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sat, 18 May 2024 22:15:41 -0700 Subject: [PATCH] Replace WithInstrumentationScope with options and and argument (#5606) Resolves #5586 Removes the import of `go.opentelemetry.io/otel/sdk` by replacing `WithInstrumentationScope` with a `name` parameters for both `NewHandler` and `NewLogger` and the added options `WithVersion` and `WithSchemaURL`. --- bridges/otellogrus/example_test.go | 2 +- bridges/otellogrus/go.mod | 1 - bridges/otellogrus/go.sum | 2 -- bridges/otellogrus/hook.go | 56 ++++++++++++++---------------- bridges/otellogrus/hook_test.go | 52 +++++++++------------------ bridges/otellogrus/version.go | 7 ---- 6 files changed, 43 insertions(+), 77 deletions(-) delete mode 100644 bridges/otellogrus/version.go diff --git a/bridges/otellogrus/example_test.go b/bridges/otellogrus/example_test.go index 4b5806a6d50..5c8cd16b10c 100644 --- a/bridges/otellogrus/example_test.go +++ b/bridges/otellogrus/example_test.go @@ -15,7 +15,7 @@ func Example() { provider := noop.NewLoggerProvider() // Create an *otellogrus.Hook and use it in your application. - hook := otellogrus.NewHook(otellogrus.WithLoggerProvider(provider)) + hook := otellogrus.NewHook("my/pkg/name", otellogrus.WithLoggerProvider(provider)) // Set the newly created hook as a global logrus hook logrus.AddHook(hook) diff --git a/bridges/otellogrus/go.mod b/bridges/otellogrus/go.mod index e187a4a0b05..73a46bb1f0a 100644 --- a/bridges/otellogrus/go.mod +++ b/bridges/otellogrus/go.mod @@ -6,7 +6,6 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/otel/log v0.2.0-alpha.0.20240517142840-ebd0adee357f - go.opentelemetry.io/otel/sdk v1.26.0 ) require ( diff --git a/bridges/otellogrus/go.sum b/bridges/otellogrus/go.sum index ac5af72922d..99b90aca013 100644 --- a/bridges/otellogrus/go.sum +++ b/bridges/otellogrus/go.sum @@ -22,8 +22,6 @@ go.opentelemetry.io/otel/log v0.2.0-alpha.0.20240517142840-ebd0adee357f h1:JmJ7s go.opentelemetry.io/otel/log v0.2.0-alpha.0.20240517142840-ebd0adee357f/go.mod h1:vbFZc65yq4c4ssvXY43y/nIqkNJLxORrqw0L85P59LA= go.opentelemetry.io/otel/metric v1.26.0 h1:7S39CLuY5Jgg9CrnA9HHiEjGMF/X2VHvoXGgSllRz30= go.opentelemetry.io/otel/metric v1.26.0/go.mod h1:SY+rHOI4cEawI9a7N1A4nIg/nTQXe1ccCNWYOJUrpX4= -go.opentelemetry.io/otel/sdk v1.26.0 h1:Y7bumHf5tAiDlRYFmGqetNcLaVUZmh4iYfmGxtmz7F8= -go.opentelemetry.io/otel/sdk v1.26.0/go.mod h1:0p8MXpqLeJ0pzcszQQN4F0S5FVjBLgypeGSngLsmirs= go.opentelemetry.io/otel/trace v1.26.0 h1:1ieeAUb4y0TE26jUFrCIXKpTuVK7uJGN9/Z/2LP5sQA= go.opentelemetry.io/otel/trace v1.26.0/go.mod h1:4iDxvGDQuUkHve82hJJ8UqrwswHYsZuWCBllGV2U2y0= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/bridges/otellogrus/hook.go b/bridges/otellogrus/hook.go index e3ac7585768..19e477a6647 100644 --- a/bridges/otellogrus/hook.go +++ b/bridges/otellogrus/hook.go @@ -38,14 +38,12 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/global" - "go.opentelemetry.io/otel/sdk/instrumentation" ) -const bridgeName = "go.opentelemetry.io/contrib/bridges/otellogrus" - type config struct { - provider log.LoggerProvider - scope instrumentation.Scope + provider log.LoggerProvider + version string + schemaURL string levels []logrus.Level } @@ -56,14 +54,6 @@ func newConfig(options []Option) config { c = opt.apply(c) } - var emptyScope instrumentation.Scope - if c.scope == emptyScope { - c.scope = instrumentation.Scope{ - Name: bridgeName, - Version: version, - } - } - if c.provider == nil { c.provider = global.GetLoggerProvider() } @@ -75,15 +65,15 @@ func newConfig(options []Option) config { return c } -func (c config) logger() log.Logger { +func (c config) logger(name string) log.Logger { var opts []log.LoggerOption - if c.scope.Version != "" { - opts = append(opts, log.WithInstrumentationVersion(c.scope.Version)) + if c.version != "" { + opts = append(opts, log.WithInstrumentationVersion(c.version)) } - if c.scope.SchemaURL != "" { - opts = append(opts, log.WithSchemaURL(c.scope.SchemaURL)) + if c.schemaURL != "" { + opts = append(opts, log.WithSchemaURL(c.schemaURL)) } - return c.provider.Logger(c.scope.Name, opts...) + return c.provider.Logger(name, opts...) } // Option configures a [Hook]. @@ -95,16 +85,22 @@ type optFunc func(config) config func (f optFunc) apply(c config) config { return f(c) } -// WithInstrumentationScope returns an [Option] that configures the scope of -// the logs that will be emitted by the configured [Hook]. -// -// By default if this Option is not provided, the Hook will use a default -// instrumentation scope describing this bridge package. It is recommended to -// provide this so log data can be associated with its source package or -// module. -func WithInstrumentationScope(scope instrumentation.Scope) Option { +// WithVersion returns an [Option] that configures the version of the +// [log.Logger] used by a [Hook]. The version should be the version of the +// package that is being logged. +func WithVersion(version string) Option { + return optFunc(func(c config) config { + c.version = version + return c + }) +} + +// WithSchemaURL returns an [Option] that configures the semantic convention +// schema URL of the [log.Logger] used by a [Hook]. The schemaURL should be +// the schema URL for the semantic conventions used in log records. +func WithSchemaURL(schemaURL string) Option { return optFunc(func(c config) config { - c.scope = scope + c.schemaURL = schemaURL return c }) } @@ -137,10 +133,10 @@ func WithLevels(l []logrus.Level) Option { // // If [WithLoggerProvider] is not provided, the returned Hook will use the // global LoggerProvider. -func NewHook(options ...Option) *Hook { +func NewHook(name string, options ...Option) *Hook { cfg := newConfig(options) return &Hook{ - logger: cfg.logger(), + logger: cfg.logger(name), levels: cfg.levels, } } diff --git a/bridges/otellogrus/hook_test.go b/bridges/otellogrus/hook_test.go index cc2142469b8..8a261f70ce6 100644 --- a/bridges/otellogrus/hook_test.go +++ b/bridges/otellogrus/hook_test.go @@ -14,7 +14,6 @@ import ( "go.opentelemetry.io/otel/log/embedded" "go.opentelemetry.io/otel/log/global" "go.opentelemetry.io/otel/log/logtest" - "go.opentelemetry.io/otel/sdk/instrumentation" ) type mockLoggerProvider struct { @@ -38,10 +37,6 @@ func TestNewConfig(t *testing.T) { name: "with no options", wantConfig: config{ - scope: instrumentation.Scope{ - Name: bridgeName, - Version: version, - }, provider: global.GetLoggerProvider(), levels: logrus.AllLevels, }, @@ -49,17 +44,11 @@ func TestNewConfig(t *testing.T) { { name: "with a custom instrumentation scope", options: []Option{ - WithInstrumentationScope(instrumentation.Scope{ - Name: "test", - Version: "42.0", - }), + WithVersion("42.0"), }, wantConfig: config{ - scope: instrumentation.Scope{ - Name: "test", - Version: "42.0", - }, + version: "42.0", provider: global.GetLoggerProvider(), levels: logrus.AllLevels, }, @@ -71,10 +60,6 @@ func TestNewConfig(t *testing.T) { }, wantConfig: config{ - scope: instrumentation.Scope{ - Name: bridgeName, - Version: version, - }, provider: customLoggerProvider, levels: logrus.AllLevels, }, @@ -86,10 +71,6 @@ func TestNewConfig(t *testing.T) { }, wantConfig: config{ - scope: instrumentation.Scope{ - Name: bridgeName, - Version: version, - }, provider: global.GetLoggerProvider(), levels: []logrus.Level{logrus.FatalLevel}, }, @@ -102,6 +83,7 @@ func TestNewConfig(t *testing.T) { } func TestNewHook(t *testing.T) { + const name = "name" provider := global.GetLoggerProvider() for _, tt := range []struct { @@ -113,26 +95,23 @@ func TestNewHook(t *testing.T) { { name: "with the default options", - wantLogger: provider.Logger(bridgeName, log.WithInstrumentationVersion(version)), + wantLogger: provider.Logger(name), }, { name: "with a schema URL", options: []Option{ - WithInstrumentationScope(instrumentation.Scope{ - Name: "test", - Version: "42.1", - SchemaURL: "https://example.com", - }), + WithVersion("42.1"), + WithSchemaURL("https://example.com"), }, - wantLogger: provider.Logger("test", + wantLogger: provider.Logger(name, log.WithInstrumentationVersion("42.1"), log.WithSchemaURL("https://example.com"), ), }, } { t.Run(tt.name, func(t *testing.T) { - hook := NewHook(tt.options...) + hook := NewHook(name, tt.options...) assert.NotNil(t, hook) assert.Equal(t, tt.wantLogger, hook.logger) @@ -160,13 +139,14 @@ func TestHookLevels(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - levels := NewHook(tt.options...).Levels() + levels := NewHook("", tt.options...).Levels() assert.Equal(t, tt.wantLevels, levels) }) } } func TestHookFire(t *testing.T) { + const name = "name" now := time.Now() for _, tt := range []struct { @@ -181,7 +161,7 @@ func TestHookFire(t *testing.T) { entry: &logrus.Entry{}, wantRecords: map[string][]log.Record{ - bridgeName: { + name: { buildRecord(log.StringValue(""), time.Time{}, 0, nil), }, }, @@ -192,7 +172,7 @@ func TestHookFire(t *testing.T) { Time: now, }, wantRecords: map[string][]log.Record{ - bridgeName: { + name: { buildRecord(log.StringValue(""), now, 0, nil), }, }, @@ -203,7 +183,7 @@ func TestHookFire(t *testing.T) { Level: logrus.FatalLevel, }, wantRecords: map[string][]log.Record{ - bridgeName: { + name: { buildRecord(log.StringValue(""), time.Time{}, log.SeverityTrace1, nil), }, }, @@ -216,7 +196,7 @@ func TestHookFire(t *testing.T) { }, }, wantRecords: map[string][]log.Record{ - bridgeName: { + name: { buildRecord(log.StringValue(""), time.Time{}, 0, []log.KeyValue{ log.String("hello", "world"), }), @@ -227,7 +207,7 @@ func TestHookFire(t *testing.T) { t.Run(tt.name, func(t *testing.T) { rec := logtest.NewRecorder() - err := NewHook(WithLoggerProvider(rec)).Fire(tt.entry) + err := NewHook(name, WithLoggerProvider(rec)).Fire(tt.entry) assert.Equal(t, tt.wantErr, err) for k, v := range tt.wantRecords { @@ -387,7 +367,7 @@ func BenchmarkHook(b *testing.B) { b.Run("Fire", func(b *testing.B) { hooks := make([]*Hook, b.N) for i := range hooks { - hooks[i] = NewHook() + hooks[i] = NewHook("") } b.ReportAllocs() diff --git a/bridges/otellogrus/version.go b/bridges/otellogrus/version.go deleted file mode 100644 index 3e82fb7278b..00000000000 --- a/bridges/otellogrus/version.go +++ /dev/null @@ -1,7 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package otellogrus // import "go.opentelemetry.io/contrib/bridges/otellogrus" - -// version is the current release version of otellogrus in use. -const version = "0.0.1-alpha"