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

Support logr.Marshaler and fmt.Stringer #4

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Support logr.Marshaler and fmt.Stringer #4

merged 1 commit into from
Oct 21, 2021

Conversation

hn8
Copy link
Contributor

@hn8 hn8 commented Sep 28, 2021

No description provided.

@hn8 hn8 marked this pull request as draft September 28, 2021 14:28
@hn8
Copy link
Contributor Author

hn8 commented Sep 28, 2021

@pohly @thockin Just an idea to support logr.Marshaler for zerologr. Users could use WithMarshaler() to optionally enable this feature with minor extra overhead. Since logr passes in interface{} variadic spread, hopefully safe to mutate interface slice shallowly in MapFilter function.

zerologr.go Outdated
return WithTransformer(l, Transformer)
}

func WithTransformer(l Logger, tf TransformerFunc) Logger {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on this particular pattern - it's going to require multiple calls to set up.

What about an options struct with a field to "support standard transformations"?

Or what about not making it an option? How much does this hurt critical benchmarks? I considered it important enough to default to "on" for funcr.

@hn8
Copy link
Contributor Author

hn8 commented Oct 1, 2021

@thockin Changed to enable standard transformations by default. Still allow global customization since users may prefer json.Marshaler (current behavior) to fmt.Stringer (new behavior) or need key renaming and value reshaping.

@hn8 hn8 changed the title WIP: support logr.Marshaler Support logr.Marshaler and fmt.Stringer Oct 20, 2021
@hn8 hn8 requested a review from thockin October 20, 2021 23:00
zerologr.go Show resolved Hide resolved
zerologr.go Show resolved Hide resolved
@thockin thockin merged commit 5a64b15 into master Oct 21, 2021
@hn8 hn8 deleted the wip branch October 21, 2021 16:17
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.

2 participants