Skip to content

Commit 663e86e

Browse files
committed
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]>
1 parent 1b5d660 commit 663e86e

File tree

4 files changed

+259
-39
lines changed

4 files changed

+259
-39
lines changed

CLAUDE.md

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## Project Overview
6+
7+
PerfSpect is a command-line performance analysis tool for Linux systems written in Go. It collects CPU metrics, system configuration data, telemetry, flamegraphs, and lock contention analysis. The tool targets both local and remote systems via SSH.
8+
9+
## Build Commands
10+
11+
**First Build:**
12+
```bash
13+
builder/build.sh # Builds dependencies and app in Docker containers
14+
```
15+
16+
**Subsequent Builds:**
17+
```bash
18+
make # Build x86_64 binary (requires Go 1.25+)
19+
make perfspect-aarch64 # Build ARM64 binary
20+
make dist # Build distribution packages for both architectures
21+
```
22+
23+
**Testing:**
24+
```bash
25+
make test # Run all unit tests
26+
```
27+
28+
**Code Quality:**
29+
```bash
30+
make format # Format code with gofmt (run before committing)
31+
make check # Run all checks: format, vet, static analysis, license, lint, vulnerabilities, tests
32+
```
33+
34+
Individual checks: `check_format`, `check_vet`, `check_static`, `check_license`, `check_lint`, `check_vuln`, `check_sec`, `check_semgrep`, `check_modernize`
35+
36+
**All Quality Checks:**
37+
```bash
38+
make check # Run all quality checks
39+
```
40+
41+
**Cleanup:**
42+
```bash
43+
make clean # Remove binaries and build artifacts
44+
make sweep # Remove temporary output directories and logs
45+
```
46+
47+
## Architecture
48+
49+
### High-Level Structure
50+
51+
PerfSpect follows a command-target-script architecture:
52+
53+
1. **Commands** (`cmd/`) - User-facing commands that coordinate execution
54+
2. **Targets** (`internal/target/`) - Abstraction for local/remote system execution
55+
3. **Scripts** (`internal/script/`) - Embedded scripts that collect data on targets
56+
57+
### Core Packages
58+
59+
**`internal/target/`** - Target abstraction layer
60+
- `Target` interface defines operations on local or remote systems
61+
- `LocalTarget` - executes commands on the local system
62+
- `RemoteTarget` - executes commands via SSH on remote systems
63+
- Handles privilege elevation, CPU detection, file operations
64+
- Key methods: `RunCommand()`, `GetFile()`, `PutFile()`, `CanElevatePrivileges()`
65+
66+
**`internal/script/`** - Script execution framework
67+
- `ScriptDefinition` - defines scripts with metadata (name, path, architecture, etc.)
68+
- `script_defs.go` - contains all script definitions used by commands
69+
- `RunScript()` / `RunScripts()` - execute scripts on targets, handle parallel/sequential execution
70+
- Scripts are embedded via `//go:embed resources` and deployed to targets at runtime
71+
- Supports Linux Kernel Modules (LKMs) for advanced data collection
72+
73+
**`internal/common/`** - Shared types and utilities
74+
- Common data structures and constants
75+
- Shared functionality across commands
76+
77+
**`internal/report/`** - Report generation
78+
- HTML, CSV, JSON, Excel output formatters
79+
- Report templates and styling
80+
81+
**`internal/progress/`** - Progress tracking
82+
- Progress bars and status updates for long-running operations
83+
84+
**`internal/util/`** - Utility functions
85+
- File operations, string manipulation, system utilities
86+
87+
**`internal/cpus/`** - CPU database
88+
- CPU family/model/stepping identification
89+
- Architecture-specific information
90+
91+
### Command Structure
92+
93+
Each command lives in `cmd/<command>/`:
94+
- `metrics/` - CPU core/uncore performance metrics (TMAM, PMU events)
95+
- `report/` - System configuration and health reports
96+
- `telemetry/` - System telemetry over time
97+
- `flame/` - Software flamegraphs via perf
98+
- `lock/` - Lock contention and cache-to-cache analysis
99+
- `config/` - System configuration modifications
100+
101+
Commands are registered in `cmd/root.go` using Cobra framework.
102+
103+
### Data Flow
104+
105+
1. User invokes command (e.g., `perfspect metrics`)
106+
2. Command parses flags and creates target(s) from CLI args or YAML
107+
3. Command selects scripts from `script_defs.go` based on what data to collect
108+
4. Scripts are deployed to target(s) and executed (parallel when possible)
109+
5. Script outputs are collected and processed
110+
6. Reports are generated in requested formats (HTML, CSV, JSON, etc.)
111+
112+
### Architecture-Specific Code
113+
114+
- Event definitions: `cmd/metrics/resources/perfmon/`, `cmd/metrics/resources/legacy/`, `cmd/metrics/resources/component/`
115+
- Tool binaries: `internal/script/resources/x86_64/`, `internal/script/resources/aarch64/`
116+
- CPU-specific logic in `internal/cpus/`
117+
- Metadata collection in `cmd/metrics/metadata.go`
118+
- Summary generation in `cmd/metrics/summary.go`
119+
120+
When adding architecture support:
121+
- Follow existing patterns from x86_64 and aarch64
122+
- Place architecture-specific code in dedicated directories
123+
- Use CPU family/model/stepping detection from `internal/cpus/`
124+
- Add event definitions for the architecture
125+
- Provide validation plan for new architectures
126+
127+
## License Requirements
128+
129+
**Every source file must include the BSD-3-Clause license header:**
130+
131+
Go files:
132+
```go
133+
// Copyright (C) 2021-2025 Intel Corporation
134+
// SPDX-License-Identifier: BSD-3-Clause
135+
```
136+
137+
Shell scripts and Makefiles:
138+
```bash
139+
# Copyright (C) 2021-2025 Intel Corporation
140+
# SPDX-License-Identifier: BSD-3-Clause
141+
```
142+
143+
Run `make check_license` to verify all files have proper headers.
144+
145+
## Development Workflow
146+
147+
1. Make changes
148+
2. Run `make check` to run all quality checks
149+
3. Commit with `git commit -s` (Developer Certificate of Origin required)
150+
151+
**IMPORTANT:** Do not include attributions to Claude Code or AI assistance in commit messages or PR descriptions. Commits and PRs should appear as normal human contributions without mentioning they were generated by AI.
152+
153+
## Key Patterns
154+
155+
### Creating a Target
156+
```go
157+
import "perfspect/internal/target"
158+
159+
// Local target
160+
t, err := target.NewLocalTarget()
161+
162+
// Remote target with credentials
163+
t, err := target.NewRemoteTarget(host, user, password, keyPath)
164+
165+
// Execute command
166+
output, err := t.RunCommand("ls -la")
167+
```
168+
169+
### Running Scripts
170+
```go
171+
import "perfspect/internal/script"
172+
173+
// Run single script
174+
output, err := script.RunScript(myTarget, scriptDef, tempDir)
175+
176+
// Run multiple scripts (parallel when possible)
177+
outputs, err := script.RunScripts(myTarget, scriptDefs, ignoreErrors, tempDir)
178+
```
179+
180+
### Logging
181+
```go
182+
import "log/slog"
183+
184+
slog.Info("message", "key", value)
185+
slog.Debug("debug info", "detail", data)
186+
slog.Error("error occurred", "error", err)
187+
```
188+
189+
## Testing
190+
191+
- Unit tests use standard `testing` package and `github.com/stretchr/testify`
192+
- Test files: `*_test.go` alongside source files
193+
- Use table-driven tests for multiple scenarios
194+
- Run specific package tests: `go test -v ./internal/target`
195+
- Mock external dependencies appropriately
196+
197+
## Profiling
198+
199+
Set `PERFSPECT_PROFILE=1` environment variable to enable CPU and memory profiling:
200+
```bash
201+
PERFSPECT_PROFILE=1 ./perfspect metrics
202+
go tool pprof -http=:8080 cpu.prof
203+
```
204+
205+
## Remote Targets
206+
207+
- Target specification via `--target`, `--user`, `--key`, `--password` flags
208+
- Multiple targets via YAML file with `--targets` flag (see `targets.yaml` example)
209+
- Remote user needs password-less sudo or root for full functionality
210+
- SSH connection handled by `internal/target/RemoteTarget`
211+
212+
## Output Formats
213+
214+
Commands support multiple output formats:
215+
- HTML (default for reports)
216+
- CSV (default for metrics)
217+
- JSON
218+
- Text
219+
- Excel (XLSX)
220+
221+
Use `--format` flag to specify output format. Use `--output` to specify output directory.
222+
223+
## Contributing
224+
225+
- Open GitHub Issues for significant changes before implementation
226+
- Provide validation plans for architecture-specific features
227+
- Named maintainer required for new features
228+
- Sign commits with DCO: `git commit -s`
229+
- See CONTRIBUTING.md for full guidelines

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: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,30 @@ 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
151175
// set output directory path (directory will be created later when needed)
@@ -213,13 +237,15 @@ func initializeApplication(cmd *cobra.Command, args []string) error {
213237
}
214238
// create the output directory now to fail fast if there are permission or disk space issues
215239
// this validates write access before any data collection begins
216-
err = common.CreateOutputDir(outputDir)
240+
created, err := createOutputDir(outputDir)
217241
if err != nil {
218242
slog.Error("failed to create output directory", slog.String("path", outputDir), slog.String("error", err.Error()))
219243
fmt.Printf("Error: failed to create output directory: %v\n", err)
220244
os.Exit(1)
221245
}
222-
slog.Debug("output directory created", slog.String("path", outputDir))
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)