Skip to content

Commit

Permalink
otelslog: Split code attributes (#6415)
Browse files Browse the repository at this point in the history
Follow semantic conventions more strictly. See the following thread:
open-telemetry/semantic-conventions#1624 (comment)

Follows
#6253

There is no noticeable performance overhead:

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/bridges/otelslog
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                               │   old.txt    │               new.txt               │
                               │    sec/op    │   sec/op     vs base                │
Handler/(WithSource).Handle-16   260.4n ± 21%   234.0n ± 5%  -10.14% (p=0.035 n=10)

                               │  old.txt   │            new.txt             │
                               │    B/op    │    B/op     vs base            │
Handler/(WithSource).Handle-16   248.0 ± 0%   248.0 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

                               │  old.txt   │            new.txt             │
                               │ allocs/op  │ allocs/op   vs base            │
Handler/(WithSource).Handle-16   2.000 ± 0%   2.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
  • Loading branch information
pellared authored Dec 11, 2024
1 parent 8e19210 commit 0abc931
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fixed the value for configuring the OTLP exporter to use `grpc` instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`. (#6338)
- 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)

<!-- Released section -->
<!-- Don't change this section unless doing release -->
Expand Down
17 changes: 16 additions & 1 deletion bridges/otelslog/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"log/slog"
"runtime"
"slices"
"strings"

"go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log/global"
Expand Down Expand Up @@ -192,9 +193,11 @@ func (h *Handler) convertRecord(r slog.Record) log.Record {
if h.source {
fs := runtime.CallersFrames([]uintptr{r.PC})
f, _ := fs.Next()
funcName, namespace := splitFuncName(f.Function)
record.AddAttributes(
log.String(string(semconv.CodeFilepathKey), f.File),
log.String(string(semconv.CodeFunctionKey), f.Function),
log.String(string(semconv.CodeFunctionKey), funcName),
log.String(string(semconv.CodeNamespaceKey), namespace),
log.Int(string(semconv.CodeLineNumberKey), f.Line),
)
}
Expand Down Expand Up @@ -476,3 +479,15 @@ func convert(v slog.Value) log.Value {
return log.StringValue(fmt.Sprintf("unhandled: (%s) %+v", v.Kind(), v.Any()))
}
}

// 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]
}
52 changes: 51 additions & 1 deletion bridges/otelslog/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ 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()
funcName, namespace := splitFuncName(runtime.FuncForPC(pc).Name())

cases := []testCase{
{
Expand Down Expand Up @@ -414,6 +414,7 @@ func TestSLogHandler(t *testing.T) {
checks: [][]check{{
hasAttr(string(semconv.CodeFilepathKey), file),
hasAttr(string(semconv.CodeFunctionKey), funcName),
hasAttr(string(semconv.CodeNamespaceKey), namespace),
hasAttr(string(semconv.CodeLineNumberKey), int64(line)),
}},
options: []Option{WithSource(true)},
Expand Down Expand Up @@ -510,6 +511,42 @@ func TestHandlerEnabled(t *testing.T) {
assert.True(t, h.Enabled(ctx, slog.LevelDebug), "context not passed")
}

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",
},
{
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 BenchmarkHandler(b *testing.B) {
var (
h slog.Handler
Expand Down Expand Up @@ -639,5 +676,18 @@ func BenchmarkHandler(b *testing.B) {
})
})

b.Run("(WithSource).Handle", func(b *testing.B) {
handlers := make([]*Handler, b.N)
for i := range handlers {
handlers[i] = NewHandler("", WithSource(true))
}

b.ReportAllocs()
b.ResetTimer()
for n := 0; n < b.N; n++ {
err = handlers[n].Handle(ctx, record)
}
})

_, _ = h, err
}

0 comments on commit 0abc931

Please sign in to comment.