From 848b814b98c5332b59369d6bda1a975087b1651c Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Tue, 5 Nov 2024 16:35:25 -0800 Subject: [PATCH 1/5] address issues raised by golangci-lint --- .gitignore | 1 + cmd/config/config.go | 31 ++++++++++-- cmd/metrics/metrics.go | 90 +++++++++++++++++++++++---------- cmd/metrics/print.go | 91 +++++++++++++++++++++++----------- cmd/report/report.go | 2 +- cmd/root.go | 5 +- cmd/telemetry/telemetry.go | 2 +- internal/common/common.go | 27 +++++++--- internal/report/report.go | 68 ++++++++++++------------- internal/script/script.go | 14 +++++- internal/script/script_test.go | 7 ++- internal/target/target.go | 5 +- 12 files changed, 237 insertions(+), 106 deletions(-) diff --git a/.gitignore b/.gitignore index dad0ed4..9e98870 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ /tools/bin /dist /internal/script/resources/x86_64 +/test \ No newline at end of file diff --git a/cmd/config/config.go b/cmd/config/config.go index fb9bf7b..b9baeb9 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -232,7 +232,13 @@ func runCmd(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true return err } - defer myTarget.RemoveDirectory(targetTempDir) + defer func() { + err = myTarget.RemoveDirectory(targetTempDir) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to remove target directory: %+v\n", err) + slog.Error(err.Error()) + } + }() } // print config prior to changes if err := printConfig(myTargets, localTempDir); err != nil { @@ -310,18 +316,33 @@ func printConfig(myTargets []target.Target, localTempDir string) (err error) { } for _, myTarget := range myTargets { multiSpinner := progress.NewMultiSpinner() - multiSpinner.AddSpinner(myTarget.GetName()) + err = multiSpinner.AddSpinner(myTarget.GetName()) + if err != nil { + err = fmt.Errorf("failed to add spinner: %v", err) + return + } multiSpinner.Start() - multiSpinner.Status(myTarget.GetName(), "collecting data") + err = multiSpinner.Status(myTarget.GetName(), "collecting data") + if err != nil { + err = fmt.Errorf("failed to set spinner status: %v", err) + return + } // run the scripts var scriptOutputs map[string]script.ScriptOutput if scriptOutputs, err = script.RunScripts(myTarget, scriptsToRun, true, localTempDir); err != nil { err = fmt.Errorf("failed to run collection scripts: %v", err) - multiSpinner.Status(myTarget.GetName(), "error collecting data") + errSpinner := multiSpinner.Status(myTarget.GetName(), "error collecting data") + if errSpinner != nil { + slog.Error(errSpinner.Error()) + } multiSpinner.Finish() return } - multiSpinner.Status(myTarget.GetName(), "collection complete") + err = multiSpinner.Status(myTarget.GetName(), "collection complete") + if err != nil { + err = fmt.Errorf("failed to set spinner status: %v", err) + return + } multiSpinner.Finish() // process the tables, i.e., get field values from raw script output tableNames := []string{report.ConfigurationTableName} diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index 2d564ea..fcfef29 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -671,7 +671,10 @@ func runCmd(cmd *cobra.Command, args []string) error { } else { finalMessage += fmt.Sprintf(" for %d seconds", flagDuration) } - multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage) + err = multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage) + if err != nil { + slog.Error("failed to set status", slog.String("target", targetContexts[i].target.GetName()), slog.String("error", err.Error())) + } } go collectOnTarget(&targetContexts[i], localTempDir, localOutputDir, channelTargetError, multiSpinner.Status) } @@ -687,7 +690,10 @@ func runCmd(cmd *cobra.Command, args []string) error { // finalize and stop the spinner for _, targetContext := range targetContexts { if targetContext.err == nil { - multiSpinner.Status(targetContext.target.GetName(), "collection complete") + err = multiSpinner.Status(targetContext.target.GetName(), "collection complete") + if err != nil { + slog.Error("failed to set status", slog.String("target", targetContext.target.GetName()), slog.String("error", err.Error())) + } } } // summarize outputs @@ -750,11 +756,15 @@ func runCmd(cmd *cobra.Command, args []string) error { func prepareTarget(targetContext *targetContext, targetTempRoot string, localTempDir string, localPerfPath string, channelError chan targetError, statusUpdate progress.MultiSpinnerUpdateFunc) { myTarget := targetContext.target - // create a temporary directory on the target - statusUpdate(myTarget.GetName(), "configuring target") var err error + // create a temporary directory on the target + if err = statusUpdate(myTarget.GetName(), "configuring target"); err != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", err.Error())) + } if targetContext.tempDir, err = myTarget.CreateTempDirectory(targetTempRoot); err != nil { - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -764,7 +774,9 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem var nmiWatchdogEnabled bool if nmiWatchdogEnabled, err = NMIWatchdogEnabled(myTarget); err != nil { err = fmt.Errorf("failed to retrieve NMI watchdog status: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -772,7 +784,9 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem if nmiWatchdogEnabled { if err = DisableNMIWatchdog(myTarget, localTempDir); err != nil { err = fmt.Errorf("failed to disable NMI watchdog: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -784,14 +798,18 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem if !flagNoRoot { if targetContext.perfMuxIntervals, err = GetMuxIntervals(myTarget, localTempDir); err != nil { err = fmt.Errorf("failed to get perf mux intervals: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return } if err = SetAllMuxIntervals(myTarget, flagPerfMuxInterval, localTempDir); err != nil { err = fmt.Errorf("failed to set all perf mux intervals: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -800,7 +818,9 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem // get the full path to the perf binary if targetContext.perfPath, err = getPerfPath(myTarget, localPerfPath); err != nil { err = fmt.Errorf("failed to find perf: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -816,11 +836,15 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr return } // load metadata - statusUpdate(myTarget.GetName(), "collecting metadata") + if statusUpdateErr := statusUpdate(myTarget.GetName(), "collecting metadata"); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } var err error if targetContext.metadata, err = LoadMetadata(myTarget, flagNoRoot, targetContext.perfPath, localTempDir); err != nil { err = fmt.Errorf("failed to load metadata: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -830,7 +854,9 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr var uncollectableEvents []string if targetContext.groupDefinitions, uncollectableEvents, err = LoadEventGroups(flagEventFilePath, targetContext.metadata); err != nil { err = fmt.Errorf("failed to load event definitions: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -838,7 +864,9 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr // load metric definitions if targetContext.metricDefinitions, err = LoadMetricDefinitions(flagMetricFilePath, flagMetricsList, uncollectableEvents, targetContext.metadata); err != nil { err = fmt.Errorf("failed to load metric definitions: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -846,7 +874,9 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr // configure metrics if err = ConfigureMetrics(targetContext.metricDefinitions, GetEvaluatorFunctions(), targetContext.metadata); err != nil { err = fmt.Errorf("failed to configure metrics: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -877,7 +907,9 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut // get the perf command if processes, perfCommand, err = getPerfCommand(myTarget, targetContext.perfPath, targetContext.groupDefinitions, localTempDir); err != nil { err = fmt.Errorf("failed to get perf command: %w", err) - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } break } beginTimestamp := time.Now() @@ -887,7 +919,9 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut if perfErr != nil { if !getSignalReceived() { err = perfErr - statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } } break } @@ -916,21 +950,21 @@ func printMetrics(frameChannel chan []MetricFrame, targetName string, outputDir var printedFiles []string // block until next set of metric frames arrives, will exit loop when channel is closed for metricFrames := range frameChannel { - fileName := printMetricsTxt(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatTxt, !flagLive && util.StringInList(formatTxt, flagOutputFormat), outputDir) - if fileName != "" { + fileName, err := printMetricsTxt(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatTxt, !flagLive && util.StringInList(formatTxt, flagOutputFormat), outputDir) + if err == nil && fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } - fileName = printMetricsJSON(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatJSON, !flagLive && util.StringInList(formatJSON, flagOutputFormat), outputDir) - if fileName != "" { + fileName, err = printMetricsJSON(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatJSON, !flagLive && util.StringInList(formatJSON, flagOutputFormat), outputDir) + if err == nil && fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } // csv is always written to file unless no files are requested -- we need it to create the summary reports - fileName = printMetricsCSV(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatCSV, !flagLive, outputDir) - if fileName != "" { + fileName, err = printMetricsCSV(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatCSV, !flagLive, outputDir) + if err == nil && fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } - fileName = printMetricsWide(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatWide, !flagLive && util.StringInList(formatWide, flagOutputFormat), outputDir) - if fileName != "" { + fileName, err = printMetricsWide(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatWide, !flagLive && util.StringInList(formatWide, flagOutputFormat), outputDir) + if err == nil && fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } } @@ -1170,7 +1204,11 @@ func runPerf(myTarget target.Target, noRoot bool, processes []Process, cmd *exec outputLines = [][]byte{} // empty it } if timeout != 0 && int(time.Since(startPerfTimestamp).Seconds()) > timeout { - localCommand.Process.Signal(os.Interrupt) + err = localCommand.Process.Signal(os.Interrupt) + if err != nil { + err = fmt.Errorf("failed to terminate perf: %v", err) + slog.Error(err.Error()) + } } } }() diff --git a/cmd/metrics/print.go b/cmd/metrics/print.go index 0a9acca..c4f3f69 100644 --- a/cmd/metrics/print.go +++ b/cmd/metrics/print.go @@ -14,9 +14,9 @@ import ( "time" ) -func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) string { +func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { if !printToStdout && !printToFile { - return "" + return "", nil } for _, metricFrame := range metricFrames { // can't Marshal NaN or Inf values in JSON, so no need to set them to a specific value @@ -33,7 +33,7 @@ func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdo if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) - return "" + return "", err } if printToStdout { fmt.Println(string(jsonBytes)) @@ -43,19 +43,24 @@ func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdo if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) - return "" + return "", err } defer file.Close() - file.WriteString(string(jsonBytes) + "\n") - return file.Name() + _, err = file.WriteString(string(jsonBytes) + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } + return file.Name(), nil } } - return "" + return "", nil } -func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) string { +func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { if !printToStdout && !printToFile { - return "" + return "", nil } var file *os.File if printToFile { @@ -65,7 +70,7 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) - return "" + return "", err } defer file.Close() } @@ -76,7 +81,12 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Print(contextHeaders) } if printToFile { - file.WriteString(contextHeaders) + _, err := file.WriteString(contextHeaders) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } } names := make([]string, 0, len(metricFrame.Metrics)) for _, metric := range metricFrame.Metrics { @@ -87,7 +97,12 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Println(metricNames) } if printToFile { - file.WriteString(metricNames + "\n") + _, err := file.WriteString(metricNames + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } } } metricContext := fmt.Sprintf("%d,%s,%s,%s,", gCollectionStartTime.Unix()+int64(metricFrame.Timestamp), metricFrame.Socket, metricFrame.CPU, metricFrame.Cgroup) @@ -100,16 +115,21 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Println(metricContext + metricValues) } if printToFile { - file.WriteString(metricContext + metricValues + "\n") - return file.Name() + _, err := file.WriteString(metricContext + metricValues + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } + return file.Name(), nil } } - return "" + return "", nil } -func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) string { +func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { if !printToStdout && !printToFile { - return "" + return "", nil } var file *os.File if printToFile { @@ -119,7 +139,7 @@ func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdo if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) - return "" + return "", err } defer file.Close() } @@ -156,7 +176,12 @@ func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdo fmt.Println(header) } if printToFile { - file.WriteString(header + "\n") + _, err := file.WriteString(header + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } } } // handle values @@ -195,16 +220,21 @@ func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdo fmt.Println(row) } if printToFile { - file.WriteString(row + "\n") - return file.Name() + _, err := file.WriteString(row + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } + return file.Name(), nil } } - return "" + return "", nil } -func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) string { +func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { if !printToStdout && !printToFile { - return "" + return "", nil } var outputLines []string if len(metricFrames) > 0 && metricFrames[0].Socket != "" { @@ -260,11 +290,16 @@ func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdou if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) - return "" + return "", err } defer file.Close() - file.WriteString(strings.Join(outputLines, "\n") + "\n") - return file.Name() + _, err = file.WriteString(strings.Join(outputLines, "\n") + "\n") + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + return "", err + } + return file.Name(), nil } - return "" + return "", nil } diff --git a/cmd/report/report.go b/cmd/report/report.go index 4ddacd0..48c189c 100644 --- a/cmd/report/report.go +++ b/cmd/report/report.go @@ -226,7 +226,7 @@ func getFlagGroups() []common.FlagGroup { } for _, cat := range categories { flags = append(flags, common.Flag{ - Name: fmt.Sprintf(cat.FlagName), + Name: cat.FlagName, Help: cat.Help, }) } diff --git a/cmd/root.go b/cmd/root.go index 4dc3b8d..754e9e7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -230,7 +230,10 @@ func initializeApplication(cmd *cobra.Command, args []string) error { go func() { sig := <-sigChannel slog.Info("received signal", slog.String("signal", sig.String())) - terminateApplication(cmd, args) + err := terminateApplication(cmd, args) + if err != nil { + slog.Error("Error terminating application", slog.String("error", err.Error())) + } fmt.Println() os.Exit(1) }() diff --git a/cmd/telemetry/telemetry.go b/cmd/telemetry/telemetry.go index c1b3003..2f00ace 100644 --- a/cmd/telemetry/telemetry.go +++ b/cmd/telemetry/telemetry.go @@ -137,7 +137,7 @@ func getFlagGroups() []common.FlagGroup { } for _, cat := range categories { flags = append(flags, common.Flag{ - Name: fmt.Sprintf(cat.FlagName), + Name: cat.FlagName, Help: cat.Help, }) } diff --git a/internal/common/common.go b/internal/common/common.go index 143f1a0..7db9709 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -377,27 +377,42 @@ func collectOnTarget(cmd *cobra.Command, myTarget target.Target, scriptsToRun [] // create a temporary directory on the target var targetTempDir string var err error - statusUpdate(myTarget.GetName(), "creating temporary directory") + if statusUpdateErr := statusUpdate(myTarget.GetName(), "creating temporary directory"); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } targetTempRoot, _ := cmd.Flags().GetString(FlagTargetTempDirName) if targetTempDir, err = myTarget.CreateTempDirectory(targetTempRoot); err != nil { - statusUpdate(myTarget.GetName(), fmt.Sprintf("error creating temporary directory: %v", err)) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("error creating temporary directory: %v", err)); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } err = fmt.Errorf("error creating temporary directory on %s: %v", myTarget.GetName(), err) channelError <- err return } // don't remove the directory if we're debugging if cmd.Parent().PersistentFlags().Lookup("debug").Value.String() != "true" { - defer myTarget.RemoveDirectory(targetTempDir) + defer func() { + err := myTarget.RemoveDirectory(targetTempDir) + if err != nil { + slog.Error("error removing target temporary directory", slog.String("error", err.Error())) + } + }() } // run the scripts on the target - statusUpdate(myTarget.GetName(), "collecting data") + if statusUpdateErr := statusUpdate(myTarget.GetName(), "collecting data"); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } scriptOutputs, err := script.RunScripts(myTarget, scriptsToRun, true, localTempDir) if err != nil { - statusUpdate(myTarget.GetName(), fmt.Sprintf("error collecting data: %v", err)) + if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("error collecting data: %v", err)); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } err = fmt.Errorf("error running data collection scripts on %s: %v", myTarget.GetName(), err) channelError <- err return } - statusUpdate(myTarget.GetName(), "collection complete") + if statusUpdateErr := statusUpdate(myTarget.GetName(), "collection complete"); statusUpdateErr != nil { + slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) + } channelTargetScriptOutputs <- TargetScriptOutputs{targetName: myTarget.GetName(), scriptOutputs: scriptOutputs} } diff --git a/internal/report/report.go b/internal/report/report.go index 17ce6cc..7ad5c56 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -234,11 +234,11 @@ func renderXlsxTable(tableValues TableValues, f *excelize.File, sheetName string Bold: true, }, }) - f.SetCellValue(sheetName, cellName(col, *row), tableValues.Name) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), tableNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), tableValues.Name) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), tableNameStyle) *row++ if len(tableValues.Fields) == 0 || len(tableValues.Fields[0].Values) == 0 { - f.SetCellValue(sheetName, cellName(col, *row), noDataFound) + _ = f.SetCellValue(sheetName, cellName(col, *row), noDataFound) *row += 2 return } @@ -269,15 +269,15 @@ func renderXlsxTableMultiTarget(tableIdx int, allTargetsTableValues [][]TableVal }, }) - f.SetCellValue(sheetName, cellName(col, *row), allTargetsTableValues[0][tableIdx].Name) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), tableNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), allTargetsTableValues[0][tableIdx].Name) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), tableNameStyle) if !allTargetsTableValues[0][tableIdx].HasRows { col += 2 // print the target names for _, targetName := range targetNames { - f.SetCellValue(sheetName, cellName(col, *row), targetName) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), targetNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), targetName) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), targetNameStyle) col++ } *row++ @@ -285,15 +285,15 @@ func renderXlsxTableMultiTarget(tableIdx int, allTargetsTableValues [][]TableVal // print the field names and values from each target for fieldIdx, field := range allTargetsTableValues[0][tableIdx].Fields { col = 2 - f.SetCellValue(sheetName, cellName(col, *row), field.Name) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), fieldNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), field.Name) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), fieldNameStyle) col++ for targetIdx := 0; targetIdx < len(targetNames); targetIdx++ { var fieldValue string if len(allTargetsTableValues[targetIdx][tableIdx].Fields[fieldIdx].Values) > 0 { fieldValue = allTargetsTableValues[targetIdx][tableIdx].Fields[fieldIdx].Values[0] } - f.SetCellValue(sheetName, cellName(col, *row), fieldValue) + _ = f.SetCellValue(sheetName, cellName(col, *row), fieldValue) col++ } *row++ @@ -302,13 +302,13 @@ func renderXlsxTableMultiTarget(tableIdx int, allTargetsTableValues [][]TableVal for targetIdx, targetName := range targetNames { // print the target name col = 2 - f.SetCellValue(sheetName, cellName(col, *row), targetName) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), targetNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), targetName) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), targetNameStyle) *row++ // if no data found, print a message and skip to the next target if len(allTargetsTableValues[targetIdx][tableIdx].Fields) == 0 || len(allTargetsTableValues[targetIdx][tableIdx].Fields[0].Values) == 0 { - f.SetCellValue(sheetName, cellName(col, *row), noDataFound) + _ = f.SetCellValue(sheetName, cellName(col, *row), noDataFound) *row += 2 continue } @@ -316,8 +316,8 @@ func renderXlsxTableMultiTarget(tableIdx int, allTargetsTableValues [][]TableVal // print the field names as column headings across the top of the table col = 2 for _, field := range allTargetsTableValues[targetIdx][tableIdx].Fields { - f.SetCellValue(sheetName, cellName(col, *row), field.Name) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), fieldNameStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), field.Name) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), fieldNameStyle) col++ } *row++ @@ -327,7 +327,7 @@ func renderXlsxTableMultiTarget(tableIdx int, allTargetsTableValues [][]TableVal col = 2 for _, field := range allTargetsTableValues[targetIdx][tableIdx].Fields { value := getValueForCell(field.Values[tableRow]) - f.SetCellValue(sheetName, cellName(col, *row), value) + _ = f.SetCellValue(sheetName, cellName(col, *row), value) col++ } *row++ @@ -354,8 +354,8 @@ func DefaultXlsxTableRendererFunc(tableValues TableValues, f *excelize.File, she // print the field names as column headings across the top of the table col := 2 for _, field := range tableValues.Fields { - f.SetCellValue(sheetName, cellName(col, *row), field.Name) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), headerStyle) + _ = f.SetCellValue(sheetName, cellName(col, *row), field.Name) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), headerStyle) col++ } col = 2 @@ -365,8 +365,8 @@ func DefaultXlsxTableRendererFunc(tableValues TableValues, f *excelize.File, she for tableRow := 0; tableRow < tableRows; tableRow++ { for _, field := range tableValues.Fields { value := getValueForCell(field.Values[tableRow]) - f.SetCellValue(sheetName, cellName(col, *row), value) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), alignLeft) + _ = f.SetCellValue(sheetName, cellName(col, *row), value) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), alignLeft) col++ } col = 2 @@ -380,11 +380,11 @@ func DefaultXlsxTableRendererFunc(tableValues TableValues, f *excelize.File, she if len(tableValues.Fields[0].Values) > 0 { fieldValue = field.Values[0] } - f.SetCellValue(sheetName, cellName(col, *row), field.Name) + _ = f.SetCellValue(sheetName, cellName(col, *row), field.Name) col++ value := getValueForCell(fieldValue) - f.SetCellValue(sheetName, cellName(col, *row), value) - f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), alignLeft) + _ = f.SetCellValue(sheetName, cellName(col, *row), value) + _ = f.SetCellStyle(sheetName, cellName(col, *row), cellName(col, *row), alignLeft) col = 1 *row++ } @@ -399,16 +399,16 @@ const ( func createXlsxReport(allTableValues []TableValues) (out []byte, err error) { f := excelize.NewFile() sheetName := XlsxPrimarySheetName - f.SetSheetName("Sheet1", sheetName) - f.SetColWidth(sheetName, "A", "A", 25) - f.SetColWidth(sheetName, "B", "L", 25) + _ = f.SetSheetName("Sheet1", sheetName) + _ = f.SetColWidth(sheetName, "A", "A", 25) + _ = f.SetColWidth(sheetName, "B", "L", 25) row := 1 for _, tableValues := range allTableValues { if tableValues.Name == SystemSummaryTableName { row := 1 sheetName := XlsxBriefSheetName - f.NewSheet(sheetName) - f.SetColWidth(sheetName, "A", "L", 25) + _, _ = f.NewSheet(sheetName) + _ = f.SetColWidth(sheetName, "A", "L", 25) renderXlsxTable(tableValues, f, sheetName, &row) } else { renderXlsxTable(tableValues, f, sheetName, &row) @@ -428,17 +428,17 @@ func createXlsxReport(allTableValues []TableValues) (out []byte, err error) { func createXlsxReportMultiTarget(allTargetsTableValues [][]TableValues, targetNames []string) (out []byte, err error) { f := excelize.NewFile() sheetName := XlsxPrimarySheetName - f.SetSheetName("Sheet1", sheetName) - f.SetColWidth(sheetName, "A", "A", 15) - f.SetColWidth(sheetName, "B", "L", 25) + _ = f.SetSheetName("Sheet1", sheetName) + _ = f.SetColWidth(sheetName, "A", "A", 15) + _ = f.SetColWidth(sheetName, "B", "L", 25) row := 1 for tableIdx, tableValues := range allTargetsTableValues[0] { if tableValues.Name == SystemSummaryTableName { row := 1 sheetName := XlsxBriefSheetName - f.NewSheet(sheetName) - f.SetColWidth(sheetName, "A", "A", 15) - f.SetColWidth(sheetName, "B", "L", 25) + _, _ = f.NewSheet(sheetName) + _ = f.SetColWidth(sheetName, "A", "A", 15) + _ = f.SetColWidth(sheetName, "B", "L", 25) renderXlsxTableMultiTarget(tableIdx, allTargetsTableValues, targetNames, f, sheetName, &row) } else { renderXlsxTableMultiTarget(tableIdx, allTargetsTableValues, targetNames, f, sheetName, &row) diff --git a/internal/script/script.go b/internal/script/script.go index 25d8acf..4eacf29 100644 --- a/internal/script/script.go +++ b/internal/script/script.go @@ -124,7 +124,12 @@ func RunScripts(myTarget target.Target, scripts []ScriptDefinition, ignoreScript return nil, err } if len(installedLkms) > 0 { - defer myTarget.UninstallLkms(installedLkms) + defer func() { + err := myTarget.UninstallLkms(installedLkms) + if err != nil { + slog.Error("error uninstalling LKMs", slog.String("lkms", strings.Join(installedLkms, ", ")), slog.String("error", err.Error())) + } + }() } // if there's only 1 parallel script, run it sequentially @@ -237,7 +242,12 @@ func RunScriptAsync(myTarget target.Target, script ScriptDefinition, localTempDi return } if len(installedLkms) != 0 { - defer myTarget.UninstallLkms(installedLkms) + defer func() { + err := myTarget.UninstallLkms(installedLkms) + if err != nil { + slog.Error("error uninstalling LKMs", slog.String("lkms", strings.Join(installedLkms, ", ")), slog.String("error", err.Error())) + } + }() } cmd := prepareCommand(script, myTarget.GetTempDirectory()) err = myTarget.RunCommandAsync(cmd, stdoutChannel, stderrChannel, exitcodeChannel, script.Timeout, cmdChannel) diff --git a/internal/script/script_test.go b/internal/script/script_test.go index 3b33457..b3ea5c9 100644 --- a/internal/script/script_test.go +++ b/internal/script/script_test.go @@ -18,7 +18,12 @@ func TestRunScript(t *testing.T) { targets = append(targets, target.NewLocalTarget()) for _, tgt := range targets { targetTempDir, err := tgt.CreateTempDirectory("/tmp") - defer tgt.RemoveDirectory(targetTempDir) + defer func() { + err := tgt.RemoveDirectory(targetTempDir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }() if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/target/target.go b/internal/target/target.go index ca7cd33..3e4331b 100644 --- a/internal/target/target.go +++ b/internal/target/target.go @@ -379,7 +379,10 @@ func (t *LocalTarget) CanElevatePrivileges() bool { stdin, _ := cmd.StdinPipe() go func() { defer stdin.Close() - io.WriteString(stdin, t.sudo+"\n") + _, err := io.WriteString(stdin, t.sudo+"\n") + if err != nil { + slog.Error("error writing sudo password", slog.String("error", err.Error())) + } }() _, _, _, err := t.RunCommand(cmd, 0) if err == nil { From 8a6fb66e899b70b6fa80ad5ebca57c7db356e82f Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 6 Nov 2024 05:42:00 -0800 Subject: [PATCH 2/5] ignore status update errors --- cmd/config/config.go | 17 +++--------- cmd/metrics/metrics.go | 56 ++++++++++----------------------------- internal/common/common.go | 20 ++++---------- 3 files changed, 22 insertions(+), 71 deletions(-) diff --git a/cmd/config/config.go b/cmd/config/config.go index b9baeb9..48eb5c0 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -322,27 +322,16 @@ func printConfig(myTargets []target.Target, localTempDir string) (err error) { return } multiSpinner.Start() - err = multiSpinner.Status(myTarget.GetName(), "collecting data") - if err != nil { - err = fmt.Errorf("failed to set spinner status: %v", err) - return - } + _ = multiSpinner.Status(myTarget.GetName(), "collecting data") // run the scripts var scriptOutputs map[string]script.ScriptOutput if scriptOutputs, err = script.RunScripts(myTarget, scriptsToRun, true, localTempDir); err != nil { err = fmt.Errorf("failed to run collection scripts: %v", err) - errSpinner := multiSpinner.Status(myTarget.GetName(), "error collecting data") - if errSpinner != nil { - slog.Error(errSpinner.Error()) - } + _ = multiSpinner.Status(myTarget.GetName(), "error collecting data") multiSpinner.Finish() return } - err = multiSpinner.Status(myTarget.GetName(), "collection complete") - if err != nil { - err = fmt.Errorf("failed to set spinner status: %v", err) - return - } + _ = multiSpinner.Status(myTarget.GetName(), "collection complete") multiSpinner.Finish() // process the tables, i.e., get field values from raw script output tableNames := []string{report.ConfigurationTableName} diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index fcfef29..917644a 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -758,13 +758,9 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem myTarget := targetContext.target var err error // create a temporary directory on the target - if err = statusUpdate(myTarget.GetName(), "configuring target"); err != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", err.Error())) - } + _ = statusUpdate(myTarget.GetName(), "configuring target") if targetContext.tempDir, err = myTarget.CreateTempDirectory(targetTempRoot); err != nil { - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -774,9 +770,7 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem var nmiWatchdogEnabled bool if nmiWatchdogEnabled, err = NMIWatchdogEnabled(myTarget); err != nil { err = fmt.Errorf("failed to retrieve NMI watchdog status: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -784,9 +778,7 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem if nmiWatchdogEnabled { if err = DisableNMIWatchdog(myTarget, localTempDir); err != nil { err = fmt.Errorf("failed to disable NMI watchdog: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -798,18 +790,14 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem if !flagNoRoot { if targetContext.perfMuxIntervals, err = GetMuxIntervals(myTarget, localTempDir); err != nil { err = fmt.Errorf("failed to get perf mux intervals: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return } if err = SetAllMuxIntervals(myTarget, flagPerfMuxInterval, localTempDir); err != nil { err = fmt.Errorf("failed to set all perf mux intervals: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -818,9 +806,7 @@ func prepareTarget(targetContext *targetContext, targetTempRoot string, localTem // get the full path to the perf binary if targetContext.perfPath, err = getPerfPath(myTarget, localPerfPath); err != nil { err = fmt.Errorf("failed to find perf: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err)) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -836,15 +822,11 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr return } // load metadata - if statusUpdateErr := statusUpdate(myTarget.GetName(), "collecting metadata"); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), "collecting metadata") var err error if targetContext.metadata, err = LoadMetadata(myTarget, flagNoRoot, targetContext.perfPath, localTempDir); err != nil { err = fmt.Errorf("failed to load metadata: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -854,9 +836,7 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr var uncollectableEvents []string if targetContext.groupDefinitions, uncollectableEvents, err = LoadEventGroups(flagEventFilePath, targetContext.metadata); err != nil { err = fmt.Errorf("failed to load event definitions: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -864,9 +844,7 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr // load metric definitions if targetContext.metricDefinitions, err = LoadMetricDefinitions(flagMetricFilePath, flagMetricsList, uncollectableEvents, targetContext.metadata); err != nil { err = fmt.Errorf("failed to load metric definitions: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -874,9 +852,7 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr // configure metrics if err = ConfigureMetrics(targetContext.metricDefinitions, GetEvaluatorFunctions(), targetContext.metadata); err != nil { err = fmt.Errorf("failed to configure metrics: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) targetContext.err = err channelError <- targetError{target: myTarget, err: err} return @@ -907,9 +883,7 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut // get the perf command if processes, perfCommand, err = getPerfCommand(myTarget, targetContext.perfPath, targetContext.groupDefinitions, localTempDir); err != nil { err = fmt.Errorf("failed to get perf command: %w", err) - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) break } beginTimestamp := time.Now() @@ -919,9 +893,7 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut if perfErr != nil { if !getSignalReceived() { err = perfErr - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error())) } break } diff --git a/internal/common/common.go b/internal/common/common.go index 7db9709..3fc5882 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -377,14 +377,10 @@ func collectOnTarget(cmd *cobra.Command, myTarget target.Target, scriptsToRun [] // create a temporary directory on the target var targetTempDir string var err error - if statusUpdateErr := statusUpdate(myTarget.GetName(), "creating temporary directory"); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), "creating temporary directory") targetTempRoot, _ := cmd.Flags().GetString(FlagTargetTempDirName) if targetTempDir, err = myTarget.CreateTempDirectory(targetTempRoot); err != nil { - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("error creating temporary directory: %v", err)); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("error creating temporary directory: %v", err)) err = fmt.Errorf("error creating temporary directory on %s: %v", myTarget.GetName(), err) channelError <- err return @@ -399,20 +395,14 @@ func collectOnTarget(cmd *cobra.Command, myTarget target.Target, scriptsToRun [] }() } // run the scripts on the target - if statusUpdateErr := statusUpdate(myTarget.GetName(), "collecting data"); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), "collecting data") scriptOutputs, err := script.RunScripts(myTarget, scriptsToRun, true, localTempDir) if err != nil { - if statusUpdateErr := statusUpdate(myTarget.GetName(), fmt.Sprintf("error collecting data: %v", err)); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), fmt.Sprintf("error collecting data: %v", err)) err = fmt.Errorf("error running data collection scripts on %s: %v", myTarget.GetName(), err) channelError <- err return } - if statusUpdateErr := statusUpdate(myTarget.GetName(), "collection complete"); statusUpdateErr != nil { - slog.Error("failed to set status", slog.String("target", myTarget.GetName()), slog.String("error", statusUpdateErr.Error())) - } + _ = statusUpdate(myTarget.GetName(), "collection complete") channelTargetScriptOutputs <- TargetScriptOutputs{targetName: myTarget.GetName(), scriptOutputs: scriptOutputs} } From 634f580858fb21fd075e289e51d6622d304b6c15 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 6 Nov 2024 05:51:47 -0800 Subject: [PATCH 3/5] two more --- cmd/metrics/metrics.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index 917644a..ded2fa6 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -671,10 +671,7 @@ func runCmd(cmd *cobra.Command, args []string) error { } else { finalMessage += fmt.Sprintf(" for %d seconds", flagDuration) } - err = multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage) - if err != nil { - slog.Error("failed to set status", slog.String("target", targetContexts[i].target.GetName()), slog.String("error", err.Error())) - } + _ = multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage) } go collectOnTarget(&targetContexts[i], localTempDir, localOutputDir, channelTargetError, multiSpinner.Status) } @@ -690,10 +687,7 @@ func runCmd(cmd *cobra.Command, args []string) error { // finalize and stop the spinner for _, targetContext := range targetContexts { if targetContext.err == nil { - err = multiSpinner.Status(targetContext.target.GetName(), "collection complete") - if err != nil { - slog.Error("failed to set status", slog.String("target", targetContext.target.GetName()), slog.String("error", err.Error())) - } + _ = multiSpinner.Status(targetContext.target.GetName(), "collection complete") } } // summarize outputs From 1bb67b55c7cd375d4973509d4d1cd45fe3d762f1 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 6 Nov 2024 06:06:12 -0800 Subject: [PATCH 4/5] refactor --- cmd/metrics/metrics.go | 20 ++++++-- cmd/metrics/print.go | 104 +++++++++++++++++------------------------ 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index ded2fa6..939df72 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -917,20 +917,32 @@ func printMetrics(frameChannel chan []MetricFrame, targetName string, outputDir // block until next set of metric frames arrives, will exit loop when channel is closed for metricFrames := range frameChannel { fileName, err := printMetricsTxt(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatTxt, !flagLive && util.StringInList(formatTxt, flagOutputFormat), outputDir) - if err == nil && fileName != "" { + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + } else if fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } fileName, err = printMetricsJSON(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatJSON, !flagLive && util.StringInList(formatJSON, flagOutputFormat), outputDir) - if err == nil && fileName != "" { + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + } else if fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } // csv is always written to file unless no files are requested -- we need it to create the summary reports fileName, err = printMetricsCSV(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatCSV, !flagLive, outputDir) - if err == nil && fileName != "" { + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + } else if fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } fileName, err = printMetricsWide(metricFrames, targetName, flagLive && flagOutputFormat[0] == formatWide, !flagLive && util.StringInList(formatWide, flagOutputFormat), outputDir) - if err == nil && fileName != "" { + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + slog.Error(err.Error()) + } else if fileName != "" { printedFiles = util.UniqueAppend(printedFiles, fileName) } } diff --git a/cmd/metrics/print.go b/cmd/metrics/print.go index c4f3f69..62b665f 100644 --- a/cmd/metrics/print.go +++ b/cmd/metrics/print.go @@ -6,7 +6,6 @@ package metrics import ( "encoding/json" "fmt" - "log/slog" "math" "os" "strconv" @@ -14,9 +13,9 @@ import ( "time" ) -func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { +func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (filename string, err error) { if !printToStdout && !printToFile { - return "", nil + return } for _, metricFrame := range metricFrames { // can't Marshal NaN or Inf values in JSON, so no need to set them to a specific value @@ -29,48 +28,42 @@ func printMetricsJSON(metricFrames []MetricFrame, targetName string, printToStdo filteredMetricFrame.Metrics = append(filteredMetricFrame.Metrics, metric) } } - jsonBytes, err := json.Marshal(filteredMetricFrame) + var jsonBytes []byte + jsonBytes, err = json.Marshal(filteredMetricFrame) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } if printToStdout { fmt.Println(string(jsonBytes)) } if printToFile { - file, err := os.OpenFile(outputDir+"/"+targetName+"_"+"metrics.json", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + var file *os.File + file, err = os.OpenFile(outputDir+"/"+targetName+"_"+"metrics.json", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } defer file.Close() _, err = file.WriteString(string(jsonBytes) + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } - return file.Name(), nil + filename = file.Name() + return // success } } - return "", nil + return } -func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { +func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (filename string, err error) { if !printToStdout && !printToFile { - return "", nil + return } var file *os.File if printToFile { // open file for writing/appending - var err error file, err = os.OpenFile(outputDir+"/"+targetName+"_"+"metrics.csv", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } defer file.Close() } @@ -81,11 +74,9 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Print(contextHeaders) } if printToFile { - _, err := file.WriteString(contextHeaders) + _, err = file.WriteString(contextHeaders) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } } names := make([]string, 0, len(metricFrame.Metrics)) @@ -97,11 +88,9 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Println(metricNames) } if printToFile { - _, err := file.WriteString(metricNames + "\n") + _, err = file.WriteString(metricNames + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } } } @@ -115,31 +104,27 @@ func printMetricsCSV(metricFrames []MetricFrame, targetName string, printToStdou fmt.Println(metricContext + metricValues) } if printToFile { - _, err := file.WriteString(metricContext + metricValues + "\n") + _, err = file.WriteString(metricContext + metricValues + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } - return file.Name(), nil + filename = file.Name() + return // success } } return "", nil } -func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { +func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (filename string, err error) { if !printToStdout && !printToFile { - return "", nil + return } var file *os.File if printToFile { // open file for writing/appending - var err error file, err = os.OpenFile(outputDir+"/"+targetName+"_"+"metrics_wide.txt", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } defer file.Close() } @@ -176,11 +161,9 @@ func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdo fmt.Println(header) } if printToFile { - _, err := file.WriteString(header + "\n") + _, err = file.WriteString(header + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } } } @@ -220,21 +203,20 @@ func printMetricsWide(metricFrames []MetricFrame, targetName string, printToStdo fmt.Println(row) } if printToFile { - _, err := file.WriteString(row + "\n") + _, err = file.WriteString(row + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } - return file.Name(), nil + filename = file.Name() + return // success } } - return "", nil + return } -func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (string, error) { +func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdout bool, printToFile bool, outputDir string) (filename string, err error) { if !printToStdout && !printToFile { - return "", nil + return } var outputLines []string if len(metricFrames) > 0 && metricFrames[0].Socket != "" { @@ -286,20 +268,18 @@ func printMetricsTxt(metricFrames []MetricFrame, targetName string, printToStdou } if printToFile { // open file for writing/appending - file, err := os.OpenFile(outputDir+"/"+targetName+"_"+"metrics.txt", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + var file *os.File + file, err = os.OpenFile(outputDir+"/"+targetName+"_"+"metrics.txt", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } defer file.Close() _, err = file.WriteString(strings.Join(outputLines, "\n") + "\n") if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - return "", err + return } - return file.Name(), nil + filename = file.Name() + return // success } - return "", nil + return } From fb2c8f8b65458e5535ef55f543d52f3b23b79468 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 6 Nov 2024 06:16:42 -0800 Subject: [PATCH 5/5] add lint check to makefile --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a7a05ea..60edc10 100644 --- a/Makefile +++ b/Makefile @@ -96,8 +96,14 @@ check_license: if ! grep -E 'Copyright \(C\) [0-9]{4}-[0-9]{4} Intel Corporation' "$$f" >/dev/null; then echo "Error: license not found: $$f"; fail=1; fi; \ done; if [ -n "$$fail" ]; then exit 1; fi +.PHONY: check_lint +check_lint: + @echo "Running golangci-lint..." + go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest + golangci-lint run + .PHONY: check -check: check_format check_vet check_static check_license +check: check_format check_vet check_static check_license check_lint .PHONY: clean clean: