Skip to content

Conversation

@ZetaoZhuang
Copy link
Contributor

@ZetaoZhuang ZetaoZhuang commented Aug 11, 2022

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 ZetaoZhuang requested a review from rbtr as a code owner August 11, 2022 23:08
@ZetaoZhuang ZetaoZhuang self-assigned this Aug 11, 2022
@rbtr
Copy link
Collaborator

rbtr commented Aug 11, 2022

@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.

@ZetaoZhuang
Copy link
Contributor Author

ZetaoZhuang commented Aug 12, 2022

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

@ZetaoZhuang ZetaoZhuang merged commit bd236e7 into master Sep 2, 2022
@ZetaoZhuang ZetaoZhuang deleted the support_zap_object branch September 2, 2022 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants