support non-clone zapcore

[sasakiyori]

Is your feature request related to a problem? Please describe. There is a common use case (such as gin + zap): I have a global zap.Logger, and every session will create a new logger like below.

logger := globalLogger.With(zap.String("session", "xxx"))
logger.Info("xxx")

I want to set different common kvs for every unique logger due to different request param. As the code goes ahead, there are more and more common kvs need to print for every log record. So I may use the With function to embed the common k-v just like the code below. But I find the With function will clone logger and inside zapcore, It is expensive for memory cost and also the gc.

func XXX() {
    logger := globalLogger.With(zap.String("session", "xxx"))
    logger = logger.With(zap.String("common_key1", "value"))
    logger.Info("xxx")
    // ... 
    logger = logger.With(zap.String("common_key2", "value"))
    logger.Info("xxx")
}

Describe the solution you'd like I think I can make a new zapcore, which is the same as ioCore except the With method.

type sessionIoCore {
    LevelEnabler
    enc Encoder
    out WriteSyncer
}

func (c *sessionIoCore) With(fields []Field) Core {
    for i := range fields {
        fields[i].AddTo(c.enc)
    }
    return c
}

So that I can use it like this:

func XXX() {
    logger := globalLogger.With(zap.String("session", "xxx"))
    logger.Core().With([]zap.Field{zap.String("common_key1", "value")})
    logger.Info("xxx")
    // ...
    logger.Core().With([]zap.Field{zap.String("common_key2", "value")})
    logger.Info("xxx")
}

Describe alternatives you've considered I saw some similar issue requests, and they prefer to solve this by context.Context (seems not merged so far). I think it is feasible but not so comfortable for coding?

Is this a breaking change? No breaking change, just add a new zapcore type.

Additional context The problem of this solution is that users should clearly understand if this zapcore can be used in their scenario, or errors may occur. Please tell me if you accept this solution. If OK, I'd glad to create a PR for this.

[abhinav]

Hey @sasakiyori, making the core behind a logger mutable isn't ideal. It'll have to be safe for concurrent use somehow, and prevent accidental use of the mutable per-request logger core in a non-cloned context. I haven't thought through the implications of this fully, but I think this might be a change we would be wary of.

However, there's an ongoing PR (#1319 by @jquirke) that would introduce a WithLazy option. It'll still clone the Core, but it'll be a lot cheaper. The fields passed to WithLazy won't be encoded until the first time that logger is used.

I know that it's not the exact solution you're looking for, but does that help address your concerns?

[sasakiyori]

It'll have to be safe for concurrent use somehow, and prevent accidental use of the mutable per-request logger core in a non-cloned context.

I agree. It is used for specific scencario, adding it into the basic package is not a good choice.

WithLazy is a good trial, but I think it cannot solve the cost problem thoroughly.

How about this: https://github.com/uber-go/zap/pull/1019. Is there any plans to accept or reject this PR? It seems to be anothoer way to solve the clone cost.