use uber/zap for logger

Signed-off-by: Akihiko Kuroda <[email protected]>

Author: akihikokuroda

go.mod

@@ -31,6 +31,8 @@ require (
 	github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
 	github.com/yosida95/uritemplate/v3 v3.0.2 // indirect
 	github.com/yusufpapurcu/wmi v1.2.4 // indirect
+	go.uber.org/multierr v1.11.0 // indirect
+	go.uber.org/zap v1.27.0 // indirect
 	golang.org/x/sys v0.25.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 )

go.sum

@@ -70,6 +70,10 @@ github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zI
 github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4=
 github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0=
 github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
+go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
+go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
+go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
+go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
 golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

internal/common/file_logger.go

@@ -10,7 +10,11 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
+
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 var (
@@ -50,7 +54,8 @@ func generateUUID() string {
 
 // FileLogger handles logging of workflow and agent activities to files
 type FileLogger struct {
-	LogDir string
+	LogDir  string
+	loggers map[string]*zap.Logger // Map of workflow IDs to loggers
 }
 
 // NewFileLogger creates a new FileLogger instance
@@ -66,17 +71,81 @@ func NewFileLogger(logDir string) (*FileLogger, error) {
 	}
 
 	return &FileLogger{
-		LogDir: dir,
+		LogDir:  dir,
+		loggers: make(map[string]*zap.Logger),
 	}, nil
 }
 
+// createLogger creates a new zap logger for a specific workflow
+func (l *FileLogger) createLogger(workflowID string) (*zap.Logger, error) {
+	logPath := filepath.Join(l.LogDir, fmt.Sprintf("maestro_run_%s.jsonl", workflowID))
+
+	// Create encoder config for JSON format
+	encoderConfig := zapcore.EncoderConfig{
+		TimeKey:        "timestamp",
+		LevelKey:       zapcore.OmitKey, // Omit log level as it's not in original format
+		NameKey:        zapcore.OmitKey,
+		CallerKey:      zapcore.OmitKey,
+		FunctionKey:    zapcore.OmitKey,
+		MessageKey:     zapcore.OmitKey, // We'll use custom fields instead of message
+		StacktraceKey:  zapcore.OmitKey,
+		LineEnding:     zapcore.DefaultLineEnding,
+		EncodeLevel:    zapcore.LowercaseLevelEncoder,
+		EncodeTime:     zapcore.ISO8601TimeEncoder,
+		EncodeDuration: zapcore.MillisDurationEncoder,
+		EncodeCaller:   zapcore.ShortCallerEncoder,
+	}
+
+	// Create file for logging
+	file, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	if err != nil {
+		return nil, fmt.Errorf("failed to open log file: %w", err)
+	}
+
+	// Create core with JSON encoder and file writer
+	core := zapcore.NewCore(
+		zapcore.NewJSONEncoder(encoderConfig),
+		zapcore.AddSync(file),
+		zap.InfoLevel,
+	)
+
+	// Create logger
+	return zap.New(core), nil
+}
+
+// getLogger gets or creates a logger for the specified workflow
+func (l *FileLogger) getLogger(workflowID string) (*zap.Logger, error) {
+	if logger, ok := l.loggers[workflowID]; ok {
+		return logger, nil
+	}
+
+	logger, err := l.createLogger(workflowID)
+	if err != nil {
+		return nil, err
+	}
+
+	l.loggers[workflowID] = logger
+	return logger, nil
+}
+
+// Close closes all loggers and releases resources
+func (l *FileLogger) Close() {
+	for _, logger := range l.loggers {
+		// Sync ensures all buffered logs are written
+		_ = logger.Sync()
+	}
+	l.loggers = make(map[string]*zap.Logger)
+}
+
 // GenerateWorkflowID generates a unique workflow ID
 func (l *FileLogger) GenerateWorkflowID() string {
 	return generateUUID()
 }
 
 // writeJSONLine writes a JSON line to the specified log file
+// Kept for backward compatibility with tests
 func (l *FileLogger) writeJSONLine(logPath string, data interface{}) error {
+	// For backward compatibility with tests, use the direct file approach
 	jsonData, err := json.Marshal(data)
 	if err != nil {
 		return fmt.Errorf("failed to marshal JSON: %w", err)
@@ -98,6 +167,96 @@ func (l *FileLogger) writeJSONLine(logPath string, data interface{}) error {
 	return nil
 }
 
+// writeJSONLineWithZap writes a JSON line to the specified log file using zap
+// This is an internal method used by the new implementation
+func (l *FileLogger) writeJSONLineWithZap(logPath string, data interface{}) error {
+	// Extract the workflow ID from the log path
+	base := filepath.Base(logPath)
+	// Expected format: maestro_run_{workflowID}.jsonl
+	workflowID := ""
+	prefix := "maestro_run_"
+	suffix := ".jsonl"
+
+	if len(base) > len(prefix) && strings.HasPrefix(base, prefix) && strings.HasSuffix(base, suffix) {
+		workflowID = base[len(prefix) : len(base)-len(suffix)]
+	} else {
+		// If we can't extract the workflow ID, create a temporary logger
+		encoderConfig := zapcore.EncoderConfig{
+			TimeKey:        "timestamp",
+			LevelKey:       zapcore.OmitKey,
+			NameKey:        zapcore.OmitKey,
+			CallerKey:      zapcore.OmitKey,
+			FunctionKey:    zapcore.OmitKey,
+			MessageKey:     zapcore.OmitKey,
+			StacktraceKey:  zapcore.OmitKey,
+			LineEnding:     zapcore.DefaultLineEnding,
+			EncodeLevel:    zapcore.LowercaseLevelEncoder,
+			EncodeTime:     zapcore.ISO8601TimeEncoder,
+			EncodeDuration: zapcore.MillisDurationEncoder,
+			EncodeCaller:   zapcore.ShortCallerEncoder,
+		}
+
+		file, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+		if err != nil {
+			return fmt.Errorf("failed to open log file: %w", err)
+		}
+		defer file.Close()
+
+		core := zapcore.NewCore(
+			zapcore.NewJSONEncoder(encoderConfig),
+			zapcore.AddSync(file),
+			zap.InfoLevel,
+		)
+
+		logger := zap.New(core)
+		defer logger.Sync()
+
+		// Convert data to zap fields
+		jsonData, err := json.Marshal(data)
+		if err != nil {
+			return fmt.Errorf("failed to marshal JSON: %w", err)
+		}
+
+		var fields map[string]interface{}
+		if err := json.Unmarshal(jsonData, &fields); err != nil {
+			return fmt.Errorf("failed to unmarshal JSON: %w", err)
+		}
+
+		zapFields := make([]zap.Field, 0, len(fields))
+		for k, v := range fields {
+			zapFields = append(zapFields, zap.Any(k, v))
+		}
+
+		logger.Info("", zapFields...)
+		return nil
+	}
+
+	// Get or create a logger for this workflow
+	logger, err := l.getLogger(workflowID)
+	if err != nil {
+		return err
+	}
+
+	// Convert data to zap fields
+	jsonData, err := json.Marshal(data)
+	if err != nil {
+		return fmt.Errorf("failed to marshal JSON: %w", err)
+	}
+
+	var fields map[string]interface{}
+	if err := json.Unmarshal(jsonData, &fields); err != nil {
+		return fmt.Errorf("failed to unmarshal JSON: %w", err)
+	}
+
+	zapFields := make([]zap.Field, 0, len(fields))
+	for k, v := range fields {
+		zapFields = append(zapFields, zap.Any(k, v))
+	}
+
+	logger.Info("", zapFields...)
+	return nil
+}
+
 // TokenUsage represents token usage information
 type TokenUsage struct {
 	PromptTokens     int `json:"prompt_tokens,omitempty"`

internal/common/file_logger_test.go

@@ -54,6 +54,10 @@ func TestNewFileLogger(t *testing.T) {
 		if logger.LogDir != tempDir {
 			t.Errorf("Expected LogDir to be %s, got %s", tempDir, logger.LogDir)
 		}
+		// Check that loggers map is initialized
+		if logger.loggers == nil {
+			t.Error("Expected loggers map to be initialized")
+		}
 	})
 
 	// Test with default log directory
@@ -124,6 +128,7 @@ func TestGenerateWorkflowID(t *testing.T) {
 		if ids[id] {
 			t.Errorf("Generated duplicate ID: %s", id)
 		}
+
 		ids[id] = true
 	}
 }
@@ -475,3 +480,89 @@ func TestLogWorkflowRun(t *testing.T) {
 }
 
 // Made with Bob
+
+func TestGetLogger(t *testing.T) {
+	// Create a temporary directory for testing
+	tempDir, err := os.MkdirTemp("", "file_logger_test")
+	if err != nil {
+		t.Fatalf("Failed to create temp directory: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	logger, err := NewFileLogger(tempDir)
+	if err != nil {
+		t.Fatalf("NewFileLogger failed: %v", err)
+	}
+	defer logger.Close() // Clean up resources
+
+	// Test getting a new logger
+	t.Run("GetNewLogger", func(t *testing.T) {
+		workflowID := "test-workflow"
+		zapLogger, err := logger.getLogger(workflowID)
+		if err != nil {
+			t.Fatalf("getLogger failed: %v", err)
+		}
+		if zapLogger == nil {
+			t.Error("Expected non-nil logger")
+		}
+
+		// Check that logger was cached
+		if cachedLogger, ok := logger.loggers[workflowID]; !ok || cachedLogger != zapLogger {
+			t.Error("Logger was not properly cached")
+		}
+	})
+
+	// Test getting an existing logger
+	t.Run("GetExistingLogger", func(t *testing.T) {
+		workflowID := "test-workflow-2"
+		firstLogger, err := logger.getLogger(workflowID)
+		if err != nil {
+			t.Fatalf("First getLogger failed: %v", err)
+		}
+
+		secondLogger, err := logger.getLogger(workflowID)
+		if err != nil {
+			t.Fatalf("Second getLogger failed: %v", err)
+		}
+
+		if firstLogger != secondLogger {
+			t.Error("Expected same logger instance to be returned")
+		}
+	})
+}
+
+func TestClose(t *testing.T) {
+	// Create a temporary directory for testing
+	tempDir, err := os.MkdirTemp("", "file_logger_test")
+	if err != nil {
+		t.Fatalf("Failed to create temp directory: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	logger, err := NewFileLogger(tempDir)
+	if err != nil {
+		t.Fatalf("NewFileLogger failed: %v", err)
+	}
+
+	// Create some loggers
+	workflowIDs := []string{"workflow1", "workflow2", "workflow3"}
+	for _, id := range workflowIDs {
+		_, err := logger.getLogger(id)
+		if err != nil {
+			t.Fatalf("Failed to get logger for %s: %v", id, err)
+		}
+	}
+
+	// Verify loggers exist
+	if len(logger.loggers) != len(workflowIDs) {
+		t.Errorf("Expected %d loggers, got %d", len(workflowIDs), len(logger.loggers))
+	}
+
+	// Close loggers
+	logger.Close()
+
+	// Verify loggers map is empty
+	if len(logger.loggers) != 0 {
+		t.Errorf("Expected empty loggers map after Close, got %d entries", len(logger.loggers))
+	}
+}