Skip to content

Commit

Permalink
[otelcol] Fix grpclogger to capture correct caller location (#10773)
Browse files Browse the repository at this point in the history
#### Description
Cleaned up Grpc logs to make them reference the location where the log
statements are written instead of zapgrpc/zapgrpc.go:176

#### Link to tracking issue
Fixes #10772
Fixes jaegertracing/jaeger#5784

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
3 people authored Aug 26, 2024
1 parent 00c8ea9 commit 93ecf69
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
2 changes: 1 addition & 1 deletion otelcol/internal/grpclog/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func SetLogger(baseLogger *zap.Logger, loglevel zapcore.Level) *zapgrpc.Logger {
c = core
}
return c.With([]zapcore.Field{zap.Bool("grpc_log", true)})
})))
}), zap.AddCallerSkip(5)))

grpclog.SetLoggerV2(logger)
return logger
Expand Down
27 changes: 22 additions & 5 deletions otelcol/internal/grpclog/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"google.golang.org/grpc/grpclog"
)

func TestGRPCLogger(t *testing.T) {
Expand Down Expand Up @@ -50,29 +51,45 @@ func TestGRPCLogger(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
obsInfo, obsWarn := false, false
callerInfo := ""
hook := zap.Hooks(func(entry zapcore.Entry) error {
switch entry.Level {
case zapcore.InfoLevel:
obsInfo = true
case zapcore.WarnLevel:
obsWarn = true
}
callerInfo = entry.Caller.String()
return nil
})

// create new collector zap logger
logger, err := test.cfg.Build(hook)
assert.NoError(t, err)

// create colGRPCLogger
// create GRPCLogger
glogger := SetLogger(logger, test.cfg.Level.Level())
assert.NotNil(t, glogger)

glogger.Info(test.name)
glogger.Warning(test.name)

// grpc does not usually call the logger directly, but through various wrappers that add extra depth
component := &mockComponent{logger: grpclog.Component("channelz")}
component.Info(test.name)
component.Warning(test.name)
assert.Equal(t, obsInfo, test.infoLogged)
assert.Equal(t, obsWarn, test.warnLogged)
// match the file name and line number of Warning() call above
assert.Contains(t, callerInfo, "internal/grpclog/logger_test.go:76")
})
}
}

type mockComponent struct {
logger grpclog.DepthLoggerV2
}

func (c *mockComponent) Info(args ...any) {
c.logger.Info(args...)
}

func (c *mockComponent) Warning(args ...any) {
c.logger.Warning(args...)
}

0 comments on commit 93ecf69

Please sign in to comment.