Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support AppendNil method for zapcore.ArrayEncoder #630

Closed
tamayika opened this issue Sep 2, 2018 · 3 comments
Closed

Add support AppendNil method for zapcore.ArrayEncoder #630

tamayika opened this issue Sep 2, 2018 · 3 comments

Comments

@tamayika
Copy link

tamayika commented Sep 2, 2018

I'm currently using custom marshaller and noticed that zapcore.ArrayEncoder does not support to append nil value.
Please see below example.

package main

import (
	"go.uber.org/zap"
	"go.uber.org/zap/zapcore"
)

type user struct {
	name string
	age  int
}

type users []*user

func (u *user) MarshalLogObject(enc zapcore.ObjectEncoder) error {
	enc.AddString("name", u.name)
	enc.AddInt("age", u.age)
	return nil
}

func (us users) MarshalLogArray(enc zapcore.ArrayEncoder) error {
	for _, u := range us {
		enc.AppendObject(u)
	}
	return nil
}

func main() {
	logger := zap.NewExample()
	defer logger.Sync()
	var users users = []*user{
		&user{"hoge", 20},
		nil,
	}
	logger.Info("users log",
		zap.Array("users", users),
	)
}

This example crashes at enc.AddString("name", u.name).

If zapcore.ArrayEncoder has AppendNil method, I can write as below

package main

import (
	"go.uber.org/zap"
	"go.uber.org/zap/zapcore"
)

type user struct {
	name string
	age  int
}

type users []*user

func (u *user) MarshalLogObject(enc zapcore.ObjectEncoder) error {
	enc.AddString("name", u.name)
	enc.AddInt("age", u.age)
	return nil
}

func (us users) MarshalLogArray(enc zapcore.ArrayEncoder) error {
	for _, u := range us {
		if u != nil {
			enc.AppendObject(u)
		} else {
			enc.AppendNil()
		}
	}
	return nil
}

func main() {
	logger := zap.NewExample()
	defer logger.Sync()
	var users users = []*user{
		&user{"hoge", 20},
		nil,
	}
	logger.Info("users log",
		zap.Array("users", users),
	)
}

Expected log is

{"level":"info","msg":"users log","users":[{"name":"hoge","age":20},null]}
@akshayjshah
Copy link
Contributor

Definitely an oversight in the zapcore package 😿

Unfortunately, adding this method to the ArrayMarshaler interface is a breaking change - it'll immediately stop all third-party implementations of ArrayMarshaler from compiling. I've added it to the 2.0 wishlist in #388, though.

Today, I know of two workarounds for this. Internally, we usually don't log null values - in your second example, you'd just skip logging nil users. If that doesn't work for you, you can use the AppendReflected method to add a nil. For JSON, serializing nil with reflection is pretty fast - it shouldn't impact performance much.

@tamayika
Copy link
Author

Thank you for the reply.
I confirmed AppendReflected works as I expected.
I'm looking forward to 2.0 release!

@akshayjshah
Copy link
Contributor

Great, glad that worked. I'm going to close this issue.

I wouldn't hold your breath waiting for a 2.0 - we're collecting issues, but we may never cut a new major version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants