contextual logging: avoid extra SetContextualLogger API

We can avoid the variant of SetLogger by indicating support via an additional
interface that the LogSink must implement. This was seen as a cleaner
approach (#296 (comment)).

contextual.go

@@ -65,22 +65,54 @@ var (
 // To remove a backing logr implemention, use ClearLogger. Setting an
 // empty logger with SetLogger(logr.Logger{}) does not work.
 //
+// If the logger instance implements the ContextualLogger interface in addition
+// to logr.Logger, then the instance will also be called directly
+// by code that retrieves a logger with FromContext, TODO, or Background.
+// In that case the logger instance should do its own verbosity checks
+// in Enabled because klog will not do that for it.
+//
+// AddSupportsContext can be used for loggers which don't know about this
+// klog specific convention.
+//
 // Modifying the logger is not thread-safe and should be done while no other
 // goroutines invoke log calls, usually during program initialization.
 func SetLogger(logger logr.Logger) {
 	globalLogger = &logger
 	contextualLogger = false
+	if contextual, ok := logger.GetSink().(ContextualLogSink); ok && contextual.SupportsContext() {
+		contextualLogger = true
+	}
 }
 
-// SetContextualLogger does the same as SetLogger, but in addition the
-// logger may also get called directly by code that retrieves it
-// with FromContext, TODO or Background. The logger therefore must
-// implements its own verbosity checking.
-func SetContextualLogger(logger logr.Logger) {
-	globalLogger = &logger
-	contextualLogger = true
+// ContextualLogSink is an extension of the logr.LogSink interface that
+// indicates to SetLogger whether the logger may get called directly by
+// application code. Only the LogSink seen by SetLogger needs to implement
+// this, clones of it as they occur during logger.V, logger.WithName or
+// logger.WithValues don't need it.
+type ContextualLogSink interface {
+	logr.LogSink
+
+	// SupportsContext returns true if the logger may get called directly
+	// by application code.
+	SupportsContext() bool
+}
+
+// AddSupportsContext wraps a logger that doesn't know about ContextualLogSink
+// so that SetLogger knows that it can also be called directly.
+func AddSupportsContext(logger logr.Logger) logr.Logger {
+	return logr.New(contextualLogSinkWrapper{logger.GetSink()})
 }
 
+type contextualLogSinkWrapper struct {
+	logr.LogSink
+}
+
+func (c contextualLogSinkWrapper) SupportsContext() bool {
+	return true
+}
+
+var _ ContextualLogSink = contextualLogSinkWrapper{}
+
 // ClearLogger removes a backing Logger implementation if one was set earlier
 // with SetLogger.
 //

contextual_test.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2022 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 klog_test
+
+import (
+	"fmt"
+
+	"github.com/go-logr/logr"
+	"k8s.io/klog/v2"
+)
+
+func ExampleSetLogger() {
+	logger := logr.Discard()
+	klog.SetLogger(logger)
+	equal := logger == klog.Background()
+	fmt.Printf("original logr.Discard logger can be used directly: %v\n", equal)
+
+	logger = klog.AddSupportsContext(logger)
+	klog.SetLogger(logger)
+	equal = logger == klog.Background()
+	fmt.Printf("wrapped logger can be used directly: %v\n", equal)
+
+	// Output:
+	// original logr.Discard logger can be used directly: false
+	// wrapped logger can be used directly: true
+}