Skip to content

Commit

Permalink
spanlogger: Remove global logger usages
Browse files Browse the repository at this point in the history
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
  • Loading branch information
aknuds1 committed Sep 15, 2021
1 parent 460bf06 commit 673a8b1
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 37 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
github.com/hashicorp/go-sockaddr v1.0.2
github.com/hashicorp/memberlist v0.2.3
github.com/opentracing-contrib/go-grpc v0.0.0-20210225150812-73cb765af46e // indirect
github.com/opentracing/opentracing-go v1.2.0
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.11.0
github.com/prometheus/common v0.26.0
Expand Down
46 changes: 16 additions & 30 deletions spanlogger/spanlogger.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"github.com/opentracing/opentracing-go/ext"
otlog "github.com/opentracing/opentracing-go/log"

"github.com/cortexproject/cortex/pkg/tenant"
util_log "github.com/cortexproject/cortex/pkg/util/log"
"github.com/grafana/dskit/dslog"
"github.com/grafana/dskit/tenant"
)

type loggerCtxMarker struct{}
Expand All @@ -29,44 +29,30 @@ type SpanLogger struct {
opentracing.Span
}

// New makes a new SpanLogger, where logs will be sent to the global logger.
func New(ctx context.Context, method string, kvps ...interface{}) (*SpanLogger, context.Context) {
return NewWithLogger(ctx, util_log.Logger, method, kvps...)
}

// NewWithLogger makes a new SpanLogger with a custom log.Logger to send logs
// to. The provided context will have the logger attached to it and can be
// retrieved with FromContext or FromContextWithFallback.
func NewWithLogger(ctx context.Context, l log.Logger, method string, kvps ...interface{}) (*SpanLogger, context.Context) {
// New makes a new SpanLogger with a log.Logger to send logs to. The provided context will have the logger attached
// to it and can be retrieved with FromContext.
func New(ctx context.Context, logger log.Logger, method string, kvps ...interface{}) (*SpanLogger, context.Context) {
span, ctx := opentracing.StartSpanFromContext(ctx, method)
if ids, _ := tenant.TenantIDs(ctx); len(ids) > 0 {
if ids, _ := tenant.IDs(ctx); len(ids) > 0 {
span.SetTag(TenantIDTagName, ids)
}
logger := &SpanLogger{
Logger: log.With(util_log.WithContext(ctx, l), "method", method),
l := &SpanLogger{
Logger: log.With(dslog.WithContext(ctx, logger), "method", method),
Span: span,
}
if len(kvps) > 0 {
level.Debug(logger).Log(kvps...)
level.Debug(l).Log(kvps...)
}

ctx = context.WithValue(ctx, loggerCtxKey, l)
return logger, ctx
}

// FromContext returns a span logger using the current parent span. If there
// is no parent span, the SpanLogger will only log to the logger
// in the context. If the context doesn't have a logger, the global logger
// is used.
func FromContext(ctx context.Context) *SpanLogger {
return FromContextWithFallback(ctx, util_log.Logger)
ctx = context.WithValue(ctx, loggerCtxKey, logger)
return l, ctx
}

// FromContextWithFallback returns a span logger using the current parent span.
// IF there is no parent span, the SpanLogger will only log to the logger
// FromContext returns a span logger using the current parent span.
// If there is no parent span, the SpanLogger will only log to the logger
// within the context. If the context doesn't have a logger, the fallback
// logger is used.
func FromContextWithFallback(ctx context.Context, fallback log.Logger) *SpanLogger {
func FromContext(ctx context.Context, fallback log.Logger) *SpanLogger {
logger, ok := ctx.Value(loggerCtxKey).(log.Logger)
if !ok {
logger = fallback
Expand All @@ -76,7 +62,7 @@ func FromContextWithFallback(ctx context.Context, fallback log.Logger) *SpanLogg
sp = defaultNoopSpan
}
return &SpanLogger{
Logger: util_log.WithContext(ctx, logger),
Logger: dslog.WithContext(ctx, logger),
Span: sp,
}
}
Expand All @@ -93,7 +79,7 @@ func (s *SpanLogger) Log(kvps ...interface{}) error {
return nil
}

// Error sets error flag and logs the error on the span, if non-nil. Returns the err passed in.
// Error sets error flag and logs the error on the span, if non-nil. Returns the err passed in.
func (s *SpanLogger) Error(err error) error {
if err == nil {
return nil
Expand Down
15 changes: 8 additions & 7 deletions spanlogger/spanlogger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import (
)

func TestSpanLogger_Log(t *testing.T) {
span, ctx := New(context.Background(), "test", "bar")
logger := log.NewNopLogger()
span, ctx := New(context.Background(), logger, "test", "bar")
_ = span.Log("foo")
newSpan := FromContext(ctx)
newSpan := FromContext(ctx, logger)
require.Equal(t, span.Span, newSpan.Span)
_ = newSpan.Log("bar")
noSpan := FromContext(context.Background())
noSpan := FromContext(context.Background(), logger)
_ = noSpan.Log("foo")
require.Error(t, noSpan.Error(errors.New("err")))
require.NoError(t, noSpan.Error(nil))
Expand All @@ -30,13 +31,13 @@ func TestSpanLogger_CustomLogger(t *testing.T) {
logged = append(logged, keyvals)
return nil
}
span, ctx := NewWithLogger(context.Background(), logger, "test")
span, ctx := New(context.Background(), logger, "test")
_ = span.Log("msg", "original spanlogger")

span = FromContextWithFallback(ctx, log.NewNopLogger())
span = FromContext(ctx, log.NewNopLogger())
_ = span.Log("msg", "restored spanlogger")

span = FromContextWithFallback(context.Background(), logger)
span = FromContext(context.Background(), logger)
_ = span.Log("msg", "fallback spanlogger")

expect := [][]interface{}{
Expand Down Expand Up @@ -64,7 +65,7 @@ func createSpan(ctx context.Context) *mocktracer.MockSpan {
mockTracer := mocktracer.New()
opentracing.SetGlobalTracer(mockTracer)

logger, _ := New(ctx, "name")
logger, _ := New(ctx, log.NewNopLogger(), "name")
return logger.Span.(*mocktracer.MockSpan)
}

Expand Down

0 comments on commit 673a8b1

Please sign in to comment.