From f05a79d2c1af510a58f2efdedb7bb4fda7425c43 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 16 Feb 2022 12:17:47 +0100 Subject: [PATCH] enhance and fix log calls Some of these changes are cosmetic (repeatedly calling klog.V instead of reusing the result), others address real issues: - Logging a message only above a certain verbosity threshold without recording that verbosity level (if klog.V().Enabled() { klog.Info... }): this matters when using a logging backend which records the verbosity level. - Passing a format string with parameters to a logging function that doesn't do string formatting. All of these locations where found by the enhanced logcheck tool from https://github.com/kubernetes/klog/pull/297. In some cases it reports false positives, but those can be suppressed with source code comments. Kubernetes-commit: edffc700a43e610f641907290a5152ca593bad79 --- gce/gce_loadbalancer_internal.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gce/gce_loadbalancer_internal.go b/gce/gce_loadbalancer_internal.go index 3fca2d76..b16e5888 100644 --- a/gce/gce_loadbalancer_internal.go +++ b/gce/gce_loadbalancer_internal.go @@ -31,7 +31,7 @@ import ( "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" "github.com/google/go-cmp/cmp" compute "google.golang.org/api/compute/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" cloudprovider "k8s.io/cloud-provider" @@ -202,9 +202,9 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v // Delete existing forwarding rule before making changes to the backend service. For example - changing protocol // of backend service without first deleting forwarding rule will throw an error since the linked forwarding // rule would show the old protocol. - if klog.V(2).Enabled() { + if klogV := klog.V(2); klogV.Enabled() { frDiff := cmp.Diff(existingFwdRule, newFwdRule) - klog.V(2).Infof("ensureInternalLoadBalancer(%v): forwarding rule changed - Existing - %+v\n, New - %+v\n, Diff(-existing, +new) - %s\n. Deleting existing forwarding rule.", loadBalancerName, existingFwdRule, newFwdRule, frDiff) + klogV.Infof("ensureInternalLoadBalancer(%v): forwarding rule changed - Existing - %+v\n, New - %+v\n, Diff(-existing, +new) - %s\n. Deleting existing forwarding rule.", loadBalancerName, existingFwdRule, newFwdRule, frDiff) } if err = ignoreNotFound(g.DeleteRegionForwardingRule(loadBalancerName, g.region)); err != nil { return nil, err