Repository: kubernetes/klog (go)

Issue:
textlogger: don't leak internal VState in the Config API
/kind bug

**What steps did you take and what happened:**

Config embeds VState, which makes VState methods `V` and `VModule` accessible. This was intentional because it enables verbosity level changes at runtime. However, it's not a good solution because the internal packages should be completely hidden.

**What did you expect to happen:**

Config should define its own API for v and vmodule changes. This might even be the same as in VState.

Advantages:
- more discoverable
- proper layering


Pull request:
ktesting + textlogger config api
**What this PR does / why we need it**:

Both loggers should have a well-documented, stable API for making runtime changes to the verbosity.

**Which issue(s) this PR fixes**:
Fixes #367 

**Special notes for your reviewer**:

This is an API break, but in a part of the code base that is still marked as experimental.

**Release note**:
```release-note
NONE
```

Patch:
diff --git a/textlogger/options.go b/textlogger/options.go
index 5803cce..db4cbeb 100644
--- a/textlogger/options.go
+++ b/textlogger/options.go
@@ -36,8 +36,22 @@ import (
 // Notice: This type is EXPERIMENTAL and may be changed or removed in a
 // later release.
 type Config struct {
-	*verbosity.VState
-	co configOptions
+	vstate *verbosity.VState
+	co     configOptions
+}
+
+// Verbosity returns a value instance that can be used to query (via String) or
+// modify (via Set) the verbosity threshold. This is thread-safe and can be
+// done at runtime.
+func (c *Config) Verbosity() flag.Value {
+	return c.vstate.V()
+}
+
+// VModule returns a value instance that can be used to query (via String) or
+// modify (via Set) the vmodule settings. This is thread-safe and can be done
+// at runtime.
+func (c *Config) VModule() flag.Value {
+	return c.vstate.VModule()
 }
 
 // ConfigOption implements functional parameters for NewConfig.
@@ -111,7 +125,7 @@ func Output(output io.Writer) ConfigOption {
 // later release.
 func NewConfig(opts ...ConfigOption) *Config {
 	c := &Config{
-		VState: verbosity.New(),
+		vstate: verbosity.New(),
 		co: configOptions{
 			verbosityFlagName: "v",
 			vmoduleFlagName:   "vmodule",
@@ -123,7 +137,7 @@ func NewConfig(opts ...ConfigOption) *Config {
 		opt(&c.co)
 	}
 
-	c.V().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10))
+	c.Verbosity().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10))
 	return c
 }
 
@@ -134,6 +148,6 @@ func NewConfig(opts ...ConfigOption) *Config {
 // Notice: This function is EXPERIMENTAL and may be changed or removed in a
 // later release.
 func (c *Config) AddFlags(fs *flag.FlagSet) {
-	fs.Var(c.V(), c.co.verbosityFlagName, "number for the log level verbosity of the testing logger")
+	fs.Var(c.Verbosity(), c.co.verbosityFlagName, "number for the log level verbosity of the testing logger")
 	fs.Var(c.VModule(), c.co.vmoduleFlagName, "comma-separated list of pattern=N log level settings for files matching the patterns")
 }
diff --git a/textlogger/textlogger.go b/textlogger/textlogger.go
index be946e0..87029c2 100644
--- a/textlogger/textlogger.go
+++ b/textlogger/textlogger.go
@@ -50,6 +50,9 @@ var (
 
 // NewLogger constructs a new logger.
 //
+// Verbosity can be modified at any time through the Config.V and
+// Config.VModule API.
+//
 // # Experimental
 //
 // Notice: This function is EXPERIMENTAL and may be changed or removed in a
@@ -82,7 +85,7 @@ func (l *tlogger) WithCallDepth(depth int) logr.LogSink {
 func (l *tlogger) Enabled(level int) bool {
 	// Skip this function and the Logger.Info call, then
 	// also any additional stack frames from WithCallDepth.
-	return l.config.Enabled(verbosity.Level(level), 2+l.callDepth)
+	return l.config.vstate.Enabled(verbosity.Level(level), 2+l.callDepth)
 }
 
 func (l *tlogger) Info(level int, msg string, kvList ...interface{}) {


Test patch:
diff --git a/ktesting/example_test.go b/ktesting/example_test.go
index 3fc688b..9d88793 100644
--- a/ktesting/example_test.go
+++ b/ktesting/example_test.go
@@ -98,3 +98,26 @@ func ExampleNewLogger() {
 	// E...] I failed err="failure" what="something" data={field:1}
 	// I...] Logged at level 5.
 }
+
+func ExampleConfig_Verbosity() {
+	var buffer ktesting.BufferTL
+	config := ktesting.NewConfig(ktesting.Verbosity(1))
+	logger := ktesting.NewLogger(&buffer, config)
+
+	logger.Info("initial verbosity", "v", config.Verbosity().String())
+	logger.V(2).Info("now you don't see me")
+	if err := config.Verbosity().Set("2"); err != nil {
+		logger.Error(err, "setting verbosity to 2")
+	}
+	logger.V(2).Info("now you see me")
+	if err := config.Verbosity().Set("1"); err != nil {
+		logger.Error(err, "setting verbosity to 1")
+	}
+	logger.V(2).Info("now I'm gone again")
+
+	fmt.Print(headerRe.ReplaceAllString(buffer.String(), "${1}...] "))
+
+	// Output:
+	// I...] initial verbosity v="1"
+	// I...] now you see me
+}
diff --git a/ktesting/options.go b/ktesting/options.go
index c9bb3da..d039c40 100644
--- a/ktesting/options.go
+++ b/ktesting/options.go
@@ -39,6 +39,20 @@ type Config struct {
 	co     configOptions
 }
 
+// Verbosity returns a value instance that can be used to query (via String) or
+// modify (via Set) the verbosity threshold. This is thread-safe and can be
+// done at runtime.
+func (c *Config) Verbosity() flag.Value {
+	return c.vstate.V()
+}
+
+// VModule returns a value instance that can be used to query (via String) or
+// modify (via Set) the vmodule settings. This is thread-safe and can be done
+// at runtime.
+func (c *Config) VModule() flag.Value {
+	return c.vstate.VModule()
+}
+
 // ConfigOption implements functional parameters for NewConfig.
 //
 // # Experimental
diff --git a/ktesting/testinglogger.go b/ktesting/testinglogger.go
index 434b153..14a22bf 100644
--- a/ktesting/testinglogger.go
+++ b/ktesting/testinglogger.go
@@ -107,6 +107,9 @@ var _ TL = &BufferTL{}
 // that output will be printed via the global klog logger with
 // `<test name> leaked goroutine` as prefix.
 //
+// Verbosity can be modified at any time through the Config.V and
+// Config.VModule API.
+//
 // # Experimental
 //
 // Notice: This type is EXPERIMENTAL and may be changed or removed in a
diff --git a/textlogger/example_test.go b/textlogger/example_test.go
new file mode 100644
index 0000000..9038d2b
--- /dev/null
+++ b/textlogger/example_test.go
@@ -0,0 +1,50 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package textlogger_test
+
+import (
+	"bytes"
+	"fmt"
+	"regexp"
+
+	"k8s.io/klog/v2/textlogger"
+)
+
+var headerRe = regexp.MustCompile(`([IE])[[:digit:]]{4} [[:digit:]]{2}:[[:digit:]]{2}:[[:digit:]]{2}\.[[:digit:]]{6}[[:space:]]+[[:digit:]]+ example_test.go:[[:digit:]]+\] `)
+
+func ExampleConfig_Verbosity() {
+	var buffer bytes.Buffer
+	config := textlogger.NewConfig(textlogger.Verbosity(1), textlogger.Output(&buffer))
+	logger := textlogger.NewLogger(config)
+
+	logger.Info("initial verbosity", "v", config.Verbosity().String())
+	logger.V(2).Info("now you don't see me")
+	if err := config.Verbosity().Set("2"); err != nil {
+		logger.Error(err, "setting verbosity to 2")
+	}
+	logger.V(2).Info("now you see me")
+	if err := config.Verbosity().Set("1"); err != nil {
+		logger.Error(err, "setting verbosity to 1")
+	}
+	logger.V(2).Info("now I'm gone again")
+
+	fmt.Print(headerRe.ReplaceAllString(buffer.String(), "${1}...] "))
+
+	// Output:
+	// I...] "initial verbosity" v="1"
+	// I...] "now you see me"
+}