Add comment about ctxzap.AddFields not being concurrent-safe

[hexdigest]

Changes

Added comment about ctxzap.AddFields not being concurrent-safe

Verification

This assignment is not concurrent-safe:

go-grpc-middleware/logging/zap/ctxzap/context.go

Line 29 in 43a1474

l.fields = append(l.fields, fields...)

This might result in an AddTo call on an empty zapcore.Field and cause panic here:
https://github.com/uber-go/zap/blob/master/zapcore/field.go#L180

panic: unknown field type: { 0 0 <nil>}

Let me know if you guys want me to actually fix the issue because I think we might not want to introduce a lock here.

[hexdigest]

This 👇 test is flaky.

[johanbrandhorst]

1. Thanks for the PR! I don't know this code that well, does it look like it should be protected? I was sort of assuming that loggers are request scoped anyway so it shouldn't matter here, but I could be wrong.
2. Yes, that test is flaky, that is troubling. If you can figure out what's wrong I'll happily review a PR :).

[hexdigest]

1. I was sort of assuming that loggers are request scoped anyway so it shouldn't matter here, but I could be wrong.

They're usually request scoped you're right. However, it doesn't mean that code that handles the request is completely linear. You can query let's say third parties and/or databases concurrently in order to handle request (and this is also something very typical). So each goroutine that participates in request handling technically can call AddFields func and cause panic. And this is something that happened to me many times already and was hard to reproduce locally, hence the PR.

The panic itself is a different topic, I believe that it shouldn't be there in the first place.

[johanbrandhorst]

I think the docs change is good for now, I don't think we want to make this concurrency safe by default. Hope that makes sense!