improve the logging

Author: behzad-mir

cns/cni-telemetry-sidecar/configmanager.go

@@ -4,104 +4,98 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"time"
 
 	"github.com/Azure/azure-container-networking/cns/configuration"
-	"github.com/Azure/azure-container-networking/cns/logger"
+	"go.uber.org/zap"
 )
 
-// ConfigManager handles Azure CNS configuration loading and validation
+// ConfigManager handles CNS configuration loading
 type ConfigManager struct {
 	configPath string
+	logger     *zap.Logger
 }
 
-// NewConfigManager creates a new configuration manager for Azure CNS
+// NewConfigManager creates a new config manager
 func NewConfigManager(configPath string) *ConfigManager {
 	return &ConfigManager{
 		configPath: configPath,
 	}
 }
 
-// LoadConfig loads and validates the Azure CNS configuration
+// SetLogger sets the zap logger for the config manager
+func (cm *ConfigManager) SetLogger(logger *zap.Logger) {
+	cm.logger = logger
+}
+
+// LoadConfig loads the CNS configuration from file
 func (cm *ConfigManager) LoadConfig() (*configuration.CNSConfig, error) {
-	logger.Printf("Loading Azure CNS configuration from: %s", cm.configPath)
+	// Use zap logger if available, otherwise create a default config
+	if cm.logger != nil {
+		cm.logger.Debug("Loading CNS configuration", zap.String("path", cm.configPath))
+	}
 
-	// Wait for configuration file to become available (Kubernetes ConfigMap mount)
-	if err := cm.waitForConfigFile(); err != nil {
-		return nil, fmt.Errorf("Azure CNS configuration file not available: %w", err)
+	// Check if config file exists
+	if _, err := os.Stat(cm.configPath); os.IsNotExist(err) {
+		if cm.logger != nil {
+			cm.logger.Info("CNS config file not found, using default configuration",
+				zap.String("path", cm.configPath))
+		}
+
+		// Return default configuration
+		return &configuration.CNSConfig{
+			TelemetrySettings: configuration.TelemetrySettings{
+				DisableAll:                    false,
+				TelemetryBatchSizeBytes:       16384,
+				TelemetryBatchIntervalInSecs:  15,
+				RefreshIntervalInSecs:         15,
+				DisableMetadataRefreshThread:  false,
+				DebugMode:                     false,
+				DisableTrace:                  false,
+				DisableMetric:                 false,
+				DisableEvent:                  false,
+				AppInsightsInstrumentationKey: "", // Will be set by environment or config
+			},
+		}, nil
 	}
 
-	// Read the configuration file from mounted volume
+	// Read the config file
 	data, err := os.ReadFile(cm.configPath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read Azure CNS configuration file: %w", err)
+		if cm.logger != nil {
+			cm.logger.Error("Failed to read CNS config file",
+				zap.String("path", cm.configPath),
+				zap.Error(err))
+		}
+		return nil, fmt.Errorf("failed to read config file %s: %w", cm.configPath, err)
 	}
 
-	// Parse JSON configuration into CNS config structure
+	// Parse the JSON configuration
 	var config configuration.CNSConfig
 	if err := json.Unmarshal(data, &config); err != nil {
-		return nil, fmt.Errorf("failed to parse Azure CNS configuration: %w", err)
-	}
-
-	// Validate configuration for Azure telemetry requirements
-	if err := cm.validateConfig(&config); err != nil {
-		return nil, fmt.Errorf("Azure CNS configuration validation failed: %w", err)
-	}
-
-	logger.Printf("Azure CNS configuration loaded and validated successfully")
-	return &config, nil
-}
-
-// waitForConfigFile waits for the configuration file to become available
-// This is important in Kubernetes environments where ConfigMaps are mounted asynchronously
-func (cm *ConfigManager) waitForConfigFile() error {
-	const maxRetries = 30
-	const retryInterval = 2 * time.Second
-
-	for i := 0; i < maxRetries; i++ {
-		if _, err := os.Stat(cm.configPath); err == nil {
-			return nil
-		}
-
-		if i == 0 {
-			logger.Printf("Waiting for Azure CNS configuration file to become available...")
+		if cm.logger != nil {
+			cm.logger.Error("Failed to parse CNS config file",
+				zap.String("path", cm.configPath),
+				zap.Error(err))
 		}
-
-		time.Sleep(retryInterval)
+		return nil, fmt.Errorf("failed to parse config file: %w", err)
 	}
 
-	return fmt.Errorf("Azure CNS configuration file not available after %d attempts (%v total wait time)",
-		maxRetries, time.Duration(maxRetries)*retryInterval)
-}
-
-// validateConfig performs Azure-specific validation of the CNS configuration
-func (cm *ConfigManager) validateConfig(config *configuration.CNSConfig) error {
-	// Validate that telemetry settings are properly configured
-	if config.TelemetrySettings.AppInsightsInstrumentationKey == "" && !config.TelemetrySettings.DisableAll {
-		logger.Printf("Warning: No Application Insights instrumentation key configured and telemetry not disabled")
+	// Set default values for telemetry settings if not specified
+	if config.TelemetrySettings.TelemetryBatchSizeBytes == 0 {
+		config.TelemetrySettings.TelemetryBatchSizeBytes = 16384
 	}
-
-	// Validate batch size settings for Azure Application Insights
-	if config.TelemetrySettings.TelemetryBatchSizeBytes <= 0 {
-		logger.Printf("Warning: Invalid telemetry batch size, using default")
+	if config.TelemetrySettings.TelemetryBatchIntervalInSecs == 0 {
+		config.TelemetrySettings.TelemetryBatchIntervalInSecs = 15
 	}
-
-	// Validate batch interval for optimal Azure ingestion
-	if config.TelemetrySettings.TelemetryBatchIntervalInSecs <= 0 {
-		logger.Printf("Warning: Invalid telemetry batch interval, using default")
+	if config.TelemetrySettings.RefreshIntervalInSecs == 0 {
+		config.TelemetrySettings.RefreshIntervalInSecs = 15
 	}
 
-	// Log configuration summary for Azure monitoring and debugging
-	logger.Printf("Azure CNS Configuration Summary:")
-	logger.Printf("  - Telemetry DisableAll: %t", config.TelemetrySettings.DisableAll)
-	logger.Printf("  - Application Insights Key Present: %t",
-		config.TelemetrySettings.AppInsightsInstrumentationKey != "")
-	logger.Printf("  - Batch Size: %d bytes", config.TelemetrySettings.TelemetryBatchSizeBytes)
-	logger.Printf("  - Batch Interval: %d seconds", config.TelemetrySettings.TelemetryBatchIntervalInSecs)
-	logger.Printf("  - Disable Trace: %t", config.TelemetrySettings.DisableTrace)
-	logger.Printf("  - Disable Metric: %t", config.TelemetrySettings.DisableMetric)
-	logger.Printf("  - Disable Event: %t", config.TelemetrySettings.DisableEvent)
-	logger.Printf("  - Debug Mode: %t", config.TelemetrySettings.DebugMode)
+	if cm.logger != nil {
+		cm.logger.Info("Successfully loaded CNS configuration",
+			zap.String("path", cm.configPath),
+			zap.Bool("telemetryDisabled", config.TelemetrySettings.DisableAll))
+	}
 
-	return nil
+	return &config, nil
 }

cns/cni-telemetry-sidecar/main.go

@@ -7,26 +7,46 @@ import (
 	"os/signal"
 	"syscall"
 
-	"github.com/Azure/azure-container-networking/cns/logger"
+	"github.com/Azure/azure-container-networking/cns/logger/v2"
+	cores "github.com/Azure/azure-container-networking/cns/logger/v2/cores"
+	"go.uber.org/zap"
 )
 
 var (
 	version    = "unknown"
 	configPath = flag.String("config", "/etc/cns/cns-config.json", "Path to CNS configuration file")
+	logLevel   = flag.String("log-level", "info", "Log level (debug, info, warn, error)")
 )
 
 func main() {
 	flag.Parse()
 
-	// Initialize logging for the CNI telemetry sidecar
-	logger.InitLogger("azure-cns-cni-telemetry-sidecar", 1, 1, "/var/log/azure-cns-telemetry")
-	defer logger.Close()
+	// Initialize main logger with correct path for shared volume
+	zapLogger, cleanup, err := logger.New(&logger.Config{
+		Level: *logLevel,
+		File: &cores.FileConfig{
+			Filepath: "/var/log/azure-cni-telemetry-sidecar.log", // This will write to host's /var/log/azure-cns/
+		},
+	})
+	if err != nil {
+		panic("Failed to initialize logger: " + err.Error())
+	}
+	defer cleanup()
 
-	logger.Printf("Starting Azure CNI Telemetry Sidecar v%s", version)
+	zapLogger.Info("Starting Azure CNI Telemetry Sidecar",
+		zap.String("version", version),
+		zap.String("configPath", *configPath),
+		zap.String("logLevel", *logLevel))
 
-	// Create telemetry sidecar service
+	// Create telemetry sidecar service and pass the logger
 	sidecar := NewTelemetrySidecar(*configPath)
 
+	// Set the logger for the sidecar to avoid nil pointer
+	if err := sidecar.SetLogger(zapLogger); err != nil {
+		zapLogger.Error("Failed to set logger for sidecar", zap.Error(err))
+		os.Exit(1)
+	}
+
 	// Setup graceful shutdown context
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -37,15 +57,17 @@ func main() {
 
 	go func() {
 		sig := <-sigCh
-		logger.Printf("Received shutdown signal %v, initiating graceful shutdown", sig)
+		zapLogger.Info("Received shutdown signal, initiating graceful shutdown",
+			zap.String("signal", sig.String()))
 		cancel()
 	}()
 
-	// Run the telemetry sidecar
+	// Run the telemetry sidecar (using the Run method from sidecar.go)
 	if err := sidecar.Run(ctx); err != nil {
-		logger.Errorf("Azure CNI Telemetry Sidecar failed: %v", err)
+		zapLogger.Error("Azure CNI Telemetry Sidecar failed",
+			zap.Error(err))
 		os.Exit(1)
 	}
 
-	logger.Printf("Azure CNI Telemetry Sidecar stopped gracefully")
+	zapLogger.Info("Azure CNI Telemetry Sidecar stopped gracefully")
 }