Refactors logging to use the zapr implementation of go-logr/logr

Author: danehans

Gopkg.lock

@@ -7,13 +7,12 @@ import (
 
 	"github.com/openshift/cluster-ingress-operator/pkg/dns"
 	awsdns "github.com/openshift/cluster-ingress-operator/pkg/dns/aws"
+	logf "github.com/openshift/cluster-ingress-operator/pkg/log"
 	"github.com/openshift/cluster-ingress-operator/pkg/operator"
 	operatorconfig "github.com/openshift/cluster-ingress-operator/pkg/operator/config"
 
 	configv1 "github.com/openshift/api/config/v1"
 
-	"github.com/sirupsen/logrus"
-
 	corev1 "k8s.io/api/core/v1"
 
 	"k8s.io/apimachinery/pkg/types"
@@ -32,54 +31,64 @@ const (
 	cloudCredentialsSecretName = "cloud-credentials"
 )
 
+var log = logf.Logger.WithName("entrypoint")
+
 func main() {
 	// Get a kube client.
 	kubeConfig, err := config.GetConfig()
 	if err != nil {
-		logrus.Fatalf("failed to get kube config: %v", err)
+		log.Error(err, "failed to get kube config")
+		os.Exit(1)
 	}
 	kubeClient, err := operator.Client(kubeConfig)
 	if err != nil {
-		logrus.Fatalf("failed to create kube client: %v", err)
+		log.Error(err, "failed to create kube client")
+		os.Exit(1)
 	}
 
 	// Collect operator configuration.
 	operatorNamespace, ok := os.LookupEnv("WATCH_NAMESPACE")
 	if !ok {
-		logrus.Fatalf("WATCH_NAMESPACE environment variable is required")
+		log.Error(fmt.Errorf("missing environment variable"), "'WATCH_NAMESPACE' environment variable must be set")
+		os.Exit(1)
 	}
 	routerImage := os.Getenv("IMAGE")
 	if len(routerImage) == 0 {
-		logrus.Fatalf("IMAGE environment variable is required")
+		log.Error(fmt.Errorf("missing environment variable"), "'IMAGE' environment variable must be set")
+		os.Exit(1)
 	}
 
 	// Retrieve the cluster infrastructure config.
 	infraConfig := &configv1.Infrastructure{}
 	err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infraConfig)
 	if err != nil {
-		logrus.Fatalf("failed to get infrastructure 'config': %v", err)
+		log.Error(err, "failed to get infrastructure 'config'")
+		os.Exit(1)
 	}
 
 	// Retrieve the cluster ingress config.
 	ingressConfig := &configv1.Ingress{}
 	err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, ingressConfig)
 	if err != nil {
-		logrus.Fatalf("failed to get ingress 'cluster': %v", err)
+		log.Error(err, "failed to get ingress 'cluster'")
+		os.Exit(1)
 	}
 	if len(ingressConfig.Spec.Domain) == 0 {
-		logrus.Warnln("cluster ingress configuration has an empty domain; default ClusterIngress will have empty ingressDomain")
+		log.Info("cluster ingress configuration has an empty domain; default ClusterIngress will have empty ingressDomain")
 	}
 
 	dnsConfig := &configv1.DNS{}
 	err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, dnsConfig)
 	if err != nil {
-		logrus.Fatalf("failed to get dns 'cluster': %v", err)
+		log.Error(err, "failed to get dns 'cluster'")
+		os.Exit(1)
 	}
 
 	// Set up the DNS manager.
 	dnsManager, err := createDNSManager(kubeClient, operatorNamespace, infraConfig, dnsConfig)
 	if err != nil {
-		logrus.Fatalf("failed to create DNS manager: %v", err)
+		log.Error(err, "failed to create DNS manager")
+		os.Exit(1)
 	}
 
 	// Set up and start the operator.
@@ -91,10 +100,12 @@ func main() {
 	}
 	op, err := operator.New(operatorConfig, dnsManager, kubeConfig)
 	if err != nil {
-		logrus.Fatalf("failed to create operator: %v", err)
+		log.Error(err, "failed to create operator")
+		os.Exit(1)
 	}
 	if err := op.Start(signals.SetupSignalHandler()); err != nil {
-		logrus.Fatal(err)
+		log.Error(err, "failed to start operator")
+		os.Exit(1)
 	}
 }
 

cmd/cluster-ingress-operator/main.go

@@ -6,9 +6,8 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/openshift/cluster-ingress-operator/pkg/dns"
+	logf "github.com/openshift/cluster-ingress-operator/pkg/log"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/arn"
@@ -25,7 +24,10 @@ import (
 	configv1 "github.com/openshift/api/config/v1"
 )
 
-var _ dns.Manager = &Manager{}
+var (
+	_   dns.Manager = &Manager{}
+	log             = logf.Logger.WithName("aws-dns-manager")
+)
 
 // Manager provides AWS DNS record management. In this implementation, calling
 // Ensure will create records in any zone specified in the DNS configuration.
@@ -79,15 +81,15 @@ func NewManager(config Config) (*Manager, error) {
 
 	region := aws.StringValue(sess.Config.Region)
 	if len(region) > 0 {
-		logrus.Infof("using region %q from shared config", region)
+		log.Info("using region from shared config", "region name", region)
 	} else {
 		metadata := ec2metadata.New(sess)
 		discovered, err := metadata.Region()
 		if err != nil {
 			return nil, fmt.Errorf("couldn't get region from metadata: %v", err)
 		}
 		region = discovered
-		logrus.Infof("discovered region %q from metadata", region)
+		log.Info("discovered region from metadata", "region name", region)
 	}
 
 	return &Manager{
@@ -167,7 +169,7 @@ func (m *Manager) getZoneID(zoneConfig configv1.DNSZone) (string, error) {
 
 	// Update the cache
 	m.idsToTags[id] = zoneConfig.Tags
-	logrus.Infof("found zone id %q for tags %v", id, zoneConfig.Tags)
+	log.Info("found hosted zone using tags", "zone id", id, "tags", zoneConfig.Tags)
 
 	return id, nil
 }
@@ -218,7 +220,7 @@ func (m *Manager) change(record *dns.Record, action action) error {
 		return fmt.Errorf("failed to describe load balancers: %v", err)
 	}
 	for _, lb := range loadBalancers.LoadBalancerDescriptions {
-		logrus.Debugf("found load balancer: %s (DNS name: %s, zone: %s)", aws.StringValue(lb.LoadBalancerName), aws.StringValue(lb.DNSName), aws.StringValue(lb.CanonicalHostedZoneNameID))
+		log.Info("found load balancer", "name", aws.StringValue(lb.LoadBalancerName), "dns name", aws.StringValue(lb.DNSName), "hosted zone ID", aws.StringValue(lb.CanonicalHostedZoneNameID))
 		if aws.StringValue(lb.CanonicalHostedZoneName) == target {
 			targetHostedZoneID = aws.StringValue(lb.CanonicalHostedZoneNameID)
 			break
@@ -235,7 +237,7 @@ func (m *Manager) change(record *dns.Record, action action) error {
 	key := zoneID + domain + target
 	// Only process updates once for now because we're not diffing.
 	if m.updatedRecords.Has(key) && action == upsertAction {
-		logrus.Infof("skipping update for record %v", record)
+		log.Info("skipping DNS record update", "record", record)
 		return nil
 	}
 	err = m.updateAlias(domain, zoneID, target, targetHostedZoneID, string(action))
@@ -245,10 +247,10 @@ func (m *Manager) change(record *dns.Record, action action) error {
 	switch action {
 	case upsertAction:
 		m.updatedRecords.Insert(key)
-		logrus.Infof("updated DNS record: %v", record)
+		log.Info("upserted DNS record", "record", record)
 	case deleteAction:
 		m.updatedRecords.Delete(key)
-		logrus.Infof("deleted DNS record: %v", record)
+		log.Info("deleted DNS record", "record", record)
 	}
 	return nil
 }
@@ -278,6 +280,6 @@ func (m *Manager) updateAlias(domain, zoneID, target, targetHostedZoneID, action
 	if err != nil {
 		return fmt.Errorf("couldn't update DNS record in zone %s: %v", zoneID, err)
 	}
-	logrus.Infof("updated DNS record in zone %s, %s -> %s: %v", zoneID, domain, target, resp)
+	log.Info("updated DNS record", "zone id", zoneID, "domain", domain, "target", target, "response", resp)
 	return nil
 }

pkg/dns/aws/dns.go

@@ -0,0 +1,24 @@
+package log
+
+import (
+	"fmt"
+
+	"github.com/go-logr/logr"
+	"github.com/go-logr/zapr"
+	"go.uber.org/zap"
+)
+
+// Logger is a simple logging interface for Go.
+var Logger logr.Logger
+
+func init() {
+	// Build a zap development logger.
+	zapLog, err := zap.NewDevelopment()
+	if err != nil {
+		panic(fmt.Sprintf("error building logger: %v", err))
+	}
+	// zapr defines an implementation of the Logger
+	// interface built on top of Zap (go.uber.org/zap).
+	Logger = zapr.NewLogger(zapLog)
+	Logger.Info("started development logger")
+}