Chaining WithContext call is broken

[twalwyn]

#81 broke clients that chain WithContext calls. According to the docs, I expect the following program to compile (lifted from #80):

package main

import (
	"context"
	"github.com/rs/zerolog"
	"os"
)

func process(ctx context.Context, id int) {
	log := zerolog.Ctx(ctx).With().Int("id", id).Logger()
	child(log.WithContext(ctx))
}

func child(ctx context.Context) {
	zerolog.Ctx(ctx).Info().Msg("hello from child")
}

func main() {
	ctx := zerolog.New(os.Stdout).WithContext(context.Background())
	zerolog.Ctx(ctx).Info().Msg("processing 1")
	process(ctx, 1)

	zerolog.Ctx(ctx).Info().Msg("processing 2")
	process(ctx, 2)

	zerolog.Ctx(ctx).Info().Msg("done")
}

but it fails:

# command-line-arguments
cmd/main.go:19:31: cannot call pointer method on zerolog.New(os.Stdout)
cmd/main.go:19:31: cannot take the address of zerolog.New(os.Stdout)

Reason being that variables in go are addressable but return values from functions are not.

The current workaround is this:

package main

import (
	"context"
	"github.com/rs/zerolog"
	"os"
)

func process(ctx context.Context, id int) {
	log := zerolog.Ctx(ctx).With().Int("id", id).Logger()
	child(log.WithContext(ctx))
}

func child(ctx context.Context) {
	zerolog.Ctx(ctx).Info().Msg("hello from child")
}

func main() {
	l := zerolog.New(os.Stdout)
	ctx := l.WithContext(context.Background())
	zerolog.Ctx(ctx).Info().Msg("processing 1")
	process(ctx, 1)

	zerolog.Ctx(ctx).Info().Msg("processing 2")
	process(ctx, 2)

	zerolog.Ctx(ctx).Info().Msg("done")
}

Output:

{"level":"info","message":"processing 1"}
{"level":"info","id":1,"message":"hello from child"}
{"level":"info","message":"processing 2"}
{"level":"info","id":2,"message":"hello from child"}
{"level":"info","message":"done"}

[sirkon]

Any idea on the issue? I like the project so far, but this stinks.

I believe Logger should stay the same (to keep backward compatibility) and another function which returns *Logger is needed. How about RefLogger()?

[smyrman]

Would it perhaps be better to change WithContext to be a non-pointer method? It can still store a pointer in context, and Ctx(context.Context) *Logger may still return a pointer to the logger from context for backwards compatibility reasons.

zerolog/ctx.go

Line 27 in 9cd6f6e

func (l *Logger) WithContext(ctx context.Context) context.Context {

Since the current code don't seam to handle the nil case anyway, this should be mostly backwards-compatible. The only slight change in semantics, and I believe that to be a good/acceptable change, would be that if modifying/replacing a logger, the logger in context won't change.

I don't think there should be much performance hit, but this can presumably be benchmarked. From what I can see, the logger is 88 bytes as of v1.11.0.

package main

import (
	"os"
	"unsafe"

	"github.com/rs/zerolog"
)

func main() {
	l := zerolog.New(os.Stdout)
	l.Info().Msgf("size if l is %d", unsafe.Sizeof(l))
	l = l.With().Str("foo", "bar").Int("baz", 42).Logger()
	l.Info().Msgf("size if l is %d", unsafe.Sizeof(l))
}

Output:

% go run example.go
{"level":"info","message":"size if l is 88"}
{"level":"info","foo":"bar","baz":42,"message":"size if l is 88"}

[ditrytus]

It's 2021. Is there any chance this issue will be fixed?

[rs]

Feel free to submit a PR.

[domino14]

Can the README at least be edited to show this part is wrong

ctx := log.With().Str("component", "module").Logger().WithContext(ctx)
log.Ctx(ctx).Info().Msg("hello world")
// Output: {"component":"module","level":"info","message":"hello world"}

[rs]

A fix would be preferred. I have no time to work on it, so any PR is welcome.

[nickcorin]

I have submitted a PR, checked have passed. I'm just waiting for a maintainer to approve and hopefully merge. 🤞🏼