Skip to content

Commit

Permalink
address issues raised by golangci-lint (#88)
Browse files Browse the repository at this point in the history
  • Loading branch information
@harp-intel
harp-intel authored Nov 6, 2024
1 parent 91707b0 commit 8ed7549
Show file tree
Hide file tree
Showing 13 changed files with 197 additions and 123 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
8 changes: 7 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 15 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,22 @@ 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")
_ = 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)
multiSpinner.Status(myTarget.GetName(), "error collecting data")
_ = multiSpinner.Status(myTarget.GetName(), "error collecting data")
multiSpinner.Finish()
return
}
multiSpinner.Status(myTarget.GetName(), "collection complete")
_ = multiSpinner.Status(myTarget.GetName(), "collection complete")
multiSpinner.Finish()
// process the tables, i.e., get field values from raw script output
tableNames := []string{report.ConfigurationTableName}
Expand Down
68 changes: 42 additions & 26 deletions cmd/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ func runCmd(cmd *cobra.Command, args []string) error {
} else {
finalMessage += fmt.Sprintf(" for %d seconds", flagDuration)
}
multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage)
_ = multiSpinner.Status(targetContexts[i].target.GetName(), finalMessage)
}
go collectOnTarget(&targetContexts[i], localTempDir, localOutputDir, channelTargetError, multiSpinner.Status)
}
Expand All @@ -687,7 +687,7 @@ 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")
_ = multiSpinner.Status(targetContext.target.GetName(), "collection complete")
}
}
// summarize outputs
Expand Down Expand Up @@ -750,11 +750,11 @@ 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
_ = statusUpdate(myTarget.GetName(), "configuring target")
if targetContext.tempDir, err = myTarget.CreateTempDirectory(targetTempRoot); err != nil {
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand All @@ -764,15 +764,15 @@ 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()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.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()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand All @@ -784,14 +784,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)
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.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)
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand All @@ -800,7 +800,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)
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %v", err))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand All @@ -816,11 +816,11 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr
return
}
// load metadata
statusUpdate(myTarget.GetName(), "collecting metadata")
_ = 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)
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand All @@ -830,23 +830,23 @@ 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()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.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()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.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()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
targetContext.err = err
channelError <- targetError{target: myTarget, err: err}
return
Expand Down Expand Up @@ -877,7 +877,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)
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
break
}
beginTimestamp := time.Now()
Expand All @@ -887,7 +887,7 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut
if perfErr != nil {
if !getSignalReceived() {
err = perfErr
statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
_ = statusUpdate(myTarget.GetName(), fmt.Sprintf("Error: %s", err.Error()))
}
break
}
Expand Down Expand Up @@ -916,21 +916,33 @@ 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 {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
slog.Error(err.Error())
} else if 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 {
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 = 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 {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
slog.Error(err.Error())
} else if 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 {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
slog.Error(err.Error())
} else if fileName != "" {
printedFiles = util.UniqueAppend(printedFiles, fileName)
}
}
Expand Down Expand Up @@ -1170,7 +1182,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 8ed7549

Please sign in to comment.