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

embed: ZapLoggerBuilder no longer exposed #11144

Closed
ChrisRx opened this issue Sep 12, 2019 · 2 comments · Fixed by #11147
Closed

embed: ZapLoggerBuilder no longer exposed #11144

ChrisRx opened this issue Sep 12, 2019 · 2 comments · Fixed by #11147

Comments

@ChrisRx
Copy link
Contributor

ChrisRx commented Sep 12, 2019

348b0d4#diff-935426888c9eba579af569553d8e7959

In this commit the field ZapLoggerBuilder in the embed.Config struct is no longer being exposed. In my program where I make use of the etcd embed package, I was using this to create a custom zap logger and have all components of my program to use this logger and create the same output (it is logfmt format instead of json).

I was wondering if there would be any issue/concern with allowing this field to still be exposed so that the underlying zap logger can be replaced?

@gyuho
Copy link
Contributor

gyuho commented Sep 12, 2019

@ChrisRx The field was never released in an official release as a public field, so I just simplified the struct by not exposing it. We can revert that change. Would you like to contribute?

@ChrisRx
Copy link
Contributor Author

ChrisRx commented Sep 13, 2019

Yes I would like to, and thank you for the consideration here, it is a huge help!

ChrisRx added a commit to ChrisRx/etcd that referenced this issue Sep 13, 2019
This exposes the ZapLoggerBuilder in the embed.Config to allow for
custom loggers to be defined and used by embedded etcd.

Fixes etcd-io#11144
spzala pushed a commit to spzala/etcd that referenced this issue Sep 13, 2019
This exposes the ZapLoggerBuilder in the embed.Config to allow for
custom loggers to be defined and used by embedded etcd.

Fixes etcd-io#11144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants