✨ Add helpers for remaining Zap options

[hasbro17]

This PR adds predefined functional options for more easily setting the logger options in pkg/log/zap.

Follow up to #560 per #560 (comment)

/cc @DirectXMan12 @alvaroaleman

[k8s-ci-robot]

Hi @hasbro17. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[hasbro17]

This covers the all the provided options but I was wondering if we should expose more predefined options for the EncoderConfig?
I guess you can still set that via the Encoder option.

import (
	"os"

	uzap "go.uber.org/zap"
	"go.uber.org/zap/zapcore"
	"sigs.k8s.io/controller-runtime/pkg/log/zap"
	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
)

var log = logf.Log.WithName("cmd")

func main() {
	ecfg := uzap.NewDevelopmentEncoderConfig()
	ecfg.EncodeTime = zapcore.EpochTimeEncoder
	ecfg.EncodeLevel = zapcore.LowercaseLevelEncoder
	encoder := zapcore.NewConsoleEncoder(ecfg)

	zapLogr := zap.New(zap.Development(true), zap.DestWritter(os.Stdout), zap.Encoder(encoder))
	logf.SetLogger(zapLogr)

	log.Info("Test message", "value", 1)

}

And having options for both Encoder and EncoderConfig might be confusing if you could set two different configs via both options.

[hasbro17]

@alvaroaleman I also just realized that the destination writer option is named DestWritter

controller-runtime/pkg/log/zap/zap.go

Line 97 in 16c93b0

DestWritter io.Writer

Is that supposed to be with a single T DestWriter? I could change that, but that might be a breaking change.

[alvaroaleman]

/ok-to-test

Is that supposed to be with a single T DestWriter? I could change that, but that might be a breaking change.

Yes, seems there is a typo. Please change it, I guess Solly will defer changing that to a new release anyways.

[alvaroaleman]

Should we probably mention the defaults in the godocs for the new funcs?

[hasbro17]

We have them defined already for the Option fields so I wasn't sure if I should repeat those here again.

controller-runtime/pkg/log/zap/zap.go

Lines 93 to 94 in 16c93b0

	// The encoder to use, defaults to console when Development is true
	// and JSON otherwise

[alvaroaleman]

Maybe RawZapOpts to avoid the linter error?

[hasbro17]

Didn't see #646 get merged which has the first two options. I'll rebase this to only add the remaining option helpers.

[hasbro17]

@DirectXMan12 Did you mean to fix this?
#646 (comment)

[joelanford]

@hasbro17 I noticed this as well, and fixed it and added tests in #653.

[alvaroaleman]

/lgtm
/assign @DirectXMan12

[hasbro17]

PTAL @mengqiy or @droot if you get a chance.

There's a small overlap with #653 so we can rebase either PR depending on what gets merged first.

[DirectXMan12]

minor nit, otherwise fine

[DirectXMan12]

nit: make this variadic

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: hasbro17

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mengqiy]

/lgtm