From e9ba47828c221d0260388d16428381c989976ae6 Mon Sep 17 00:00:00 2001 From: Cheng-Zhen Yang Date: Sun, 6 Oct 2024 01:40:52 +1300 Subject: [PATCH 1/7] otellogr: Implement LevelSeverity --- bridges/otellogr/example_test.go | 16 +++++++- bridges/otellogr/logsink.go | 68 +++++++++++++++++++++++++------- bridges/otellogr/logsink_test.go | 54 ++++++++++++++++++++++--- 3 files changed, 117 insertions(+), 21 deletions(-) diff --git a/bridges/otellogr/example_test.go b/bridges/otellogr/example_test.go index 4b544a0b2b5..28486e3c186 100644 --- a/bridges/otellogr/example_test.go +++ b/bridges/otellogr/example_test.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" "go.opentelemetry.io/contrib/bridges/otellogr" + "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/noop" ) @@ -17,6 +18,17 @@ 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)), - ) + otellogr.WithLoggerProvider(provider), + // Optionally, set the log level severity mapping. + otellogr.WithLevelSeverity(func(i int) log.Severity { + switch i { + case 0: + return log.SeverityInfo + case 1: + return log.SeverityWarn + default: + return log.SeverityFatal + } + }), + )) } diff --git a/bridges/otellogr/logsink.go b/bridges/otellogr/logsink.go index 1acd6c55e56..9ca60f36cbb 100644 --- a/bridges/otellogr/logsink.go +++ b/bridges/otellogr/logsink.go @@ -10,13 +10,26 @@ // way: // // - Message is set as the Body using a [log.StringValue]. -// - TODO: Level +// - Level is transformed and set as the Severity. The SeverityText is not +// set. // - KeyAndValues are transformed and set as Attributes. // - The [context.Context] value in KeyAndValues is propagated to OpenTelemetry // log record. All non-nested [context.Context] values are ignored and not // added as attributes. If there are multiple [context.Context] the last one // is used. // +// The Level is transformed by using the [WithLevelSeverity] option. If this is +// not provided it would default to a function that adds an offset to the logr +// such that [logr.Info] is transformed to [log.SeverityInfo]. For example: +// +// - [logr.Info] is transformed to [log.SeverityInfo]. +// - [logr.V(0)] is transformed to [log.SeverityInfo]. +// - [logr.V(1)] is transformed to [log.SeverityInfo2]. +// - [logr.V(2)] is transformed to [log.SeverityInfo3]. +// - ... +// - [logr.V(15)] is transformed to [log.SeverityFatal4]. +// - [logr.Error] is transformed to [log.SeverityError]. +// // KeysAndValues values are transformed based on their type. The following types are // supported: // @@ -56,6 +69,8 @@ type config struct { provider log.LoggerProvider version string schemaURL string + + levelSeverity func(int) log.Severity } func newConfig(options []Option) config { @@ -68,6 +83,13 @@ func newConfig(options []Option) config { c.provider = global.GetLoggerProvider() } + if c.levelSeverity == nil { + c.levelSeverity = func(level int) log.Severity { + const sevOffset = int(log.SeverityInfo) + return log.Severity(level + sevOffset) + } + } + return c } @@ -112,6 +134,20 @@ func WithLoggerProvider(provider log.LoggerProvider) Option { }) } +// WithLevelSeverity returns an [Option] that configures the function used to +// convert logr levels to OpenTelemetry log severities. +// +// By default if this Option is not provided, the LogSink will use a default +// conversion function which adds an offset to the logr level to get the +// OpenTelemetry severity. The offset is such that logr.Info("message") +// converts to OpenTelemetry [log.SeverityInfo]. +func WithLevelSeverity(f func(int) log.Severity) Option { + return optFunc(func(c config) config { + c.levelSeverity = f + return c + }) +} + // NewLogSink returns a new [LogSink] to be used as a [logr.LogSink]. // // If [WithLoggerProvider] is not provided, the returned [LogSink] will use the @@ -128,10 +164,11 @@ func NewLogSink(name string, options ...Option) *LogSink { } return &LogSink{ - name: name, - provider: c.provider, - logger: c.provider.Logger(name, opts...), - opts: opts, + name: name, + provider: c.provider, + logger: c.provider.Logger(name, opts...), + levelSeverity: c.levelSeverity, + opts: opts, } } @@ -141,12 +178,13 @@ type LogSink struct { // Ensure forward compatibility by explicitly making this not comparable. noCmp [0]func() //nolint: unused // This is indeed used. - name string - provider log.LoggerProvider - logger log.Logger - opts []log.LoggerOption - attr []log.KeyValue - ctx context.Context + name string + provider log.LoggerProvider + logger log.Logger + levelSeverity func(int) log.Severity + opts []log.LoggerOption + attr []log.KeyValue + ctx context.Context } // Compile-time check *Handler implements logr.LogSink. @@ -156,8 +194,10 @@ var _ logr.LogSink = (*LogSink)(nil) // For example, commandline flags might be used to set the logging // verbosity and disable some info logs. func (l *LogSink) Enabled(level int) bool { - // TODO - return true + var param log.EnabledParameters + param.SetSeverity(l.levelSeverity(level)) + ctx := context.Background() + return l.logger.Enabled(ctx, param) } // Error logs an error, with the given message and key/value pairs. @@ -169,7 +209,7 @@ func (l *LogSink) Error(err error, msg string, keysAndValues ...any) { func (l *LogSink) Info(level int, msg string, keysAndValues ...any) { var record log.Record record.SetBody(log.StringValue(msg)) - record.SetSeverity(log.SeverityInfo) // TODO: level + record.SetSeverity(l.levelSeverity(level)) record.AddAttributes(l.attr...) diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index 8b47f871313..df6a2c18fc6 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -63,7 +63,9 @@ func TestNewConfig(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.wantConfig, newConfig(tt.options)) + config := newConfig(tt.options) + config.levelSeverity = nil // Ignore asserting level severity function, assert.Equal does not support function comparison + assert.Equal(t, tt.wantConfig, config) }) } } @@ -116,9 +118,10 @@ func TestLogSink(t *testing.T) { const name = "name" for _, tt := range []struct { - name string - f func(*logr.Logger) - wantRecords map[string][]log.Record + name string + f func(*logr.Logger) + levelSeverity func(int) log.Severity + wantRecords map[string][]log.Record }{ { name: "no_log", @@ -138,6 +141,44 @@ func TestLogSink(t *testing.T) { }, }, }, + { + name: "info_with_level_severity", + f: func(l *logr.Logger) { + l.V(1).Info("msg") + l.V(4).Info("msg") + }, + wantRecords: map[string][]log.Record{ + name: { + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityInfo2, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityWarn, nil), + }, + }, + }, + { + name: "info_with_custom_level_severity", + f: func(l *logr.Logger) { + l.Info("msg") + l.V(1).Info("msg") + l.V(2).Info("msg") + }, + levelSeverity: func(level int) log.Severity { + switch level { + case 1: + return log.SeverityError + case 2: + return log.SeverityWarn + default: + return log.SeverityInfo + } + }, + wantRecords: map[string][]log.Record{ + name: { + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityInfo, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityError, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityWarn, nil), + }, + }, + }, { name: "info_multi_attrs", f: func(l *logr.Logger) { @@ -219,7 +260,10 @@ func TestLogSink(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { rec := logtest.NewRecorder() - ls := NewLogSink(name, WithLoggerProvider(rec)) + ls := NewLogSink(name, + WithLoggerProvider(rec), + WithLevelSeverity(tt.levelSeverity), + ) l := logr.New(ls) tt.f(&l) From 2830a0a6aaa06f2eab34cbb5f6befb6f2a53026f Mon Sep 17 00:00:00 2001 From: Cheng-Zhen Yang Date: Sun, 6 Oct 2024 01:46:20 +1300 Subject: [PATCH 2/7] Add TestLogSinkEnabled --- bridges/otellogr/logsink_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index df6a2c18fc6..2b2e71a541c 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -3,6 +3,7 @@ package otellogr import ( + "context" "testing" "time" @@ -288,6 +289,33 @@ func TestLogSink(t *testing.T) { } } +func TestLogSinkEnabled(t *testing.T) { + enabledFunc := func(ctx context.Context, param log.EnabledParameters) bool { + lvl, ok := param.Severity() + if !ok { + return true + } + return lvl == log.SeverityInfo + } + + rec := logtest.NewRecorder(logtest.WithEnabledFunc(enabledFunc)) + ls := NewLogSink( + "name", + WithLoggerProvider(rec), + WithLevelSeverity(func(i int) log.Severity { + switch i { + case 0: + return log.SeverityDebug + default: + return log.SeverityInfo + } + }), + ) + + assert.False(t, ls.Enabled(0)) + assert.True(t, ls.Enabled(1)) +} + func buildRecord(body log.Value, timestamp time.Time, severity log.Severity, attrs []log.KeyValue) log.Record { var record log.Record record.SetBody(body) From 7e0528ac88ea912abc1c4e6210c12a6b2baabeb1 Mon Sep 17 00:00:00 2001 From: Cheng-Zhen Yang Date: Sat, 19 Oct 2024 20:29:53 +1300 Subject: [PATCH 3/7] Inverse log level translation and add documentation --- bridges/otellogr/example_test.go | 4 ++-- bridges/otellogr/logsink.go | 25 ++++++++++++++++++------- bridges/otellogr/logsink_test.go | 10 ++++++++-- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/bridges/otellogr/example_test.go b/bridges/otellogr/example_test.go index 28486e3c186..10cfda271c9 100644 --- a/bridges/otellogr/example_test.go +++ b/bridges/otellogr/example_test.go @@ -25,9 +25,9 @@ func Example() { case 0: return log.SeverityInfo case 1: - return log.SeverityWarn + return log.SeverityDebug default: - return log.SeverityFatal + return log.SeverityTrace } }), )) diff --git a/bridges/otellogr/logsink.go b/bridges/otellogr/logsink.go index 6e1e8ba1522..bac072a2063 100644 --- a/bridges/otellogr/logsink.go +++ b/bridges/otellogr/logsink.go @@ -18,18 +18,29 @@ // added as attributes. If there are multiple [context.Context] the last one // is used. // -// The Level is transformed by using the [WithLevelSeverity] option. If this is -// not provided it would default to a function that adds an offset to the logr -// such that [logr.Info] is transformed to [log.SeverityInfo]. For example: +// The V-level is transformed by using the [WithLevelSeverity] option. If this is +// not provided it would default to a function that inverses the level with an +// offset such that [logr.Info] is transformed to [log.SeverityInfo] as the higher +// the V-level of a log line, the less critical it is considered. By default, +// V-level higher than 8 is not supported and will result in negative severity. +// For example: // // - [logr.Info] is transformed to [log.SeverityInfo]. // - [logr.V(0)] is transformed to [log.SeverityInfo]. -// - [logr.V(1)] is transformed to [log.SeverityInfo2]. -// - [logr.V(2)] is transformed to [log.SeverityInfo3]. +// - [logr.V(1)] is transformed to [log.SeverityDebug4]. +// - [logr.V(2)] is transformed to [log.SeverityDebug3]. // - ... -// - [logr.V(15)] is transformed to [log.SeverityFatal4]. +// - [logr.V(8)] is transformed to [log.SeverityTrace]. +// - [logr.V(9)] is transformed to [log.SeverityUndefined]. +// - [logr.V(10)] is transformed to log.Severity(-1). // - [logr.Error] is transformed to [log.SeverityError]. // +// If possible, look at alternative log bridges that provide less abstract log +// level mapping such as [go.opentelemetry.io/contrib/bridges/otelslog], +// [go.opentelemetry.io/contrib/bridges/otelzap], or +// [go.opentelemetry.io/contrib/bridges/otellogrus]. Which provide more direct +// translation of log levels. +// // KeysAndValues values are transformed based on their type. The following types are // supported: // @@ -87,7 +98,7 @@ func newConfig(options []Option) config { if c.levelSeverity == nil { c.levelSeverity = func(level int) log.Severity { const sevOffset = int(log.SeverityInfo) - return log.Severity(level + sevOffset) + return log.Severity(sevOffset - level) } } diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index 3e4afdfdeb9..30820ea181f 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -147,11 +147,17 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.V(1).Info("msg") l.V(4).Info("msg") + l.V(8).Info("msg") + l.V(9).Info("msg") + l.V(10).Info("msg") }, wantRecords: map[string][]log.Record{ name: { - buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityInfo2, nil), - buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityWarn, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityDebug4, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityDebug, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityTrace, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityUndefined, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.Severity(-1), nil), }, }, }, From be7773d48118387fdcec31a04753450f4fe0e959 Mon Sep 17 00:00:00 2001 From: Cheng-Zhen Yang Date: Sun, 27 Oct 2024 02:00:22 +1300 Subject: [PATCH 4/7] Simpler level transformer --- bridges/otellogr/example_test.go | 4 ++-- bridges/otellogr/logsink.go | 32 ++++++++++++++------------------ bridges/otellogr/logsink_test.go | 12 +++++------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/bridges/otellogr/example_test.go b/bridges/otellogr/example_test.go index 10cfda271c9..e8858a7521f 100644 --- a/bridges/otellogr/example_test.go +++ b/bridges/otellogr/example_test.go @@ -20,8 +20,8 @@ func Example() { "my/pkg/name", otellogr.WithLoggerProvider(provider), // Optionally, set the log level severity mapping. - otellogr.WithLevelSeverity(func(i int) log.Severity { - switch i { + otellogr.WithLevelSeverity(func(level int) log.Severity { + switch level { case 0: return log.SeverityInfo case 1: diff --git a/bridges/otellogr/logsink.go b/bridges/otellogr/logsink.go index bac072a2063..7e51e815a30 100644 --- a/bridges/otellogr/logsink.go +++ b/bridges/otellogr/logsink.go @@ -18,27 +18,17 @@ // added as attributes. If there are multiple [context.Context] the last one // is used. // -// The V-level is transformed by using the [WithLevelSeverity] option. If this is -// not provided it would default to a function that inverses the level with an -// offset such that [logr.Info] is transformed to [log.SeverityInfo] as the higher -// the V-level of a log line, the less critical it is considered. By default, -// V-level higher than 8 is not supported and will result in negative severity. -// For example: +// The V-level is transformed by using the [WithLevelSeverity] option. If option is +// not provided then V-level is transformed in the following way: // -// - [logr.Info] is transformed to [log.SeverityInfo]. -// - [logr.V(0)] is transformed to [log.SeverityInfo]. -// - [logr.V(1)] is transformed to [log.SeverityDebug4]. -// - [logr.V(2)] is transformed to [log.SeverityDebug3]. -// - ... -// - [logr.V(8)] is transformed to [log.SeverityTrace]. -// - [logr.V(9)] is transformed to [log.SeverityUndefined]. -// - [logr.V(10)] is transformed to log.Severity(-1). -// - [logr.Error] is transformed to [log.SeverityError]. +// - logr.Info and logr.V(0) are transformed to [log.SeverityInfo]. +// - logr.V(1) is transformed to [log.SeverityDebug]. +// - logr.V(2) and higher are transformed to [log.SeverityTrace]. // // If possible, look at alternative log bridges that provide less abstract log // level mapping such as [go.opentelemetry.io/contrib/bridges/otelslog], // [go.opentelemetry.io/contrib/bridges/otelzap], or -// [go.opentelemetry.io/contrib/bridges/otellogrus]. Which provide more direct +// [go.opentelemetry.io/contrib/bridges/otellogrus]. They provide more direct // translation of log levels. // // KeysAndValues values are transformed based on their type. The following types are @@ -97,8 +87,14 @@ func newConfig(options []Option) config { if c.levelSeverity == nil { c.levelSeverity = func(level int) log.Severity { - const sevOffset = int(log.SeverityInfo) - return log.Severity(sevOffset - level) + switch level { + case 0: + return log.SeverityInfo + case 1: + return log.SeverityDebug + default: + return log.SeverityTrace + } } } diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index 30820ea181f..9e310e443e0 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -145,19 +145,17 @@ func TestLogSink(t *testing.T) { { name: "info_with_level_severity", f: func(l *logr.Logger) { + l.V(0).Info("msg") l.V(1).Info("msg") - l.V(4).Info("msg") - l.V(8).Info("msg") - l.V(9).Info("msg") - l.V(10).Info("msg") + l.V(2).Info("msg") + l.V(3).Info("msg") }, wantRecords: map[string][]log.Record{ name: { - buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityDebug4, nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityInfo, nil), buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityDebug, nil), buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityTrace, nil), - buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityUndefined, nil), - buildRecord(log.StringValue("msg"), time.Time{}, log.Severity(-1), nil), + buildRecord(log.StringValue("msg"), time.Time{}, log.SeverityTrace, nil), }, }, }, From 0e56af11bc12fafe3b5bb8cdec80de6d364615b5 Mon Sep 17 00:00:00 2001 From: Cheng-Zhen Yang Date: Sun, 27 Oct 2024 02:02:50 +1300 Subject: [PATCH 5/7] Fix comments on WithLevelSeverity --- bridges/otellogr/logsink.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bridges/otellogr/logsink.go b/bridges/otellogr/logsink.go index 7e51e815a30..2bd4cb2efb1 100644 --- a/bridges/otellogr/logsink.go +++ b/bridges/otellogr/logsink.go @@ -146,9 +146,11 @@ func WithLoggerProvider(provider log.LoggerProvider) Option { // convert logr levels to OpenTelemetry log severities. // // By default if this Option is not provided, the LogSink will use a default -// conversion function which adds an offset to the logr level to get the -// OpenTelemetry severity. The offset is such that logr.Info("message") -// converts to OpenTelemetry [log.SeverityInfo]. +// conversion function that transforms in the following way: +// +// - logr.Info and logr.V(0) are transformed to [log.SeverityInfo]. +// - logr.V(1) is transformed to [log.SeverityDebug]. +// - logr.V(2) and higher are transformed to [log.SeverityTrace]. func WithLevelSeverity(f func(int) log.Severity) Option { return optFunc(func(c config) config { c.levelSeverity = f From 2059b878f0af1e8e1d7920999ef338d43c06daf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 28 Oct 2024 09:31:45 +0100 Subject: [PATCH 6/7] Update bridges/otellogr/logsink_test.go --- bridges/otellogr/logsink_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index 9e310e443e0..b63a201c412 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -324,15 +324,15 @@ func TestLogSinkEnabled(t *testing.T) { WithLevelSeverity(func(i int) log.Severity { switch i { case 0: - return log.SeverityDebug - default: return log.SeverityInfo + default: + return log.SeverityDebug } }), ) - assert.False(t, ls.Enabled(0)) - assert.True(t, ls.Enabled(1)) + assert.True(t, ls.Enabled(0)) + assert.False(t, ls.Enabled(1)) } func buildRecord(body log.Value, timestamp time.Time, severity log.Severity, attrs []log.KeyValue) log.Record { From b171aad6ab794e21a60f27c355ea87fe38e7175d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 28 Oct 2024 11:05:41 +0100 Subject: [PATCH 7/7] Update logsink.go --- bridges/otellogr/logsink.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bridges/otellogr/logsink.go b/bridges/otellogr/logsink.go index 2bd4cb2efb1..4e36fe18e23 100644 --- a/bridges/otellogr/logsink.go +++ b/bridges/otellogr/logsink.go @@ -25,12 +25,6 @@ // - logr.V(1) is transformed to [log.SeverityDebug]. // - logr.V(2) and higher are transformed to [log.SeverityTrace]. // -// If possible, look at alternative log bridges that provide less abstract log -// level mapping such as [go.opentelemetry.io/contrib/bridges/otelslog], -// [go.opentelemetry.io/contrib/bridges/otelzap], or -// [go.opentelemetry.io/contrib/bridges/otellogrus]. They provide more direct -// translation of log levels. -// // KeysAndValues values are transformed based on their type. The following types are // supported: //