From 5afea321e38e75ef3108b6d76d22a1ad787c070b Mon Sep 17 00:00:00 2001 From: Jesse Michael Date: Mon, 14 Oct 2024 21:06:30 -0700 Subject: [PATCH 1/7] feat(otelslog): add WithSource option Add a slogbridge option to include the source file location in the log attributes --- bridges/otelslog/handler.go | 28 +++++++++++++++- bridges/otelslog/handler_test.go | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/bridges/otelslog/handler.go b/bridges/otelslog/handler.go index dc3e985a897..493f1377f47 100644 --- a/bridges/otelslog/handler.go +++ b/bridges/otelslog/handler.go @@ -49,6 +49,7 @@ import ( "context" "fmt" "log/slog" + "runtime" "slices" "go.opentelemetry.io/otel/log" @@ -65,6 +66,7 @@ type config struct { provider log.LoggerProvider version string schemaURL string + source bool } func newConfig(options []Option) config { @@ -132,6 +134,15 @@ func WithLoggerProvider(provider log.LoggerProvider) Option { }) } +// WithSource returns an [Option] that configures the [log.Logger] to include +// the source location of the log record in log attributes. +func WithSource(source bool) Option { + return optFunc(func(c config) config { + c.source = source + return c + }) +} + // Handler is an [slog.Handler] that sends all logging records it receives to // OpenTelemetry. See package documentation for how conversions are made. type Handler struct { @@ -141,6 +152,8 @@ type Handler struct { attrs *kvBuffer group *group logger log.Logger + + source bool } // Compile-time check *Handler implements slog.Handler. @@ -156,7 +169,10 @@ var _ slog.Handler = (*Handler)(nil) // [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(name)} + return &Handler{ + logger: cfg.logger(name), + source: cfg.source, + } } // Handle handles the passed record. @@ -173,6 +189,16 @@ func (h *Handler) convertRecord(r slog.Record) log.Record { const sevOffset = slog.Level(log.SeverityDebug) - slog.LevelDebug record.SetSeverity(log.Severity(r.Level + sevOffset)) + if h.source { + fs := runtime.CallersFrames([]uintptr{r.PC}) + f, _ := fs.Next() + record.AddAttributes(log.Map("source", + log.String("function", f.Function), + log.String("file", f.File), + log.Int("line", f.Line)), + ) + } + if h.attrs.Len() > 0 { record.AddAttributes(h.attrs.KeyValues()...) } diff --git a/bridges/otelslog/handler_test.go b/bridges/otelslog/handler_test.go index febc496bcf2..d5080c7413a 100644 --- a/bridges/otelslog/handler_test.go +++ b/bridges/otelslog/handler_test.go @@ -457,6 +457,7 @@ func TestNewHandlerConfiguration(t *testing.T) { WithLoggerProvider(r), WithVersion("ver"), WithSchemaURL("url"), + WithSource(true), ) }) require.NotNil(t, h.logger) @@ -613,3 +614,57 @@ func BenchmarkHandler(b *testing.B) { _, _ = h, err } + +func TestHandler_convertRecord(t *testing.T) { + // Capture the PC of this line + pc, file, line, _ := runtime.Caller(0) + funcName := runtime.FuncForPC(pc).Name() + + tests := []struct { + name string + handler Handler + wantAttrs []log.KeyValue + }{ + { + name: "empty", + handler: Handler{}, + wantAttrs: []log.KeyValue{}, + }, + { + name: "with source", + handler: Handler{source: true}, + wantAttrs: []log.KeyValue{ + {Key: "source", Value: log.MapValue( + log.String("function", funcName), + log.String("file", file), + log.Int("line", line), + )}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + slogRecord := slog.NewRecord(time.Now(), slog.LevelInfo, "body", pc) + record := tt.handler.convertRecord(slogRecord) + + // Validate attributes + attrMap := make(map[string]bool) + for _, attr := range tt.wantAttrs { + attrMap[attr.String()] = true + } + + record.WalkAttributes(func(kv log.KeyValue) bool { + if !attrMap[kv.String()] { + t.Errorf("Unexpected attribute: %v", kv) + return false + } + delete(attrMap, kv.String()) + return true + }) + + if len(attrMap) > 0 { + t.Errorf("Missing expected attributes: %v", attrMap) + } + }) + } +} From c7fc7dc9be3957183badb3335da570d7aceb0878 Mon Sep 17 00:00:00 2001 From: Jesse Michael Date: Mon, 14 Oct 2024 21:16:31 -0700 Subject: [PATCH 2/7] docs(changelog): update changelog.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e6f953d79..105976bf56c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Transform nil attribute values to `log.Value` zero value instead of panicking in `go.opentelemetry.io/contrib/bridges/otelzap`. (#6237) - Transform nil attribute values to `log.Value` zero value instead of `log.StringValue("")` in `go.opentelemetry.io/contrib/bridges/otelslog`. (#6246) +### Added + +- Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `source` attribute in the log record that includes the source location where the record was emitted. (#6253) + From 9eeb939a29095841c1e8f3de74d7d52659d844c6 Mon Sep 17 00:00:00 2001 From: Jesse Michael Date: Tue, 15 Oct 2024 18:36:46 -0700 Subject: [PATCH 3/7] test: test WithSource handler with recorder --- bridges/otelslog/handler_test.go | 78 ++++++++++---------------------- 1 file changed, 23 insertions(+), 55 deletions(-) diff --git a/bridges/otelslog/handler_test.go b/bridges/otelslog/handler_test.go index d5080c7413a..da0290c6731 100644 --- a/bridges/otelslog/handler_test.go +++ b/bridges/otelslog/handler_test.go @@ -149,6 +149,8 @@ type testCase struct { // checks is a list of checks to run on the result. Each item is a slice of // checks that will be evaluated for the corresponding record emitted. checks [][]check + // options are passed to the Handler constructed for this test case. + options []Option } // copied from slogtest (1.22.1). @@ -225,6 +227,10 @@ func (h *wrapper) Handle(ctx context.Context, r slog.Record) error { } func TestSLogHandler(t *testing.T) { + // Capture the PC of this line + pc, file, line, _ := runtime.Caller(0) + funcName := runtime.FuncForPC(pc).Name() + cases := []testCase{ { name: "Values", @@ -392,13 +398,29 @@ func TestSLogHandler(t *testing.T) { inGroup("G", missingKey("a")), }}, }, + { + name: "WithSource", + explanation: withSource("a Handler using the WithSource Option should include a source attribute containing the source location of where the file was emitted"), + f: func(l *slog.Logger) { + l.Info("msg") + }, + mod: func(r *slog.Record) { + // Assign the PC of record to the one captured above. + r.PC = pc + }, + checks: [][]check{{ + hasAttr("source", map[string]any{"function": funcName, "file": file, "line": int64(line)}), + }}, + options: []Option{WithSource(true)}, + }, } // Based on slogtest.Run. for _, c := range cases { t.Run(c.name, func(t *testing.T) { r := new(recorder) - var h slog.Handler = NewHandler("", WithLoggerProvider(r)) + opts := append([]Option{WithLoggerProvider(r)}, c.options...) + var h slog.Handler = NewHandler("", opts...) if c.mod != nil { h = &wrapper{h, c.mod} } @@ -614,57 +636,3 @@ func BenchmarkHandler(b *testing.B) { _, _ = h, err } - -func TestHandler_convertRecord(t *testing.T) { - // Capture the PC of this line - pc, file, line, _ := runtime.Caller(0) - funcName := runtime.FuncForPC(pc).Name() - - tests := []struct { - name string - handler Handler - wantAttrs []log.KeyValue - }{ - { - name: "empty", - handler: Handler{}, - wantAttrs: []log.KeyValue{}, - }, - { - name: "with source", - handler: Handler{source: true}, - wantAttrs: []log.KeyValue{ - {Key: "source", Value: log.MapValue( - log.String("function", funcName), - log.String("file", file), - log.Int("line", line), - )}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - slogRecord := slog.NewRecord(time.Now(), slog.LevelInfo, "body", pc) - record := tt.handler.convertRecord(slogRecord) - - // Validate attributes - attrMap := make(map[string]bool) - for _, attr := range tt.wantAttrs { - attrMap[attr.String()] = true - } - - record.WalkAttributes(func(kv log.KeyValue) bool { - if !attrMap[kv.String()] { - t.Errorf("Unexpected attribute: %v", kv) - return false - } - delete(attrMap, kv.String()) - return true - }) - - if len(attrMap) > 0 { - t.Errorf("Missing expected attributes: %v", attrMap) - } - }) - } -} From 9193d4e7c9b0e5ddec847d815bf622ffeb836e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 16 Oct 2024 08:12:31 +0200 Subject: [PATCH 4/7] Update CHANGELOG.md --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 105976bf56c..ec894266622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,16 +8,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `source` attribute in the log record that includes the source location where the record was emitted. (#6253) + ### Fixed - Transform nil attribute values to `log.Value` zero value instead of panicking in `go.opentelemetry.io/contrib/bridges/otellogrus`. (#6237) - Transform nil attribute values to `log.Value` zero value instead of panicking in `go.opentelemetry.io/contrib/bridges/otelzap`. (#6237) - Transform nil attribute values to `log.Value` zero value instead of `log.StringValue("")` in `go.opentelemetry.io/contrib/bridges/otelslog`. (#6246) -### Added - -- Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `source` attribute in the log record that includes the source location where the record was emitted. (#6253) - From d8404fc1e8b1956a31bd458144d8395361ca963e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 16 Oct 2024 08:27:05 +0200 Subject: [PATCH 5/7] Update bridges/otelslog/handler.go --- bridges/otelslog/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelslog/handler.go b/bridges/otelslog/handler.go index 37cdf8a3e76..b2d8cf791c1 100644 --- a/bridges/otelslog/handler.go +++ b/bridges/otelslog/handler.go @@ -133,7 +133,7 @@ func WithLoggerProvider(provider log.LoggerProvider) Option { }) } -// WithSource returns an [Option] that configures the [log.Logger] to include +// WithSource returns an [Option] that configures the [Handler] to include // the source location of the log record in log attributes. func WithSource(source bool) Option { return optFunc(func(c config) config { From 870a16db6c6625646c93da6de8329a93b03cd7fa Mon Sep 17 00:00:00 2001 From: Jesse Michael Date: Wed, 16 Oct 2024 07:18:37 -0700 Subject: [PATCH 6/7] fix: use semconv keys for source attributes --- bridges/otelslog/go.mod | 2 +- bridges/otelslog/handler.go | 9 +++++---- bridges/otelslog/handler_test.go | 7 +++++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/bridges/otelslog/go.mod b/bridges/otelslog/go.mod index 3166cc845b3..5041613bf90 100644 --- a/bridges/otelslog/go.mod +++ b/bridges/otelslog/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 ) @@ -12,7 +13,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 gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/bridges/otelslog/handler.go b/bridges/otelslog/handler.go index b2d8cf791c1..6d40533a0b5 100644 --- a/bridges/otelslog/handler.go +++ b/bridges/otelslog/handler.go @@ -53,6 +53,7 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/global" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) // NewLogger returns a new [slog.Logger] backed by a new [Handler]. See @@ -191,10 +192,10 @@ func (h *Handler) convertRecord(r slog.Record) log.Record { if h.source { fs := runtime.CallersFrames([]uintptr{r.PC}) f, _ := fs.Next() - record.AddAttributes(log.Map("source", - log.String("function", f.Function), - log.String("file", f.File), - log.Int("line", f.Line)), + record.AddAttributes( + log.String(string(semconv.CodeFilepathKey), f.File), + log.String(string(semconv.CodeFunctionKey), f.Function), + log.Int(string(semconv.CodeLineNumberKey), f.Line), ) } diff --git a/bridges/otelslog/handler_test.go b/bridges/otelslog/handler_test.go index da64d8b10f3..9ca30603cdb 100644 --- a/bridges/otelslog/handler_test.go +++ b/bridges/otelslog/handler_test.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/embedded" "go.opentelemetry.io/otel/log/global" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) var now = time.Now() @@ -402,7 +403,7 @@ func TestSLogHandler(t *testing.T) { }, { name: "WithSource", - explanation: withSource("a Handler using the WithSource Option should include a source attribute containing the source location of where the file was emitted"), + explanation: withSource("a Handler using the WithSource Option should include file attributes from where the log was emitted"), f: func(l *slog.Logger) { l.Info("msg") }, @@ -411,7 +412,9 @@ func TestSLogHandler(t *testing.T) { r.PC = pc }, checks: [][]check{{ - hasAttr("source", map[string]any{"function": funcName, "file": file, "line": int64(line)}), + hasAttr(string(semconv.CodeFilepathKey), file), + hasAttr(string(semconv.CodeFunctionKey), funcName), + hasAttr(string(semconv.CodeLineNumberKey), int64(line)), }}, options: []Option{WithSource(true)}, }, From 8c8de993721a3e0342f8a99afb343956126d7726 Mon Sep 17 00:00:00 2001 From: Jesse Michael Date: Wed, 16 Oct 2024 07:26:32 -0700 Subject: [PATCH 7/7] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ccd4e3ddc3..137256dbab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,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 `source` attribute in the log record that includes the source location where the record was emitted. (#6253) +- 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) ### Fixed