Skip to content

Commit fe58b1f

Browse files
authored
fix: create output directory if it doesn't exist (#567)
* 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 <[email protected]> Signed-off-by: Harper, Jason M <[email protected]> * 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 <[email protected]> Signed-off-by: Harper, Jason M <[email protected]> --------- Signed-off-by: Jason Harper <[email protected]> Signed-off-by: Harper, Jason M <[email protected]>
1 parent ea021c9 commit fe58b1f

File tree

3 files changed

+39
-48
lines changed

3 files changed

+39
-48
lines changed

cmd/metrics/metrics.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -842,16 +842,8 @@ func runCmd(cmd *cobra.Command, args []string) error {
842842
signalMgr := newSignalManager()
843843
// short circuit when --input flag is set
844844
if flagInput != "" {
845-
// create output directory
846-
err := common.CreateOutputDir(localOutputDir)
847-
if err != nil {
848-
err = fmt.Errorf("failed to create output directory: %w", err)
849-
fmt.Fprintf(os.Stderr, "Error: %+v\n", err)
850-
cmd.SilenceUsage = true
851-
return err
852-
}
853845
// skip data collection and use raw data for reports
854-
err = processRawData(localOutputDir)
846+
err := processRawData(localOutputDir)
855847
if err != nil {
856848
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
857849
slog.Error(err.Error())
@@ -1057,16 +1049,6 @@ func runCmd(cmd *cobra.Command, args []string) error {
10571049
createPrometheusMetrics(targetContext.metricDefinitions)
10581050
}
10591051
}
1060-
// create the local output directory
1061-
if !flagLive && !flagPrometheusServer {
1062-
err = common.CreateOutputDir(localOutputDir)
1063-
if err != nil {
1064-
err = fmt.Errorf("failed to create output directory: %w", err)
1065-
fmt.Fprintf(os.Stderr, "Error: %+v\n", err)
1066-
cmd.SilenceUsage = true
1067-
return err
1068-
}
1069-
}
10701052
// start the metric production for each target
10711053
collectOnTargetWG := sync.WaitGroup{}
10721054
for i := range targetContexts {

cmd/root.go

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,33 @@ func Execute() {
146146
}
147147
}
148148

149+
// createOutputDir creates the output directory if it does not exist.
150+
// Returns true if the directory was created, false if it already existed.
151+
func createOutputDir(outputDir string) (bool, error) {
152+
// Check if directory already exists
153+
info, err := os.Stat(outputDir)
154+
if err == nil {
155+
// Path exists, verify it's a directory
156+
if !info.IsDir() {
157+
return false, fmt.Errorf("output path exists but is not a directory: %s", outputDir)
158+
}
159+
return false, nil // Already exists
160+
}
161+
// If error is not "not exists", something else is wrong
162+
if !os.IsNotExist(err) {
163+
return false, fmt.Errorf("failed to check output directory: %w", err)
164+
}
165+
// Directory doesn't exist, create it
166+
err = os.MkdirAll(outputDir, 0755) // #nosec G301
167+
if err != nil {
168+
return false, fmt.Errorf("failed to create output directory: %w", err)
169+
}
170+
return true, nil // Created successfully
171+
}
172+
149173
func initializeApplication(cmd *cobra.Command, args []string) error {
150174
timestamp := time.Now().Local().Format("2006-01-02_15-04-05") // app startup time
151-
// verify requested output directory exists or create an output directory
175+
// set output directory path (directory will be created later when needed)
152176
var outputDir string
153177
if flagOutputDir != "" {
154178
var err error
@@ -157,17 +181,8 @@ func initializeApplication(cmd *cobra.Command, args []string) error {
157181
fmt.Printf("Error: failed to expand output dir %v\n", err)
158182
os.Exit(1)
159183
}
160-
exists, err := util.DirectoryExists(outputDir)
161-
if err != nil {
162-
fmt.Printf("Error: failed to determine if output dir exists: %v\n", err)
163-
os.Exit(1)
164-
}
165-
if !exists {
166-
fmt.Printf("Error: requested output dir, %s, does not exist\n", outputDir)
167-
os.Exit(1)
168-
}
169184
} else {
170-
// set output dir path to app name + timestamp (dont' create the directory)
185+
// set output dir path to app name + timestamp
171186
outputDirName := common.AppName + "_" + timestamp
172187
var err error
173188
// outputDir will be in current working directory
@@ -220,6 +235,17 @@ func initializeApplication(cmd *cobra.Command, args []string) error {
220235
if gLogFile != nil {
221236
logFilePath = gLogFile.Name()
222237
}
238+
// create the output directory now to fail fast if there are permission or disk space issues
239+
// this validates write access before any data collection begins
240+
created, err := createOutputDir(outputDir)
241+
if err != nil {
242+
slog.Error("failed to create output directory", slog.String("path", outputDir), slog.String("error", err.Error()))
243+
fmt.Printf("Error: failed to create output directory: %v\n", err)
244+
os.Exit(1)
245+
}
246+
if created {
247+
slog.Debug("output directory created", slog.String("path", outputDir))
248+
}
223249
// set app context
224250
cmd.Parent().SetContext(
225251
context.WithValue(

internal/common/common.go

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -204,17 +204,9 @@ func (rc *ReportingCommand) Run() error {
204204
return err
205205
}
206206
}
207-
// we have output data so create the output directory
208-
err := CreateOutputDir(outputDir)
209-
if err != nil {
210-
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
211-
slog.Error(err.Error())
212-
rc.Cmd.SilenceUsage = true
213-
return err
214-
}
215207
// create the raw report before processing the data, so that we can save the raw data even if there is an error while processing
216208
var rawReports []string
217-
rawReports, err = rc.createRawReports(appContext, orderedTargetScriptOutputs)
209+
rawReports, err := rc.createRawReports(appContext, orderedTargetScriptOutputs)
218210
if err != nil {
219211
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
220212
slog.Error(err.Error())
@@ -289,15 +281,6 @@ func (rc *ReportingCommand) Run() error {
289281
return nil
290282
}
291283

292-
// CreateOutputDir creates the output directory if it does not exist
293-
func CreateOutputDir(outputDir string) error {
294-
err := os.MkdirAll(outputDir, 0755) // #nosec G301
295-
if err != nil {
296-
return fmt.Errorf("failed to create output directory: %w", err)
297-
}
298-
return nil
299-
}
300-
301284
// DefaultInsightsFunc returns the insights table values from the table values
302285
func DefaultInsightsFunc(allTableValues []report.TableValues, scriptOutputs map[string]script.ScriptOutput) report.TableValues {
303286
insightsTableValues := report.TableValues{

0 commit comments

Comments
 (0)