From 1b5d660168a83e4df1dce16350e43b11b52f9cf4 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Sat, 22 Nov 2025 09:17:34 -0800 Subject: [PATCH 1/2] fix: allow non-existent output directories with --output flag When using the --output flag, PerfSpect now accepts and creates directories that don't exist yet, rather than requiring them to exist beforehand. The directory is created early during initialization to validate write permissions before data collection begins, preventing wasted effort if the directory cannot be created. Changes: - Removed existence check that prevented specifying new directories - Added early directory creation after logging setup - Validates write access before any data collection starts - Provides clear error messages if creation fails Benefits: - Improved usability - no manual directory creation needed - Fail-fast validation - catches permission/disk issues immediately - No wasted data collection - validates before work begins - Creates nested directories automatically (e.g., /path/to/new/dir) - Consistent with default output directory behavior The directory is created at an optimal point in initialization: - After basic setup (logging) is complete, so errors can be logged - Before setting app context and starting any data collection - Early enough to fail fast and save user time Common failure modes handled gracefully: - Permission denied - clear error before any work - Disk full - detected immediately - Read-only filesystem - fails at start - Path conflicts (file exists) - caught early Fixes #556 Signed-off-by: Jason Harper Signed-off-by: Harper, Jason M --- cmd/root.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 87b5f78b..44ca3299 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -148,7 +148,7 @@ func Execute() { func initializeApplication(cmd *cobra.Command, args []string) error { timestamp := time.Now().Local().Format("2006-01-02_15-04-05") // app startup time - // verify requested output directory exists or create an output directory + // set output directory path (directory will be created later when needed) var outputDir string if flagOutputDir != "" { var err error @@ -157,17 +157,8 @@ func initializeApplication(cmd *cobra.Command, args []string) error { fmt.Printf("Error: failed to expand output dir %v\n", err) os.Exit(1) } - exists, err := util.DirectoryExists(outputDir) - if err != nil { - fmt.Printf("Error: failed to determine if output dir exists: %v\n", err) - os.Exit(1) - } - if !exists { - fmt.Printf("Error: requested output dir, %s, does not exist\n", outputDir) - os.Exit(1) - } } else { - // set output dir path to app name + timestamp (dont' create the directory) + // set output dir path to app name + timestamp outputDirName := common.AppName + "_" + timestamp var err error // outputDir will be in current working directory @@ -220,6 +211,15 @@ func initializeApplication(cmd *cobra.Command, args []string) error { if gLogFile != nil { logFilePath = gLogFile.Name() } + // create the output directory now to fail fast if there are permission or disk space issues + // this validates write access before any data collection begins + err = common.CreateOutputDir(outputDir) + if err != nil { + slog.Error("failed to create output directory", slog.String("path", outputDir), slog.String("error", err.Error())) + fmt.Printf("Error: failed to create output directory: %v\n", err) + os.Exit(1) + } + slog.Debug("output directory created", slog.String("path", outputDir)) // set app context cmd.Parent().SetContext( context.WithValue( From 2cf99e044c2253064f32acf6be6b2f98a8454c8c Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Sat, 22 Nov 2025 13:43:33 -0800 Subject: [PATCH 2/2] fix: allow non-existent output directories with --output flag When using the --output flag, PerfSpect now accepts and creates directories that don't exist yet, rather than requiring them to exist beforehand. The directory is created once, early during initialization, to validate write permissions before data collection begins. Changes: - Removed existence check in initializeApplication() - Added createOutputDir() function in root.go (single use, kept local) - Create output directory early (after logging, before data collection) - Only log creation when directory is actually created - Removed redundant directory creation calls from commands - Detects if output path exists as a file and fails with clear error Benefits: - Improved usability - no manual directory creation needed - Fail-fast validation - catches permission/disk issues immediately - No wasted data collection - validates write access before work begins - Single point of directory creation - simpler, more maintainable - Cleaner logs - only logs when directory is actually created - Creates nested directories automatically (e.g., /path/to/new/dir) Directory creation timing: - After logging is configured (errors can be logged) - Before app context is set and data collection starts - Early enough to fail fast and save time Error handling: - Permission denied - clear error before any work - Disk full - detected immediately - Read-only filesystem - fails at start - Path exists as file - caught with clear error - Path already exists as directory - succeeds silently Fixes #556 Signed-off-by: Jason Harper Signed-off-by: Harper, Jason M --- cmd/metrics/metrics.go | 20 +------------------- cmd/root.go | 30 ++++++++++++++++++++++++++++-- internal/common/common.go | 19 +------------------ 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index 9985f296..57c4ac9d 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -842,16 +842,8 @@ func runCmd(cmd *cobra.Command, args []string) error { signalMgr := newSignalManager() // short circuit when --input flag is set if flagInput != "" { - // create output directory - err := common.CreateOutputDir(localOutputDir) - if err != nil { - err = fmt.Errorf("failed to create output directory: %w", err) - fmt.Fprintf(os.Stderr, "Error: %+v\n", err) - cmd.SilenceUsage = true - return err - } // skip data collection and use raw data for reports - err = processRawData(localOutputDir) + err := processRawData(localOutputDir) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) @@ -1057,16 +1049,6 @@ func runCmd(cmd *cobra.Command, args []string) error { createPrometheusMetrics(targetContext.metricDefinitions) } } - // create the local output directory - if !flagLive && !flagPrometheusServer { - err = common.CreateOutputDir(localOutputDir) - if err != nil { - err = fmt.Errorf("failed to create output directory: %w", err) - fmt.Fprintf(os.Stderr, "Error: %+v\n", err) - cmd.SilenceUsage = true - return err - } - } // start the metric production for each target collectOnTargetWG := sync.WaitGroup{} for i := range targetContexts { diff --git a/cmd/root.go b/cmd/root.go index 44ca3299..f723935a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -146,6 +146,30 @@ func Execute() { } } +// createOutputDir creates the output directory if it does not exist. +// Returns true if the directory was created, false if it already existed. +func createOutputDir(outputDir string) (bool, error) { + // Check if directory already exists + info, err := os.Stat(outputDir) + if err == nil { + // Path exists, verify it's a directory + if !info.IsDir() { + return false, fmt.Errorf("output path exists but is not a directory: %s", outputDir) + } + return false, nil // Already exists + } + // If error is not "not exists", something else is wrong + if !os.IsNotExist(err) { + return false, fmt.Errorf("failed to check output directory: %w", err) + } + // Directory doesn't exist, create it + err = os.MkdirAll(outputDir, 0755) // #nosec G301 + if err != nil { + return false, fmt.Errorf("failed to create output directory: %w", err) + } + return true, nil // Created successfully +} + func initializeApplication(cmd *cobra.Command, args []string) error { timestamp := time.Now().Local().Format("2006-01-02_15-04-05") // app startup time // set output directory path (directory will be created later when needed) @@ -213,13 +237,15 @@ func initializeApplication(cmd *cobra.Command, args []string) error { } // create the output directory now to fail fast if there are permission or disk space issues // this validates write access before any data collection begins - err = common.CreateOutputDir(outputDir) + created, err := createOutputDir(outputDir) if err != nil { slog.Error("failed to create output directory", slog.String("path", outputDir), slog.String("error", err.Error())) fmt.Printf("Error: failed to create output directory: %v\n", err) os.Exit(1) } - slog.Debug("output directory created", slog.String("path", outputDir)) + if created { + slog.Debug("output directory created", slog.String("path", outputDir)) + } // set app context cmd.Parent().SetContext( context.WithValue( diff --git a/internal/common/common.go b/internal/common/common.go index 1c77c992..2c5db882 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -204,17 +204,9 @@ func (rc *ReportingCommand) Run() error { return err } } - // we have output data so create the output directory - err := CreateOutputDir(outputDir) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - slog.Error(err.Error()) - rc.Cmd.SilenceUsage = true - return err - } // create the raw report before processing the data, so that we can save the raw data even if there is an error while processing var rawReports []string - rawReports, err = rc.createRawReports(appContext, orderedTargetScriptOutputs) + rawReports, err := rc.createRawReports(appContext, orderedTargetScriptOutputs) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) slog.Error(err.Error()) @@ -289,15 +281,6 @@ func (rc *ReportingCommand) Run() error { return nil } -// CreateOutputDir creates the output directory if it does not exist -func CreateOutputDir(outputDir string) error { - err := os.MkdirAll(outputDir, 0755) // #nosec G301 - if err != nil { - return fmt.Errorf("failed to create output directory: %w", err) - } - return nil -} - // DefaultInsightsFunc returns the insights table values from the table values func DefaultInsightsFunc(allTableValues []report.TableValues, scriptOutputs map[string]script.ScriptOutput) report.TableValues { insightsTableValues := report.TableValues{