From b9b73e7658e714c3ad42be487f744ce746ffed9a Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Fri, 2 Oct 2020 15:21:18 -0400 Subject: [PATCH] Switch logging to promlog --- .golangci.errcheck-exclude | 4 ++ .golangci.yml | 3 ++ cgroup_exporter.go | 85 +++++++++++++++++++++----------------- cgroup_exporter_test.go | 35 +++++++++++----- go.mod | 2 +- go.sum | 6 +-- 6 files changed, 83 insertions(+), 52 deletions(-) create mode 100644 .golangci.errcheck-exclude create mode 100644 .golangci.yml diff --git a/.golangci.errcheck-exclude b/.golangci.errcheck-exclude new file mode 100644 index 0000000..ad914f4 --- /dev/null +++ b/.golangci.errcheck-exclude @@ -0,0 +1,4 @@ +// Used in HTTP handlers, any error is handled by the server itself. +(net/http.ResponseWriter).Write +// Never check for logger errors. +(github.com/go-kit/kit/log.Logger).Log diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..c1d979d --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,3 @@ +linters-settings: + errcheck: + exclude: .golangci.errcheck-exclude diff --git a/cgroup_exporter.go b/cgroup_exporter.go index 36c95c1..c8467b6 100644 --- a/cgroup_exporter.go +++ b/cgroup_exporter.go @@ -26,9 +26,12 @@ import ( "strings" "github.com/containerd/cgroups" + "github.com/go-kit/kit/log" + "github.com/go-kit/kit/log/level" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/prometheus/common/log" + "github.com/prometheus/common/promlog" + "github.com/prometheus/common/promlog/flag" "github.com/prometheus/common/version" "github.com/prometheus/procfs" "gopkg.in/alecthomas/kingpin.v2" @@ -92,6 +95,7 @@ type Exporter struct { memswFailCount *prometheus.Desc info *prometheus.Desc processExec *prometheus.Desc + logger log.Logger } func fileExists(filename string) bool { @@ -120,19 +124,19 @@ func subsystem() ([]cgroups.Subsystem, error) { return s, nil } -func getCPUs(name string) ([]string, error) { +func getCPUs(name string, logger log.Logger) ([]string, error) { cpusPath := fmt.Sprintf("%s/cpuset%s/cpuset.cpus", *cgroupRoot, name) if !fileExists(cpusPath) { return nil, nil } cpusData, err := ioutil.ReadFile(cpusPath) if err != nil { - log.Errorf("Error reading %s: %s", cpusPath, err.Error()) + level.Error(logger).Log("msg", "Error reading cpuset", "cpuset", cpusPath, "err", err) return nil, err } cpus, err := parseCpuSet(strings.TrimSuffix(string(cpusData), "\n")) if err != nil { - log.Errorf("Error parsing cpu set %s", err.Error()) + level.Error(logger).Log("msg", "Error parsing cpu set", "cpuset", cpusPath, "err", err) return nil, err } return cpus, nil @@ -172,7 +176,7 @@ func parseCpuSet(cpuset string) ([]string, error) { return cpus, nil } -func getInfo(name string, metric *CgroupMetric) { +func getInfo(name string, metric *CgroupMetric, logger log.Logger) { pathBase := filepath.Base(name) userSlicePattern := regexp.MustCompile("^user-([0-9]+).slice$") userSliceMatch := userSlicePattern.FindStringSubmatch(pathBase) @@ -181,7 +185,7 @@ func getInfo(name string, metric *CgroupMetric) { metric.uid = userSliceMatch[1] user, err := user.LookupId(metric.uid) if err != nil { - log.Errorf("Error looking up user slice uid %s: %s", metric.uid, err.Error()) + level.Error(logger).Log("msg", "Error looking up user slice uid", "uid", metric.uid, "err", err) } else { metric.username = user.Username } @@ -195,7 +199,7 @@ func getInfo(name string, metric *CgroupMetric) { metric.jobid = slurmMatch[2] user, err := user.LookupId(metric.uid) if err != nil { - log.Errorf("Error looking up slurm uid %s: %s", metric.uid, err.Error()) + level.Error(logger).Log("msg", "Error looking up slurm uid", "uid", metric.uid, "err", err) } else { metric.username = user.Username } @@ -209,26 +213,26 @@ func getInfo(name string, metric *CgroupMetric) { } } -func getProcInfo(pids []int, metric *CgroupMetric) { +func getProcInfo(pids []int, metric *CgroupMetric, logger log.Logger) { executables := make(map[string]float64) procFS, err := procfs.NewFS(*procRoot) if err != nil { - log.Errorf("Unable to open procfs at %s", *procRoot) + level.Error(logger).Log("msg", "Unable to open procfs", "path", *procRoot) return } for _, pid := range pids { proc, err := procFS.Proc(pid) if err != nil { - log.Errorf("Unable to read PID=%d", pid) + level.Error(logger).Log("msg", "Unable to read PID", "pid", pid) continue } executable, err := proc.Executable() if err != nil { - log.Errorf("Unable to get executable for PID=%d", pid) + level.Error(logger).Log("msg", "Unable to get executable for PID", "pid", pid) continue } if len(executable) > *collectProcMaxExec { - log.Debugf("Executable will be truncated executable=%s len=%d pid=%d", executable, len(executable), pid) + level.Debug(logger).Log("msg", "Executable will be truncated", "executable", executable, "len", len(executable), "pid", pid) executable = executable[len(executable)-*collectProcMaxExec:] executable = fmt.Sprintf("...%s", executable) } @@ -237,12 +241,12 @@ func getProcInfo(pids []int, metric *CgroupMetric) { metric.processExec = executables } -func getName(p cgroups.Process, path string) (string, error) { +func getName(p cgroups.Process, path string, logger log.Logger) (string, error) { cpuacctPath := filepath.Join(*cgroupRoot, "cpuacct") name := strings.TrimPrefix(p.Path, cpuacctPath) name = strings.TrimSuffix(name, "/") dirs := strings.Split(name, "/") - log.Debugf("cgroup name dirs %v", dirs) + level.Debug(logger).Log("msg", "cgroup name", "dirs", fmt.Sprintf("%v", dirs)) // Handle user.slice, system.slice and torque if len(dirs) == 3 { return name, nil @@ -262,7 +266,7 @@ func getName(p cgroups.Process, path string) (string, error) { return name, nil } -func NewExporter(paths []string) *Exporter { +func NewExporter(paths []string, logger log.Logger) *Exporter { return &Exporter{ paths: paths, cpuUser: prometheus.NewDesc(prometheus.BuildFQName(namespace, "cpu", "user_seconds"), @@ -297,6 +301,7 @@ func NewExporter(paths []string) *Exporter { "Count of instances of a given process", []string{"cgroup", "exec"}, nil), collectError: prometheus.NewDesc(prometheus.BuildFQName(namespace, "exporter", "collect_error"), "Indicates collection error, 0=no error, 1=error", []string{"cgroup"}, nil), + logger: logger, } } @@ -304,28 +309,28 @@ func (e *Exporter) collect() ([]CgroupMetric, error) { var names []string var metrics []CgroupMetric for _, path := range e.paths { - log.Debugf("Loading cgroup path %v", path) + level.Debug(e.logger).Log("msg", "Loading cgroup", "path", path) control, err := cgroups.Load(subsystem, cgroups.StaticPath(path)) if err != nil { - log.Errorf("Error loading cgroup subsystem path %s: %s", path, err.Error()) + level.Error(e.logger).Log("msg", "Error loading cgroup subsystem", "path", path, "err", err) metric := CgroupMetric{name: path, err: true} metrics = append(metrics, metric) continue } processes, err := control.Processes(cgroups.Cpuacct, true) if err != nil { - log.Errorf("Error loading cgroup processes for path %s: %s", path, err.Error()) + level.Error(e.logger).Log("msg", "Error loading cgroup processes", "path", path, "err", err) metric := CgroupMetric{name: path, err: true} metrics = append(metrics, metric) continue } - log.Debugf("Found %d processes", len(processes)) + level.Debug(e.logger).Log("msg", "Found processes", "processes", len(processes)) pids := make(map[string][]int) for _, p := range processes { - log.Debugf("Get name of process=%s pid=%d path=%s", p.Path, p.Pid, path) - name, err := getName(p, path) + level.Debug(e.logger).Log("msg", "Get Name", "process", p.Path, "pid", p.Pid, "path", path) + name, err := getName(p, path, e.logger) if err != nil { - log.Errorf("Error getting cgroup name for for process %s at path %s: %s", p.Path, path, err.Error()) + level.Error(e.logger).Log("msg", "Error getting cgroup name for process", "process", p.Path, "path", path, "err", err) continue } if !sliceContains(names, name) { @@ -342,12 +347,12 @@ func (e *Exporter) collect() ([]CgroupMetric, error) { } for _, name := range names { metric := CgroupMetric{name: name} - log.Debugf("Loading cgroup path %s", name) + level.Debug(e.logger).Log("msg", "Loading cgroup", "path", name) ctrl, err := cgroups.Load(subsystem, func(subsystem cgroups.Name) (string, error) { return name, nil }) if err != nil { - log.Errorf("Failed to load cgroups for %s: %s", name, err.Error()) + level.Error(e.logger).Log("msg", "Failed to load cgroups", "path", name, "err", err) metric.err = true metrics = append(metrics, metric) continue @@ -364,17 +369,17 @@ func (e *Exporter) collect() ([]CgroupMetric, error) { metric.memswUsed = float64(stats.Memory.Swap.Usage) metric.memswTotal = float64(stats.Memory.Swap.Limit) metric.memswFailCount = float64(stats.Memory.Swap.Failcnt) - if cpus, err := getCPUs(name); err == nil { + if cpus, err := getCPUs(name, e.logger); err == nil { metric.cpus = len(cpus) metric.cpu_list = strings.Join(cpus, ",") } - getInfo(name, &metric) + getInfo(name, &metric, e.logger) if *collectProc { if val, ok := pids[name]; ok { - log.Debugf("Get process info for pids=%v", val) - getProcInfo(val, &metric) + level.Debug(e.logger).Log("msg", "Get process info", "pids", fmt.Sprintf("%v", val)) + getProcInfo(val, &metric, e.logger) } else { - log.Errorf("Unable to get PIDs for %s", name) + level.Error(e.logger).Log("msg", "Unable to get PIDs", "path", name) } } metrics = append(metrics, metric) @@ -434,13 +439,13 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) { } } -func metricsHandler() http.HandlerFunc { +func metricsHandler(logger log.Logger) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { registry := prometheus.NewRegistry() paths := strings.Split(*configPaths, ",") - exporter := NewExporter(paths) + exporter := NewExporter(paths, logger) registry.MustRegister(exporter) gatherers := prometheus.Gatherers{registry} @@ -456,14 +461,16 @@ func metricsHandler() http.HandlerFunc { func main() { metricsEndpoint := "/metrics" - log.AddFlags(kingpin.CommandLine) + promlogConfig := &promlog.Config{} + flag.AddFlags(kingpin.CommandLine, promlogConfig) kingpin.Version(version.Print("cgroup_exporter")) kingpin.HelpFlag.Short('h') kingpin.Parse() - log.Infoln("Starting cgroup_exporter", version.Info()) - log.Infoln("Build context", version.BuildContext()) - log.Infoln("Starting Server:", *listenAddress) + logger := promlog.New(promlogConfig) + level.Info(logger).Log("msg", "Starting cgroup_exporter", "version", version.Info()) + level.Info(logger).Log("msg", "Build context", "build_context", version.BuildContext()) + level.Info(logger).Log("msg", "Starting Server", "address", *listenAddress) http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { //nolint:errcheck @@ -475,6 +482,10 @@ func main() { `)) }) - http.Handle(metricsEndpoint, metricsHandler()) - log.Fatal(http.ListenAndServe(*listenAddress, nil)) + http.Handle(metricsEndpoint, metricsHandler(logger)) + err := http.ListenAndServe(*listenAddress, nil) + if err != nil { + level.Error(logger).Log("err", err) + os.Exit(1) + } } diff --git a/cgroup_exporter_test.go b/cgroup_exporter_test.go index 2542ba7..a72854b 100644 --- a/cgroup_exporter_test.go +++ b/cgroup_exporter_test.go @@ -15,8 +15,6 @@ package main import ( "fmt" - "github.com/prometheus/common/log" - kingpin "gopkg.in/alecthomas/kingpin.v2" "io/ioutil" "net/http" "os" @@ -26,6 +24,9 @@ import ( "strings" "testing" "time" + + "github.com/go-kit/kit/log" + kingpin "gopkg.in/alecthomas/kingpin.v2" ) const ( @@ -34,7 +35,7 @@ const ( func TestMain(m *testing.M) { if _, err := kingpin.CommandLine.Parse([]string{"--config.paths=/user.slice"}); err != nil { - log.Fatal(err) + os.Exit(1) } _, filename, _, _ := runtime.Caller(0) dir := filepath.Dir(filename) @@ -45,10 +46,14 @@ func TestMain(m *testing.M) { varTrue := true disableExporterMetrics = &varTrue collectProc = &varTrue - _ = log.Base().SetLevel("debug") + w := log.NewSyncWriter(os.Stderr) + logger := log.NewLogfmtLogger(w) go func() { - http.Handle("/metrics", metricsHandler()) - log.Fatal(http.ListenAndServe(address, nil)) + http.Handle("/metrics", metricsHandler(logger)) + err := http.ListenAndServe(address, nil) + if err != nil { + os.Exit(1) + } }() time.Sleep(1 * time.Second) @@ -80,7 +85,9 @@ func TestParseCpuSet(t *testing.T) { func TestGetProcInfo(t *testing.T) { metric := CgroupMetric{} - getProcInfo([]int{95521, 95525}, &metric) + w := log.NewSyncWriter(os.Stderr) + logger := log.NewLogfmtLogger(w) + getProcInfo([]int{95521, 95525}, &metric, logger) if val, ok := metric.processExec["/bin/bash"]; !ok { t.Errorf("Process /bin/bash not in metrics") return @@ -91,7 +98,7 @@ func TestGetProcInfo(t *testing.T) { } varLen := 4 collectProcMaxExec = &varLen - getProcInfo([]int{95521, 95525}, &metric) + getProcInfo([]int{95521, 95525}, &metric, logger) if val, ok := metric.processExec["...bash"]; !ok { t.Errorf("Process /bin/bash not in metrics, found: %v", metric.processExec) return @@ -105,7 +112,9 @@ func TestGetProcInfo(t *testing.T) { func TestCollectUserSlice(t *testing.T) { varFalse := false collectProc = &varFalse - exporter := NewExporter([]string{"/user.slice"}) + w := log.NewSyncWriter(os.Stderr) + logger := log.NewLogfmtLogger(w) + exporter := NewExporter([]string{"/user.slice"}, logger) metrics, err := exporter.collect() if err != nil { t.Errorf("Unexpected error: %s", err.Error()) @@ -161,7 +170,9 @@ func TestCollectSLURM(t *testing.T) { collectProc = &varTrue varLen := 100 collectProcMaxExec = &varLen - exporter := NewExporter([]string{"/slurm"}) + w := log.NewSyncWriter(os.Stderr) + logger := log.NewLogfmtLogger(w) + exporter := NewExporter([]string{"/slurm"}, logger) metrics, err := exporter.collect() if err != nil { t.Errorf("Unexpected error: %s", err.Error()) @@ -225,7 +236,9 @@ func TestCollectSLURM(t *testing.T) { func TestCollectTorque(t *testing.T) { varFalse := false collectProc = &varFalse - exporter := NewExporter([]string{"/torque"}) + w := log.NewSyncWriter(os.Stderr) + logger := log.NewLogfmtLogger(w) + exporter := NewExporter([]string{"/torque"}, logger) metrics, err := exporter.collect() if err != nil { t.Errorf("Unexpected error: %s", err.Error()) diff --git a/go.mod b/go.mod index 061b5bd..94c52aa 100644 --- a/go.mod +++ b/go.mod @@ -5,10 +5,10 @@ go 1.14 require ( github.com/containerd/cgroups v0.0.0-20200824123100-0b889c03f102 github.com/coreos/go-systemd/v22 v22.1.0 // indirect + github.com/go-kit/kit v0.10.0 github.com/prometheus/client_golang v1.7.1 github.com/prometheus/common v0.14.0 github.com/prometheus/procfs v0.2.0 - github.com/sirupsen/logrus v1.7.0 // indirect golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f // indirect google.golang.org/protobuf v1.25.0 // indirect gopkg.in/alecthomas/kingpin.v2 v2.2.6 diff --git a/go.sum b/go.sum index 42b0347..e5fc196 100644 --- a/go.sum +++ b/go.sum @@ -72,11 +72,14 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= +github.com/go-kit/kit v0.10.0 h1:dXFJfIHVvUcpSgDOV+Ne6t7jXri8Tfv2uOLHUZ2XNuo= github.com/go-kit/kit v0.10.0/go.mod h1:xUsJbQ/Fp4kEt7AFgCuvyX4a71u8h9jB8tj/ORgOZ7o= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= +github.com/go-logfmt/logfmt v0.5.0 h1:TrB8swr/68K7m9CcGut2g3UOihhbcbiMAYiuTXdEih4= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= +github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/godbus/dbus/v5 v5.0.3 h1:ZqHaoEF7TBzh4jzPmqVhE/5A1z9of6orkAe5uHoAeME= github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= @@ -268,8 +271,6 @@ github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPx github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= -github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM= -github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM= @@ -356,7 +357,6 @@ golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191220142924-d4481acd189f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200106162015-b016eb3dc98e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200124204421-9fbb57f87de9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=