From 70a4c4fe4f22104b961d874379c06ed205483e67 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 13:14:40 +0200 Subject: [PATCH 01/12] added caller to record attributes --- bridges/otelzap/core.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 4e222e37a6a..40c20453b2b 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -200,6 +200,13 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { r.SetSeverityText(ent.Level.String()) r.AddAttributes(o.attr...) + if ent.Caller.Defined { + r.AddAttributes( + log.String("file", ent.Caller.File), + log.Int("line", ent.Caller.Line), + log.String("function", ent.Caller.Function), + ) + } if len(fields) > 0 { ctx, attrbuf := convertField(fields) if ctx != nil { From 0951147d79dfba291b5bba5d9bee2a80de0c15b7 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 13:49:04 +0200 Subject: [PATCH 02/12] added changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab91768f636..c2092eb974c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Transform raw (`slog.KindAny`) attribute values to matching `log.Value` types. For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) - Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `code.*` attributes in the log record that includes the source location where the record was emitted. (#6253) +- Add caller information in otelzap to the `log.Record` if the `zap.Logger` was created with the `AddCaller()` option. (#6268) ### Fixed From 253487ce1ff174beaec2a86980f25c4db3531b71 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 13:49:41 +0200 Subject: [PATCH 03/12] adjusted to follow semconv conventions --- bridges/otelzap/core.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 40c20453b2b..71be0441c9a 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -41,6 +41,7 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/global" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) type config struct { @@ -202,9 +203,9 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { r.AddAttributes(o.attr...) if ent.Caller.Defined { r.AddAttributes( - log.String("file", ent.Caller.File), - log.Int("line", ent.Caller.Line), - log.String("function", ent.Caller.Function), + log.String(string(semconv.CodeFilepathKey), ent.Caller.File), + log.Int(string(semconv.CodeLineNumberKey), ent.Caller.Line), + log.String(string(semconv.CodeFunctionKey), ent.Caller.Function), ) } if len(fields) > 0 { From 1bbaa6bf55ef14b9b7daa382a0ba985a80a1d877 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 14:00:59 +0200 Subject: [PATCH 04/12] added stack trace to log record if persistent in log entry --- CHANGELOG.md | 1 + bridges/otelzap/core.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2092eb974c..5843d03b8e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) - Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `code.*` attributes in the log record that includes the source location where the record was emitted. (#6253) - Add caller information in otelzap to the `log.Record` if the `zap.Logger` was created with the `AddCaller()` option. (#6268) +- Add stack trace in otelzap to the `log.Record` if the `zap.Logger` was created with the `AddStackStrace()` option. (#6268) ### Fixed diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 71be0441c9a..f2c25569152 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -208,6 +208,9 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { log.String(string(semconv.CodeFunctionKey), ent.Caller.Function), ) } + if ent.Stack != "" { + r.AddAttributes(log.String(string(semconv.CodeStacktraceKey), ent.Stack)) + } if len(fields) > 0 { ctx, attrbuf := convertField(fields) if ctx != nil { From 2a0d9be1fafc2846ed2b29dc7c0fd9eb0320e504 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 14:05:21 +0200 Subject: [PATCH 05/12] applied changelog review suggestion --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5843d03b8e4..10369be93cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Transform raw (`slog.KindAny`) attribute values to matching `log.Value` types. For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) - Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `code.*` attributes in the log record that includes the source location where the record was emitted. (#6253) -- Add caller information in otelzap to the `log.Record` if the `zap.Logger` was created with the `AddCaller()` option. (#6268) -- Add stack trace in otelzap to the `log.Record` if the `zap.Logger` was created with the `AddStackStrace()` option. (#6268) +- Set the `code.*` attributes in `go.opentelemetry.io/contrib/bridges/otelzap` if the `zap.Logger` was created with the `AddCaller` and/or `AddStacktrace(level)` option. (#6268) ### Fixed From 335a4b4bb9db4f60cb0a75aea693f885a3171ebc Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 14:40:43 +0200 Subject: [PATCH 06/12] added tests --- bridges/otelzap/core_test.go | 77 ++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 0d00794b409..8dd7ccaa9cc 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -6,6 +6,7 @@ package otelzap import ( "context" "fmt" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "testing" "time" @@ -164,6 +165,82 @@ func TestCoreEnabled(t *testing.T) { assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) } +func TestCoreWithCaller(t *testing.T) { + rec := logtest.NewRecorder() + zc := NewCore(loggerName, WithLoggerProvider(rec)) + + t.Run("WithAddCaller", func(t *testing.T) { + logger := zap.New(zc, zap.AddCaller()) + logger.Info(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) + assert.Equal(t, 3, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + switch kv.Key { + case string(semconv.CodeFilepathKey): + assert.Contains(t, kv.Value.AsString(), "core_test.go") + case string(semconv.CodeLineNumberKey): + assert.Greater(t, kv.Value.AsInt64(), int64(0)) + case string(semconv.CodeFunctionKey): + assert.Contains(t, kv.Value.AsString(), "TestCoreWithCaller") + default: + assert.Fail(t, "unexpected attribute key", kv.Key) + } + return true + }) + }) + + rec.Reset() + + t.Run("Default", func(t *testing.T) { + logger := zap.New(zc) + logger.Info(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) + assert.Equal(t, 0, got.AttributesLen()) + }) +} + +func TestCoreWithStacktrace(t *testing.T) { + rec := logtest.NewRecorder() + zc := NewCore(loggerName, WithLoggerProvider(rec)) + logger := zap.New(zc, zap.AddStacktrace(zapcore.ErrorLevel)) + + t.Run("Error", func(t *testing.T) { + logger.Error(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityError, got.Severity()) + assert.Equal(t, zap.ErrorLevel.String(), got.SeverityText()) + assert.Equal(t, 1, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, string(semconv.CodeStacktraceKey), kv.Key) + assert.NotEmpty(t, kv.Value.AsString()) + return true + }) + }) + + rec.Reset() + + t.Run("Warn", func(t *testing.T) { + logger.Warn(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityWarn, got.Severity()) + assert.Equal(t, zap.WarnLevel.String(), got.SeverityText()) + assert.Equal(t, 0, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + println(kv.Key) + println(kv.Value.AsString()) + return true + }) + }) +} + func TestNewCoreConfiguration(t *testing.T) { t.Run("Default", func(t *testing.T) { r := logtest.NewRecorder() From 54969a412653316cde5413533911ab5cdf05787a Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Mon, 21 Oct 2024 14:42:51 +0200 Subject: [PATCH 07/12] reformat code --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 8dd7ccaa9cc..5d7ad31230c 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -6,7 +6,6 @@ package otelzap import ( "context" "fmt" - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "testing" "time" @@ -18,6 +17,7 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/global" "go.opentelemetry.io/otel/log/logtest" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) var ( From 35d5e279450629e7e5feca4a8b525c3c00a4693b Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Tue, 22 Oct 2024 09:32:59 +0200 Subject: [PATCH 08/12] removed debug print statements --- bridges/otelzap/core_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 5d7ad31230c..46b1458e895 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -233,11 +233,6 @@ func TestCoreWithStacktrace(t *testing.T) { assert.Equal(t, log.SeverityWarn, got.Severity()) assert.Equal(t, zap.WarnLevel.String(), got.SeverityText()) assert.Equal(t, 0, got.AttributesLen()) - got.WalkAttributes(func(kv log.KeyValue) bool { - println(kv.Key) - println(kv.Value.AsString()) - return true - }) }) } From 3f0d61b36302c8a345415196629a18c9c9ca4053 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Tue, 22 Oct 2024 09:37:34 +0200 Subject: [PATCH 09/12] applied linting suggestion --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 46b1458e895..a8cb791eb49 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -182,7 +182,7 @@ func TestCoreWithCaller(t *testing.T) { case string(semconv.CodeFilepathKey): assert.Contains(t, kv.Value.AsString(), "core_test.go") case string(semconv.CodeLineNumberKey): - assert.Greater(t, kv.Value.AsInt64(), int64(0)) + assert.Positive(t, kv.Value.AsInt64()) case string(semconv.CodeFunctionKey): assert.Contains(t, kv.Value.AsString(), "TestCoreWithCaller") default: From f2bc56c72267bbfc2c854eacdf446b8919459310 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Tue, 22 Oct 2024 14:33:14 +0200 Subject: [PATCH 10/12] removed unnecessary tests --- bridges/otelzap/core_test.go | 85 ++++++++++++------------------------ 1 file changed, 29 insertions(+), 56 deletions(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index a8cb791eb49..bcd7130b714 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -168,40 +168,26 @@ func TestCoreEnabled(t *testing.T) { func TestCoreWithCaller(t *testing.T) { rec := logtest.NewRecorder() zc := NewCore(loggerName, WithLoggerProvider(rec)) + logger := zap.New(zc, zap.AddCaller()) - t.Run("WithAddCaller", func(t *testing.T) { - logger := zap.New(zc, zap.AddCaller()) - logger.Info(testMessage) - got := rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityInfo, got.Severity()) - assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) - assert.Equal(t, 3, got.AttributesLen()) - got.WalkAttributes(func(kv log.KeyValue) bool { - switch kv.Key { - case string(semconv.CodeFilepathKey): - assert.Contains(t, kv.Value.AsString(), "core_test.go") - case string(semconv.CodeLineNumberKey): - assert.Positive(t, kv.Value.AsInt64()) - case string(semconv.CodeFunctionKey): - assert.Contains(t, kv.Value.AsString(), "TestCoreWithCaller") - default: - assert.Fail(t, "unexpected attribute key", kv.Key) - } - return true - }) - }) - - rec.Reset() - - t.Run("Default", func(t *testing.T) { - logger := zap.New(zc) - logger.Info(testMessage) - got := rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityInfo, got.Severity()) - assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) - assert.Equal(t, 0, got.AttributesLen()) + logger.Info(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, zap.InfoLevel.String(), got.SeverityText()) + assert.Equal(t, 3, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + switch kv.Key { + case string(semconv.CodeFilepathKey): + assert.Contains(t, kv.Value.AsString(), "core_test.go") + case string(semconv.CodeLineNumberKey): + assert.Positive(t, kv.Value.AsInt64()) + case string(semconv.CodeFunctionKey): + assert.Contains(t, kv.Value.AsString(), "TestCoreWithCaller") + default: + assert.Fail(t, "unexpected attribute key", kv.Key) + } + return true }) } @@ -210,29 +196,16 @@ func TestCoreWithStacktrace(t *testing.T) { zc := NewCore(loggerName, WithLoggerProvider(rec)) logger := zap.New(zc, zap.AddStacktrace(zapcore.ErrorLevel)) - t.Run("Error", func(t *testing.T) { - logger.Error(testMessage) - got := rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityError, got.Severity()) - assert.Equal(t, zap.ErrorLevel.String(), got.SeverityText()) - assert.Equal(t, 1, got.AttributesLen()) - got.WalkAttributes(func(kv log.KeyValue) bool { - assert.Equal(t, string(semconv.CodeStacktraceKey), kv.Key) - assert.NotEmpty(t, kv.Value.AsString()) - return true - }) - }) - - rec.Reset() - - t.Run("Warn", func(t *testing.T) { - logger.Warn(testMessage) - got := rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityWarn, got.Severity()) - assert.Equal(t, zap.WarnLevel.String(), got.SeverityText()) - assert.Equal(t, 0, got.AttributesLen()) + logger.Error(testMessage) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityError, got.Severity()) + assert.Equal(t, zap.ErrorLevel.String(), got.SeverityText()) + assert.Equal(t, 1, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, string(semconv.CodeStacktraceKey), kv.Key) + assert.NotEmpty(t, kv.Value.AsString()) + return true }) } From 59d480bb7c932fe2bebf6d06e7e9766ea1dec336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 22 Oct 2024 14:37:09 +0200 Subject: [PATCH 11/12] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1077e4c02ec..301e53afdc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) - Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `code.*` attributes in the log record that includes the source location where the record was emitted. (#6253) - Add `ContextWithStartTime` and `StartTimeFromContext` to `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`, which allows setting the start time using go context. (#6137) -- Set the `code.*` attributes in `go.opentelemetry.io/contrib/bridges/otelzap` if the `zap.Logger` was created with the `AddCaller` and/or `AddStacktrace(level)` option. (#6268) +- Set the `code.*` attributes in `go.opentelemetry.io/contrib/bridges/otelzap` if the `zap.Logger` was created with the `AddCaller` or `AddStacktrace` option. (#6268) ### Fixed From 354511595cfc9d7306c2f3857465d6795b90f231 Mon Sep 17 00:00:00 2001 From: Frederik Enste Date: Tue, 22 Oct 2024 15:06:06 +0200 Subject: [PATCH 12/12] updated go mod file --- bridges/otelzap/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/go.mod b/bridges/otelzap/go.mod index abea363927a..68da34e7733 100644 --- a/bridges/otelzap/go.mod +++ b/bridges/otelzap/go.mod @@ -4,6 +4,7 @@ go 1.22 require ( github.com/stretchr/testify v1.9.0 + go.opentelemetry.io/otel v1.31.0 go.opentelemetry.io/otel/log v0.7.0 go.uber.org/zap v1.27.0 ) @@ -13,7 +14,6 @@ require ( github.com/go-logr/logr v1.4.2 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.opentelemetry.io/otel v1.31.0 // indirect go.opentelemetry.io/otel/metric v1.31.0 // indirect go.opentelemetry.io/otel/trace v1.31.0 // indirect go.uber.org/multierr v1.11.0 // indirect