otelslog: Replace WithInstrumentationScope with options and and argum…

…ent (#5588)

Part of #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`.

CHANGELOG.md

@@ -19,12 +19,25 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add a repository Code Ownership Policy. (#5555)
 - The `go.opentelemetry.io/contrib/bridges/otellogrus` module.
   This module provides an OpenTelemetry logging bridge for `github.com/sirupsen/logrus`. (#5355)
+- The `WithVersion` option function in `go.opentelemetry.io/contrib/bridges/otelslog`.
+  This option function is used as a replacement of `WithInstrumentationScope` to specify the logged package version. (#5588)
+- The `WithSchemaURL` option function in `go.opentelemetry.io/contrib/bridges/otelslog`.
+  This option function is used as a replacement of `WithInstrumentationScope` to specify the semantic convention schema URL for the logged records. (#5588)
 
 ### Changed
 
 - The gRPC trace `Filter` for interceptor is renamed to `InterceptorFilter`. (#5196)
 - The gRPC trace filter functions `Any`, `All`, `None`, `Not`, `MethodName`, `MethodPrefix`, `FullMethodName`, `ServiceName`, `ServicePrefix` and `HealthCheck` for interceptor are moved to `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor`.
   With this change, the filters in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` are now working for stats handler. (#5196)
+- `NewLogger` now accepts a `name` `string` as the first argument.
+  This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the underlying `Handler`. (#5588)
+- `NewHandler` now accepts a `name` `string` as the first argument.
+  This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the returned `Handler`. (#5588)
+
+### Removed
+
+- The `WithInstrumentationScope` option function in `go.opentelemetry.io/contrib/bridges/otelslog` is removed.
+  Use the `name` parameter added to `NewHandler` and `NewLogger` as well as `WithVersion` and `WithSchema` as replacements. (#5588)
 
 ### Deprecated
 

bridges/otelslog/example_test.go

@@ -13,5 +13,5 @@ func Example() {
 	provider := noop.NewLoggerProvider()
 
 	// Create an *slog.Logger and use it in your application.
-	otelslog.NewLogger(otelslog.WithLoggerProvider(provider))
+	otelslog.NewLogger("my/pkg/name", otelslog.WithLoggerProvider(provider))
 }

bridges/otelslog/go.mod

@@ -5,7 +5,6 @@ go 1.21
 require (
 	github.com/stretchr/testify v1.9.0
 	go.opentelemetry.io/otel/log v0.2.0-alpha
-	go.opentelemetry.io/otel/sdk v1.26.0
 )
 
 require (

bridges/otelslog/go.sum

@@ -17,8 +17,6 @@ go.opentelemetry.io/otel/log v0.2.0-alpha h1:ixOPvMzserpqA07SENHvRzkZOsnG0XbPr74
 go.opentelemetry.io/otel/log v0.2.0-alpha/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=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=

bridges/otelslog/handler.go

@@ -53,20 +53,18 @@ 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/otelslog"
-
 // NewLogger returns a new [slog.Logger] backed by a new [Handler]. See
 // [NewHandler] for details on how the backing Handler is created.
-func NewLogger(options ...Option) *slog.Logger {
-	return slog.New(NewHandler(options...))
+func NewLogger(name string, options ...Option) *slog.Logger {
+	return slog.New(NewHandler(name, options...))
 }
 
 type config struct {
-	provider log.LoggerProvider
-	scope    instrumentation.Scope
+	provider  log.LoggerProvider
+	version   string
+	schemaURL string
 }
 
 func newConfig(options []Option) config {
@@ -75,30 +73,22 @@ 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()
 	}
 
 	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 [Handler].
@@ -110,16 +100,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 [log.Logger] used by a [Handler].
-//
-// By default if this Option is not provided, the Handler 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 [Handler]. 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 [Handler]. 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
 	})
 }
@@ -155,13 +151,12 @@ var _ slog.Handler = (*Handler)(nil)
 // If [WithLoggerProvider] is not provided, the returned Handler will use the
 // global LoggerProvider.
 //
-// By default the returned Handler will use a [log.Logger] that is identified
-// with this bridge package information. [WithInstrumentationScope] should be
-// used to override this with details about the package or module the handler
-// will instrument.
-func NewHandler(options ...Option) *Handler {
+// The provided name needs to uniquely identify the code being logged. This is
+// most commonly the package name of the code. If name is empty, the
+// [log.Logger] implementation may override this value with a default.
+func NewHandler(name string, options ...Option) *Handler {
 	cfg := newConfig(options)
-	return &Handler{logger: cfg.logger()}
+	return &Handler{logger: cfg.logger(name)}
 }
 
 // Handle handles the passed record.

bridges/otelslog/handler_test.go

@@ -23,19 +23,22 @@ import (
 	"go.opentelemetry.io/otel/log"
 	"go.opentelemetry.io/otel/log/embedded"
 	"go.opentelemetry.io/otel/log/global"
-	"go.opentelemetry.io/otel/sdk/instrumentation"
 )
 
 var now = time.Now()
 
 func TestNewLogger(t *testing.T) {
-	assert.IsType(t, &Handler{}, NewLogger().Handler())
+	assert.IsType(t, &Handler{}, NewLogger("").Handler())
 }
 
 // embeddedLogger is a type alias so the embedded.Logger type doesn't conflict
 // with the Logger method of the recorder when it is embedded.
 type embeddedLogger = embedded.Logger // nolint:unused  // Used below.
 
+type scope struct {
+	Name, Version, SchemaURL string
+}
+
 // recorder records all [log.Record]s it is ased to emit.
 type recorder struct {
 	embedded.LoggerProvider
@@ -45,7 +48,7 @@ type recorder struct {
 	Records []log.Record
 
 	// Scope is the Logger scope recorder received when Logger was called.
-	Scope instrumentation.Scope
+	Scope scope
 
 	// MinSeverity is the minimum severity the recorder will return true for
 	// when Enabled is called (unless enableKey is set).
@@ -55,7 +58,7 @@ type recorder struct {
 func (r *recorder) Logger(name string, opts ...log.LoggerOption) log.Logger {
 	cfg := log.NewLoggerConfig(opts...)
 
-	r.Scope = instrumentation.Scope{
+	r.Scope = scope{
 		Name:      name,
 		Version:   cfg.InstrumentationVersion(),
 		SchemaURL: cfg.SchemaURL(),
@@ -389,7 +392,7 @@ func TestSLogHandler(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			r := new(recorder)
-			var h slog.Handler = NewHandler(WithLoggerProvider(r))
+			var h slog.Handler = NewHandler("", WithLoggerProvider(r))
 			if c.mod != nil {
 				h = &wrapper{h, c.mod}
 			}
@@ -411,7 +414,7 @@ func TestSLogHandler(t *testing.T) {
 
 	t.Run("slogtest.TestHandler", func(t *testing.T) {
 		r := new(recorder)
-		h := NewHandler(WithLoggerProvider(r))
+		h := NewHandler("", WithLoggerProvider(r))
 
 		// TODO: use slogtest.Run when Go 1.21 is no longer supported.
 		err := slogtest.TestHandler(h, r.Results)
@@ -422,36 +425,39 @@ func TestSLogHandler(t *testing.T) {
 }
 
 func TestNewHandlerConfiguration(t *testing.T) {
+	name := "name"
 	t.Run("Default", func(t *testing.T) {
 		r := new(recorder)
 		prev := global.GetLoggerProvider()
 		defer global.SetLoggerProvider(prev)
 		global.SetLoggerProvider(r)
 
 		var h *Handler
-		require.NotPanics(t, func() { h = NewHandler() })
+		require.NotPanics(t, func() { h = NewHandler(name) })
 		require.NotNil(t, h.logger)
 		require.IsType(t, &recorder{}, h.logger)
 
 		l := h.logger.(*recorder)
-		want := instrumentation.Scope{Name: bridgeName, Version: version}
+		want := scope{Name: name}
 		assert.Equal(t, want, l.Scope)
 	})
 
 	t.Run("Options", func(t *testing.T) {
 		r := new(recorder)
-		scope := instrumentation.Scope{Name: "name", Version: "ver", SchemaURL: "url"}
 		var h *Handler
 		require.NotPanics(t, func() {
 			h = NewHandler(
+				name,
 				WithLoggerProvider(r),
-				WithInstrumentationScope(scope),
+				WithVersion("ver"),
+				WithSchemaURL("url"),
 			)
 		})
 		require.NotNil(t, h.logger)
 		require.IsType(t, &recorder{}, h.logger)
 
 		l := h.logger.(*recorder)
+		scope := scope{Name: "name", Version: "ver", SchemaURL: "url"}
 		assert.Equal(t, scope, l.Scope)
 	})
 }
@@ -460,7 +466,7 @@ func TestHandlerEnabled(t *testing.T) {
 	r := new(recorder)
 	r.MinSeverity = log.SeverityInfo
 
-	h := NewHandler(WithLoggerProvider(r))
+	h := NewHandler("name", WithLoggerProvider(r))
 
 	ctx := context.Background()
 	assert.False(t, h.Enabled(ctx, slog.LevelDebug), "level conversion: permissive")
@@ -495,7 +501,7 @@ func BenchmarkHandler(b *testing.B) {
 	b.Run("Handle", func(b *testing.B) {
 		handlers := make([]*Handler, b.N)
 		for i := range handlers {
-			handlers[i] = NewHandler()
+			handlers[i] = NewHandler("")
 		}
 
 		b.ReportAllocs()
@@ -509,7 +515,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("5", func(b *testing.B) {
 			handlers := make([]*Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler()
+				handlers[i] = NewHandler("")
 			}
 
 			b.ReportAllocs()
@@ -521,7 +527,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("10", func(b *testing.B) {
 			handlers := make([]*Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler()
+				handlers[i] = NewHandler("")
 			}
 
 			b.ReportAllocs()
@@ -535,7 +541,7 @@ func BenchmarkHandler(b *testing.B) {
 	b.Run("WithGroup", func(b *testing.B) {
 		handlers := make([]*Handler, b.N)
 		for i := range handlers {
-			handlers[i] = NewHandler()
+			handlers[i] = NewHandler("")
 		}
 
 		b.ReportAllocs()
@@ -549,7 +555,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("5", func(b *testing.B) {
 			handlers := make([]*Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler()
+				handlers[i] = NewHandler("")
 			}
 
 			b.ReportAllocs()
@@ -561,7 +567,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("10", func(b *testing.B) {
 			handlers := make([]*Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler()
+				handlers[i] = NewHandler("")
 			}
 
 			b.ReportAllocs()
@@ -576,7 +582,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("5", func(b *testing.B) {
 			handlers := make([]slog.Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler().WithGroup("group").WithAttrs(attrs5)
+				handlers[i] = NewHandler("").WithGroup("group").WithAttrs(attrs5)
 			}
 
 			b.ReportAllocs()
@@ -588,7 +594,7 @@ func BenchmarkHandler(b *testing.B) {
 		b.Run("10", func(b *testing.B) {
 			handlers := make([]slog.Handler, b.N)
 			for i := range handlers {
-				handlers[i] = NewHandler().WithGroup("group").WithAttrs(attrs10)
+				handlers[i] = NewHandler("").WithGroup("group").WithAttrs(attrs10)
 			}
 
 			b.ReportAllocs()