feat: add migration shim and config for v2 logger

Signed-off-by: Evan Baker <[email protected]

Author: rbtr

cns/configuration/configuration.go

@@ -30,6 +30,7 @@ type CNSConfig struct {
 	EnableCNIConflistGeneration bool
 	EnableIPAMv2                bool
 	EnableK8sDevicePlugin       bool
+	EnableLoggerV2              bool
 	EnablePprof                 bool
 	EnableStateMigration        bool
 	EnableSubnetScarcity        bool

cns/logger/cnslogger.go

@@ -17,7 +17,7 @@ import (
 // wait time for closing AI telemetry session.
 const waitTimeInSecs = 10
 
-type CNSLogger struct {
+type logger struct {
 	logger    *log.Logger
 	zapLogger *zap.Logger
 	th        ai.TelemetryHandle
@@ -30,7 +30,8 @@ type CNSLogger struct {
 	metadata map[string]string
 }
 
-func New(fileName string, logLevel, logTarget int, logDir string) (*CNSLogger, error) {
+// Deprecated: The v1 logger is deprecated. Migrate to zap using the cns/logger/v2 package.
+func New(fileName string, logLevel, logTarget int, logDir string) (loggershim, error) {
 	l, err := log.NewLoggerE(fileName, logLevel, logTarget, logDir)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get new logger")
@@ -46,18 +47,18 @@ func New(fileName string, logLevel, logTarget int, logDir string) (*CNSLogger, e
 	}
 	zapLogger := zap.New(platformCore, zap.AddCaller()).With(zap.Int("pid", os.Getpid()))
 
-	return &CNSLogger{
+	return &logger{
 		logger:    l,
 		zapLogger: zapLogger,
 		metadata:  map[string]string{},
 	}, nil
 }
 
-func (c *CNSLogger) InitAI(aiConfig ai.AIConfig, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
+func (c *logger) InitAI(aiConfig ai.AIConfig, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
 	c.InitAIWithIKey(aiConfig, aiMetadata, disableTraceLogging, disableMetricLogging, disableEventLogging)
 }
 
-func (c *CNSLogger) InitAIWithIKey(aiConfig ai.AIConfig, instrumentationKey string, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
+func (c *logger) InitAIWithIKey(aiConfig ai.AIConfig, instrumentationKey string, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
 	th, err := ai.NewAITelemetry("", instrumentationKey, aiConfig)
 	if err != nil {
 		c.logger.Errorf("Error initializing AI Telemetry:%v", err)
@@ -70,28 +71,28 @@ func (c *CNSLogger) InitAIWithIKey(aiConfig ai.AIConfig, instrumentationKey stri
 	c.disableEventLogging = disableEventLogging
 }
 
-func (c *CNSLogger) Close() {
+func (c *logger) Close() {
 	c.logger.Close()
 	if c.th != nil {
 		c.th.Close(waitTimeInSecs)
 	}
 }
 
-func (c *CNSLogger) SetContextDetails(orchestrator, nodeID string) {
+func (c *logger) SetContextDetails(orchestrator, nodeID string) {
 	c.logger.Logf("SetContext details called with: %v orchestrator nodeID %v", orchestrator, nodeID)
 	c.m.Lock()
 	c.metadata[orchestratorTypeKey] = orchestrator
 	c.metadata[nodeIDKey] = nodeID
 	c.m.Unlock()
 }
 
-func (c *CNSLogger) SetAPIServer(apiserver string) {
+func (c *logger) SetAPIServer(apiserver string) {
 	c.m.Lock()
 	c.metadata[apiServerKey] = apiserver
 	c.m.Unlock()
 }
 
-func (c *CNSLogger) Printf(format string, args ...any) {
+func (c *logger) Printf(format string, args ...any) {
 	c.logger.Logf(format, args...)
 	c.zapLogger.Info(fmt.Sprintf(format, args...))
 	if c.th == nil || c.disableTraceLogging {
@@ -101,7 +102,7 @@ func (c *CNSLogger) Printf(format string, args ...any) {
 	c.sendTraceInternal(msg, ai.InfoLevel)
 }
 
-func (c *CNSLogger) Debugf(format string, args ...any) {
+func (c *logger) Debugf(format string, args ...any) {
 	c.logger.Debugf(format, args...)
 	c.zapLogger.Debug(fmt.Sprintf(format, args...))
 	if c.th == nil || c.disableTraceLogging {
@@ -111,7 +112,7 @@ func (c *CNSLogger) Debugf(format string, args ...any) {
 	c.sendTraceInternal(msg, ai.DebugLevel)
 }
 
-func (c *CNSLogger) Warnf(format string, args ...any) {
+func (c *logger) Warnf(format string, args ...any) {
 	c.logger.Warnf(format, args...)
 	c.zapLogger.Warn(fmt.Sprintf(format, args...))
 	if c.th == nil || c.disableTraceLogging {
@@ -121,7 +122,7 @@ func (c *CNSLogger) Warnf(format string, args ...any) {
 	c.sendTraceInternal(msg, ai.WarnLevel)
 }
 
-func (c *CNSLogger) Errorf(format string, args ...any) {
+func (c *logger) Errorf(format string, args ...any) {
 	c.logger.Errorf(format, args...)
 	c.zapLogger.Error(fmt.Sprintf(format, args...))
 	if c.th == nil || c.disableTraceLogging {
@@ -131,7 +132,7 @@ func (c *CNSLogger) Errorf(format string, args ...any) {
 	c.sendTraceInternal(msg, ai.ErrorLevel)
 }
 
-func (c *CNSLogger) Request(tag string, request any, err error) {
+func (c *logger) Request(tag string, request any, err error) {
 	c.logger.Request(tag, request, err)
 	if c.th == nil || c.disableTraceLogging {
 		return
@@ -147,7 +148,7 @@ func (c *CNSLogger) Request(tag string, request any, err error) {
 	c.sendTraceInternal(msg, lvl)
 }
 
-func (c *CNSLogger) Response(tag string, response any, returnCode types.ResponseCode, err error) {
+func (c *logger) Response(tag string, response any, returnCode types.ResponseCode, err error) {
 	c.logger.Response(tag, response, int(returnCode), returnCode.String(), err)
 	if c.th == nil || c.disableTraceLogging {
 		return
@@ -166,7 +167,7 @@ func (c *CNSLogger) Response(tag string, response any, returnCode types.Response
 	c.sendTraceInternal(msg, lvl)
 }
 
-func (c *CNSLogger) ResponseEx(tag string, request, response any, returnCode types.ResponseCode, err error) {
+func (c *logger) ResponseEx(tag string, request, response any, returnCode types.ResponseCode, err error) {
 	c.logger.ResponseEx(tag, request, response, int(returnCode), returnCode.String(), err)
 	if c.th == nil || c.disableTraceLogging {
 		return
@@ -185,7 +186,7 @@ func (c *CNSLogger) ResponseEx(tag string, request, response any, returnCode typ
 	c.sendTraceInternal(msg, lvl)
 }
 
-func (c *CNSLogger) sendTraceInternal(msg string, lvl ai.Level) {
+func (c *logger) sendTraceInternal(msg string, lvl ai.Level) {
 	report := ai.Report{
 		Message:          msg,
 		Level:            lvl,
@@ -198,7 +199,7 @@ func (c *CNSLogger) sendTraceInternal(msg string, lvl ai.Level) {
 	c.th.TrackLog(report)
 }
 
-func (c *CNSLogger) LogEvent(event ai.Event) {
+func (c *logger) LogEvent(event ai.Event) {
 	if c.th == nil || c.disableEventLogging {
 		return
 	}
@@ -208,7 +209,7 @@ func (c *CNSLogger) LogEvent(event ai.Event) {
 	c.th.TrackEvent(event)
 }
 
-func (c *CNSLogger) SendMetric(metric ai.Metric) {
+func (c *logger) SendMetric(metric ai.Metric) {
 	if c.th == nil || c.disableMetricLogging {
 		return
 	}

cns/logger/log.go

@@ -6,66 +6,95 @@ import (
 	"github.com/Azure/azure-container-networking/cns/types"
 )
 
+type loggershim interface {
+	Close()
+	InitAI(aitelemetry.AIConfig, bool, bool, bool)
+	InitAIWithIKey(aitelemetry.AIConfig, string, bool, bool, bool)
+	SetContextDetails(string, string)
+	SetAPIServer(string)
+	Printf(string, ...any)
+	Debugf(string, ...any)
+	Warnf(string, ...any)
+	LogEvent(aitelemetry.Event)
+	Errorf(string, ...any)
+	Request(string, any, error)
+	Response(string, any, types.ResponseCode, error)
+	ResponseEx(string, any, any, types.ResponseCode, error)
+	SendMetric(aitelemetry.Metric)
+}
+
 var (
-	Log             *CNSLogger
-	aiMetadata      string // this var is set at build time.
+	Log             loggershim
 	AppInsightsIKey = aiMetadata
+	aiMetadata      string // this var is set at build time.
 )
 
-// todo: the functions below should be removed. CNSLogger should be injected where needed and not used from package level scope.
-
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Close() {
 	Log.Close()
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func InitLogger(fileName string, logLevel, logTarget int, logDir string) {
 	Log, _ = New(fileName, logLevel, logTarget, logDir)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func InitAI(aiConfig aitelemetry.AIConfig, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
 	Log.InitAI(aiConfig, disableTraceLogging, disableMetricLogging, disableEventLogging)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func InitAIWithIKey(aiConfig aitelemetry.AIConfig, instrumentationKey string, disableTraceLogging, disableMetricLogging, disableEventLogging bool) {
 	Log.InitAIWithIKey(aiConfig, instrumentationKey, disableTraceLogging, disableMetricLogging, disableEventLogging)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func SetContextDetails(orchestrator, nodeID string) {
 	Log.SetContextDetails(orchestrator, nodeID)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Printf(format string, args ...any) {
 	Log.Printf(format, args...)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Debugf(format string, args ...any) {
 	Log.Debugf(format, args...)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Warnf(format string, args ...any) {
 	Log.Warnf(format, args...)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func LogEvent(event aitelemetry.Event) {
 	Log.LogEvent(event)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Errorf(format string, args ...any) {
 	Log.Errorf(format, args...)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Request(tag string, request any, err error) {
 	Log.Request(tag, request, err)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func Response(tag string, response any, returnCode types.ResponseCode, err error) {
 	Log.Response(tag, response, returnCode, err)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func ResponseEx(tag string, request, response any, returnCode types.ResponseCode, err error) {
 	Log.ResponseEx(tag, request, response, returnCode, err)
 }
 
+// Deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead.
 func SendMetric(metric aitelemetry.Metric) {
 	Log.SendMetric(metric)
 }

cns/logger/v2/logger.go

@@ -1,3 +1,5 @@
+// Package logger provides an opinionated logger for CNS which knows how to
+// log to Application Insights, file, stdout and ETW (based on platform).
 package logger
 
 import (

cns/logger/v2/shim.go

@@ -0,0 +1,64 @@
+package logger
+
+import (
+	"github.com/Azure/azure-container-networking/aitelemetry"
+	"github.com/Azure/azure-container-networking/cns/types"
+	"go.uber.org/zap"
+)
+
+// shim wraps the Zap logger to provide a compatible interface to the
+// legacy CNS logger. This is temporary and exists to make migration
+// feasible and optional.
+type shim struct {
+	z      *zap.Logger
+	closer func()
+}
+
+func (s *shim) Close() {
+	_ = s.z.Sync()
+	s.closer()
+}
+
+func (s *shim) Printf(format string, a ...any) {
+	s.z.Sugar().Infof(format, a...)
+}
+
+func (s *shim) Debugf(format string, a ...any) {
+	s.z.Sugar().Debugf(format, a...)
+}
+
+func (s *shim) Warnf(format string, a ...any) {
+	s.z.Sugar().Warnf(format, a...)
+}
+
+func (s *shim) Errorf(format string, a ...any) {
+	s.z.Sugar().Errorf(format, a...)
+}
+
+func (s *shim) Request(msg string, data any, err error) {
+	s.z.Sugar().Infow("Request", "message", msg, "data", data, "error", err)
+}
+
+func (s *shim) Response(msg string, data any, code types.ResponseCode, err error) {
+	s.z.Sugar().Infow("Response", "message", msg, "data", data, "code", code, "error", err)
+}
+
+func (s *shim) ResponseEx(msg string, request, response any, code types.ResponseCode, err error) {
+	s.z.Sugar().Infow("ResponseEx", "message", msg, "request", request, "response", response, "code", code, "error", err)
+}
+
+func (*shim) InitAI(aitelemetry.AIConfig, bool, bool, bool) {}
+
+func (*shim) InitAIWithIKey(aitelemetry.AIConfig, string, bool, bool, bool) {}
+
+func (s *shim) SetContextDetails(string, string) {}
+
+func (s *shim) SetAPIServer(string) {}
+
+func (s *shim) SendMetric(aitelemetry.Metric) {}
+
+func (s *shim) LogEvent(aitelemetry.Event) {}
+
+func AsV1(z *zap.Logger, closer func()) *shim { //nolint:revive // I want it to be annoying to use.
+	return &shim{z: z, closer: closer}
+}

cns/service/main.go

@@ -41,6 +41,7 @@ import (
 	nncctrl "github.com/Azure/azure-container-networking/cns/kubecontroller/nodenetworkconfig"
 	podctrl "github.com/Azure/azure-container-networking/cns/kubecontroller/pod"
 	"github.com/Azure/azure-container-networking/cns/logger"
+	loggerv2 "github.com/Azure/azure-container-networking/cns/logger/v2"
 	"github.com/Azure/azure-container-networking/cns/metric"
 	"github.com/Azure/azure-container-networking/cns/middlewares"
 	"github.com/Azure/azure-container-networking/cns/multitenantcontroller"
@@ -69,7 +70,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
 	"golang.org/x/time/rate"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -130,7 +130,6 @@ const (
 var (
 	rootCtx   context.Context
 	rootErrCh chan error
-	z         *zap.Logger
 )
 
 // Version is populated by make during build.
@@ -629,13 +628,18 @@ func main() {
 		}
 	}
 
-	// configure zap logger
-	zconfig := zap.NewProductionConfig()
-	zconfig.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
-	if z, err = zconfig.Build(); err != nil {
+	// build the zap logger
+	z, c, err := loggerv2.New(&loggerv2.Config{})
+	defer c()
+	if err != nil {
 		fmt.Printf("failed to create logger: %v", err)
 		os.Exit(1)
 	}
+	// set the logger to the global logger if configured
+	if cnsconfig.EnableLoggerV2 {
+		logger.Printf("hotswapping logger v2")
+		logger.Log = loggerv2.AsV1(z, c)
+	}
 
 	// start the healthz/readyz/metrics server
 	readyCh := make(chan interface{})
@@ -866,7 +870,7 @@ func main() {
 
 		logger.Printf("Set GlobalPodInfoScheme %v (InitializeFromCNI=%t)", cns.GlobalPodInfoScheme, cnsconfig.InitializeFromCNI)
 
-		err = InitializeCRDState(rootCtx, httpRemoteRestService, cnsconfig)
+		err = InitializeCRDState(rootCtx, z, httpRemoteRestService, cnsconfig)
 		if err != nil {
 			logger.Errorf("Failed to start CRD Controller, err:%v.\n", err)
 			return
@@ -1369,7 +1373,7 @@ func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter,
 }
 
 // InitializeCRDState builds and starts the CRD controllers.
-func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cnsconfig *configuration.CNSConfig) error {
+func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns.HTTPService, cnsconfig *configuration.CNSConfig) error {
 	// convert interface type to implementation type
 	httpRestServiceImplementation, ok := httpRestService.(*restserver.HTTPRestService)
 	if !ok {