Skip to content

Commit

Permalink
remove scope and implement changes
Browse files Browse the repository at this point in the history
  • Loading branch information
scorpionknifes committed Jul 19, 2024
1 parent 755982b commit f5b5d3d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 92 deletions.
1 change: 1 addition & 0 deletions bridges/otellogr/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func Example() {

// Create an logr.Logger with *otellogr.LogSink and use it in your application.
logr.New(otellogr.NewLogSink(
"my/pkg/name",
otellogr.WithLoggerProvider(provider),
// Optionally, set the log level severity mapping.
otellogr.WithLevelSeverity(func(i int) log.Severity {
Expand Down
9 changes: 4 additions & 5 deletions bridges/otellogr/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ go 1.21
require (
github.com/go-logr/logr v1.4.2
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/otel/log v0.3.0
go.opentelemetry.io/otel/sdk v1.27.0
go.opentelemetry.io/otel/log v0.4.0
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opentelemetry.io/otel v1.27.0 // indirect
go.opentelemetry.io/otel/metric v1.27.0 // indirect
go.opentelemetry.io/otel/trace v1.27.0 // indirect
go.opentelemetry.io/otel v1.28.0 // indirect
go.opentelemetry.io/otel/metric v1.28.0 // indirect
go.opentelemetry.io/otel/trace v1.28.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
18 changes: 8 additions & 10 deletions bridges/otellogr/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
go.opentelemetry.io/otel v1.27.0 h1:9BZoF3yMK/O1AafMiQTVu0YDj5Ea4hPhxCs7sGva+cg=
go.opentelemetry.io/otel v1.27.0/go.mod h1:DMpAK8fzYRzs+bi3rS5REupisuqTheUlSZJ1WnZaPAQ=
go.opentelemetry.io/otel/log v0.3.0 h1:kJRFkpUFYtny37NQzL386WbznUByZx186DpEMKhEGZs=
go.opentelemetry.io/otel/log v0.3.0/go.mod h1:ziCwqZr9soYDwGNbIL+6kAvQC+ANvjgG367HVcyR/ys=
go.opentelemetry.io/otel/metric v1.27.0 h1:hvj3vdEKyeCi4YaYfNjv2NUje8FqKqUY8IlF0FxV/ik=
go.opentelemetry.io/otel/metric v1.27.0/go.mod h1:mVFgmRlhljgBiuk/MP/oKylr4hs85GZAylncepAX/ak=
go.opentelemetry.io/otel/sdk v1.27.0 h1:mlk+/Y1gLPLn84U4tI8d3GNJmGT/eXe3ZuOXN9kTWmI=
go.opentelemetry.io/otel/sdk v1.27.0/go.mod h1:Ha9vbLwJE6W86YstIywK2xFfPjbWlCuwPtMkKdz/Y4A=
go.opentelemetry.io/otel/trace v1.27.0 h1:IqYb813p7cmbHk0a5y6pD5JPakbVfftRXABGt5/Rscw=
go.opentelemetry.io/otel/trace v1.27.0/go.mod h1:6RiD1hkAprV4/q+yd2ln1HG9GoPx39SuvvstaLBl+l4=
go.opentelemetry.io/otel v1.28.0 h1:/SqNcYk+idO0CxKEUOtKQClMK/MimZihKYMruSMViUo=
go.opentelemetry.io/otel v1.28.0/go.mod h1:q68ijF8Fc8CnMHKyzqL6akLO46ePnjkgfIMIjUIX9z4=
go.opentelemetry.io/otel/log v0.4.0 h1:/vZ+3Utqh18e8TPjuc3ecg284078KWrR8BRz+PQAj3o=
go.opentelemetry.io/otel/log v0.4.0/go.mod h1:DhGnQvky7pHy82MIRV43iXh3FlKN8UUKftn0KbLOq6I=
go.opentelemetry.io/otel/metric v1.28.0 h1:f0HGvSl1KRAU1DLgLGFjrwVyismPlnuU6JD6bOeuA5Q=
go.opentelemetry.io/otel/metric v1.28.0/go.mod h1:Fb1eVBFZmLVTMb6PPohq3TO9IIhUisDsbJoL/+uQW4s=
go.opentelemetry.io/otel/trace v1.28.0 h1:GhQ9cUuQGmNDd5BTCP2dAvv75RdMxEfTmYejp+lkx9g=
go.opentelemetry.io/otel/trace v1.28.0/go.mod h1:jPyXzNPg6da9+38HEwElrQiHlVMTnVfM3/yv2OlIHaI=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
Expand Down
72 changes: 31 additions & 41 deletions bridges/otellogr/logsink.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,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/otellogr"
// errorKey is used to log the error parameter of Error as an additional attribute.
errorKey = "error"
)

type config struct {
provider log.LoggerProvider
scope instrumentation.Scope
provider log.LoggerProvider
version string
schemaURL string

levelSeverity func(int) log.Severity
}

Expand All @@ -93,14 +93,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()
}
Expand All @@ -115,15 +107,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 [LogSink].
Expand All @@ -135,16 +127,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 [LogSink].
//
// By default if this Option is not provided, the LogSink 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.scope = scope
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.schemaURL = schemaURL
return c
})
}
Expand Down Expand Up @@ -179,11 +177,11 @@ func WithLevelSeverity(f func(int) log.Severity) Option {
//
// If [WithLoggerProvider] is not provided, the returned LogSink will use the
// global LoggerProvider.
func NewLogSink(options ...Option) *LogSink {
func NewLogSink(name string, options ...Option) *LogSink {
c := newConfig(options)
return &LogSink{
config: c,
logger: c.logger(),
newLogger: c.logger,
logger: c.logger(name),
levelSeverity: c.levelSeverity,
}
}
Expand All @@ -194,7 +192,7 @@ type LogSink struct {
// Ensure forward compatibility by explicitly making this not comparable.
noCmp [0]func() //nolint: unused // This is indeed used.

config config
newLogger func(name string) log.Logger
logger log.Logger
levelSeverity func(int) log.Severity
values []log.KeyValue
Expand All @@ -206,7 +204,6 @@ var _ logr.LogSink = (*LogSink)(nil)
// log sends a log record to the OpenTelemetry logger.
func (l *LogSink) log(err error, msg string, serverity log.Severity, kvList ...any) {
var record log.Record
record.SetTimestamp(time.Now())
record.SetBody(log.StringValue(msg))
record.SetSeverity(serverity)

Expand Down Expand Up @@ -259,15 +256,8 @@ func (l *LogSink) Init(info logr.RuntimeInfo) {

// WithName returns a new LogSink with the specified name appended.
func (l LogSink) WithName(name string) logr.LogSink {
newConfig := l.config
newConfig.scope.Name = fmt.Sprintf("%s/%s", l.config.scope.Name, name)

return &LogSink{
config: newConfig,
logger: newConfig.logger(),
levelSeverity: newConfig.levelSeverity,
values: l.values,
}
l.logger = l.newLogger(name)
return &l
}

// WithValues returns a new LogSink with additional key/value pairs.
Expand Down
49 changes: 20 additions & 29 deletions bridges/otellogr/logsink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log/global"
"go.opentelemetry.io/otel/log/logtest"
"go.opentelemetry.io/otel/sdk/instrumentation"
)

type expectedRecord struct {
Expand All @@ -36,24 +35,20 @@ func TestNewLogSinkConfiguration(t *testing.T) {
global.SetLoggerProvider(rec)

var ls *LogSink
assert.NotPanics(t, func() { ls = NewLogSink() })
assert.NotPanics(t, func() { ls = NewLogSink("name") })
assert.NotNil(t, ls)
want := &logtest.ScopeRecords{
Name: bridgeName,
Version: version,
}
got := rec.Result()[0]
assert.Equal(t, want, got)
assert.Equal(t, ls.newLogger("name"), ls.logger)
})

t.Run("with_options", func(t *testing.T) {
rec := logtest.NewRecorder()
scope := instrumentation.Scope{Name: "name", Version: "ver", SchemaURL: "url"}
var ls *LogSink
assert.NotPanics(t, func() {
ls = NewLogSink(
"name",
WithVersion("42.0"),
WithSchemaURL("https://example.com"),
WithLoggerProvider(rec),
WithInstrumentationScope(scope),
WithLevelSeverity(func(i int) log.Severity {
return log.SeverityFatal
}),
Expand All @@ -62,30 +57,25 @@ func TestNewLogSinkConfiguration(t *testing.T) {
assert.NotNil(t, ls)
assert.NotNil(t, ls.levelSeverity)
assert.Equal(t, log.SeverityFatal, ls.levelSeverity(0))

want := &logtest.ScopeRecords{
Name: "name",
Version: "ver",
SchemaURL: "url",
}
got := rec.Result()[0]
assert.Equal(t, want, got)
assert.Equal(t, ls.newLogger("name"), ls.logger)
})

t.Run("with_name", func(t *testing.T) {
rec := logtest.NewRecorder()
var ls *LogSink
var lsWithName logr.LogSink
assert.NotPanics(t, func() {
NewLogSink(
ls = NewLogSink(
"name",
WithLoggerProvider(rec),
).WithName("test")
)
lsWithName = ls.WithName("test")
})

want := &logtest.ScopeRecords{
Name: bridgeName + "/test",
Version: version,
}
got := rec.Result()[1]
assert.Equal(t, want, got)
assert.NotNil(t, ls)
assert.NotNil(t, lsWithName)
assert.NotEqual(t, ls, lsWithName)
assert.Equal(t, ls.newLogger("name"), ls.logger)
assert.Equal(t, ls.newLogger("test"), lsWithName.(*LogSink).logger)
})
}

Expand Down Expand Up @@ -244,7 +234,7 @@ func TestLogSink(t *testing.T) {
} {
t.Run(tt.name, func(t *testing.T) {
rec := logtest.NewRecorder()
ls := NewLogSink(WithLoggerProvider(rec))
ls := NewLogSink("name", WithLoggerProvider(rec))
l := logr.New(ls)
tt.f(&l)

Expand All @@ -268,7 +258,7 @@ func TestLogSink(t *testing.T) {

func TestLogSinkWithName(t *testing.T) {
rec := logtest.NewRecorder()
ls := NewLogSink(WithLoggerProvider(rec))
ls := NewLogSink("name", WithLoggerProvider(rec))
lsWithName := ls.WithName("test")
require.NotEqual(t, ls, lsWithName)

Expand All @@ -282,6 +272,7 @@ func TestLogSinkEnabled(t *testing.T) {
}),
)
ls := NewLogSink(
"name",
WithLoggerProvider(rec),
WithLevelSeverity(func(i int) log.Severity {
switch i {
Expand Down
7 changes: 0 additions & 7 deletions bridges/otellogr/version.go

This file was deleted.

0 comments on commit f5b5d3d

Please sign in to comment.