Missing SugaredLogger methods

[mrsufgi]

trafficstars

Hey,

I've been trying otelzap as a wrapper for zap. I've also created a thin wrapper to make logger usage more simplified across my packages:

package log

import (
	"context"

	"github.com/uptrace/opentelemetry-go-extra/otelzap"
	"go.uber.org/zap"
)

// std is the name of the standard logger in stdlib `log`
var std *otelzap.Logger

func New() *otelzap.Logger {
	logger := otelzap.New(zap.L(),
		otelzap.WithTraceIDField(true),
		otelzap.WithMinLevel(zap.DebugLevel))

	std = logger
	return logger
}

func L(ctx ...context.Context) *otelzap.LoggerWithCtx {
	var c context.Context
	if len(ctx) > 0 {
		c = ctx[0]
	}
	return WithCtx(c)
}

func S(ctx ...context.Context) *otelzap.SugaredLoggerWithCtx {
	var c context.Context
	if len(ctx) > 0 {
		c = ctx[0]
	}
	return SugaredWithCtx(c)
}

func WithCtx(ctx context.Context) *otelzap.LoggerWithCtx {
	ctxl := std.Ctx(ctx)
	return &ctxl
}

func SugaredWithCtx(ctx context.Context) *otelzap.SugaredLoggerWithCtx {
	ctxl := std.Sugar().Ctx(ctx)
	return &ctxl
}

The plan is importing my log wrapper instead of zap (easy migration) However, there are methods missing from SugaredLogger that do exist on zap.

.Info()
.Fatal()
.Debug()
.Error()
...

(the ones without formatting)

were there a reason why you didnt add them?

Im guessing it should look like:

// Info uses fmt.Sprint to construct and log a message.
func (s SugaredLoggerWithCtx) Info(msg string) {
	s.s.logArgs(s.ctx, zap.InfoLevel, msg, nil)
	s.s.Info(msg)
} 

Should I go ahead and implement those for all levels? Cheers :clinking_glasses:

[vmihailenco]

I guess you mean this:

// Info uses fmt.Sprint to construct and log a message.
func (s *SugaredLogger) Info(args ...interface{}) {
	s.log(InfoLevel, "", args, nil)
}

Such logs are not structured and back-ends will have troubles parsing and grouping them. I guess we should support them, but such logs are neither efficient/fast nor structured so it is strange that Zap supports them...

Should I go ahead and implement those for all levels?

Yes, please.

I've also created a thin wrapper to make logger usage more simplified across my packages:

That looks good. otelzap already supports otelzap.L, otelzap.S, and otelzap.ReplaceGlobals. I guess we should also add otelzap.Ctx shortcut with some docs.

SugaredWithCtx is not much better than otelzap.S().Ctx(ctx) so I would leave it alone...

[mrsufgi]

Sure I'll submit a PR later today probably.

I don't understand why .Info is on Sugared but that's zap API haha.

Regarding replaceGlobals, usage is the same as zap? Can you share how to use otelzap L S and ReplaceGlobals ?