Skip to content

Commit

Permalink
logging: replace glog with logr/zap
Browse files Browse the repository at this point in the history
Replace all usage of glog with logr/zap consistent with the ingress
and DNS operators.
  • Loading branch information
ironcladlou committed Oct 7, 2019
1 parent 4573eb7 commit dc68ebf
Show file tree
Hide file tree
Showing 27 changed files with 283 additions and 294 deletions.
37 changes: 0 additions & 37 deletions cmd/openshift-router/glog.go

This file was deleted.

2 changes: 0 additions & 2 deletions cmd/openshift-router/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,5 @@ func CommandFor(basename string) *cobra.Command {
os.Exit(1)
}

GLog(cmd.PersistentFlags())

return cmd
}
20 changes: 20 additions & 0 deletions log/log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package log

import (
"github.com/go-logr/logr"
"github.com/go-logr/zapr"
"go.uber.org/zap"
)

// Logger is the root logger which should be used by all
// other packages in the codebase.
var Logger logr.Logger

func init() {
// Set up logging.
zapLogger, err := zap.NewDevelopment(zap.AddCallerSkip(1), zap.AddStacktrace(zap.FatalLevel))
if err != nil {
panic(err)
}
Logger = zapr.NewLogger(zapLogger).WithName("router")
}
22 changes: 12 additions & 10 deletions pkg/cmd/infra/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"strings"
"time"

"github.com/golang/glog"
"github.com/spf13/cobra"
"github.com/spf13/pflag"

Expand All @@ -22,10 +21,13 @@ import (
routev1 "github.com/openshift/api/route/v1"
projectclient "github.com/openshift/client-go/project/clientset/versioned/typed/project/v1"
routeclientset "github.com/openshift/client-go/route/clientset/versioned"
logf "github.com/openshift/router/log"
"github.com/openshift/router/pkg/router/controller"
controllerfactory "github.com/openshift/router/pkg/router/controller/factory"
)

var log = logf.Logger.WithName("router")

// RouterSelection controls what routes and resources on the server are considered
// part of this router.
type RouterSelection struct {
Expand Down Expand Up @@ -115,7 +117,7 @@ func (o *RouterSelection) RouteUpdate(route *routev1.Route) {
}

s = strings.Trim(s, "\"'")
glog.V(4).Infof("changing route %s to %s", route.Spec.Host, s)
log.V(4).Info("changing route", "fromHost", route.Spec.Host, "toHost", s)
route.Spec.Host = s
}

Expand All @@ -125,18 +127,18 @@ func (o *RouterSelection) AdmissionCheck(route *routev1.Route) error {
}

if hostInDomainList(route.Spec.Host, o.BlacklistedDomains) {
glog.V(4).Infof("host %s in list of denied domains", route.Spec.Host)
log.V(4).Info("host in list of denied domains", "routeName", route.Name, "host", route.Spec.Host)
return fmt.Errorf("host in list of denied domains")
}

if o.WhitelistedDomains.Len() > 0 {
glog.V(4).Infof("Checking if host %s is in the list of allowed domains", route.Spec.Host)
log.V(4).Info("checking if host is in the list of allowed domains", "routeName", route.Name, "host", route.Spec.Host)
if hostInDomainList(route.Spec.Host, o.WhitelistedDomains) {
glog.V(4).Infof("host %s admitted - in the list of allowed domains", route.Spec.Host)
log.V(4).Info("host admitted - in the list of allowed domains", "routeName", route.Name, "host", route.Spec.Host)
return nil
}

glog.V(4).Infof("host %s rejected - not in the list of allowed domains", route.Spec.Host)
log.V(4).Info("host rejected - not in the list of allowed domains", "routeName", route.Name, "host", route.Spec.Host)
return fmt.Errorf("host not in the allowed list of domains")
}
return nil
Expand Down Expand Up @@ -244,15 +246,15 @@ func (o *RouterSelection) NewFactory(routeclient routeclientset.Interface, proje
factory.ResyncInterval = o.ResyncInterval
switch {
case o.NamespaceLabels != nil:
glog.Infof("Router is only using routes in namespaces matching %s", o.NamespaceLabels)
log.V(0).Info("router is only using routes in namespaces matching labels", "labels", o.NamespaceLabels)
factory.NamespaceLabels = o.NamespaceLabels
case o.ProjectLabels != nil:
glog.Infof("Router is only using routes in projects matching %s", o.ProjectLabels)
log.V(0).Info("router is only using routes in projects matching labels", "labels", o.ProjectLabels)
factory.ProjectLabels = o.ProjectLabels
case len(factory.Namespace) > 0:
glog.Infof("Router is only using resources in namespace %s", factory.Namespace)
log.V(0).Info("router is only using resources in namespace", "namespace", factory.Namespace)
default:
glog.Infof("Router is including routes in all namespaces")
log.V(0).Info("router is including routes in all namespaces")
}
return factory
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/infra/router/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"time"

"github.com/MakeNowJust/heredoc"
"github.com/golang/glog"

"github.com/spf13/cobra"
"github.com/spf13/pflag"

Expand Down Expand Up @@ -140,7 +140,7 @@ func getIntervalFromEnv(name string, defaultValSecs int) time.Duration {

value, err := time.ParseDuration(interval)
if err != nil {
glog.Warningf("Invalid %q %q, using default value %v ...", name, interval, defaultValSecs)
log.V(0).Info("invalid interval, using default", "name", name, "interval", interval, "default", defaultValSecs)
value = time.Duration(time.Duration(defaultValSecs) * time.Second)
}
return value
Expand Down Expand Up @@ -296,7 +296,7 @@ func (o *TemplateRouterOptions) Validate() error {

// Run launches a template router using the provided options. It never exits.
func (o *TemplateRouterOptions) Run() error {
glog.Infof("Starting template router (%s)", version.Get())
log.V(0).Info("starting router", "version", version.Get())
var ptrTemplatePlugin *templateplugin.TemplatePlugin

var reloadCallbacks []func()
Expand Down
20 changes: 9 additions & 11 deletions pkg/router/controller/contention.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"sync"
"time"

"github.com/golang/glog"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/cache"

Expand Down Expand Up @@ -149,9 +147,9 @@ func (t *SimpleContentionTracker) flush() {
}
}
if t.contentions > 0 && len(t.message) > 0 {
glog.Warning(t.message)
log.V(0).Info(t.message)
}
glog.V(5).Infof("Flushed contention tracker (%s): %d out of %d removed, %d total contentions", t.expires*2, removed, removed+len(t.ids), t.contentions)
log.V(5).Info("flushed contention tracker", "expires", t.expires*2, "removed", removed, "total", removed+len(t.ids), "contentions", t.contentions)
t.contentions = contentions
}

Expand All @@ -165,7 +163,7 @@ func (t *SimpleContentionTracker) Changed(id string, current *routev1.RouteIngre

// we have detected a sufficient number of conflicts to skip all updates for this interval
if t.contentions > t.maxContentions {
glog.V(4).Infof("Reached max contentions, stop tracking changes")
log.V(4).Info("reached max contentions, stop tracking changes")
return
}

Expand All @@ -177,19 +175,19 @@ func (t *SimpleContentionTracker) Changed(id string, current *routev1.RouteIngre
state: stateCandidate,
last: current,
}
glog.V(4).Infof("Object %s is a candidate for contention", id)
log.V(4).Info("object is a candidate for contention", "id", id)
return
}

// the previous state matches the current state, nothing to do
if ingressEqual(last.last, current) {
glog.V(4).Infof("Object %s is unchanged", id)
log.V(4).Info("object is unchanged", "id", id)
return
}

if last.state == stateContended {
t.contentions++
glog.V(4).Infof("Object %s is contended and has been modified by another writer", id)
log.V(4).Info("object is contended and has been modified by another writer", "id", id)
return
}

Expand All @@ -201,7 +199,7 @@ func (t *SimpleContentionTracker) Changed(id string, current *routev1.RouteIngre
last: current,
}
t.contentions++
glog.V(4).Infof("Object %s has been modified by another writer", id)
log.V(4).Info("object has been modified by another writer", "id", id)
return
}
}
Expand All @@ -212,7 +210,7 @@ func (t *SimpleContentionTracker) IsChangeContended(id string, now time.Time, cu

// we have detected a sufficient number of conflicts to skip all updates for this interval
if t.contentions > t.maxContentions {
glog.V(4).Infof("Reached max contentions, rejecting all update attempts until the next interval")
log.V(4).Info("reached max contentions, rejecting all update attempts until the next interval")
return true
}

Expand All @@ -224,7 +222,7 @@ func (t *SimpleContentionTracker) IsChangeContended(id string, now time.Time, cu

// if the object is contended, exit early
if last.state == stateContended {
glog.V(4).Infof("Object %s is being contended by another writer", id)
log.V(4).Info("object is being contended by another writer", "id", id)
return true
}

Expand Down
12 changes: 4 additions & 8 deletions pkg/router/controller/extended_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package controller
import (
"fmt"

"github.com/golang/glog"
kapi "k8s.io/api/core/v1"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/watch"

Expand Down Expand Up @@ -47,14 +47,10 @@ func (p *ExtendedValidator) HandleEndpoints(eventType watch.EventType, endpoints
func (p *ExtendedValidator) HandleRoute(eventType watch.EventType, route *routev1.Route) error {
// Check if previously seen route and its Spec is unchanged.
routeName := routeNameKey(route)
if errs := routeapihelpers.ExtendedValidateRoute(route); len(errs) > 0 {
errmsg := ""
for i := 0; i < len(errs); i++ {
errmsg = errmsg + "\n - " + errs[i].Error()
}
glog.Errorf("Skipping route %s due to invalid configuration: %s", routeName, errmsg)
if err := routeapihelpers.ExtendedValidateRoute(route).ToAggregate(); err != nil {
log.Error(err, "skipping route due to invalid configuration", "route", routeName)

p.recorder.RecordRouteRejection(route, "ExtendedValidationFailed", errmsg)
p.recorder.RecordRouteRejection(route, "ExtendedValidationFailed", err.Error())
p.plugin.HandleRoute(watch.Deleted, route)
return fmt.Errorf("invalid route configuration")
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/router/controller/factory/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"sort"
"time"

"github.com/golang/glog"

kapi "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -22,16 +20,21 @@ import (
routev1 "github.com/openshift/api/route/v1"
projectclient "github.com/openshift/client-go/project/clientset/versioned/typed/project/v1"
routeclientset "github.com/openshift/client-go/route/clientset/versioned"
informerfactory "k8s.io/client-go/informers"

"github.com/openshift/router/pkg/router"
routercontroller "github.com/openshift/router/pkg/router/controller"
"github.com/openshift/router/pkg/router/routeapihelpers"
informerfactory "k8s.io/client-go/informers"

logf "github.com/openshift/router/log"
)

const (
DefaultResyncInterval = 30 * time.Minute
)

var log = logf.Logger.WithName("controller_factory")

// RouterControllerFactory initializes and manages the watches that drive a router
// controller. It supports optional scoping on Namespace, Labels, and Fields of routes.
// If Namespace is empty, it means "all namespaces".
Expand Down Expand Up @@ -299,13 +302,13 @@ func (f *RouterControllerFactory) registerSharedInformerEventHandlers(obj runtim
if objType != reflect.TypeOf(obj) {
tombstone, ok := obj.(kcache.DeletedFinalStateUnknown)
if !ok {
glog.Errorf("Couldn't get object from tombstone: %+v", obj)
log.Error(nil, "couldn't get object from tombstone", "object", obj)
return
}

obj = tombstone.Obj
if objType != reflect.TypeOf(obj) {
glog.Errorf("Tombstone contained object that is not a %s: %+v", objType, obj)
log.Error(nil, "tombstone contained unexpected object type", "type", objType, "object", objType, obj)
return
}
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/router/controller/host_admitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package controller
import (
"fmt"

"github.com/golang/glog"
kapi "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/watch"
Expand Down Expand Up @@ -134,7 +133,7 @@ func (p *HostAdmitter) HandleRoute(eventType watch.EventType, route *routev1.Rou
}

if err := p.admitter(route); err != nil {
glog.V(4).Infof("Route %s not admitted: %s", routeNameKey(route), err.Error())
log.V(4).Info("route not admitted: %s", "routeNameKey", routeNameKey(route), "error", err.Error())
p.recorder.RecordRouteRejection(route, "RouteNotAdmitted", err.Error())
p.plugin.HandleRoute(watch.Deleted, route)
return err
Expand All @@ -144,7 +143,7 @@ func (p *HostAdmitter) HandleRoute(eventType watch.EventType, route *routev1.Rou
switch eventType {
case watch.Added, watch.Modified:
if err := p.addRoute(route); err != nil {
glog.Errorf("Route %s not admitted: %s", routeNameKey(route), err.Error())
log.Error(err, "route not admitted", "routeNameKey", routeNameKey(route))
return err
}

Expand Down

0 comments on commit dc68ebf

Please sign in to comment.