From 2a0c960cd60c36fedcf90ebbdda41f715b21ebde Mon Sep 17 00:00:00 2001 From: Adam Harwayne Date: Wed, 1 May 2019 13:11:09 -0700 Subject: [PATCH 1/3] Remove unneeded log keys. knative.dev/controller is set by pkg/reconciler just before calling our reconciler, so adding a second one in main is redundant and produced entries with: { "knative.dev/controller": "controller", "knative.dev/controller": "trigger-controller", } The reconcile key is already saved as 'knative.dev/key' by pkg/reconciler, so we don't need to add 'key' as well. --- cmd/controller/main.go | 2 -- pkg/reconciler/eventtype/eventtype.go | 1 - pkg/reconciler/trigger/trigger.go | 2 -- pkg/reconciler/v1alpha1/broker/broker.go | 1 - 4 files changed, 6 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 37afc799c65..ffc61e60a64 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -40,7 +40,6 @@ import ( "github.com/knative/eventing/pkg/reconciler/v1alpha1/broker" "github.com/knative/pkg/configmap" kncontroller "github.com/knative/pkg/controller" - "github.com/knative/pkg/logging/logkey" "github.com/knative/pkg/signals" "go.uber.org/zap" controllerruntime "sigs.k8s.io/controller-runtime/pkg/client/config" @@ -57,7 +56,6 @@ func main() { logger, atomicLevel := setupLogger() defer logger.Sync() - logger = logger.With(zap.String(logkey.ControllerType, logconfig.Controller)) // set up signals so we handle the first shutdown signal gracefully stopCh := signals.SetupSignalHandler() diff --git a/pkg/reconciler/eventtype/eventtype.go b/pkg/reconciler/eventtype/eventtype.go index a1aa28113a8..0b614732133 100644 --- a/pkg/reconciler/eventtype/eventtype.go +++ b/pkg/reconciler/eventtype/eventtype.go @@ -106,7 +106,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { r.Logger.Errorf("invalid resource key: %s", key) return nil } - ctx = logging.WithLogger(ctx, r.Logger.Desugar().With(zap.String("key", key))) // Get the EventType resource with this namespace/name original, err := r.eventTypeLister.EventTypes(namespace).Get(name) diff --git a/pkg/reconciler/trigger/trigger.go b/pkg/reconciler/trigger/trigger.go index d046ce10a9f..ac6d9927dea 100644 --- a/pkg/reconciler/trigger/trigger.go +++ b/pkg/reconciler/trigger/trigger.go @@ -135,8 +135,6 @@ func NewController( // converge the two. It then updates the Status block of the Trigger resource // with the current status of the resource. func (r *Reconciler) Reconcile(ctx context.Context, key string) error { - ctx = logging.WithLogger(ctx, r.Logger.Desugar().With(zap.String("key", key))) - // Convert the namespace/name string into a distinct namespace and name. namespace, name, err := cache.SplitMetaNamespaceKey(key) if err != nil { diff --git a/pkg/reconciler/v1alpha1/broker/broker.go b/pkg/reconciler/v1alpha1/broker/broker.go index d0751b786e1..f9abad9183e 100644 --- a/pkg/reconciler/v1alpha1/broker/broker.go +++ b/pkg/reconciler/v1alpha1/broker/broker.go @@ -146,7 +146,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { r.Logger.Errorf("invalid resource key: %s", key) return nil } - ctx = logging.WithLogger(ctx, r.Logger.Desugar().With(zap.String("key", key))) // Get the Broker resource with this namespace/name original, err := r.brokerLister.Brokers(namespace).Get(name) From 8c1d90de22f7eb67c43db5f89451257c4bd9bedb Mon Sep 17 00:00:00 2001 From: Adam Harwayne Date: Wed, 1 May 2019 13:16:12 -0700 Subject: [PATCH 2/3] Remove explict adds of key. --- pkg/reconciler/channel/channel.go | 6 +++--- pkg/reconciler/namespace/namespace.go | 6 +++--- pkg/reconciler/subscription/subscription.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/reconciler/channel/channel.go b/pkg/reconciler/channel/channel.go index 9ea395ee854..4e1d10ee925 100644 --- a/pkg/reconciler/channel/channel.go +++ b/pkg/reconciler/channel/channel.go @@ -86,7 +86,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { original, err := r.channelLister.Channels(namespace).Get(name) if apierrs.IsNotFound(err) { // The resource may no longer exist, in which case we stop processing. - logging.FromContext(ctx).Error("channel key in work queue no longer exists", zap.Any("key", key)) + logging.FromContext(ctx).Error("channel key in work queue no longer exists") return nil } else if err != nil { return err @@ -106,12 +106,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { if err != nil { logging.FromContext(ctx).Warn("Error reconciling Channel", zap.Error(err)) } else { - logging.FromContext(ctx).Debug("Successfully reconciled Channel", zap.Any("key", key)) + logging.FromContext(ctx).Debug("Successfully reconciled Channel") r.Recorder.Eventf(channel, corev1.EventTypeNormal, channelReconciled, "Channel reconciled: %s", key) } if _, updateStatusErr := r.updateStatus(ctx, channel.DeepCopy()); updateStatusErr != nil { - logging.FromContext(ctx).Warn("Error updating Channel status", zap.Any("key", key), zap.Error(updateStatusErr)) + logging.FromContext(ctx).Warn("Error updating Channel status", zap.Error(updateStatusErr)) r.Recorder.Eventf(channel, corev1.EventTypeWarning, channelUpdateStatusFailed, "Failed to update channel status: %s", key) return updateStatusErr } diff --git a/pkg/reconciler/namespace/namespace.go b/pkg/reconciler/namespace/namespace.go index d696fb8d4f0..11df3a5e2b2 100644 --- a/pkg/reconciler/namespace/namespace.go +++ b/pkg/reconciler/namespace/namespace.go @@ -130,7 +130,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { original, err := r.namespaceLister.Get(name) if apierrs.IsNotFound(err) { // The resource may no longer exist, in which case we stop processing. - logging.FromContext(ctx).Error("namespace key in work queue no longer exists", zap.Any("key", key)) + logging.FromContext(ctx).Error("namespace key in work queue no longer exists") return nil } else if err != nil { return err @@ -149,9 +149,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { // whether the reconcile error out. err = r.reconcile(ctx, ns) if err != nil { - logging.FromContext(ctx).Error("Error reconciling Namespace", zap.Error(err), zap.Any("key", key)) + logging.FromContext(ctx).Error("Error reconciling Namespace", zap.Error(err)) } else { - logging.FromContext(ctx).Debug("Namespace reconciled", zap.Any("key", key)) + logging.FromContext(ctx).Debug("Namespace reconciled") } // Requeue if the resource is not ready: diff --git a/pkg/reconciler/subscription/subscription.go b/pkg/reconciler/subscription/subscription.go index 4444539cc8a..ba99c76b5f7 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -107,7 +107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { original, err := r.subscriptionLister.Subscriptions(namespace).Get(name) if apierrs.IsNotFound(err) { // The resource may no longer exist, in which case we stop processing. - logging.FromContext(ctx).Error("subscription key in work queue no longer exists", zap.Any("key", key)) + logging.FromContext(ctx).Error("subscription key in work queue no longer exists") return nil } else if err != nil { return err From 3bbbeb2a3f7327575101a69251f6c10d7c343bea Mon Sep 17 00:00:00 2001 From: Adam Harwayne Date: Wed, 1 May 2019 14:00:04 -0700 Subject: [PATCH 3/3] PR comments. --- pkg/reconciler/channel/channel.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/channel/channel.go b/pkg/reconciler/channel/channel.go index 4e1d10ee925..f437eb5cbfc 100644 --- a/pkg/reconciler/channel/channel.go +++ b/pkg/reconciler/channel/channel.go @@ -86,7 +86,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { original, err := r.channelLister.Channels(namespace).Get(name) if apierrs.IsNotFound(err) { // The resource may no longer exist, in which case we stop processing. - logging.FromContext(ctx).Error("channel key in work queue no longer exists") + logging.FromContext(ctx).Error("Channel key in work queue no longer exists") return nil } else if err != nil { return err @@ -104,7 +104,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { // updates regardless of whether the reconcile error out. err = r.reconcile(ctx, channel) if err != nil { - logging.FromContext(ctx).Warn("Error reconciling Channel", zap.Error(err)) + logging.FromContext(ctx).Error("Error reconciling Channel", zap.Error(err)) } else { logging.FromContext(ctx).Debug("Successfully reconciled Channel") r.Recorder.Eventf(channel, corev1.EventTypeNormal, channelReconciled, "Channel reconciled: %s", key)