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`.
open-telemetry · May 19, 2024 · 7352c6e · 7352c6e
1 parent a481b61
commit 7352c6e
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 77 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,28 +37,18 @@ func TestNewConfig(t *testing.T) {
 			name: "with no options",
 
 			wantConfig: config{
-				scope: instrumentation.Scope{
-					Name:    bridgeName,
-					Version: version,
-				},
 				provider: global.GetLoggerProvider(),
 				levels:   logrus.AllLevels,
 			},
 		},
 		{
 			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