Skip to content

Commit

Permalink
Move logger into context (#208)
Browse files Browse the repository at this point in the history
The logger was previously available on the Config object as well as from
the context via `logr.FromContext(ctx)`. While they are the same root
logger, each has a different set of names and values applied to each log
line.

Loading the logger from the context is the preferred approach moving
forward. The logger on the Config is still available, but is deprecated
and will be removed in a future release. The first use of the `c.Log`
methods will trigger an error printed to the log.

Each reconciler in the call chain can enrich the logger as it is passed
deeper in the reconciler hierarchy. Many reconciler have a new `Name`
field that is appended to the logger's name for all log statements under
it. `ParentReconciler`, `ChildReconciler` and `CastParent` provide
default names based on the type they are reconciling. Unique names are
recommended, but are not required, since they only appear in logs and do
not affect the runtime behavior.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>
  • Loading branch information
scothis committed Apr 18, 2022
1 parent d3021c6 commit 5b2e5a8
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 57 deletions.
32 changes: 14 additions & 18 deletions README.md
Expand Up @@ -52,9 +52,8 @@ Parent reconcilers tend to be quite simple, as they delegate their work to sub r

```go
func FunctionReconciler(c reconcilers.Config) *reconcilers.ParentReconciler {
c.Log = c.Log.WithName("Function")

return &reconcilers.ParentReconciler{
Name: "Function",
Type: &buildv1alpha1.Function{},
Reconciler: reconcilers.Sequence{
FunctionTargetImageReconciler(c),
Expand All @@ -81,10 +80,11 @@ While sync reconcilers have the ability to do anything a reconciler can do, it's

```go
func FunctionTargetImageReconciler(c reconcilers.Config) reconcilers.SubReconciler {
c.Log = c.Log.WithName("TargetImage")

return &reconcilers.SyncReconciler{
Name: "TargetImage",
Sync: func(ctx context.Context, parent *buildv1alpha1.Function) error {
log := logr.FromContextOrDiscard(ctx)

targetImage, err := resolveTargetImage(ctx, c.Client, parent)
if err != nil {
return err
Expand Down Expand Up @@ -125,9 +125,8 @@ Now it's time to create the child Image resource that will do the work of buildi

```go
func FunctionChildImageReconciler(c reconcilers.Config) reconcilers.SubReconciler {
c.Log = c.Log.WithName("ChildImage")

return &reconcilers.ChildReconciler{
Name: "ChildImage",
ChildType: &kpackbuildv1alpha1.Image{},
ChildListType: &kpackbuildv1alpha1.ImageList{},

Expand Down Expand Up @@ -214,9 +213,8 @@ JSON encoding is used as the intermediate representation. Operations on a cast p

```go
func FunctionReconciler(c reconcilers.Config) *reconcilers.ParentReconciler {
c.Log = c.Log.WithName("Function")

return &reconcilers.ParentReconciler{
Name: "Function",
Type: &buildv1alpha1.Function{},
Reconciler: reconcilers.Sequence{
&reconcilers.CastParent{
Expand Down Expand Up @@ -247,9 +245,8 @@ A Sequence is commonly used in a ParentReconciler, but may be used anywhere a Su

```go
func FunctionReconciler(c reconcilers.Config) *reconcilers.ParentReconciler {
c.Log = c.Log.WithName("Function")

return &reconcilers.ParentReconciler{
Name: "Function",
Type: &buildv1alpha1.Function{},
Reconciler: reconcilers.Sequence{
FunctionTargetImageReconciler(c),
Expand Down Expand Up @@ -391,9 +388,8 @@ The stash allows passing arbitrary state between sub reconcilers within the scop
const exampleStashKey reconcilers.StashKey = "example"

func StashExampleSubReconciler(c reconcilers.Config) reconcilers.SubReconciler {
c.Log = c.Log.WithName("StashExample")

return &reconcilers.SyncReconciler{
Name: "StashExample",
Sync: func(ctx context.Context, resource *examplev1.MyExample) error {
value := Example{} // something we want to expose to a sub reconciler later in this chain
reconcilers.StashValue(ctx, exampleStashKey, *value)
Expand All @@ -406,9 +402,8 @@ func StashExampleSubReconciler(c reconcilers.Config) reconcilers.SubReconciler {


func StashExampleSubReconciler(c reconcilers.Config) reconcilers.SubReconciler {
c.Log = c.Log.WithName("StashExample")

return &reconcilers.SyncReconciler{
Name: "StashExample",
Sync: func(ctx context.Context, resource *examplev1.MyExample) error {
value, ok := reconcilers.RetrieveValue(ctx, exampleStashKey).(Example)
if !ok {
Expand All @@ -432,10 +427,11 @@ The stream gateways in projectriff fetch the image references they use to run fr

```go
func InMemoryGatewaySyncConfigReconciler(c reconcilers.Config, namespace string) reconcilers.SubReconciler {
c.Log = c.Log.WithName("SyncConfig")

return &reconcilers.SyncReconciler{
Name: "SyncConfig",
Sync: func(ctx context.Context, parent *streamingv1alpha1.InMemoryGateway) error {
log := logr.FromContextOrDiscard(ctx)

var config corev1.ConfigMap
key := types.NamespacedName{Namespace: namespace, Name: inmemoryGatewayImages}
// track config for new images
Expand All @@ -456,13 +452,13 @@ func InMemoryGatewaySyncConfigReconciler(c reconcilers.Config, namespace string)
},

Config: c,
Setup: func(mgr reconcilers.Manager, bldr *reconcilers.Builder) error {
Setup: func(ctx context.Context, mgr reconcilers.Manager, bldr *reconcilers.Builder) error {
// enqueue the tracking resource for reconciliation from changes to
// tracked ConfigMaps. Internally `EnqueueTracked` sets up an
// Informer to watch to changes of the target resource. When the
// informer emits an event, the tracking resources are looked up
// from the tracker and enqueded for reconciliation.
bldr.Watches(&source.Kind{Type: &corev1.ConfigMap{}}, reconcilers.EnqueueTracked(&corev1.ConfigMap{}, c.Tracker, c.Scheme))
bldr.Watches(&source.Kind{Type: &corev1.ConfigMap{}}, reconcilers.EnqueueTracked(ctx, &corev1.ConfigMap{}, c.Tracker, c.Scheme))
return nil
},
}
Expand Down
6 changes: 4 additions & 2 deletions reconcilers/enqueuer.go
Expand Up @@ -6,6 +6,8 @@ SPDX-License-Identifier: Apache-2.0
package reconcilers

import (
"context"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -15,7 +17,7 @@ import (
"github.com/vmware-labs/reconciler-runtime/tracker"
)

func EnqueueTracked(by client.Object, t tracker.Tracker, s *runtime.Scheme) handler.EventHandler {
func EnqueueTracked(ctx context.Context, by client.Object, t tracker.Tracker, s *runtime.Scheme) handler.EventHandler {
return handler.EnqueueRequestsFromMapFunc(
func(a client.Object) []reconcile.Request {
var requests []reconcile.Request
Expand All @@ -29,7 +31,7 @@ func EnqueueTracked(by client.Object, t tracker.Tracker, s *runtime.Scheme) hand
gvks[0],
types.NamespacedName{Namespace: a.GetNamespace(), Name: a.GetName()},
)
for _, item := range t.Lookup(key) {
for _, item := range t.Lookup(ctx, key) {
requests = append(requests, reconcile.Request{NamespacedName: item})
}

Expand Down
68 changes: 68 additions & 0 deletions reconcilers/logger.go
@@ -0,0 +1,68 @@
/*
Copyright 2022 VMware, Inc.
SPDX-License-Identifier: Apache-2.0
*/

package reconcilers

import (
"fmt"
"sync"

"github.com/go-logr/logr"
)

var (
_ logr.LogSink = (*warnOnceLogSink)(nil)
)

// Deprecated
func newWarnOnceLogger(log logr.Logger) logr.Logger {
return logr.New(&warnOnceLogSink{
sink: log.GetSink(),
})
}

// Deprecated
type warnOnceLogSink struct {
sink logr.LogSink
once sync.Once
}

func (s *warnOnceLogSink) Init(info logr.RuntimeInfo) {
s.sink.Init(info)
}

func (s *warnOnceLogSink) Enabled(level int) bool {
return s.sink.Enabled(level)
}

func (s *warnOnceLogSink) Info(level int, msg string, keysAndValues ...interface{}) {
s.warn()
s.sink.Info(level, msg, keysAndValues...)
}

func (s *warnOnceLogSink) Error(err error, msg string, keysAndValues ...interface{}) {
s.warn()
s.sink.Error(err, msg, keysAndValues...)
}

func (s *warnOnceLogSink) WithValues(keysAndValues ...interface{}) logr.LogSink {
return &warnOnceLogSink{
sink: s.sink.WithValues(keysAndValues...),
once: s.once,
}
}

func (s *warnOnceLogSink) WithName(name string) logr.LogSink {
return &warnOnceLogSink{
sink: s.sink.WithName(name),
once: s.once,
}
}

func (s *warnOnceLogSink) warn() {
s.once.Do(func() {
s.sink.Error(fmt.Errorf("Config.Log is deprecated"), "use a logger from the context: `log := logr.FromContext(ctx)`")
})
}

0 comments on commit 5b2e5a8

Please sign in to comment.