azure-container-networking icon indicating copy to clipboard operation
azure-container-networking copied to clipboard

implement ObjectEncoder

Open ZetaoZhuang opened this issue 2 years ago • 2 comments

feat: support zap.Object in zapai

Reason for Change:

currently the zapai evaluates a zapcore.Field into a best-effort string, per the fieldStringer , this bring some troubles to log the zap object. Ex, while logging zap.Object("ex1", &ex1), the result looks like this "ex1":"&{ex1 2 0 0xc0000ef880}" instead of logging the actual object fields ,so I would like to make zapai support zap.Object by implementing the ObjectEncoder interface, and leverage ObjectMarshaler to help adding object fields into logging context. However the appinsights.TraceTelemetry.BaseTelemetry only take (string: string) pair as custom properties,, that is why I only implement the AddObject and AddString in the interface for now. But I can still implement the rest of methods and convert those non-string and non-object type into a best-effort string.

Requirements:

Notes:

ZetaoZhuang avatar Aug 11 '22 23:08 ZetaoZhuang

@ZetaoZhuang you need to provide much more context in the description on the why and how of this change. It looks like you're polluting this interface with a bunch of unimplemented methods - I don't understand what the point is, and you're going to need to convince me this is the best way to do it.

rbtr avatar Aug 11 '22 23:08 rbtr

@rbtr I updated the description with more contexts. Let me know if you have any feedbacks. Thanks

ZetaoZhuang avatar Aug 12 '22 17:08 ZetaoZhuang