From 9b253847881839afb80e1e4fb721d1ad3eacf093 Mon Sep 17 00:00:00 2001 From: Janelle Law Date: Mon, 27 Nov 2023 20:47:00 +0000 Subject: [PATCH] share lock among log file writers run linter remove unused file desciptors --- pkg/agent/v2/agent.go | 2 +- pkg/logger/logger.go | 2 +- pkg/logger/plugin_logger.go | 49 ++++++++++++++++----------------- pkg/logger/remotelogs/server.go | 6 ++-- 4 files changed, 28 insertions(+), 31 deletions(-) diff --git a/pkg/agent/v2/agent.go b/pkg/agent/v2/agent.go index 8541a054d..a0a65be57 100644 --- a/pkg/agent/v2/agent.go +++ b/pkg/agent/v2/agent.go @@ -140,7 +140,7 @@ func New(ctx context.Context, conf *v1beta1.AgentConfig, opts ...AgentOption) (* ctx = logger.WithAgentId(ctx, id) ctx = logger.WithMode(ctx, meta.ModeAgent) - lg := logger.New(logger.WithLogLevel(level), logger.WithFileWriter(logger.WriteOnlyFile(logger.GetLogFileName(ctx)))).WithGroup("agent") + lg := logger.New(logger.WithLogLevel(level), logger.WithFileWriter(logger.WriteOnlyFile(logger.GetLogFileName(id)))).WithGroup("agent") lg.Debug(fmt.Sprintf("using log level: %s", level.String())) var pl *plugins.PluginLoader diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index ce32508a3..aa708b3f1 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -37,7 +37,7 @@ var ( DefaultTimeFormat = "2006 Jan 02 15:04:05" logSampler = &sampler{} levelString = []string{"DEBUG", "INFO", "WARN", "ERROR"} - fileDesc gpkgsync.Map[string, afero.File] + fileDesc gpkgsync.Map[string, *FileWriter] ) func init() { diff --git a/pkg/logger/plugin_logger.go b/pkg/logger/plugin_logger.go index 5dd29e20d..28fe8addd 100644 --- a/pkg/logger/plugin_logger.go +++ b/pkg/logger/plugin_logger.go @@ -8,6 +8,7 @@ import ( "os" "path" "sync" + "testing" controlv1 "github.com/rancher/opni/pkg/apis/control/v1" "github.com/rancher/opni/pkg/plugins/meta" @@ -55,30 +56,32 @@ func PluginLoggerFromContext(ctx context.Context) *slog.Logger { return logger.(*slog.Logger) } -func ReadOnlyFile(filename string) afero.File { - f, err := logFs.OpenFile(filename, os.O_RDONLY|os.O_CREATE, 0666) +func ReadFile(filename string) afero.File { + f, err := logFs.OpenFile(filename, os.O_CREATE|os.O_RDONLY, 0666) if err != nil { panic(err) } return f } -func WriteOnlyFile(filename string) afero.File { - newFile, err := logFs.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666) - if err != nil { - panic(err) +func WriteOnlyFile(filename string) *FileWriter { + fileWriter, ok := fileDesc.Load(filename) + if ok { + return fileWriter } - fd, loaded := fileDesc.LoadOrStore(filename, newFile) - if loaded { - newFile.Close() + newFile, err := logFs.OpenFile(filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666) + if err != nil { + panic(err) } + newFileWriter := newFileWriter(newFile) + fileDesc.Store(filename, newFileWriter) - return fd + return newFileWriter } -func GetLogFileName(ctx context.Context) string { - return fmt.Sprintf("plugin_%s_%s", meta.ModeAgent, getAgentId(ctx)) +func GetLogFileName(agentId string) string { + return fmt.Sprintf("plugin_%s_%s", meta.ModeAgent, agentId) } func WithMode(ctx context.Context, mode meta.PluginMode) context.Context { @@ -92,12 +95,12 @@ func WithAgentId(ctx context.Context, agentId string) context.Context { // writer used for agent loggers and plugin loggers type RemotePluginWriter struct { textWriter *slog.Logger - fileWriter *fileWriter + fileWriter *FileWriter protoWriter io.Writer } func newPluginWriter(ctx context.Context) *RemotePluginWriter { - if isInProcessPluginLogger(ctx) { + if isInProcessPluginLogger() { mode := getMode(ctx) if mode == meta.ModeAgent { return NewPluginFileWriter(ctx) @@ -112,7 +115,7 @@ func newPluginWriter(ctx context.Context) *RemotePluginWriter { func NewPluginFileWriter(ctx context.Context) *RemotePluginWriter { return &RemotePluginWriter{ textWriter: New(WithWriter(os.Stderr), WithDisableCaller()), - fileWriter: newLogFileWriter(ctx), + fileWriter: WriteOnlyFile(GetLogFileName(getAgentId(ctx))), protoWriter: io.Discard, } } @@ -207,23 +210,19 @@ func (w *RemotePluginWriter) writeProtoToText(b []byte) (int, error) { } // stores agent and agent plugin logs, retrieved with debug cli -type fileWriter struct { +type FileWriter struct { file afero.File mu *sync.RWMutex } -func newLogFileWriter(ctx context.Context) *fileWriter { - return newFileWriter(WriteOnlyFile(GetLogFileName(ctx))) -} - -func newFileWriter(f afero.File) *fileWriter { - return &fileWriter{ +func newFileWriter(f afero.File) *FileWriter { + return &FileWriter{ file: f, mu: &sync.RWMutex{}, } } -func (f fileWriter) Write(b []byte) (int, error) { +func (f *FileWriter) Write(b []byte) (int, error) { if f.file == nil { return 0, nil } @@ -241,8 +240,8 @@ func getAgentId(ctx context.Context) string { return "" } -func isInProcessPluginLogger(ctx context.Context) bool { - return (getModuleBasename() == "testenv") +func isInProcessPluginLogger() bool { + return getModuleBasename() == "testenv" || testing.Testing() } func getModuleBasename() string { diff --git a/pkg/logger/remotelogs/server.go b/pkg/logger/remotelogs/server.go index f10adb1f1..bfba941e7 100644 --- a/pkg/logger/remotelogs/server.go +++ b/pkg/logger/remotelogs/server.go @@ -62,10 +62,8 @@ func (ls *LogServer) StreamLogs(req *controlv1.LogStreamRequest, server controlv nameFilters := req.Filters.NamePattern follow := req.Follow - ctx := logger.WithAgentId(server.Context(), cluster.StreamAuthorizedID(server.Context())) - f := logger.ReadOnlyFile(logger.GetLogFileName(ctx)) - ls.logger.Warn("streaming from file...", "name", logger.GetLogFileName(ctx)) - + filename := logger.GetLogFileName(cluster.StreamAuthorizedID(server.Context())) + f := logger.ReadFile(filename) defer f.Close() for {