Confusing behavior with context.Context integration

[rs]

Confusing behavior with context.Context integration

[rs]

The rational is that a request context should only have a single instance of the logger so your requests can add fields that are visible to parent middleware for instance. This is particularly useful with the hlog.AccessHandler which can be augmented with request specific fields.

This is also better for performance. Storing a new instance of the logger in the context for each update would end up with a pretty narrow context tree that would impact lookup time for every context.Value calls.

[bluekeyes]

Thanks. I think I understand the reasoning here, but I'm now confused by the difference between (*Logger) UpdateContext and (Logger) WithContext. It seems that UpdateContext also achieves what you describe (fields visible to parent middleware, no new context), so is there a reason to use WithContext instead of calling zerolog.Ctx(ctx).UpdateContext(...) in places that want this behavior?

To give a more concrete example, I have an application that responds to a GitHub webhook and performs an action on every open pull request in the repository. In the handler, I loop over the open PRs and call updatePullRequest(context.Context, *github.PullRequest). This does some logging of its own (and may call other functions that log), so I want to attach the PR number that is being processed to the logger in the context that I pass.

In your view, is it better to pass the modified logger explicitly as an argument or re-add the necessary fields at each level instead of passing a modified context?

[rs]

The (*Logger) UpdateContext will update the context of the logger with no copy of the logger (in place). If the (Logger) WithContext was just storing the pointer without trying to reuse the existing one, it would store the same pointer over and over in the context for each call of WithContext.

What I can do though, is only updating the existing pointer if the currently stored logger is the same as the one you are trying to store. This way, if you copy the logger instead of calling WithContext, it will behave the way you expect.

[rs]

Please review #81.