restruct zap logger for CNI (#2184)

* restruct zap logger

Author: paulyufan2

cni/api/api.go

@@ -9,6 +9,11 @@ import (
 	"go.uber.org/zap"
 )
 
+var (
+	loggerName = "azure-vnet"
+	logger     = log.InitZapLogCNI(loggerName, "azure-vnet.log")
+)
+
 type PodNetworkInterfaceInfo struct {
 	PodName       string
 	PodNamespace  string
@@ -24,13 +29,13 @@ type AzureCNIState struct {
 func (a *AzureCNIState) PrintResult() error {
 	b, err := json.MarshalIndent(a, "", "    ")
 	if err != nil {
-		log.Logger.Error("Failed to unmarshall Azure CNI state", zap.Error(err))
+		logger.Error("Failed to unmarshall Azure CNI state", zap.Error(err))
 	}
 
 	// write result to stdout to be captured by caller
 	_, err = os.Stdout.Write(b)
 	if err != nil {
-		log.Logger.Error("Failed to write response to stdout", zap.Error(err))
+		logger.Error("Failed to write response to stdout", zap.Error(err))
 		return err
 	}
 

cni/client/client.go

@@ -8,13 +8,19 @@ import (
 
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/api"
-	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/cni/log"
 	"github.com/Azure/azure-container-networking/platform"
 	semver "github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	utilexec "k8s.io/utils/exec"
 )
 
+var (
+	loggerName = "azure-vnet-client"
+	logger     = log.InitZapLogCNI(loggerName, "azure-vnet.log")
+)
+
 type client struct {
 	exec utilexec.Interface
 }
@@ -30,7 +36,7 @@ func (c *client) GetEndpointState() (*api.AzureCNIState, error) {
 	cmd.SetDir(CNIExecDir)
 	envs := os.Environ()
 	cmdenv := fmt.Sprintf("%s=%s", cni.Cmd, cni.CmdGetEndpointsState)
-	log.Printf("Setting cmd to %s", cmdenv)
+	logger.Info("Setting cmd to", zap.String("cmdenv", cmdenv))
 	envs = append(envs, cmdenv)
 	cmd.SetEnv(envs)
 

cni/ipam/ipam.go

@@ -21,6 +21,11 @@ import (
 
 const ipamV6 = "azure-vnet-ipamv6"
 
+var (
+	loggerName = "azure-vnet-ipam"
+	logger     = log.InitZapLogCNI(loggerName, "azure-ipam.log")
+)
+
 var ipv4DefaultRouteDstPrefix = net.IPNet{
 	IP:   net.IPv4zero,
 	Mask: net.IPv4Mask(0, 0, 0, 0),
@@ -62,25 +67,25 @@ func (plugin *ipamPlugin) Start(config *common.PluginConfig) error {
 	// Initialize base plugin.
 	err := plugin.Initialize(config)
 	if err != nil {
-		log.Logger.Error("Failed to initialize base plugin.", zap.Error(err))
+		logger.Error("Failed to initialize base plugin.", zap.Error(err))
 		return err
 	}
 
 	// Log platform information.
-	log.Logger.Info("Plugin version.", zap.String("name", plugin.Name),
+	logger.Info("Plugin version.", zap.String("name", plugin.Name),
 		zap.String("version", plugin.Version))
-	log.Logger.Info("Running on",
+	logger.Info("Running on",
 		zap.String("platform", platform.GetOSInfo()))
 
 	// Initialize address manager. rehyrdration not required on reboot for cni ipam plugin
 	err = plugin.am.Initialize(config, false, plugin.Options)
 	if err != nil {
-		log.Logger.Error("Failed to initialize address manager",
-			zap.String("error", err.Error()))
+		logger.Error("Failed to initialize address manager",
+			zap.Error(err))
 		return err
 	}
 
-	log.Logger.Info("Plugin started")
+	logger.Info("Plugin started")
 
 	return nil
 }
@@ -89,7 +94,7 @@ func (plugin *ipamPlugin) Start(config *common.PluginConfig) error {
 func (plugin *ipamPlugin) Stop() {
 	plugin.am.Uninitialize()
 	plugin.Uninitialize()
-	log.Logger.Info("Plugin stopped")
+	logger.Info("Plugin stopped")
 }
 
 // Configure parses and applies the given network configuration.
@@ -100,7 +105,7 @@ func (plugin *ipamPlugin) Configure(stdinData []byte) (*cni.NetworkConfig, error
 		return nil, err
 	}
 
-	log.Logger.Info("Read network configuration",
+	logger.Info("Read network configuration",
 		zap.Any("config", nwCfg))
 
 	// Apply IPAM configuration.
@@ -140,7 +145,7 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	var result *cniTypesCurr.Result
 	var err error
 
-	log.Logger.Info("Processing ADD command",
+	logger.Info("Processing ADD command",
 		zap.String("ContainerId", args.ContainerID),
 		zap.String("Netns", args.Netns),
 		zap.String("IfName", args.IfName),
@@ -149,9 +154,9 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 		zap.ByteString("StdinData", args.StdinData))
 
 	defer func() {
-		log.Logger.Info("ADD command completed",
+		logger.Info("ADD command completed",
 			zap.Any("result", result),
-			zap.Any("error:", err))
+			zap.Error(err))
 	}()
 
 	// Parse network configuration from stdin.
@@ -188,14 +193,14 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 		// On failure, release the address pool.
 		defer func() {
 			if err != nil && poolID != "" {
-				log.Logger.Info("Releasing pool",
+				logger.Info("Releasing pool",
 					zap.String("poolId", poolID))
 				_ = plugin.am.ReleasePool(nwCfg.IPAM.AddrSpace, poolID)
 			}
 		}()
 
 		nwCfg.IPAM.Subnet = subnet
-		log.Logger.Info("Allocated address with subnet",
+		logger.Info("Allocated address with subnet",
 			zap.String("poolId", poolID),
 			zap.String("subnet", subnet))
 	}
@@ -210,12 +215,12 @@ func (plugin *ipamPlugin) Add(args *cniSkel.CmdArgs) error {
 	// On failure, release the address.
 	defer func() {
 		if err != nil && address != "" {
-			log.Logger.Info("Releasing address", zap.String("address", address))
+			logger.Info("Releasing address", zap.String("address", address))
 			_ = plugin.am.ReleaseAddress(nwCfg.IPAM.AddrSpace, nwCfg.IPAM.Subnet, address, options)
 		}
 	}()
 
-	log.Logger.Info("Allocated address", zap.String("address", address))
+	logger.Info("Allocated address", zap.String("address", address))
 
 	// Parse IP address.
 	ipAddress, err := platform.ConvertStringToIPNet(address)
@@ -280,7 +285,7 @@ func (plugin *ipamPlugin) Get(args *cniSkel.CmdArgs) error {
 func (plugin *ipamPlugin) Delete(args *cniSkel.CmdArgs) error {
 	var err error
 
-	log.Logger.Info("[cni-ipam] Processing DEL command",
+	logger.Info("[cni-ipam] Processing DEL command",
 		zap.String("ContainerId", args.ContainerID),
 		zap.String("Netns", args.Netns),
 		zap.String("IfName", args.IfName),
@@ -289,7 +294,7 @@ func (plugin *ipamPlugin) Delete(args *cniSkel.CmdArgs) error {
 		zap.ByteString("StdinData", args.StdinData))
 
 	defer func() {
-		log.Logger.Info("[cni-ipam] DEL command completed",
+		logger.Info("[cni-ipam] DEL command completed",
 			zap.Error(err))
 	}()
 

cni/ipam/plugin/main.go

@@ -4,31 +4,22 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"os"
 
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/ipam"
-	zaplog "github.com/Azure/azure-container-networking/cni/log"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
-	"go.uber.org/zap/zapcore"
 )
 
-const (
-	name               = "azure-vnet-ipam"
-	maxLogFileSizeInMb = 5
-	maxLogFileCount    = 8
-	component          = "cni"
-)
+const name = "azure-vnet-ipam"
 
 // Version is populated by make during build.
 var version string
 
 // Main is the entry point for CNI IPAM plugin.
 func main() {
-	ctx, cancel := context.WithCancel(context.Background())
 	var config common.PluginConfig
 	config.Version = version
 
@@ -43,16 +34,6 @@ func main() {
 
 	defer log.Close()
 
-	loggerCfg := &zaplog.Config{
-		Level:       zapcore.DebugLevel,
-		LogPath:     zaplog.LogPath + "azure-ipam.log",
-		MaxSizeInMB: maxLogFileSizeInMb,
-		MaxBackups:  maxLogFileCount,
-		Name:        name,
-		Component:   component,
-	}
-	zaplog.Initialize(ctx, loggerCfg)
-
 	ipamPlugin, err := ipam.NewPlugin(name, &config)
 	if err != nil {
 		fmt.Printf("Failed to create IPAM plugin, err:%v.\n", err)
@@ -83,7 +64,6 @@ func main() {
 	err = ipamPlugin.Execute(cni.PluginApi(ipamPlugin))
 
 	ipamPlugin.Stop()
-	cancel()
 
 	if err != nil {
 		panic("ipam plugin fatal error")

cni/ipam/pluginv6/main.go

@@ -4,31 +4,22 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"os"
 
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/ipam"
-	zaplog "github.com/Azure/azure-container-networking/cni/log"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
-	"go.uber.org/zap/zapcore"
 )
 
-const (
-	name               = "azure-vnet-ipamv6"
-	maxLogFileSizeInMb = 5
-	maxLogFileCount    = 8
-	component          = "cni"
-)
+const name = "azure-vnet-ipamv6"
 
 // Version is populated by make during build.
 var version string
 
 // Main is the entry point for CNI IPAM plugin.
 func main() {
-	ctx, cancel := context.WithCancel(context.Background())
 	var config common.PluginConfig
 	config.Version = version
 
@@ -43,16 +34,6 @@ func main() {
 
 	defer log.Close()
 
-	loggerCfg := &zaplog.Config{
-		Level:       zapcore.DebugLevel,
-		LogPath:     zaplog.LogPath + "azure-ipam.log",
-		MaxSizeInMB: maxLogFileSizeInMb,
-		MaxBackups:  maxLogFileCount,
-		Name:        name,
-		Component:   component,
-	}
-	zaplog.Initialize(ctx, loggerCfg)
-
 	ipamPlugin, err := ipam.NewPlugin(name, &config)
 	if err != nil {
 		fmt.Printf("Failed to create IPAM plugin, err:%v.\n", err)
@@ -83,7 +64,6 @@ func main() {
 	err = ipamPlugin.Execute(cni.PluginApi(ipamPlugin))
 
 	ipamPlugin.Stop()
-	cancel()
 
 	if err != nil {
 		panic("ipam plugin fatal error")

cni/log/logger.go

@@ -1,55 +1,38 @@
 package log
 
 import (
-	"context"
-	"fmt"
 	"os"
 
+	"github.com/Azure/azure-container-networking/zaplog"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
-	"gopkg.in/natefinch/lumberjack.v2"
 )
 
-type Config struct {
-	Level       zapcore.Level
-	LogPath     string
-	MaxSizeInMB int
-	MaxBackups  int
-	Name        string
-	Component   string
-}
-
-var Logger *zap.Logger
+const (
+	maxLogFileSizeInMb = 5
+	maxLogFileCount    = 8
+)
 
-// Initializes a Zap logger and returns a cleanup function so logger can be cleaned up from caller
-func Initialize(ctx context.Context, cfg *Config) {
-	Logger = newFileLogger(cfg)
+var (
+	loggerName string
+	loggerFile string
+)
 
-	go func() {
-		<-ctx.Done()
-		err := Logger.Sync()
-		if err != nil {
-			fmt.Println("failed to sync logger")
-		}
-	}()
+var LoggerCfg = &zaplog.Config{
+	Level:       zapcore.DebugLevel,
+	LogPath:     loggerFile,
+	MaxSizeInMB: maxLogFileSizeInMb,
+	MaxBackups:  maxLogFileCount,
+	Name:        loggerName,
 }
 
-func newFileLogger(cfg *Config) *zap.Logger {
-	logFileWriter := zapcore.AddSync(&lumberjack.Logger{
-		Filename:   cfg.LogPath,
-		MaxSize:    cfg.MaxSizeInMB,
-		MaxBackups: cfg.MaxBackups,
-	})
-
-	encoderConfig := zap.NewProductionEncoderConfig()
-	encoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
-	jsonEncoder := zapcore.NewJSONEncoder(encoderConfig)
-	logLevel := cfg.Level
-
-	core := zapcore.NewCore(jsonEncoder, logFileWriter, logLevel)
-	Logger = zap.New(core)
-	Logger = Logger.With(zap.Int("pid", os.Getpid()))
-	Logger = Logger.With(zap.String("component", cfg.Component))
+func InitZapLogCNI(loggerName, loggerFile string) *zap.Logger {
+	LoggerCfg.Name = loggerName
+	LoggerCfg.LogPath = LogPath + loggerFile
+	logger := zaplog.InitZapLog(LoggerCfg)
 
-	return Logger
+	// only log process id on CNI package
+	logger = logger.With(zap.Int("pid", os.Getpid()))
+	logger = logger.With(zap.String("component", "cni"))
+	return logger
 }

cni/log/logger_mock.go

@@ -1,7 +1,5 @@
 package log
 
-import "go.uber.org/zap"
-
 func InitializeMock() {
-	Logger = zap.NewNop()
+	InitZapLogCNI("azure-vnet", "")
 }