Repository: kubernetes/klog
Pull request #306 (2022-03-16 11:16:34)

SetLoggerWithOptions: support flushing
**What this PR does / why we need it**:

When setting a logger that buffers data in memory, we want klog.Flush to flush
that data. For that the caller has to provide an additional callback because
logr.Logger has no API for flushing.

**Special notes for your reviewer**:

Planned usage in Kubernetes: https://github.com/pohly/kubernetes/pull/new/klog-flush

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**Release note**:
```release-note
SetLoggerWithOptions: support flushing the logger through klog.Flush and klog.FlushAndExit
```

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/306#" title="Author self-approved">pohly</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **brancz** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @brancz` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fklog).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes/klog/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["brancz"]} -->
dims: /approve
/lgtm
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/306#issuecomment-1069015468" title="LGTM">dims</a>*, *<a href="https://github.com/kubernetes/klog/pull/306#" title="Author self-approved">pohly</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fklog).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes/klog/blob/main/OWNERS)~~ [dims]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
dims: `FAIL: k8s.io/klog/v2 contains incompatible changes:
- loggerOptions: old is comparable, new is not`
pohly: `loggerOptions` is not exported, why is this an API problem? I believe we can ignore this.

k8s-ci-robot: New changes are detected. LGTM label has been removed.
dims: merging this by hand per conversation on slack - https://kubernetes.slack.com/archives/CG3518SFJ/p1647431609619139

`Looks like false positive in apidiff to me.` from @pohly 

Patch:
diff --git a/contextual.go b/contextual.go
index 6749267..bb03808 100644
--- a/contextual.go
+++ b/contextual.go
@@ -42,6 +42,10 @@ var (
 	// none is available.
 	globalLogger *Logger
 
+	// globalLoggerOptions contains the options that were supplied for
+	// globalLogger.
+	globalLoggerOptions loggerOptions
+
 	// contextualLogger defines whether globalLogger may get called
 	// directly.
 	contextualLogger bool
@@ -87,11 +91,10 @@ func SetLogger(logger logr.Logger) {
 // later release.
 func SetLoggerWithOptions(logger logr.Logger, opts ...LoggerOption) {
 	globalLogger = &logger
-	var o loggerOptions
+	globalLoggerOptions = loggerOptions{}
 	for _, opt := range opts {
-		opt(&o)
+		opt(&globalLoggerOptions)
 	}
-	contextualLogger = o.contextualLogger
 }
 
 // ContextualLogger determines whether the logger passed to
@@ -108,6 +111,18 @@ func ContextualLogger(enabled bool) LoggerOption {
 	}
 }
 
+// FlushLogger provides a callback for flushing data buffered by the logger.
+//
+// Experimental
+//
+// Notice: This function is EXPERIMENTAL and may be changed or removed in a
+// later release.
+func FlushLogger(flush func()) LoggerOption {
+	return func(o *loggerOptions) {
+		o.flush = flush
+	}
+}
+
 // LoggerOption implements the functional parameter paradigm for
 // SetLoggerWithOptions.
 //
@@ -119,6 +134,7 @@ type LoggerOption func(o *loggerOptions)
 
 type loggerOptions struct {
 	contextualLogger bool
+	flush            func()
 }
 
 // SetContextualLogger does the same as SetLogger, but in addition the
@@ -137,6 +153,7 @@ func SetContextualLogger(logger logr.Logger) {
 // goroutines invoke log calls, usually during program initialization.
 func ClearLogger() {
 	globalLogger = nil
+	globalLoggerOptions = loggerOptions{}
 }
 
 // EnableContextualLogging controls whether contextual logging is enabled.
@@ -196,7 +213,9 @@ func TODO() Logger {
 // Notice: This function is EXPERIMENTAL and may be changed or removed in a
 // later release.
 func Background() Logger {
-	if globalLogger != nil && contextualLogger {
+	if globalLoggerOptions.contextualLogger {
+		// Is non-nil because globalLoggerOptions.contextualLogger is
+		// only true if a logger was set.
 		return *globalLogger
 	}
 
diff --git a/klog.go b/klog.go
index 7eae519..15710be 100644
--- a/klog.go
+++ b/klog.go
@@ -1097,6 +1097,9 @@ func (l *loggingT) flushAll() {
 			file.Sync()  // ignore error
 		}
 	}
+	if globalLoggerOptions.flush != nil {
+		globalLoggerOptions.flush()
+	}
 }
 
 // CopyStandardLogTo arranges for messages written to the Go "log" package's


Test patch:
diff --git a/contextual_test.go b/contextual_test.go
index cb58867..80e14fa 100644
--- a/contextual_test.go
+++ b/contextual_test.go
@@ -43,3 +43,16 @@ func ExampleSetLogger() {
 	// logger after SetLoggerWithOptions with ContextualLogger(false): *klog.klogger
 	// logger after SetLoggerWithOptions with ContextualLogger(true): logr.discardLogSink
 }
+
+func ExampleFlushLogger() {
+	defer klog.ClearLogger()
+
+	// This simple logger doesn't need flushing, but others might.
+	klog.SetLoggerWithOptions(logr.Discard(), klog.FlushLogger(func() {
+		fmt.Print("flushing...")
+	}))
+	klog.Flush()
+
+	// Output:
+	// flushing...
+}