From 8d201a82fd937e21af81e90484ae0f0042455cb9 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 15 Jan 2025 09:02:53 +0100 Subject: [PATCH] agent: Fixed a bug where syslog error messages marked as notice. (#24820) The mapping between Nomad log level identifiers and syslog priorities did not handle the error level string correctly. --- .changelog/24820.txt | 3 ++ command/agent/command.go | 9 +---- command/agent/syslog.go | 29 ++++++++------ command/agent/syslog_test.go | 78 +++++++++++++++++++++++++++--------- 4 files changed, 80 insertions(+), 39 deletions(-) create mode 100644 .changelog/24820.txt diff --git a/.changelog/24820.txt b/.changelog/24820.txt new file mode 100644 index 000000000..6a0fb0bd9 --- /dev/null +++ b/.changelog/24820.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: Fixed a bug where Nomad error log messages within syslog showed via the notice priority +``` diff --git a/command/agent/command.go b/command/agent/command.go index 0f7e9613f..8077b4ea8 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -574,20 +574,13 @@ func SetupLoggers(ui cli.Ui, config *Config) (*logutils.LevelFilter, *gatedwrite // Create a log writer, and wrap a logOutput around it writers := []io.Writer{logFilter} logLevel := strings.ToUpper(config.LogLevel) - logLevelMap := map[string]gsyslog.Priority{ - "ERROR": gsyslog.LOG_ERR, - "WARN": gsyslog.LOG_WARNING, - "INFO": gsyslog.LOG_INFO, - "DEBUG": gsyslog.LOG_DEBUG, - "TRACE": gsyslog.LOG_DEBUG, - } if logLevel == "OFF" { config.EnableSyslog = false } // Check if syslog is enabled if config.EnableSyslog { ui.Output(fmt.Sprintf("Config enable_syslog is `true` with log_level=%v", config.LogLevel)) - l, err := gsyslog.NewLogger(logLevelMap[logLevel], config.SyslogFacility, "nomad") + l, err := gsyslog.NewLogger(getSysLogPriority(logLevel), config.SyslogFacility, "nomad") if err != nil { ui.Error(fmt.Sprintf("Syslog setup failed: %v", err)) return nil, nil, nil diff --git a/command/agent/syslog.go b/command/agent/syslog.go index 3c8a860eb..81900dca7 100644 --- a/command/agent/syslog.go +++ b/command/agent/syslog.go @@ -10,15 +10,26 @@ import ( "github.com/hashicorp/logutils" ) -// levelPriority is used to map a log level to a -// syslog priority level +// levelPriority is used to map a log level to a syslog priority level. The +// level strings should match those described within LevelFilter except for +// "OFF" which disables syslog. var levelPriority = map[string]gsyslog.Priority{ "TRACE": gsyslog.LOG_DEBUG, "DEBUG": gsyslog.LOG_INFO, "INFO": gsyslog.LOG_NOTICE, "WARN": gsyslog.LOG_WARNING, - "ERR": gsyslog.LOG_ERR, - "CRIT": gsyslog.LOG_CRIT, + "ERROR": gsyslog.LOG_ERR, +} + +// getSysLogPriority returns the syslog priority value associated to the passed +// log level. If the log level does not have a known mapping, the notice +// priority is returned. +func getSysLogPriority(level string) gsyslog.Priority { + priority, ok := levelPriority[level] + if !ok { + priority = gsyslog.LOG_NOTICE + } + return priority } // SyslogWrapper is used to cleanup log messages before @@ -48,13 +59,7 @@ func (s *SyslogWrapper) Write(p []byte) (int, error) { } } - // Each log level will be handled by a specific syslog priority - priority, ok := levelPriority[level] - if !ok { - priority = gsyslog.LOG_NOTICE - } - - // Attempt the write - err := s.l.WriteLevel(priority, afterLevel) + // Attempt to write using the converted syslog priority. + err := s.l.WriteLevel(getSysLogPriority(level), afterLevel) return len(p), err } diff --git a/command/agent/syslog_test.go b/command/agent/syslog_test.go index 759500826..ae63e6328 100644 --- a/command/agent/syslog_test.go +++ b/command/agent/syslog_test.go @@ -4,46 +4,86 @@ package agent import ( - "os" "runtime" "testing" gsyslog "github.com/hashicorp/go-syslog" - "github.com/hashicorp/logutils" "github.com/hashicorp/nomad/ci" + "github.com/shoenig/test/must" ) +func Test_getSysLogPriority(t *testing.T) { + ci.Parallel(t) + + if runtime.GOOS == "windows" { + t.Skip("Syslog not supported on Windows") + } + + testCases := []struct { + name string + inputLogLevel string + expectedSyslogPriority gsyslog.Priority + }{ + { + name: "trace", + inputLogLevel: "TRACE", + expectedSyslogPriority: gsyslog.LOG_DEBUG, + }, + { + name: "debug", + inputLogLevel: "DEBUG", + expectedSyslogPriority: gsyslog.LOG_INFO, + }, + { + name: "info", + inputLogLevel: "INFO", + expectedSyslogPriority: gsyslog.LOG_NOTICE, + }, + { + name: "warn", + inputLogLevel: "WARN", + expectedSyslogPriority: gsyslog.LOG_WARNING, + }, + { + name: "error", + inputLogLevel: "ERROR", + expectedSyslogPriority: gsyslog.LOG_ERR, + }, + { + name: "unknown", + inputLogLevel: "UNKNOWN", + expectedSyslogPriority: gsyslog.LOG_NOTICE, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actualPriority := getSysLogPriority(tc.inputLogLevel) + must.Eq(t, tc.expectedSyslogPriority, actualPriority) + }) + } +} + func TestSyslogFilter(t *testing.T) { ci.Parallel(t) if runtime.GOOS == "windows" { t.Skip("Syslog not supported on Windows") } - if os.Getenv("TRAVIS") == "true" { - t.Skip("Syslog not supported on travis-ci") - } - l, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, "LOCAL0", "consul") - if err != nil { - t.Fatalf("err: %s", err) - } + l, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, "LOCAL0", "nomad") + must.NoError(t, err) filt := LevelFilter() - filt.MinLevel = logutils.LogLevel("INFO") + filt.MinLevel = "INFO" s := &SyslogWrapper{l, filt} n, err := s.Write([]byte("[INFO] test")) if err != nil { t.Fatalf("err: %s", err) } - if n == 0 { - t.Fatalf("should have logged") - } + must.NonZero(t, n) n, err = s.Write([]byte("[DEBUG] test")) - if err != nil { - t.Fatalf("err: %s", err) - } - if n != 0 { - t.Fatalf("should not have logged") - } + must.NoError(t, err) + must.Zero(t, n) }