🐛 Fix allowed levels for --zap-log-levels flag to align with logr levels

[bharathi-tenneti]

--Current implementation in flags.go has a bug, which is leading to incorrect zap log level to be set.
--This PR also focuses on aligning --zap-log-level with logr levels.
Example: --zap-log-level=2 should map to using increased verbosity logr.V(2).Info().
-- This PR also disables Sampling for debug levels with increased verbosity, as they will cause index out of bounds errors in sampling code.

Closes: #990

[k8s-ci-robot]

Hi @bharathi-tenneti. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[bharathi-tenneti]

/cc @joelanford @hasbro17

[bharathi-tenneti]

Please use unit test cases, to test this fix.

[hasbro17]

Need to clarify what --zap-stacktrace-level=disabled is for.
Plus some nits on the test cases.

[hasbro17]

/ok-to-test

[joelanford]

I think its technically okay to use sampling with zapcore.DebugLevel. It only needs to be disabled starting at zapcore.Level(-2).

Also, I don't think we want to force development mode based on the log level.

Given the above details, does the following work?

Suggested change

	if o.Level.Enabled(zapcore.DebugLevel) {
	o.ZapOpts = append(o.ZapOpts, zap.Development())
	} else {
	o.ZapOpts = append(o.ZapOpts,
	zap.WrapCore(func(core zapcore.Core) zapcore.Core {
	return zapcore.NewSampler(core, time.Second, 100, 100)
	}))
	}
	if !o.Level.Enabled(zapcore.Level(-2)) {
	o.ZapOpts = append(o.ZapOpts,
	zap.WrapCore(func(core zapcore.Core) zapcore.Core {
	return zapcore.NewSampler(core, time.Second, 100, 100)
	}))
	}

[bharathi-tenneti]

Wouldn't this only cover Level(-2) ,We still need to handle Level(-3) or below.

[bharathi-tenneti]

Agree , We can remove the developement mode, and it can be as shown below.

	// Disable sampling when we are in debug mode. Otherwise, this will
		// cause index out of bounds errors in the sampling code.
		if o.Level.Enabled(zapcore.DebugLevel) {
			o.ZapOpts = append(o.ZapOpts)
		} else {
			o.ZapOpts = append(o.ZapOpts,
				zap.WrapCore(func(core zapcore.Core) zapcore.Core {
					return zapcore.NewSampler(core, time.Second, 100, 100)
				}))
		}

[camilamacedo86]

It shows /lgtm /approve for me 👍

[bharathi-tenneti]

/retest

[vincepri]

/cc @joelanford @DirectXMan12

[joelanford]

/lgtm

[vincepri]

/approve
/milestone v0.6.x

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bharathi-tenneti, vincepri

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 [vincepri]

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

[vincepri]

/label tide/merge-method-squash

[bharathi-tenneti]

/retest