Skip to content

Commit 30416ac

Browse files
runakashrunakash
authored
Improve Logging (#109)
* Reduce the noise in the logging by adding a wrapper around the logr.Logger to take the advantage of zapr logging levels. zapr provides development mode which sets to logLevel=Debug, and the production mode to logLevel=Info. By default, production mode is enabled, unless explicit argument to set the development mode. * Another iteration of refactorings to cleanup the init method of the common.Logger
1 parent 7214c2c commit 30416ac

16 files changed

+120
-71
lines changed

‎.github/.codecov.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,6 @@ comment:
2323
ignore:
2424
- "config/**/*"
2525
- "pkg/api/**/*"
26-
- "mocks/**/*"
26+
- "mocks/**/*"
27+
- "integration/scenarios/**/*"
28+
- "pkg/common/logger.go"

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ build: manifests generate generate-mocks fmt vet ## Build manager binary.
8484
go build -ldflags="-s -w -X ${PKG}.GitVersion=${GIT_TAG} -X ${PKG}.GitCommit=${GIT_COMMIT}" -o bin/manager main.go
8585

8686
run: manifests generate generate-mocks fmt vet ## Run a controller from your host.
87-
go run -ldflags="-s -w -X ${PKG}.GitVersion=${GIT_TAG} -X ${PKG}.GitCommit=${GIT_COMMIT}" ./main.go
87+
go run -ldflags="-s -w -X ${PKG}.GitVersion=${GIT_TAG} -X ${PKG}.GitCommit=${GIT_COMMIT}" ./main.go --zap-devel=true
8888

8989
docker-build: test ## Build docker image with the manager.
9090
docker build --no-cache -t ${IMG} .

‎main.go‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package main
1919
import (
2020
"context"
2121
"flag"
22+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
2223
"os"
2324

2425
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap"
@@ -42,8 +43,8 @@ import (
4243
)
4344

4445
var (
45-
scheme = runtime.NewScheme()
46-
setupLog = ctrl.Log.WithName("setup")
46+
scheme = runtime.NewScheme()
47+
log = ctrl.Log.WithName("main")
4748
)
4849

4950
func init() {
@@ -62,16 +63,18 @@ func main() {
6263
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
6364
"Enable leader election for controller manager. "+
6465
"Enabling this will ensure there is only one active controller manager.")
65-
opts := zap.Options{
66-
Development: true,
67-
}
66+
67+
// Add the zap logger flag set to the CLI. The flag set must
68+
// be added before calling flag.Parse().
69+
opts := zap.Options{}
6870
opts.BindFlags(flag.CommandLine)
71+
6972
flag.Parse()
7073

7174
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
7275

7376
v := version.GetVersion()
74-
setupLog.Info("starting AWS Cloud Map MCS Controller for K8s", "version", v)
77+
log.Info("starting AWS Cloud Map MCS Controller for K8s", "version", v)
7578

7679
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
7780
Scheme: scheme,
@@ -82,56 +85,56 @@ func main() {
8285
LeaderElectionID: "db692913.x-k8s.io",
8386
})
8487
if err != nil {
85-
setupLog.Error(err, "unable to start manager")
88+
log.Error(err, "unable to start manager")
8689
os.Exit(1)
8790
}
88-
setupLog.Info("configuring AWS session")
91+
log.Info("configuring AWS session")
8992
// GO sdk will look for region in order 1) AWS_REGION env var, 2) ~/.aws/config file, 3) EC2 IMDS
9093
awsCfg, err := config.LoadDefaultConfig(context.TODO(), config.WithEC2IMDSRegion())
9194

9295
if err != nil || awsCfg.Region == "" {
93-
setupLog.Error(err, "unable to configure AWS session", "AWS_REGION", awsCfg.Region)
96+
log.Error(err, "unable to configure AWS session", "AWS_REGION", awsCfg.Region)
9497
os.Exit(1)
9598
}
9699

97-
setupLog.Info("Running with AWS region", "AWS_REGION", awsCfg.Region)
100+
log.Info("Running with AWS region", "AWS_REGION", awsCfg.Region)
98101

99102
serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg)
100103
if err = (&controllers.ServiceExportReconciler{
101104
Client: mgr.GetClient(),
102-
Log: ctrl.Log.WithName("controllers").WithName("ServiceExport"),
105+
Log: common.NewLogger("controllers", "ServiceExport"),
103106
Scheme: mgr.GetScheme(),
104107
CloudMap: serviceDiscoveryClient,
105108
}).SetupWithManager(mgr); err != nil {
106-
setupLog.Error(err, "unable to create controller", "controller", "ServiceExport")
109+
log.Error(err, "unable to create controller", "controller", "ServiceExport")
107110
os.Exit(1)
108111
}
109112

110113
cloudMapReconciler := &controllers.CloudMapReconciler{
111114
Client: mgr.GetClient(),
112115
Cloudmap: serviceDiscoveryClient,
113-
Logger: ctrl.Log.WithName("controllers").WithName("CloudMap"),
116+
Log: common.NewLogger("controllers", "Cloudmap"),
114117
}
115118

116119
if err = mgr.Add(cloudMapReconciler); err != nil {
117-
setupLog.Error(err, "unable to create controller", "controller", "CloudMap")
120+
log.Error(err, "unable to create controller", "controller", "CloudMap")
118121
os.Exit(1)
119122
}
120123

121124
//+kubebuilder:scaffold:builder
122125

123126
if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
124-
setupLog.Error(err, "unable to set up health check")
127+
log.Error(err, "unable to set up health check")
125128
os.Exit(1)
126129
}
127130
if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
128-
setupLog.Error(err, "unable to set up ready check")
131+
log.Error(err, "unable to set up ready check")
129132
os.Exit(1)
130133
}
131134

132-
setupLog.Info("starting manager")
135+
log.Info("starting manager")
133136
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
134-
setupLog.Error(err, "problem running manager")
137+
log.Error(err, "problem running manager")
135138
os.Exit(1)
136139
}
137140
}

‎pkg/cloudmap/api.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
78
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
89
"github.com/aws/aws-sdk-go-v2/aws"
910
sd "github.com/aws/aws-sdk-go-v2/service/servicediscovery"
1011
"github.com/aws/aws-sdk-go-v2/service/servicediscovery/types"
11-
"github.com/go-logr/logr"
1212
"k8s.io/apimachinery/pkg/util/wait"
13-
ctrl "sigs.k8s.io/controller-runtime"
1413
)
1514

1615
const (
@@ -52,14 +51,14 @@ type ServiceDiscoveryApi interface {
5251
}
5352

5453
type serviceDiscoveryApi struct {
55-
log logr.Logger
54+
log common.Logger
5655
awsFacade AwsFacade
5756
}
5857

5958
// NewServiceDiscoveryApiFromConfig creates a new AWS Cloud Map API connection manager from an AWS client config.
6059
func NewServiceDiscoveryApiFromConfig(cfg *aws.Config) ServiceDiscoveryApi {
6160
return &serviceDiscoveryApi{
62-
log: ctrl.Log.WithName("cloudmap"),
61+
log: common.NewLogger("cloudmap"),
6362
awsFacade: NewAwsFacadeFromConfig(cfg),
6463
}
6564
}

‎pkg/cloudmap/api_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap"
8+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
89
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
910
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test"
1011
"github.com/aws/aws-sdk-go-v2/aws"
@@ -319,7 +320,7 @@ func TestServiceDiscoveryApi_PollNamespaceOperation_HappyCase(t *testing.T) {
319320

320321
func getServiceDiscoveryApi(t *testing.T, awsFacade *cloudmap.MockAwsFacade) ServiceDiscoveryApi {
321322
return &serviceDiscoveryApi{
322-
log: testingLogger.TestLogger{T: t},
323+
log: common.NewLoggerWithLogr(testingLogger.TestLogger{T: t}),
323324
awsFacade: awsFacade,
324325
}
325326
}

‎pkg/cloudmap/cache.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ package cloudmap
33
import (
44
"errors"
55
"fmt"
6+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
67
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
7-
"github.com/go-logr/logr"
88
"k8s.io/apimachinery/pkg/util/cache"
9-
ctrl "sigs.k8s.io/controller-runtime"
109
"time"
1110
)
1211

@@ -33,7 +32,7 @@ type ServiceDiscoveryClientCache interface {
3332
}
3433

3534
type sdCache struct {
36-
log logr.Logger
35+
log common.Logger
3736
cache *cache.LRUExpireCache
3837
config *SdCacheConfig
3938
}
@@ -46,7 +45,7 @@ type SdCacheConfig struct {
4645

4746
func NewServiceDiscoveryClientCache(cacheConfig *SdCacheConfig) ServiceDiscoveryClientCache {
4847
return &sdCache{
49-
log: ctrl.Log.WithName("cloudmap"),
48+
log: common.NewLogger("cloudmap"),
5049
cache: cache.NewLRUExpireCache(defaultCacheSize),
5150
config: cacheConfig,
5251
}

‎pkg/cloudmap/client.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ package cloudmap
33
import (
44
"context"
55
"fmt"
6+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
67
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
78
"github.com/aws/aws-sdk-go-v2/aws"
8-
"github.com/go-logr/logr"
9-
ctrl "sigs.k8s.io/controller-runtime"
109
)
1110

1211
// ServiceDiscoveryClient provides the service endpoint management functionality required by the AWS Cloud Map
@@ -29,7 +28,7 @@ type ServiceDiscoveryClient interface {
2928
}
3029

3130
type serviceDiscoveryClient struct {
32-
log logr.Logger
31+
log common.Logger
3332
sdApi ServiceDiscoveryApi
3433
cache ServiceDiscoveryClientCache
3534
}
@@ -38,15 +37,15 @@ type serviceDiscoveryClient struct {
3837
// from a given AWS client config.
3938
func NewDefaultServiceDiscoveryClient(cfg *aws.Config) ServiceDiscoveryClient {
4039
return &serviceDiscoveryClient{
41-
log: ctrl.Log.WithName("cloudmap"),
40+
log: common.NewLogger("cloudmap"),
4241
sdApi: NewServiceDiscoveryApiFromConfig(cfg),
4342
cache: NewDefaultServiceDiscoveryClientCache(),
4443
}
4544
}
4645

4746
func NewServiceDiscoveryClientWithCustomCache(cfg *aws.Config, cacheConfig *SdCacheConfig) ServiceDiscoveryClient {
4847
return &serviceDiscoveryClient{
49-
log: ctrl.Log.WithName("cloudmap"),
48+
log: common.NewLogger("cloudmap"),
5049
sdApi: NewServiceDiscoveryApiFromConfig(cfg),
5150
cache: NewServiceDiscoveryClientCache(cacheConfig),
5251
}

‎pkg/cloudmap/client_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap"
7+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
78
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
89
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test"
910
"github.com/aws/aws-sdk-go-v2/aws"
@@ -410,7 +411,7 @@ func getTestSdClient(t *testing.T) *testSdClient {
410411
mockApi := cloudmap.NewMockServiceDiscoveryApi(mockController)
411412
return &testSdClient{
412413
client: &serviceDiscoveryClient{
413-
log: testing2.TestLogger{T: t},
414+
log: common.NewLoggerWithLogr(testing2.TestLogger{T: t}),
414415
sdApi: mockApi,
415416
cache: mockCache,
416417
},

‎pkg/cloudmap/operation_collector.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
package cloudmap
22

33
import (
4-
"github.com/go-logr/logr"
5-
ctrl "sigs.k8s.io/controller-runtime"
4+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
65
"sync"
76
)
87

@@ -22,7 +21,7 @@ type OperationCollector interface {
2221
}
2322

2423
type opCollector struct {
25-
log logr.Logger
24+
log common.Logger
2625
opChan chan opResult
2726
wg sync.WaitGroup
2827
startTime int64
@@ -36,7 +35,7 @@ type opResult struct {
3635

3736
func NewOperationCollector() OperationCollector {
3837
return &opCollector{
39-
log: ctrl.Log.WithName("cloudmap"),
38+
log: common.NewLogger("cloudmap"),
4039
opChan: make(chan opResult),
4140
startTime: Now(),
4241
createOpsSuccess: true,

‎pkg/cloudmap/operation_poller.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@ package cloudmap
33
import (
44
"context"
55
"errors"
6+
"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
67
"github.com/aws/aws-sdk-go-v2/aws"
78
"github.com/aws/aws-sdk-go-v2/service/servicediscovery/types"
8-
"github.com/go-logr/logr"
99
"k8s.io/apimachinery/pkg/util/wait"
10-
ctrl "sigs.k8s.io/controller-runtime"
1110
"strconv"
1211
"time"
1312
)
@@ -29,7 +28,7 @@ type OperationPoller interface {
2928
}
3029

3130
type operationPoller struct {
32-
log logr.Logger
31+
log common.Logger
3332
sdApi ServiceDiscoveryApi
3433
timeout time.Duration
3534

@@ -41,7 +40,7 @@ type operationPoller struct {
4140

4241
func newOperationPoller(sdApi ServiceDiscoveryApi, svcId string, opIds []string, startTime int64) operationPoller {
4342
return operationPoller{
44-
log: ctrl.Log.WithName("cloudmap"),
43+
log: common.NewLogger("cloudmap"),
4544
sdApi: sdApi,
4645
timeout: defaultOperationPollTimeout,
4746

0 commit comments

Comments
 (0)