Skip to content

Commit

Permalink
address issues raised by golangci-lint
Browse files Browse the repository at this point in the history
  • Loading branch information
@harp-intel
harp-intel committed Nov 6, 2024
1 parent 91707b0 commit 848b814
Show file tree
Hide file tree
Showing 12 changed files with 237 additions and 106 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
/tools/bin
/dist
/internal/script/resources/x86_64
/test
31 changes: 26 additions & 5 deletions cmd/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
Expand Down
90 changes: 64 additions & 26 deletions cmd/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -764,15 +774,19 @@ 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
}
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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -830,23 +854,29 @@ 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
}
// 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
}
// 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
Expand Down Expand Up @@ -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()
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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())
}
}
}
}()
Expand Down
Loading

0 comments on commit 848b814

Please sign in to comment.