Change bytes encoding in JSON Encoder

[adambabik]

By default, jsonEncoder encodes bytes using base64. In my case, it would be much more useful if bytes are encoded using hex encoding.

I tried to build a wrapper around jsonEncoder:

type jsonHexEncoder struct {
	zapcore.Encoder
}

func NewJSONHexEncoder(cfg zapcore.EncoderConfig) zapcore.Encoder {
	jsonEncoder := zapcore.NewJSONEncoder(cfg)
	return &jsonHexEncoder{
		Encoder: jsonEncoder,
	}
}

func (enc *jsonHexEncoder) AddBinary(key string, val []byte) {
	enc.AddString(key, "0x"+hex.EncodeToString(val))
}

The problem is that not only Clone() needs to be overridden (this is easy) but also EncodeEntry() which is not really possible without copying a huge chunk of code.

I see two options:

• create a new field type zap.BinaryHex,
• configure bytes encoding in the NewJSONEncoder() constructor, or, for backward compatibility, provide a new one NewJSONHexEncoder() or something similar.

I would like to hear your opinion about this. Maybe there is a simpler solution I haven't noticed so far.

[jcorbin]

Of the two options you describe, adding a new zap.BinaryHex field is right out: separation of field definition from encoding is a primary design goal of zap.

For the 2nd option, this should be achievable by adding a new field to the existing zapcore.EncoderConfig to control binary-to-text encoding. If we ever end up adding a true binary encoder, it'd necessarily ignore that bit of config, but that also holds for all the other forms of encoder config, which really only apply to more-or-less textual encoders (e.g JSON).

Adding a new struct field, that defaults to base64 encoding, would be a backwards compatible change, that we should be able to review and merge as far asI can see.

cc @abhinav @prashantv

[adambabik]

Of the two options you describe, adding a new zap.BinaryHex field is right out: separation of field definition from encoding is a primary design goal of zap.

That makes sense.

Regarding the second option, that was my only concern that a new field in zapcore.EncoderConfig will be useless for some encoders but if you think that's not a big deal, it's a better proposal than a custom JSON encoder constructor because it's almost never used.

I can work on a PR if we decide that's a proper direction for this feature.

[jcorbin]

Regarding the second option, that was my only concern that a new field in zapcore.EncoderConfig will be useless for some encoders but if you think that's not a big deal, it's a better proposal than a custom JSON encoder constructor because it's almost never used.

That's already the case with some of its fields: if we ever write an encoder for something with a native encoding for times or durations, the "how to to text encode it" fields will be irrelevant to them (e.g. a mythical msgpack encoder); there may be a similar reasoning about any encoder with a native enum encoding and level, but that's more of a stretch.