From a71bdc8ed84ecebcbb0769912e50200805b1fc7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 12 Dec 2024 07:25:15 +0100 Subject: [PATCH] otelzap: Split code attributes (#6423) Same as https://github.com/open-telemetry/opentelemetry-go-contrib/pull/6415 for otelzap --- CHANGELOG.md | 1 + bridges/otelslog/handler_test.go | 6 ++++ bridges/otelzap/core.go | 17 ++++++++++- bridges/otelzap/core_test.go | 48 ++++++++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4354cc4115d..2567a9a6d66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Allow marshaling types in `go.opentelemetry.io/contrib/config`. (#6347) - Removed the redundant handling of panic from the `HTML` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6373) - The `code.function` attribute emitted by `go.opentelemetry.io/contrib/bridges/otelslog` now stores just the function name instead the package path-qualified function name. The `code.namespace` attribute now stores the package path. (#6415) +- The `code.function` attribute emitted by `go.opentelemetry.io/contrib/bridges/otelzap` now stores just the function name instead the package path-qualified function name. The `code.namespace` attribute now stores the package path. (#6423) diff --git a/bridges/otelslog/handler_test.go b/bridges/otelslog/handler_test.go index 41c3679f206..2f4bc70fcc9 100644 --- a/bridges/otelslog/handler_test.go +++ b/bridges/otelslog/handler_test.go @@ -522,6 +522,12 @@ func TestSplitFuncName(t *testing.T) { wantFuncName: "foo", wantNamespace: "github.com/my/repo/pkg", }, + { + // anonymous function + fullFuncName: "github.com/my/repo/pkg.foo.func5", + wantFuncName: "func5", + wantNamespace: "github.com/my/repo/pkg.foo", + }, { fullFuncName: "net/http.Get", wantFuncName: "Get", diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index f2c25569152..65b3958ed89 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -36,6 +36,7 @@ package otelzap // import "go.opentelemetry.io/contrib/bridges/otelzap" import ( "context" "slices" + "strings" "go.uber.org/zap/zapcore" @@ -202,10 +203,12 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { r.AddAttributes(o.attr...) if ent.Caller.Defined { + funcName, namespace := splitFuncName(ent.Caller.Function) r.AddAttributes( log.String(string(semconv.CodeFilepathKey), ent.Caller.File), log.Int(string(semconv.CodeLineNumberKey), ent.Caller.Line), - log.String(string(semconv.CodeFunctionKey), ent.Caller.Function), + log.String(string(semconv.CodeFunctionKey), funcName), + log.String(string(semconv.CodeNamespaceKey), namespace), ) } if ent.Stack != "" { @@ -262,3 +265,15 @@ func convertLevel(level zapcore.Level) log.Severity { return log.SeverityUndefined } } + +// splitFuncName splits package path-qualified function name into +// function name and package full name (namespace). E.g. it splits +// "github.com/my/repo/pkg.foo" into +// "foo" and "github.com/my/repo/pkg". +func splitFuncName(f string) (string, string) { + i := strings.LastIndexByte(f, '.') + if i < 0 { + return "", "" + } + return f[i+1:], f[:i] +} diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index bcd7130b714..65e33c39c92 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -175,7 +175,7 @@ func TestCoreWithCaller(t *testing.T) { 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()) + assert.Equal(t, 4, got.AttributesLen()) got.WalkAttributes(func(kv log.KeyValue) bool { switch kv.Key { case string(semconv.CodeFilepathKey): @@ -183,7 +183,9 @@ func TestCoreWithCaller(t *testing.T) { case string(semconv.CodeLineNumberKey): assert.Positive(t, kv.Value.AsInt64()) case string(semconv.CodeFunctionKey): - assert.Contains(t, kv.Value.AsString(), "TestCoreWithCaller") + assert.Equal(t, t.Name(), kv.Value.AsString()) + case string(semconv.CodeNamespaceKey): + assert.Equal(t, "go.opentelemetry.io/contrib/bridges/otelzap", kv.Value.AsString()) default: assert.Fail(t, "unexpected attribute key", kv.Key) } @@ -269,6 +271,48 @@ func TestConvertLevel(t *testing.T) { } } +func TestSplitFuncName(t *testing.T) { + testCases := []struct { + fullFuncName string + wantFuncName string + wantNamespace string + }{ + { + fullFuncName: "github.com/my/repo/pkg.foo", + wantFuncName: "foo", + wantNamespace: "github.com/my/repo/pkg", + }, + { + // anonymous function + fullFuncName: "github.com/my/repo/pkg.foo.func5", + wantFuncName: "func5", + wantNamespace: "github.com/my/repo/pkg.foo", + }, + { + fullFuncName: "net/http.Get", + wantFuncName: "Get", + wantNamespace: "net/http", + }, + { + fullFuncName: "invalid", + wantFuncName: "", + wantNamespace: "", + }, + { + fullFuncName: ".", + wantFuncName: "", + wantNamespace: "", + }, + } + for _, tc := range testCases { + t.Run(tc.fullFuncName, func(t *testing.T) { + gotFuncName, gotNamespace := splitFuncName(tc.fullFuncName) + assert.Equal(t, tc.wantFuncName, gotFuncName) + assert.Equal(t, tc.wantNamespace, gotNamespace) + }) + } +} + func BenchmarkCoreWrite(b *testing.B) { benchmarks := []struct { name string