From 6b0709bc32acc87dd541ac2f6d10e122ec09c8d1 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Thu, 6 Nov 2025 08:04:20 +0100 Subject: [PATCH 01/20] feat(rdb): add benchmarks for instance get, backup get/list, and database list --- .../rdb/v1/custom_benchmark_test.go | 420 ++++++++++++++++++ internal/namespaces/rdb/v1/helper_test.go | 109 +++++ .../rdb/v1/testdata/benchmark.baseline | 10 + 3 files changed, 539 insertions(+) create mode 100644 internal/namespaces/rdb/v1/custom_benchmark_test.go create mode 100644 internal/namespaces/rdb/v1/testdata/benchmark.baseline diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go new file mode 100644 index 0000000000..9ded21ec87 --- /dev/null +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -0,0 +1,420 @@ +package rdb_test + +import ( + "bytes" + "context" + "os" + "sort" + "testing" + "time" + + "github.com/scaleway/scaleway-cli/v2/core" + "github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1" + rdbSDK "github.com/scaleway/scaleway-sdk-go/api/rdb/v1" + "github.com/scaleway/scaleway-sdk-go/scw" +) + +// Benchmarks for RDB commands. +// +// Baseline stored in testdata/benchmark.baseline (like golden files). +// +// To compare performance: +// +// benchstat testdata/benchmark.baseline <(CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x .) +// +// To update baseline: +// +// CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x . > testdata/benchmark.baseline + +const ( + defaultCmdTimeout = 30 * time.Second + instanceReadyTimeout = 3 * time.Minute +) + +func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) { + b.Helper() + + clientOpts := []scw.ClientOption{ + scw.WithDefaultRegion(scw.RegionFrPar), + scw.WithDefaultZone(scw.ZoneFrPar1), + scw.WithUserAgent("cli-benchmark-test"), + scw.WithEnv(), + } + + config, err := scw.LoadConfig() + if err == nil { + activeProfile, err := config.GetActiveProfile() + if err == nil { + envProfile := scw.LoadEnvProfile() + profile := scw.MergeProfiles(activeProfile, envProfile) + clientOpts = append(clientOpts, scw.WithProfile(profile)) + } + } + + client, err := scw.NewClient(clientOpts...) + if err != nil { + b.Fatalf( + "Failed to create Scaleway client: %v\nMake sure you have configured your credentials with 'scw config'", + err, + ) + } + + meta := core.TestMetadata{ + "t": b, + } + + executeCmd := func(args []string) any { + stdoutBuffer := &bytes.Buffer{} + stderrBuffer := &bytes.Buffer{} + _, result, err := core.Bootstrap(&core.BootstrapConfig{ + Args: args, + Commands: rdb.GetCommands().Copy(), + BuildInfo: nil, + Stdout: stdoutBuffer, + Stderr: stderrBuffer, + Client: client, + DisableTelemetry: true, + DisableAliases: true, + OverrideEnv: map[string]string{}, + Ctx: context.Background(), + }) + if err != nil { + b.Errorf("error executing cmd (%s): %v\nstdout: %s\nstderr: %s", + args, err, stdoutBuffer.String(), stderrBuffer.String()) + } + + return result + } + + return client, meta, executeCmd +} + +func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn func() error) { + b.Helper() + + if err := cleanupFn(); err != nil { + b.Logf("cleanup failed (%s=%s): %v; retrying...", name, resourceID, err) + time.Sleep(2 * time.Second) + if err2 := cleanupFn(); err2 != nil { + b.Errorf("final cleanup failure (%s=%s): %v", name, resourceID, err2) + } + } +} + +type benchmarkStats struct { + timings []time.Duration + enabled bool +} + +func newBenchmarkStats() *benchmarkStats { + return &benchmarkStats{ + enabled: os.Getenv("CLI_BENCH_TRACE") == "true", + timings: make([]time.Duration, 0, 1000), + } +} + +func (s *benchmarkStats) record(d time.Duration) { + s.timings = append(s.timings, d) +} + +func (s *benchmarkStats) getMean() time.Duration { + if len(s.timings) == 0 { + return 0 + } + + var sum time.Duration + for _, t := range s.timings { + sum += t + } + + return sum / time.Duration(len(s.timings)) +} + +func (s *benchmarkStats) report(b *testing.B) { + b.Helper() + + if !s.enabled || len(s.timings) == 0 { + return + } + + sort.Slice(s.timings, func(i, j int) bool { + return s.timings[i] < s.timings[j] + }) + + minVal := s.timings[0] + maxVal := s.timings[len(s.timings)-1] + median := s.timings[len(s.timings)/2] + p95 := s.timings[int(float64(len(s.timings))*0.95)] + mean := s.getMean() + + b.Logf("Distribution (n=%d): min=%v median=%v mean=%v p95=%v max=%v", + len(s.timings), minVal, median, mean, p95, maxVal) +} + +func BenchmarkInstanceGet(b *testing.B) { + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + client, meta, executeCmd := setupBenchmark(b) + + ctx := &core.BeforeFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + err := createInstanceDirect(engine)(ctx) + if err != nil { + b.Fatalf("Failed to create instance: %v", err) + } + + instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + + b.Cleanup(func() { + afterCtx := &core.AfterFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + cleanupWithRetry(b, "instance", instance.ID, func() error { + return deleteInstanceDirect()(afterCtx) + }) + }) + + stats := newBenchmarkStats() + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + start := time.Now() + + ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) + done := make(chan any, 1) + + go func() { + done <- executeCmd([]string{"scw", "rdb", "instance", "get", instance.ID}) + }() + + select { + case <-done: + stats.record(time.Since(start)) + case <-ctx.Done(): + cancel() + b.Fatalf("command timeout after %v", defaultCmdTimeout) + } + cancel() + } + + b.StopTimer() + stats.report(b) +} + +func BenchmarkBackupGet(b *testing.B) { + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + client, meta, executeCmd := setupBenchmark(b) + + ctx := &core.BeforeFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + err := createInstanceDirect(engine)(ctx) + if err != nil { + b.Fatalf("Failed to create instance: %v", err) + } + + instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + b.Fatalf("Instance not ready: %v", err) + } + + err = createBackupDirect("Backup")(ctx) + if err != nil { + b.Fatalf("Failed to create backup: %v", err) + } + + backup := meta["Backup"].(*rdbSDK.DatabaseBackup) + + b.Cleanup(func() { + afterCtx := &core.AfterFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + cleanupWithRetry(b, "backup", backup.ID, func() error { + return deleteBackupDirect("Backup")(afterCtx) + }) + cleanupWithRetry(b, "instance", instance.ID, func() error { + return deleteInstanceDirect()(afterCtx) + }) + }) + + stats := newBenchmarkStats() + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + start := time.Now() + + ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) + done := make(chan any, 1) + + go func() { + done <- executeCmd([]string{"scw", "rdb", "backup", "get", backup.ID}) + }() + + select { + case <-done: + stats.record(time.Since(start)) + case <-ctx.Done(): + cancel() + b.Fatalf("command timeout after %v", defaultCmdTimeout) + } + cancel() + } + + b.StopTimer() + stats.report(b) +} + +func BenchmarkBackupList(b *testing.B) { + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + client, meta, executeCmd := setupBenchmark(b) + + ctx := &core.BeforeFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + err := createInstanceDirect(engine)(ctx) + if err != nil { + b.Fatalf("Failed to create instance: %v", err) + } + + instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + b.Fatalf("Instance not ready: %v", err) + } + + err = createBackupDirect("Backup1")(ctx) + if err != nil { + b.Fatalf("Failed to create backup 1: %v", err) + } + err = createBackupDirect("Backup2")(ctx) + if err != nil { + b.Fatalf("Failed to create backup 2: %v", err) + } + + backup1 := meta["Backup1"].(*rdbSDK.DatabaseBackup) + backup2 := meta["Backup2"].(*rdbSDK.DatabaseBackup) + + b.Cleanup(func() { + afterCtx := &core.AfterFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + cleanupWithRetry(b, "backup1", backup1.ID, func() error { + return deleteBackupDirect("Backup1")(afterCtx) + }) + cleanupWithRetry(b, "backup2", backup2.ID, func() error { + return deleteBackupDirect("Backup2")(afterCtx) + }) + cleanupWithRetry(b, "instance", instance.ID, func() error { + return deleteInstanceDirect()(afterCtx) + }) + }) + + stats := newBenchmarkStats() + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + start := time.Now() + + ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) + done := make(chan any, 1) + + go func() { + done <- executeCmd([]string{"scw", "rdb", "backup", "list", "instance-id=" + instance.ID}) + }() + + select { + case <-done: + stats.record(time.Since(start)) + case <-ctx.Done(): + cancel() + b.Fatalf("command timeout after %v", defaultCmdTimeout) + } + cancel() + } + + b.StopTimer() + stats.report(b) +} + +func BenchmarkDatabaseList(b *testing.B) { + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + client, meta, executeCmd := setupBenchmark(b) + + ctx := &core.BeforeFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + err := createInstanceDirect(engine)(ctx) + if err != nil { + b.Fatalf("Failed to create instance: %v", err) + } + + instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + + b.Cleanup(func() { + afterCtx := &core.AfterFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + cleanupWithRetry(b, "instance", instance.ID, func() error { + return deleteInstanceDirect()(afterCtx) + }) + }) + + stats := newBenchmarkStats() + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + start := time.Now() + + ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) + done := make(chan any, 1) + + go func() { + done <- executeCmd([]string{"scw", "rdb", "database", "list", "instance-id=" + instance.ID}) + }() + + select { + case <-done: + stats.record(time.Since(start)) + case <-ctx.Done(): + cancel() + b.Fatalf("command timeout after %v", defaultCmdTimeout) + } + cancel() + } + + b.StopTimer() + stats.report(b) +} diff --git a/internal/namespaces/rdb/v1/helper_test.go b/internal/namespaces/rdb/v1/helper_test.go index b1067f4886..6296726557 100644 --- a/internal/namespaces/rdb/v1/helper_test.go +++ b/internal/namespaces/rdb/v1/helper_test.go @@ -1,8 +1,10 @@ package rdb_test import ( + "context" "errors" "fmt" + "time" "github.com/scaleway/scaleway-cli/v2/core" "github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1" @@ -113,3 +115,110 @@ func deleteInstance() core.AfterFunc { func deleteInstanceAndWait() core.AfterFunc { return core.ExecAfterCmd("scw rdb instance delete {{ .Instance.ID }} --wait") } + +func createInstanceDirect(_ string) core.BeforeFunc { + return func(ctx *core.BeforeFuncCtx) error { + result := ctx.ExecuteCmd([]string{ + "scw", "rdb", "instance", "create", + "node-type=DB-DEV-S", + "is-ha-cluster=false", + "name=" + name, + "engine=" + engine, + "user-name=" + user, + "password=" + password, + "--wait", + }) + ctx.Meta["Instance"] = result + + return nil + } +} + +func createBackupDirect(metaKey string) core.BeforeFunc { + return func(ctx *core.BeforeFuncCtx) error { + instanceResult := ctx.Meta["Instance"].(rdb.CreateInstanceResult) + instance := instanceResult.Instance + + result := ctx.ExecuteCmd([]string{ + "scw", "rdb", "backup", "create", + "name=cli-test-backup", + "expires-at=2032-01-02T15:04:05-07:00", + "instance-id=" + instance.ID, + "database-name=rdb", + "--wait", + }) + ctx.Meta[metaKey] = result + + return nil + } +} + +func deleteBackupDirect(metaKey string) core.AfterFunc { + return func(ctx *core.AfterFuncCtx) error { + backup := ctx.Meta[metaKey].(*rdbSDK.DatabaseBackup) + ctx.ExecuteCmd([]string{ + "scw", "rdb", "backup", "delete", + backup.ID, + }) + + return nil + } +} + +func deleteInstanceDirect() core.AfterFunc { + return func(ctx *core.AfterFuncCtx) error { + instance := ctx.Meta["Instance"].(rdb.CreateInstanceResult).Instance + ctx.ExecuteCmd([]string{ + "scw", "rdb", "instance", "delete", + instance.ID, + }) + + return nil + } +} + +func waitForInstanceReady( + executeCmd func([]string) any, + instanceID string, + maxWait time.Duration, +) error { + ctx, cancel := context.WithTimeout(context.Background(), maxWait) + defer cancel() + + backoff := time.Second + for { + select { + case <-ctx.Done(): + return fmt.Errorf( + "timeout waiting for instance %s to be ready for operations", + instanceID, + ) + default: + result := executeCmd([]string{"scw", "rdb", "instance", "get", instanceID}) + + // Try direct type assertion first + if instance, ok := result.(*rdbSDK.Instance); ok { + if instance.Status == rdbSDK.InstanceStatusReady { + time.Sleep(5 * time.Second) + + return nil + } + } else { + v := result.(struct { + *rdbSDK.Instance + ACLs []*rdbSDK.ACLRule `json:"acls"` + }) + if v.Instance != nil && v.Instance.Status == rdbSDK.InstanceStatusReady { + time.Sleep(5 * time.Second) + + return nil + } + } + + time.Sleep(backoff) + if backoff < 10*time.Second { + backoff *= 2 + } + } + } +} diff --git a/internal/namespaces/rdb/v1/testdata/benchmark.baseline b/internal/namespaces/rdb/v1/testdata/benchmark.baseline new file mode 100644 index 0000000000..4037c89950 --- /dev/null +++ b/internal/namespaces/rdb/v1/testdata/benchmark.baseline @@ -0,0 +1,10 @@ +goos: darwin +goarch: amd64 +pkg: github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1 +cpu: VirtualApple @ 2.50GHz +BenchmarkInstanceGet-11 100 239648044 ns/op 350232 B/op 4121 allocs/op +BenchmarkBackupGet-11 100 85116384 ns/op 271182 B/op 2839 allocs/op +BenchmarkBackupList-11 100 118312364 ns/op 284095 B/op 2996 allocs/op +BenchmarkDatabaseList-11 100 136427978 ns/op 283705 B/op 3046 allocs/op +PASS + From 8009fe65599975e1c2bd35e85120bc6247c311f8 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 05:24:02 +0100 Subject: [PATCH 02/20] feat(rdb): add benchmark comparison tool with regression detection --- cmd/scw-benchstat/main.go | 354 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 354 insertions(+) create mode 100644 cmd/scw-benchstat/main.go diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go new file mode 100644 index 0000000000..a9bdcd3e9e --- /dev/null +++ b/cmd/scw-benchstat/main.go @@ -0,0 +1,354 @@ +package main + +import ( + "bufio" + "context" + "encoding/csv" + "flag" + "fmt" + "io" + "log" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "time" +) + +const ( + benchstatVersion = "v0.0.0-20251023143056-3684bd442cc8" +) + +type config struct { + bench string + benchtime string + count int + benchmem bool + failMetrics []string + threshold float64 + installTool bool + targetDirs []string + verbose bool +} + +type benchResult struct { + name string + timePerOp float64 + bytesPerOp int64 + allocsPerOp int64 +} + +func main() { + cfg := parseFlags() + + if cfg.installTool { + if err := installBenchstat(); err != nil { + log.Fatalf("failed to install benchstat: %v", err) + } + } + + if !isBenchstatAvailable() { + log.Fatalf("benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat@%s in your environment or run with --install-benchstat", benchstatVersion) + } + + if len(cfg.targetDirs) == 0 { + cfg.targetDirs = findBenchmarkDirs() + } + + if len(cfg.targetDirs) == 0 { + log.Fatal("no benchmark directories found") + } + + for _, dir := range cfg.targetDirs { + if err := runBenchmarksForDir(cfg, dir); err != nil { + log.Printf("failed to run benchmarks for %s: %v", dir, err) + os.Exit(1) + } + } +} + +func parseFlags() config { + cfg := config{} + + flag.StringVar(&cfg.bench, "bench", ".", "benchmark pattern to run") + flag.StringVar(&cfg.benchtime, "benchtime", "1s", "benchmark time") + flag.IntVar(&cfg.count, "count", 5, "number of benchmark runs") + flag.BoolVar(&cfg.benchmem, "benchmem", false, "include memory allocation stats") + flag.Float64Var(&cfg.threshold, "threshold", 1.5, "performance regression threshold (e.g., 1.5 = 50% slower)") + flag.BoolVar(&cfg.installTool, "install-benchstat", false, "install benchstat tool if not found") + flag.BoolVar(&cfg.verbose, "verbose", false, "verbose output") + + var failMetricsStr string + flag.StringVar(&failMetricsStr, "fail-metrics", "", "comma-separated list of metrics to check for regressions (time/op,B/op,allocs/op)") + + var targetDirsStr string + flag.StringVar(&targetDirsStr, "target-dirs", "", "comma-separated list of directories to benchmark") + + flag.Parse() + + if failMetricsStr != "" { + cfg.failMetrics = strings.Split(failMetricsStr, ",") + } + + if targetDirsStr != "" { + cfg.targetDirs = strings.Split(targetDirsStr, ",") + } + + return cfg +} + +func installBenchstat() error { + fmt.Printf("Installing benchstat@%s...\n", benchstatVersion) + cmd := exec.Command("go", "install", fmt.Sprintf("golang.org/x/perf/cmd/benchstat@%s", benchstatVersion)) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() +} + +func isBenchstatAvailable() bool { + _, err := exec.LookPath("benchstat") + return err == nil +} + +func findBenchmarkDirs() []string { + var dirs []string + + err := filepath.WalkDir("internal/namespaces", func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + if strings.HasSuffix(d.Name(), "_benchmark_test.go") { + dir := filepath.Dir(path) + dirs = append(dirs, dir) + } + + return nil + }) + + if err != nil { + log.Printf("error scanning for benchmark directories: %v", err) + } + + return dirs +} + +func runBenchmarksForDir(cfg config, dir string) error { + fmt.Printf(">>> Running benchmarks for %s\n", dir) + + baselineFile := filepath.Join(dir, "testdata", "benchmark.baseline") + + // Run benchmarks + newResults, err := runBenchmarks(cfg, dir) + if err != nil { + return fmt.Errorf("failed to run benchmarks: %w", err) + } + + // Check if baseline exists + if _, err := os.Stat(baselineFile); os.IsNotExist(err) { + fmt.Printf("No baseline found at %s. Creating new baseline.\n", baselineFile) + if err := saveBaseline(baselineFile, newResults); err != nil { + return fmt.Errorf("failed to save baseline: %w", err) + } + fmt.Printf("Baseline saved to %s\n", baselineFile) + return nil + } + + // Compare with baseline + return compareWithBaseline(cfg, baselineFile, newResults) +} + +func runBenchmarks(cfg config, dir string) (string, error) { + args := []string{"test", "-bench=" + cfg.bench, "-benchtime=" + cfg.benchtime, "-count=" + strconv.Itoa(cfg.count)} + + if cfg.benchmem { + args = append(args, "-benchmem") + } + + args = append(args, "./"+dir) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + + cmd := exec.CommandContext(ctx, "go", args...) + cmd.Env = append(os.Environ(), "CLI_RUN_BENCHMARKS=true") + + if cfg.verbose { + fmt.Printf("Running: go %s\n", strings.Join(args, " ")) + } + + output, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("benchmark execution failed: %w\nOutput: %s", err, output) + } + + return string(output), nil +} + +func saveBaseline(filename, content string) error { + dir := filepath.Dir(filename) + if err := os.MkdirAll(dir, 0755); err != nil { + return err + } + + return os.WriteFile(filename, []byte(content), 0644) +} + +func compareWithBaseline(cfg config, baselineFile, newResults string) error { + // Create temporary file for new results + tmpFile, err := os.CreateTemp("", "benchmark-new-*.txt") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + defer os.Remove(tmpFile.Name()) + defer tmpFile.Close() + + if _, err := tmpFile.WriteString(newResults); err != nil { + return fmt.Errorf("failed to write new results: %w", err) + } + tmpFile.Close() + + // Run benchstat comparison + cmd := exec.Command("benchstat", "-format=csv", baselineFile, tmpFile.Name()) + output, err := cmd.Output() + if err != nil { + return fmt.Errorf("failed to compare with benchstat for %s: %w", filepath.Dir(baselineFile), err) + } + + // Parse CSV output and check for regressions + return checkForRegressions(cfg, string(output)) +} + +func checkForRegressions(cfg config, csvOutput string) error { + reader := csv.NewReader(strings.NewReader(csvOutput)) + records, err := reader.ReadAll() + if err != nil { + return fmt.Errorf("failed to parse benchstat CSV output: %w", err) + } + + if len(records) < 2 { + fmt.Println("No benchmark comparisons found") + return nil + } + + // Find column indices + header := records[0] + nameIdx := findColumnIndex(header, "name") + oldTimeIdx := findColumnIndex(header, "old time/op") + newTimeIdx := findColumnIndex(header, "new time/op") + oldBytesIdx := findColumnIndex(header, "old B/op") + newBytesIdx := findColumnIndex(header, "new B/op") + oldAllocsIdx := findColumnIndex(header, "old allocs/op") + newAllocsIdx := findColumnIndex(header, "new allocs/op") + + if nameIdx == -1 { + return fmt.Errorf("could not find 'name' column in benchstat output") + } + + var regressions []string + + for i, record := range records[1:] { + if len(record) <= nameIdx { + continue + } + + benchName := record[nameIdx] + + // Check time/op regression + if contains(cfg.failMetrics, "time/op") && oldTimeIdx != -1 && newTimeIdx != -1 { + if regression := checkMetricRegression(record, oldTimeIdx, newTimeIdx, cfg.threshold); regression != "" { + regressions = append(regressions, fmt.Sprintf("%s: time/op %s", benchName, regression)) + } + } + + // Check B/op regression + if contains(cfg.failMetrics, "B/op") && oldBytesIdx != -1 && newBytesIdx != -1 { + if regression := checkMetricRegression(record, oldBytesIdx, newBytesIdx, cfg.threshold); regression != "" { + regressions = append(regressions, fmt.Sprintf("%s: B/op %s", benchName, regression)) + } + } + + // Check allocs/op regression + if contains(cfg.failMetrics, "allocs/op") && oldAllocsIdx != -1 && newAllocsIdx != -1 { + if regression := checkMetricRegression(record, oldAllocsIdx, newAllocsIdx, cfg.threshold); regression != "" { + regressions = append(regressions, fmt.Sprintf("%s: allocs/op %s", benchName, regression)) + } + } + + if cfg.verbose && i < 5 { // Show first few comparisons + fmt.Printf(" %s: compared\n", benchName) + } + } + + // Print full benchstat output + fmt.Println("Benchmark comparison results:") + fmt.Println(csvOutput) + + if len(regressions) > 0 { + fmt.Printf("\n❌ Performance regressions detected (threshold: %.1fx):\n", cfg.threshold) + for _, regression := range regressions { + fmt.Printf(" - %s\n", regression) + } + return fmt.Errorf("performance regressions detected") + } + + fmt.Printf("✅ No significant performance regressions detected (threshold: %.1fx)\n", cfg.threshold) + return nil +} + +func findColumnIndex(header []string, columnName string) int { + for i, col := range header { + if strings.Contains(strings.ToLower(col), strings.ToLower(columnName)) { + return i + } + } + return -1 +} + +func checkMetricRegression(record []string, oldIdx, newIdx int, threshold float64) string { + if oldIdx >= len(record) || newIdx >= len(record) { + return "" + } + + oldVal, err1 := parseMetricValue(record[oldIdx]) + newVal, err2 := parseMetricValue(record[newIdx]) + + if err1 != nil || err2 != nil || oldVal == 0 { + return "" + } + + ratio := newVal / oldVal + if ratio > threshold { + return fmt.Sprintf("%.2fx slower (%.2f → %.2f)", ratio, oldVal, newVal) + } + + return "" +} + +func parseMetricValue(s string) (float64, error) { + // Remove common suffixes and parse + s = strings.TrimSpace(s) + s = strings.ReplaceAll(s, "ns", "") + s = strings.ReplaceAll(s, "B", "") + s = strings.TrimSpace(s) + + if s == "" || s == "-" { + return 0, fmt.Errorf("empty value") + } + + return strconv.ParseFloat(s, 64) +} + +func contains(slice []string, item string) bool { + for _, s := range slice { + if s == item { + return true + } + } + return false +} From 75e9d57612a2caa12b5c21777b717404a157fb53 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 05:49:48 +0100 Subject: [PATCH 03/20] perf(rdb): reuse single shared instance across all benchmarks --- .../rdb/v1/custom_benchmark_test.go | 190 +++++++++++------- 1 file changed, 121 insertions(+), 69 deletions(-) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 9ded21ec87..2854d2bbd7 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -3,8 +3,10 @@ package rdb_test import ( "bytes" "context" + "fmt" "os" "sort" + "sync" "testing" "time" @@ -31,6 +33,18 @@ const ( instanceReadyTimeout = 3 * time.Minute ) +var ( + sharedInstance *rdbSDK.Instance + sharedInstanceMu sync.Mutex +) + +// TestMain ensures shared instance cleanup +func TestMain(m *testing.M) { + code := m.Run() + cleanupSharedInstance() + os.Exit(code) +} + func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) { b.Helper() @@ -151,35 +165,118 @@ func (s *benchmarkStats) report(b *testing.B) { len(s.timings), minVal, median, mean, p95, maxVal) } -func BenchmarkInstanceGet(b *testing.B) { - if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { - b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") - } +func getOrCreateSharedInstance(b *testing.B, client *scw.Client, executeCmd func([]string) any, meta core.TestMetadata) *rdbSDK.Instance { + b.Helper() - client, meta, executeCmd := setupBenchmark(b) + sharedInstanceMu.Lock() + defer sharedInstanceMu.Unlock() + + if sharedInstance != nil { + b.Log("Reusing existing shared RDB instance") + return sharedInstance + } + b.Log("Creating shared RDB instance for all benchmarks...") ctx := &core.BeforeFuncCtx{ Client: client, ExecuteCmd: executeCmd, Meta: meta, } - err := createInstanceDirect(engine)(ctx) - if err != nil { - b.Fatalf("Failed to create instance: %v", err) + + if err := createInstanceDirect(engine)(ctx); err != nil { + b.Fatalf("Failed to create shared instance: %v", err) } instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + sharedInstance = instance - b.Cleanup(func() { - afterCtx := &core.AfterFuncCtx{ - Client: client, - ExecuteCmd: executeCmd, - Meta: meta, + b.Logf("Shared RDB instance created: %s", instance.ID) + + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + b.Fatalf("Shared instance not ready: %v", err) + } + + b.Log("Shared instance is ready") + return sharedInstance +} + +func cleanupSharedInstance() { + sharedInstanceMu.Lock() + defer sharedInstanceMu.Unlock() + + if sharedInstance == nil { + return + } + + fmt.Printf("Cleaning up shared RDB instance: %s\n", sharedInstance.ID) + + client, err := scw.NewClient( + scw.WithDefaultRegion(scw.RegionFrPar), + scw.WithDefaultZone(scw.ZoneFrPar1), + scw.WithEnv(), + ) + if err != nil { + fmt.Printf("Error creating client for cleanup: %v\n", err) + return + } + + config, err := scw.LoadConfig() + if err == nil { + activeProfile, err := config.GetActiveProfile() + if err == nil { + envProfile := scw.LoadEnvProfile() + profile := scw.MergeProfiles(activeProfile, envProfile) + client, _ = scw.NewClient( + scw.WithDefaultRegion(scw.RegionFrPar), + scw.WithDefaultZone(scw.ZoneFrPar1), + scw.WithProfile(profile), + scw.WithEnv(), + ) } - cleanupWithRetry(b, "instance", instance.ID, func() error { - return deleteInstanceDirect()(afterCtx) + } + + executeCmd := func(args []string) any { + _, result, _ := core.Bootstrap(&core.BootstrapConfig{ + Args: args, + Commands: rdb.GetCommands().Copy(), + BuildInfo: &core.BuildInfo{}, + Client: client, + DisableTelemetry: true, + DisableAliases: true, + OverrideEnv: map[string]string{}, + Ctx: context.Background(), }) - }) + return result + } + + meta := core.TestMetadata{ + "Instance": rdb.CreateInstanceResult{Instance: sharedInstance}, + } + + afterCtx := &core.AfterFuncCtx{ + Client: client, + ExecuteCmd: executeCmd, + Meta: meta, + } + + if err := deleteInstanceDirect()(afterCtx); err != nil { + fmt.Printf("Error deleting shared instance: %v\n", err) + time.Sleep(2 * time.Second) + if err2 := deleteInstanceDirect()(afterCtx); err2 != nil { + fmt.Printf("Final cleanup failure: %v\n", err2) + } + } + + sharedInstance = nil +} + +func BenchmarkInstanceGet(b *testing.B) { + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + client, meta, executeCmd := setupBenchmark(b) + instance := getOrCreateSharedInstance(b, client, executeCmd, meta) stats := newBenchmarkStats() b.ResetTimer() @@ -215,25 +312,17 @@ func BenchmarkBackupGet(b *testing.B) { } client, meta, executeCmd := setupBenchmark(b) + instance := getOrCreateSharedInstance(b, client, executeCmd, meta) ctx := &core.BeforeFuncCtx{ Client: client, ExecuteCmd: executeCmd, Meta: meta, } - err := createInstanceDirect(engine)(ctx) - if err != nil { - b.Fatalf("Failed to create instance: %v", err) - } - instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { - b.Fatalf("Instance not ready: %v", err) - } - - err = createBackupDirect("Backup")(ctx) - if err != nil { + if err := createBackupDirect("Backup")(ctx); err != nil { b.Fatalf("Failed to create backup: %v", err) } @@ -248,9 +337,6 @@ func BenchmarkBackupGet(b *testing.B) { cleanupWithRetry(b, "backup", backup.ID, func() error { return deleteBackupDirect("Backup")(afterCtx) }) - cleanupWithRetry(b, "instance", instance.ID, func() error { - return deleteInstanceDirect()(afterCtx) - }) }) stats := newBenchmarkStats() @@ -287,29 +373,20 @@ func BenchmarkBackupList(b *testing.B) { } client, meta, executeCmd := setupBenchmark(b) + instance := getOrCreateSharedInstance(b, client, executeCmd, meta) ctx := &core.BeforeFuncCtx{ Client: client, ExecuteCmd: executeCmd, Meta: meta, } - err := createInstanceDirect(engine)(ctx) - if err != nil { - b.Fatalf("Failed to create instance: %v", err) - } - instance := meta["Instance"].(rdb.CreateInstanceResult).Instance + meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { - b.Fatalf("Instance not ready: %v", err) - } - - err = createBackupDirect("Backup1")(ctx) - if err != nil { + if err := createBackupDirect("Backup1")(ctx); err != nil { b.Fatalf("Failed to create backup 1: %v", err) } - err = createBackupDirect("Backup2")(ctx) - if err != nil { + if err := createBackupDirect("Backup2")(ctx); err != nil { b.Fatalf("Failed to create backup 2: %v", err) } @@ -328,9 +405,6 @@ func BenchmarkBackupList(b *testing.B) { cleanupWithRetry(b, "backup2", backup2.ID, func() error { return deleteBackupDirect("Backup2")(afterCtx) }) - cleanupWithRetry(b, "instance", instance.ID, func() error { - return deleteInstanceDirect()(afterCtx) - }) }) stats := newBenchmarkStats() @@ -367,29 +441,7 @@ func BenchmarkDatabaseList(b *testing.B) { } client, meta, executeCmd := setupBenchmark(b) - - ctx := &core.BeforeFuncCtx{ - Client: client, - ExecuteCmd: executeCmd, - Meta: meta, - } - err := createInstanceDirect(engine)(ctx) - if err != nil { - b.Fatalf("Failed to create instance: %v", err) - } - - instance := meta["Instance"].(rdb.CreateInstanceResult).Instance - - b.Cleanup(func() { - afterCtx := &core.AfterFuncCtx{ - Client: client, - ExecuteCmd: executeCmd, - Meta: meta, - } - cleanupWithRetry(b, "instance", instance.ID, func() error { - return deleteInstanceDirect()(afterCtx) - }) - }) + instance := getOrCreateSharedInstance(b, client, executeCmd, meta) stats := newBenchmarkStats() b.ResetTimer() From 9fed4c5d5fd03503c2fe1c10187970a5afa88579 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 05:52:48 +0100 Subject: [PATCH 04/20] fix(benchstat): remove unused imports --- cmd/scw-benchstat/main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index a9bdcd3e9e..5a7cb79d3e 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -1,12 +1,10 @@ package main import ( - "bufio" "context" "encoding/csv" "flag" "fmt" - "io" "log" "os" "os/exec" From cfaaddda872b766ee6c07173fd71fe7b9d6bd3a5 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 06:12:12 +0100 Subject: [PATCH 05/20] fix(rdb): add stdout/stderr buffers to cleanup bootstrap --- internal/namespaces/rdb/v1/custom_benchmark_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 2854d2bbd7..9db0354a35 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -236,10 +236,14 @@ func cleanupSharedInstance() { } executeCmd := func(args []string) any { + stdoutBuffer := &bytes.Buffer{} + stderrBuffer := &bytes.Buffer{} _, result, _ := core.Bootstrap(&core.BootstrapConfig{ Args: args, Commands: rdb.GetCommands().Copy(), BuildInfo: &core.BuildInfo{}, + Stdout: stdoutBuffer, + Stderr: stderrBuffer, Client: client, DisableTelemetry: true, DisableAliases: true, From cbeacc654db8f7ca6326db727daa09266a8a4622 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 06:25:26 +0100 Subject: [PATCH 06/20] fix(rdb): prevent concurrent backup operations with mutex --- .../namespaces/rdb/v1/custom_benchmark_test.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 9db0354a35..7288d65173 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -36,6 +36,7 @@ const ( var ( sharedInstance *rdbSDK.Instance sharedInstanceMu sync.Mutex + backupOpMu sync.Mutex ) // TestMain ensures shared instance cleanup @@ -326,7 +327,11 @@ func BenchmarkBackupGet(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := createBackupDirect("Backup")(ctx); err != nil { + backupOpMu.Lock() + err := createBackupDirect("Backup")(ctx) + backupOpMu.Unlock() + + if err != nil { b.Fatalf("Failed to create backup: %v", err) } @@ -387,10 +392,16 @@ func BenchmarkBackupList(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := createBackupDirect("Backup1")(ctx); err != nil { + backupOpMu.Lock() + err := createBackupDirect("Backup1")(ctx) + if err != nil { + backupOpMu.Unlock() b.Fatalf("Failed to create backup 1: %v", err) } - if err := createBackupDirect("Backup2")(ctx); err != nil { + err = createBackupDirect("Backup2")(ctx) + backupOpMu.Unlock() + + if err != nil { b.Fatalf("Failed to create backup 2: %v", err) } From 05706814cb087cf49d466dc36bc659a168e47b28 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 06:35:51 +0100 Subject: [PATCH 07/20] fix(rdb): wait for instance ready state before backup creation --- internal/namespaces/rdb/v1/custom_benchmark_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 7288d65173..d17ffc99a8 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -328,6 +328,10 @@ func BenchmarkBackupGet(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} backupOpMu.Lock() + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + backupOpMu.Unlock() + b.Fatalf("Instance not ready before backup: %v", err) + } err := createBackupDirect("Backup")(ctx) backupOpMu.Unlock() @@ -393,11 +397,19 @@ func BenchmarkBackupList(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} backupOpMu.Lock() + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + backupOpMu.Unlock() + b.Fatalf("Instance not ready before backup 1: %v", err) + } err := createBackupDirect("Backup1")(ctx) if err != nil { backupOpMu.Unlock() b.Fatalf("Failed to create backup 1: %v", err) } + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + backupOpMu.Unlock() + b.Fatalf("Instance not ready before backup 2: %v", err) + } err = createBackupDirect("Backup2")(ctx) backupOpMu.Unlock() From d8032f73c69ef8ab74d7ab2373667196ce9caf82 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 09:39:09 +0100 Subject: [PATCH 08/20] fix(rdb): retry cleanup on 409 conflicts with exponential backoff --- .../rdb/v1/custom_benchmark_test.go | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index d17ffc99a8..203202da26 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "sort" + "strings" "sync" "testing" "time" @@ -107,12 +108,26 @@ func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []s func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn func() error) { b.Helper() - if err := cleanupFn(); err != nil { - b.Logf("cleanup failed (%s=%s): %v; retrying...", name, resourceID, err) - time.Sleep(2 * time.Second) - if err2 := cleanupFn(); err2 != nil { - b.Errorf("final cleanup failure (%s=%s): %v", name, resourceID, err2) + maxRetries := 5 + for i := 0; i < maxRetries; i++ { + err := cleanupFn() + if err == nil { + return + } + + // Check if it's a 409 conflict (resource in transient state) + errMsg := err.Error() + if strings.Contains(errMsg, "409") || strings.Contains(errMsg, "Conflict") || strings.Contains(errMsg, "transient state") { + if i < maxRetries-1 { + waitTime := time.Duration(2*(i+1)) * time.Second + b.Logf("cleanup conflict for %s=%s (attempt %d/%d), waiting %v: %v", name, resourceID, i+1, maxRetries, waitTime, err) + time.Sleep(waitTime) + continue + } } + + b.Errorf("cleanup failure (%s=%s) after %d attempts: %v", name, resourceID, i+1, err) + return } } From f1f891423faf6feee90697ed8a5962af3e602eb8 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 10:23:01 +0100 Subject: [PATCH 09/20] docs: add comprehensive benchmarking guide --- docs/BENCHMARKS.md | 426 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 426 insertions(+) create mode 100644 docs/BENCHMARKS.md diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md new file mode 100644 index 0000000000..5e9a7a4c07 --- /dev/null +++ b/docs/BENCHMARKS.md @@ -0,0 +1,426 @@ +# Performance Benchmarks + +This document describes the performance benchmarking system for the Scaleway CLI. + +## Overview + +The benchmarking system allows you to: +- **Measure performance** of CLI commands over time +- **Detect regressions** automatically before merging code +- **Track performance trends** across releases +- **Compare implementations** between branches + +## Architecture + +The system consists of two main components: + +1. **Benchmark tests** (`*_benchmark_test.go`) - Go benchmark functions that measure command performance +2. **Benchstat tool** (`cmd/scw-benchstat`) - Wrapper that runs benchmarks and detects regressions using [benchstat](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) + +### Workflow + +``` +┌─────────────────────────────────────────────────────────────┐ +│ 1. Scan project for *_benchmark_test.go files │ +└──────────────────┬──────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ 2. Run benchmarks: go test -bench=. -benchtime=1s -count=10│ +└──────────────────┬──────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ 3. Save/Load baseline from testdata/benchmark.baseline │ +└──────────────────┬──────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ 4. Compare with benchstat (statistical analysis) │ +└──────────────────┬──────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────┐ +│ 5. Fail CI if regression > threshold (e.g., 1.5x slower) │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Quick Start + +### Running Benchmarks + +```bash +# Run all benchmarks for all namespaces +SCW_PROFILE=cli \ +CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat + +# Run benchmarks for a specific namespace +SCW_PROFILE=cli \ +CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat --target-dirs=internal/namespaces/rdb/v1 +``` + +### Creating a Baseline + +The first time you run benchmarks, a baseline file is automatically created at: +``` +internal/namespaces///testdata/benchmark.baseline +``` + +Example for RDB: +``` +internal/namespaces/rdb/v1/testdata/benchmark.baseline +``` + +**Commit this baseline file** to track performance over time: + +```bash +git add internal/namespaces/rdb/v1/testdata/benchmark.baseline +git commit -S -m "chore(rdb): add performance benchmark baseline" +``` + +### Updating the Baseline + +When you intentionally make performance changes, update the baseline: + +```bash +# Run benchmarks and save new baseline +CLI_RUN_BENCHMARKS=true go test \ + -bench=. \ + -benchtime=1s \ + -count=10 \ + ./internal/namespaces/rdb/v1 \ + > internal/namespaces/rdb/v1/testdata/benchmark.baseline + +# Commit the updated baseline +git add internal/namespaces/rdb/v1/testdata/benchmark.baseline +git commit -S -m "chore(rdb): update benchmark baseline after optimization" +``` + +## Command Options + +### Basic Usage + +```bash +go run ./cmd/scw-benchstat [OPTIONS] +``` + +### Available Options + +| Option | Default | Description | +|--------|---------|-------------| +| `--bench` | `.` | Benchmark pattern (Go regex) | +| `--benchtime` | `1s` | Duration per benchmark run | +| `--count` | `5` | Number of benchmark runs (for statistical accuracy) | +| `--benchmem` | `false` | Measure memory allocations | +| `--fail-metrics` | - | Comma-separated metrics to check: `time/op`, `B/op`, `allocs/op` | +| `--threshold` | `1.5` | Regression threshold (1.5 = fail if 50% slower) | +| `--install-benchstat` | `false` | Auto-install benchstat if not found | +| `--target-dirs` | (all) | Comma-separated directories to benchmark | +| `--verbose` | `false` | Enable verbose output | + +### Examples + +#### Run specific benchmarks only +```bash +SCW_PROFILE=cli CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat --bench="BenchmarkInstance.*" +``` + +#### Strict regression detection (20% threshold) +```bash +SCW_PROFILE=cli CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat \ + --threshold=1.2 \ + --fail-metrics=time/op,B/op +``` + +#### Quick run with fewer iterations +```bash +SCW_PROFILE=cli CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat --count=3 --benchtime=500ms +``` + +#### Precise measurement with more runs +```bash +SCW_PROFILE=cli CLI_RUN_BENCHMARKS=true \ +go run ./cmd/scw-benchstat --count=20 --benchtime=3s +``` + +## Benchmark Metrics + +Each benchmark reports three key metrics: + +| Metric | Unit | Description | +|--------|------|-------------| +| **time/op** | ns/op | Nanoseconds per operation (execution time) | +| **B/op** | bytes | Bytes allocated per operation (memory usage) | +| **allocs/op** | count | Number of memory allocations per operation | + +### Example Output + +``` +BenchmarkInstanceGet-11 5 235361983 ns/op 379590 B/op 4369 allocs/op +BenchmarkBackupGet-11 15 70244775 ns/op 272052 B/op 2845 allocs/op +BenchmarkBackupList-11 12 92052913 ns/op 284125 B/op 2994 allocs/op +BenchmarkDatabaseList-11 9 164681597 ns/op 299008 B/op 3152 allocs/op +``` + +**Reading the output:** +- **Column 1**: Benchmark name + number of parallel workers +- **Column 2**: Number of iterations executed +- **Column 3**: Average time per operation (in nanoseconds) +- **Column 4**: Average memory allocated per operation +- **Column 5**: Average number of allocations per operation + +## Interpreting Results + +### Comparison Output (benchstat) + +When comparing with a baseline, `benchstat` shows statistical differences: + +``` +name old time/op new time/op delta +InstanceGet-11 235ms ± 2% 220ms ± 3% -6.38% (p=0.000 n=10+10) +BackupGet-11 70.2ms ± 5% 72.1ms ± 4% +2.70% (p=0.043 n=10+10) + +name old alloc/op new alloc/op delta +InstanceGet-11 380kB ± 1% 383kB ± 1% +0.79% (p=0.000 n=10+10) +``` + +**Understanding the columns:** +- `±` : Standard deviation (variance in measurements) +- `delta` : Percentage change (negative = improvement, positive = regression) +- `p=0.000` : Statistical significance (p < 0.05 = significant change) +- `n=10+10` : Number of samples in each measurement + +### Regression Detection + +The tool fails if any metric exceeds the threshold: + +```bash +❌ Performance regressions detected (threshold: 1.5x): + - BenchmarkInstanceGet: time/op 2.1x slower (235ms → 493ms) + - BenchmarkBackupList: B/op 1.8x more memory (284KB → 512KB) +``` + + +## Writing Benchmarks + +### Basic Structure + +Create a file named `custom_benchmark_test.go` in your namespace: + +```go +package mynamespace_test + +import ( + "os" + "testing" + "time" +) + +func BenchmarkMyCommand(b *testing.B) { + // Skip unless explicitly enabled + if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") + } + + // Setup: create resources, clients, etc. + client, meta, executeCmd := setupBenchmark(b) + + // Reset timer to exclude setup time + b.ResetTimer() + b.ReportAllocs() // Track memory allocations + + // Benchmark loop (Go adjusts b.N automatically) + for range b.N { + executeCmd([]string{"scw", "my-namespace", "my-command", "arg1"}) + } + + b.StopTimer() +} +``` + +### Best Practices + +#### 1. **Resource Management** + +Reuse expensive resources across benchmarks: + +```go +var ( + sharedInstance *MyResource + sharedInstanceMu sync.Mutex +) + +func getOrCreateSharedInstance(b *testing.B) *MyResource { + b.Helper() + sharedInstanceMu.Lock() + defer sharedInstanceMu.Unlock() + + if sharedInstance != nil { + b.Log("Reusing existing shared instance") + return sharedInstance + } + + b.Log("Creating shared instance...") + sharedInstance = createExpensiveResource() + return sharedInstance +} + +func TestMain(m *testing.M) { + code := m.Run() + cleanupSharedInstance() + os.Exit(code) +} +``` + +#### 2. **Proper Timing** + +Exclude setup and cleanup from measurements: + +```go +func BenchmarkMyCommand(b *testing.B) { + // Setup (not timed) + resource := createResource() + b.Cleanup(func() { deleteResource(resource) }) + + b.ResetTimer() // ⏱️ Start timing here + + for range b.N { + executeCmd(...) + } + + b.StopTimer() // ⏹️ Stop timing before cleanup +} +``` + +#### 3. **Avoid Interference** + +Serialize operations that cannot run in parallel: + +```go +var operationMu sync.Mutex + +func BenchmarkOperation(b *testing.B) { + operationMu.Lock() + defer operationMu.Unlock() + + // Only one benchmark can run this at a time + performExclusiveOperation() +} +``` + +## Advanced Usage + +### Custom Statistics + +Enable detailed statistics with `CLI_BENCH_TRACE`: + +```bash +CLI_BENCH_TRACE=true CLI_RUN_BENCHMARKS=true go test -bench=. -v +``` + +Output: +``` +Distribution (n=100): min=200ms median=235ms mean=238ms p95=280ms max=320ms +``` + +### CPU Profiling + +Profile a slow benchmark: + +```bash +CLI_RUN_BENCHMARKS=true go test -bench=BenchmarkSlowCommand \ + -cpuprofile=cpu.prof + +go tool pprof -http=:8080 cpu.prof +``` + +### Memory Profiling + +Analyze memory allocations: + +```bash +CLI_RUN_BENCHMARKS=true go test -bench=BenchmarkMemoryHeavy \ + -memprofile=mem.prof + +go tool pprof -http=:8080 mem.prof +``` + +### Comparing Branches + +```bash +# Run on main branch +git checkout main +CLI_RUN_BENCHMARKS=true go test -bench=. -count=10 > /tmp/main.txt + +# Run on feature branch +git checkout feature/my-optimization +CLI_RUN_BENCHMARKS=true go test -bench=. -count=10 > /tmp/feature.txt + +# Compare +benchstat /tmp/main.txt /tmp/feature.txt +``` + +## Troubleshooting + +### "benchstat not found" + +Install benchstat: +```bash +go install golang.org/x/perf/cmd/benchstat@latest +``` + +Or use auto-install: +```bash +go run ./cmd/scw-benchstat --install-benchstat +``` + +### "signal: killed" + +The benchmark process was killed (timeout or OOM). Try: +- Reduce `--count` or `--benchtime` +- Run specific benchmarks only with `--bench` +- Check system resources (RAM, CPU) + +### "409 Conflict" errors + +Resources are in a transient state. The cleanup retry mechanism should handle this automatically. If it persists: +- Increase retry attempts in `cleanupWithRetry` +- Add more wait time between operations + +### Inconsistent Results + +Run with more iterations for better statistical accuracy: +```bash +go run ./cmd/scw-benchstat --count=20 --benchtime=3s +``` + +## FAQ + +**Q: How often should I update the baseline?** +A: Update it after intentional performance changes or major refactors. Don't update for every PR unless you've specifically optimized performance. + +**Q: What's a good threshold value?** +A: Start with 1.5 (50% regression). Adjust based on your tolerance: +- 1.2 (20%) = strict +- 1.5 (50%) = balanced +- 2.0 (100%) = lenient + +**Q: Should I commit baseline files?** +A: Yes! Baselines should be tracked in git to enable comparison across branches and time. + +**Q: How do I run benchmarks locally?** +A: Use the same command as CI with your local credentials: +```bash +SCW_PROFILE=cli CLI_RUN_BENCHMARKS=true go run ./cmd/scw-benchstat +``` + +**Q: Can I benchmark other namespaces (instance, vpc, etc.)?** +A: Yes! Create a `custom_benchmark_test.go` file in any namespace directory following the same pattern as RDB. + +## References + +- [Go Benchmarking Documentation](https://pkg.go.dev/testing#hdr-Benchmarks) +- [benchstat Tool](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) +- [Performance Profiling in Go](https://go.dev/blog/pprof) + From 389781018d1feff37e0932c89a1aa0f0614777d8 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 10:44:06 +0100 Subject: [PATCH 10/20] feat(env): add type-safe environment variable utilities --- internal/env/env.go | 85 ++++++++++ internal/env/env_test.go | 158 ++++++++++++++++++ .../rdb/v1/custom_benchmark_test.go | 75 ++------- internal/testhelpers/helpers_benchmark.go | 70 ++++++++ 4 files changed, 329 insertions(+), 59 deletions(-) create mode 100644 internal/env/env.go create mode 100644 internal/env/env_test.go create mode 100644 internal/testhelpers/helpers_benchmark.go diff --git a/internal/env/env.go b/internal/env/env.go new file mode 100644 index 0000000000..f7ce927c80 --- /dev/null +++ b/internal/env/env.go @@ -0,0 +1,85 @@ +// Package env provides utilities for reading environment variables with defaults and type safety. +package env + +import ( + "os" + "strconv" + "time" +) + +// GetBool reads a boolean environment variable. +// Returns defaultValue if the variable is not set or invalid. +func GetBool(key string, defaultValue bool) bool { + value := os.Getenv(key) + if value == "" { + return defaultValue + } + + switch value { + case "1", "true", "TRUE", "True", "yes", "YES", "Yes": + return true + case "0", "false", "FALSE", "False", "no", "NO", "No": + return false + default: + return defaultValue + } +} + +// GetString reads a string environment variable. +// Returns defaultValue if the variable is not set. +func GetString(key string, defaultValue string) string { + value := os.Getenv(key) + if value == "" { + return defaultValue + } + return value +} + +// GetInt reads an integer environment variable. +// Returns defaultValue if the variable is not set or invalid. +func GetInt(key string, defaultValue int) int { + value := os.Getenv(key) + if value == "" { + return defaultValue + } + + intValue, err := strconv.Atoi(value) + if err != nil { + return defaultValue + } + + return intValue +} + +// GetDuration reads a duration environment variable. +// Accepts values like "5s", "10m", "1h30m". +// Returns defaultValue if the variable is not set or invalid. +func GetDuration(key string, defaultValue time.Duration) time.Duration { + value := os.Getenv(key) + if value == "" { + return defaultValue + } + + duration, err := time.ParseDuration(value) + if err != nil { + return defaultValue + } + + return duration +} + +// IsSet returns true if the environment variable is set (even if empty). +func IsSet(key string) bool { + _, exists := os.LookupEnv(key) + return exists +} + +// MustGetString reads a string environment variable and panics if not set. +// Use this for required configuration values. +func MustGetString(key string) string { + value := os.Getenv(key) + if value == "" { + panic("environment variable " + key + " is required but not set") + } + return value +} diff --git a/internal/env/env_test.go b/internal/env/env_test.go new file mode 100644 index 0000000000..9c10db6ad3 --- /dev/null +++ b/internal/env/env_test.go @@ -0,0 +1,158 @@ +package env + +import ( + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetBool(t *testing.T) { + tests := []struct { + name string + value string + defaultValue bool + expected bool + }{ + {"true", "true", false, true}, + {"TRUE", "TRUE", false, true}, + {"1", "1", false, true}, + {"yes", "yes", false, true}, + {"false", "false", true, false}, + {"FALSE", "FALSE", true, false}, + {"0", "0", true, false}, + {"no", "no", true, false}, + {"empty", "", false, false}, + {"empty_default_true", "", true, true}, + {"invalid", "invalid", false, false}, + {"invalid_default_true", "invalid", true, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := "TEST_BOOL_" + tt.name + if tt.value != "" { + os.Setenv(key, tt.value) + defer os.Unsetenv(key) + } + result := GetBool(key, tt.defaultValue) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetString(t *testing.T) { + tests := []struct { + name string + value string + defaultValue string + expected string + }{ + {"set", "value", "default", "value"}, + {"empty", "", "default", "default"}, + {"spaces", " spaces ", "default", " spaces "}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := "TEST_STRING_" + tt.name + if tt.value != "" { + os.Setenv(key, tt.value) + defer os.Unsetenv(key) + } + result := GetString(key, tt.defaultValue) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetInt(t *testing.T) { + tests := []struct { + name string + value string + defaultValue int + expected int + }{ + {"positive", "42", 0, 42}, + {"negative", "-10", 0, -10}, + {"zero", "0", 100, 0}, + {"empty", "", 99, 99}, + {"invalid", "not_a_number", 50, 50}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := "TEST_INT_" + tt.name + if tt.value != "" { + os.Setenv(key, tt.value) + defer os.Unsetenv(key) + } + result := GetInt(key, tt.defaultValue) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetDuration(t *testing.T) { + tests := []struct { + name string + value string + defaultValue time.Duration + expected time.Duration + }{ + {"seconds", "5s", time.Minute, 5 * time.Second}, + {"minutes", "10m", time.Second, 10 * time.Minute}, + {"hours", "2h", time.Second, 2 * time.Hour}, + {"combined", "1h30m", time.Second, 90 * time.Minute}, + {"empty", "", time.Minute, time.Minute}, + {"invalid", "invalid", 5 * time.Second, 5 * time.Second}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := "TEST_DURATION_" + tt.name + if tt.value != "" { + os.Setenv(key, tt.value) + defer os.Unsetenv(key) + } + result := GetDuration(key, tt.defaultValue) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsSet(t *testing.T) { + key := "TEST_IS_SET" + + // Not set + assert.False(t, IsSet(key)) + + // Set to empty + os.Setenv(key, "") + defer os.Unsetenv(key) + assert.True(t, IsSet(key)) + + // Set to value + os.Setenv(key, "value") + assert.True(t, IsSet(key)) +} + +func TestMustGetString(t *testing.T) { + t.Run("set", func(t *testing.T) { + key := "TEST_MUST_GET_SET" + os.Setenv(key, "value") + defer os.Unsetenv(key) + + result := MustGetString(key) + assert.Equal(t, "value", result) + }) + + t.Run("not_set_panics", func(t *testing.T) { + key := "TEST_MUST_GET_NOT_SET" + require.Panics(t, func() { + MustGetString(key) + }) + }) +} diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 203202da26..484cee009e 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -12,7 +12,9 @@ import ( "time" "github.com/scaleway/scaleway-cli/v2/core" + "github.com/scaleway/scaleway-cli/v2/internal/env" "github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1" + "github.com/scaleway/scaleway-cli/v2/internal/testhelpers" rdbSDK "github.com/scaleway/scaleway-sdk-go/api/rdb/v1" "github.com/scaleway/scaleway-sdk-go/scw" ) @@ -21,6 +23,10 @@ import ( // // Baseline stored in testdata/benchmark.baseline (like golden files). // +// Install benchstat (required for comparison): +// +// go install golang.org/x/perf/cmd/benchstat@latest +// // To compare performance: // // benchstat testdata/benchmark.baseline <(CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x .) @@ -28,6 +34,10 @@ import ( // To update baseline: // // CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x . > testdata/benchmark.baseline +// +// Or use the automated tool (installs benchstat automatically): +// +// go run ./cmd/scw-benchstat --install-benchstat --bench=. --count=10 const ( defaultCmdTimeout = 30 * time.Second @@ -49,60 +59,7 @@ func TestMain(m *testing.M) { func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) { b.Helper() - - clientOpts := []scw.ClientOption{ - scw.WithDefaultRegion(scw.RegionFrPar), - scw.WithDefaultZone(scw.ZoneFrPar1), - scw.WithUserAgent("cli-benchmark-test"), - scw.WithEnv(), - } - - config, err := scw.LoadConfig() - if err == nil { - activeProfile, err := config.GetActiveProfile() - if err == nil { - envProfile := scw.LoadEnvProfile() - profile := scw.MergeProfiles(activeProfile, envProfile) - clientOpts = append(clientOpts, scw.WithProfile(profile)) - } - } - - client, err := scw.NewClient(clientOpts...) - if err != nil { - b.Fatalf( - "Failed to create Scaleway client: %v\nMake sure you have configured your credentials with 'scw config'", - err, - ) - } - - meta := core.TestMetadata{ - "t": b, - } - - executeCmd := func(args []string) any { - stdoutBuffer := &bytes.Buffer{} - stderrBuffer := &bytes.Buffer{} - _, result, err := core.Bootstrap(&core.BootstrapConfig{ - Args: args, - Commands: rdb.GetCommands().Copy(), - BuildInfo: nil, - Stdout: stdoutBuffer, - Stderr: stderrBuffer, - Client: client, - DisableTelemetry: true, - DisableAliases: true, - OverrideEnv: map[string]string{}, - Ctx: context.Background(), - }) - if err != nil { - b.Errorf("error executing cmd (%s): %v\nstdout: %s\nstderr: %s", - args, err, stdoutBuffer.String(), stderrBuffer.String()) - } - - return result - } - - return client, meta, executeCmd + return testhelpers.SetupBenchmark(b, rdb.GetCommands()) } func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn func() error) { @@ -138,7 +95,7 @@ type benchmarkStats struct { func newBenchmarkStats() *benchmarkStats { return &benchmarkStats{ - enabled: os.Getenv("CLI_BENCH_TRACE") == "true", + enabled: env.GetBool("CLI_BENCH_TRACE", false), timings: make([]time.Duration, 0, 1000), } } @@ -291,7 +248,7 @@ func cleanupSharedInstance() { } func BenchmarkInstanceGet(b *testing.B) { - if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + if !env.GetBool("CLI_RUN_BENCHMARKS", false) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -327,7 +284,7 @@ func BenchmarkInstanceGet(b *testing.B) { } func BenchmarkBackupGet(b *testing.B) { - if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + if !env.GetBool("CLI_RUN_BENCHMARKS", false) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -396,7 +353,7 @@ func BenchmarkBackupGet(b *testing.B) { } func BenchmarkBackupList(b *testing.B) { - if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + if !env.GetBool("CLI_RUN_BENCHMARKS", false) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -478,7 +435,7 @@ func BenchmarkBackupList(b *testing.B) { } func BenchmarkDatabaseList(b *testing.B) { - if os.Getenv("CLI_RUN_BENCHMARKS") != "true" { + if !env.GetBool("CLI_RUN_BENCHMARKS", false) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } diff --git a/internal/testhelpers/helpers_benchmark.go b/internal/testhelpers/helpers_benchmark.go new file mode 100644 index 0000000000..fe5e4eb7fa --- /dev/null +++ b/internal/testhelpers/helpers_benchmark.go @@ -0,0 +1,70 @@ +package testhelpers + +import ( + "bytes" + "context" + "testing" + + "github.com/scaleway/scaleway-cli/v2/core" + "github.com/scaleway/scaleway-sdk-go/scw" +) + +// SetupBenchmark initializes a Scaleway client and test metadata for benchmarks. +// It loads credentials from the active profile and environment variables. +func SetupBenchmark(b *testing.B, commands *core.Commands) (*scw.Client, core.TestMetadata, func(args []string) any) { + b.Helper() + + clientOpts := []scw.ClientOption{ + scw.WithDefaultRegion(scw.RegionFrPar), + scw.WithDefaultZone(scw.ZoneFrPar1), + scw.WithUserAgent("cli-benchmark-test"), + scw.WithEnv(), + } + + config, err := scw.LoadConfig() + if err == nil { + activeProfile, err := config.GetActiveProfile() + if err == nil { + envProfile := scw.LoadEnvProfile() + profile := scw.MergeProfiles(activeProfile, envProfile) + clientOpts = append(clientOpts, scw.WithProfile(profile)) + } + } + + client, err := scw.NewClient(clientOpts...) + if err != nil { + b.Fatalf( + "Failed to create Scaleway client: %v\nMake sure you have configured your credentials with 'scw config'", + err, + ) + } + + meta := core.TestMetadata{ + "t": b, + } + + executeCmd := func(args []string) any { + stdoutBuffer := &bytes.Buffer{} + stderrBuffer := &bytes.Buffer{} + _, result, err := core.Bootstrap(&core.BootstrapConfig{ + Args: args, + Commands: commands.Copy(), + BuildInfo: nil, + Stdout: stdoutBuffer, + Stderr: stderrBuffer, + Client: client, + DisableTelemetry: true, + DisableAliases: true, + OverrideEnv: map[string]string{}, + Ctx: context.Background(), + }) + if err != nil { + b.Errorf("error executing cmd (%s): %v\nstdout: %s\nstderr: %s", + args, err, stdoutBuffer.String(), stderrBuffer.String()) + } + + return result + } + + return client, meta, executeCmd +} From 1a1e30e6dfd182b07b70b2273b29d669812793df Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 21 Nov 2025 10:57:11 +0100 Subject: [PATCH 11/20] refactor(env): simplify package to only contain constant definitions --- internal/env/env.go | 91 ++-------- internal/env/env_test.go | 158 ------------------ .../rdb/v1/custom_benchmark_test.go | 10 +- 3 files changed, 16 insertions(+), 243 deletions(-) delete mode 100644 internal/env/env_test.go diff --git a/internal/env/env.go b/internal/env/env.go index f7ce927c80..fca4e69cf4 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -1,85 +1,16 @@ -// Package env provides utilities for reading environment variables with defaults and type safety. +// Package env contains a list of environment variables used to modify the behaviour of the CLI. package env -import ( - "os" - "strconv" - "time" -) - -// GetBool reads a boolean environment variable. -// Returns defaultValue if the variable is not set or invalid. -func GetBool(key string, defaultValue bool) bool { - value := os.Getenv(key) - if value == "" { - return defaultValue - } - - switch value { - case "1", "true", "TRUE", "True", "yes", "YES", "Yes": - return true - case "0", "false", "FALSE", "False", "no", "NO", "No": - return false - default: - return defaultValue - } -} - -// GetString reads a string environment variable. -// Returns defaultValue if the variable is not set. -func GetString(key string, defaultValue string) string { - value := os.Getenv(key) - if value == "" { - return defaultValue - } - return value -} - -// GetInt reads an integer environment variable. -// Returns defaultValue if the variable is not set or invalid. -func GetInt(key string, defaultValue int) int { - value := os.Getenv(key) - if value == "" { - return defaultValue - } +const ( + // RunBenchmarks if set to "true" will enable benchmark execution + RunBenchmarks = "CLI_RUN_BENCHMARKS" - intValue, err := strconv.Atoi(value) - if err != nil { - return defaultValue - } + // BenchTrace if set to "true" will enable detailed benchmark statistics (min, median, mean, p95, max) + BenchTrace = "CLI_BENCH_TRACE" - return intValue -} + // UpdateCassettes if set to "true" will trigger the cassettes to be recorded + UpdateCassettes = "CLI_UPDATE_CASSETTES" -// GetDuration reads a duration environment variable. -// Accepts values like "5s", "10m", "1h30m". -// Returns defaultValue if the variable is not set or invalid. -func GetDuration(key string, defaultValue time.Duration) time.Duration { - value := os.Getenv(key) - if value == "" { - return defaultValue - } - - duration, err := time.ParseDuration(value) - if err != nil { - return defaultValue - } - - return duration -} - -// IsSet returns true if the environment variable is set (even if empty). -func IsSet(key string) bool { - _, exists := os.LookupEnv(key) - return exists -} - -// MustGetString reads a string environment variable and panics if not set. -// Use this for required configuration values. -func MustGetString(key string) string { - value := os.Getenv(key) - if value == "" { - panic("environment variable " + key + " is required but not set") - } - return value -} + // UpdateGoldens if set to "true" will update golden files during tests + UpdateGoldens = "CLI_UPDATE_GOLDENS" +) diff --git a/internal/env/env_test.go b/internal/env/env_test.go deleted file mode 100644 index 9c10db6ad3..0000000000 --- a/internal/env/env_test.go +++ /dev/null @@ -1,158 +0,0 @@ -package env - -import ( - "os" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGetBool(t *testing.T) { - tests := []struct { - name string - value string - defaultValue bool - expected bool - }{ - {"true", "true", false, true}, - {"TRUE", "TRUE", false, true}, - {"1", "1", false, true}, - {"yes", "yes", false, true}, - {"false", "false", true, false}, - {"FALSE", "FALSE", true, false}, - {"0", "0", true, false}, - {"no", "no", true, false}, - {"empty", "", false, false}, - {"empty_default_true", "", true, true}, - {"invalid", "invalid", false, false}, - {"invalid_default_true", "invalid", true, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - key := "TEST_BOOL_" + tt.name - if tt.value != "" { - os.Setenv(key, tt.value) - defer os.Unsetenv(key) - } - result := GetBool(key, tt.defaultValue) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestGetString(t *testing.T) { - tests := []struct { - name string - value string - defaultValue string - expected string - }{ - {"set", "value", "default", "value"}, - {"empty", "", "default", "default"}, - {"spaces", " spaces ", "default", " spaces "}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - key := "TEST_STRING_" + tt.name - if tt.value != "" { - os.Setenv(key, tt.value) - defer os.Unsetenv(key) - } - result := GetString(key, tt.defaultValue) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestGetInt(t *testing.T) { - tests := []struct { - name string - value string - defaultValue int - expected int - }{ - {"positive", "42", 0, 42}, - {"negative", "-10", 0, -10}, - {"zero", "0", 100, 0}, - {"empty", "", 99, 99}, - {"invalid", "not_a_number", 50, 50}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - key := "TEST_INT_" + tt.name - if tt.value != "" { - os.Setenv(key, tt.value) - defer os.Unsetenv(key) - } - result := GetInt(key, tt.defaultValue) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestGetDuration(t *testing.T) { - tests := []struct { - name string - value string - defaultValue time.Duration - expected time.Duration - }{ - {"seconds", "5s", time.Minute, 5 * time.Second}, - {"minutes", "10m", time.Second, 10 * time.Minute}, - {"hours", "2h", time.Second, 2 * time.Hour}, - {"combined", "1h30m", time.Second, 90 * time.Minute}, - {"empty", "", time.Minute, time.Minute}, - {"invalid", "invalid", 5 * time.Second, 5 * time.Second}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - key := "TEST_DURATION_" + tt.name - if tt.value != "" { - os.Setenv(key, tt.value) - defer os.Unsetenv(key) - } - result := GetDuration(key, tt.defaultValue) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestIsSet(t *testing.T) { - key := "TEST_IS_SET" - - // Not set - assert.False(t, IsSet(key)) - - // Set to empty - os.Setenv(key, "") - defer os.Unsetenv(key) - assert.True(t, IsSet(key)) - - // Set to value - os.Setenv(key, "value") - assert.True(t, IsSet(key)) -} - -func TestMustGetString(t *testing.T) { - t.Run("set", func(t *testing.T) { - key := "TEST_MUST_GET_SET" - os.Setenv(key, "value") - defer os.Unsetenv(key) - - result := MustGetString(key) - assert.Equal(t, "value", result) - }) - - t.Run("not_set_panics", func(t *testing.T) { - key := "TEST_MUST_GET_NOT_SET" - require.Panics(t, func() { - MustGetString(key) - }) - }) -} diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 484cee009e..5ba8768449 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -95,7 +95,7 @@ type benchmarkStats struct { func newBenchmarkStats() *benchmarkStats { return &benchmarkStats{ - enabled: env.GetBool("CLI_BENCH_TRACE", false), + enabled: os.Getenv(env.BenchTrace) == "true", timings: make([]time.Duration, 0, 1000), } } @@ -248,7 +248,7 @@ func cleanupSharedInstance() { } func BenchmarkInstanceGet(b *testing.B) { - if !env.GetBool("CLI_RUN_BENCHMARKS", false) { + if os.Getenv(env.RunBenchmarks) != "true" { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -284,7 +284,7 @@ func BenchmarkInstanceGet(b *testing.B) { } func BenchmarkBackupGet(b *testing.B) { - if !env.GetBool("CLI_RUN_BENCHMARKS", false) { + if os.Getenv(env.RunBenchmarks) != "true" { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -353,7 +353,7 @@ func BenchmarkBackupGet(b *testing.B) { } func BenchmarkBackupList(b *testing.B) { - if !env.GetBool("CLI_RUN_BENCHMARKS", false) { + if os.Getenv(env.RunBenchmarks) != "true" { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } @@ -435,7 +435,7 @@ func BenchmarkBackupList(b *testing.B) { } func BenchmarkDatabaseList(b *testing.B) { - if !env.GetBool("CLI_RUN_BENCHMARKS", false) { + if os.Getenv(env.RunBenchmarks) != "true" { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } From 9b249fe757ca32419f77475a0166af2062453541 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Mon, 24 Nov 2025 06:25:21 +0100 Subject: [PATCH 12/20] refactor(rdb): remove unnecessary mutexes from benchmarks --- .../rdb/v1/custom_benchmark_test.go | 128 +++++------------- 1 file changed, 31 insertions(+), 97 deletions(-) diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 5ba8768449..df8c6c65b1 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -3,11 +3,12 @@ package rdb_test import ( "bytes" "context" + "errors" "fmt" + "net/http" "os" "sort" "strings" - "sync" "testing" "time" @@ -45,9 +46,7 @@ const ( ) var ( - sharedInstance *rdbSDK.Instance - sharedInstanceMu sync.Mutex - backupOpMu sync.Mutex + sharedInstance *rdbSDK.Instance ) // TestMain ensures shared instance cleanup @@ -72,15 +71,21 @@ func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn fu return } - // Check if it's a 409 conflict (resource in transient state) - errMsg := err.Error() - if strings.Contains(errMsg, "409") || strings.Contains(errMsg, "Conflict") || strings.Contains(errMsg, "transient state") { - if i < maxRetries-1 { - waitTime := time.Duration(2*(i+1)) * time.Second - b.Logf("cleanup conflict for %s=%s (attempt %d/%d), waiting %v: %v", name, resourceID, i+1, maxRetries, waitTime, err) - time.Sleep(waitTime) - continue - } + // Check if it's a 409 Conflict using typed error + var respErr *scw.ResponseError + isConflict := errors.As(err, &respErr) && respErr.StatusCode == http.StatusConflict + + // Fallback: check error message for transient state keywords + if !isConflict { + errMsg := err.Error() + isConflict = strings.Contains(errMsg, "transient state") || strings.Contains(errMsg, "backuping") + } + + if isConflict && i < maxRetries-1 { + waitTime := time.Duration(2*(i+1)) * time.Second + b.Logf("cleanup conflict for %s=%s (attempt %d/%d), waiting %v: %v", name, resourceID, i+1, maxRetries, waitTime, err) + time.Sleep(waitTime) + continue } b.Errorf("cleanup failure (%s=%s) after %d attempts: %v", name, resourceID, i+1, err) @@ -141,9 +146,6 @@ func (s *benchmarkStats) report(b *testing.B) { func getOrCreateSharedInstance(b *testing.B, client *scw.Client, executeCmd func([]string) any, meta core.TestMetadata) *rdbSDK.Instance { b.Helper() - sharedInstanceMu.Lock() - defer sharedInstanceMu.Unlock() - if sharedInstance != nil { b.Log("Reusing existing shared RDB instance") return sharedInstance @@ -174,9 +176,6 @@ func getOrCreateSharedInstance(b *testing.B, client *scw.Client, executeCmd func } func cleanupSharedInstance() { - sharedInstanceMu.Lock() - defer sharedInstanceMu.Unlock() - if sharedInstance == nil { return } @@ -261,22 +260,8 @@ func BenchmarkInstanceGet(b *testing.B) { for range b.N { start := time.Now() - - ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) - done := make(chan any, 1) - - go func() { - done <- executeCmd([]string{"scw", "rdb", "instance", "get", instance.ID}) - }() - - select { - case <-done: - stats.record(time.Since(start)) - case <-ctx.Done(): - cancel() - b.Fatalf("command timeout after %v", defaultCmdTimeout) - } - cancel() + executeCmd([]string{"scw", "rdb", "instance", "get", instance.ID}) + stats.record(time.Since(start)) } b.StopTimer() @@ -299,15 +284,11 @@ func BenchmarkBackupGet(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - backupOpMu.Lock() if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { - backupOpMu.Unlock() b.Fatalf("Instance not ready before backup: %v", err) } - err := createBackupDirect("Backup")(ctx) - backupOpMu.Unlock() - if err != nil { + if err := createBackupDirect("Backup")(ctx); err != nil { b.Fatalf("Failed to create backup: %v", err) } @@ -330,22 +311,8 @@ func BenchmarkBackupGet(b *testing.B) { for range b.N { start := time.Now() - - ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) - done := make(chan any, 1) - - go func() { - done <- executeCmd([]string{"scw", "rdb", "backup", "get", backup.ID}) - }() - - select { - case <-done: - stats.record(time.Since(start)) - case <-ctx.Done(): - cancel() - b.Fatalf("command timeout after %v", defaultCmdTimeout) - } - cancel() + executeCmd([]string{"scw", "rdb", "backup", "get", backup.ID}) + stats.record(time.Since(start)) } b.StopTimer() @@ -368,24 +335,19 @@ func BenchmarkBackupList(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - backupOpMu.Lock() if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { - backupOpMu.Unlock() b.Fatalf("Instance not ready before backup 1: %v", err) } - err := createBackupDirect("Backup1")(ctx) - if err != nil { - backupOpMu.Unlock() + + if err := createBackupDirect("Backup1")(ctx); err != nil { b.Fatalf("Failed to create backup 1: %v", err) } + if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { - backupOpMu.Unlock() b.Fatalf("Instance not ready before backup 2: %v", err) } - err = createBackupDirect("Backup2")(ctx) - backupOpMu.Unlock() - if err != nil { + if err := createBackupDirect("Backup2")(ctx); err != nil { b.Fatalf("Failed to create backup 2: %v", err) } @@ -412,22 +374,8 @@ func BenchmarkBackupList(b *testing.B) { for range b.N { start := time.Now() - - ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) - done := make(chan any, 1) - - go func() { - done <- executeCmd([]string{"scw", "rdb", "backup", "list", "instance-id=" + instance.ID}) - }() - - select { - case <-done: - stats.record(time.Since(start)) - case <-ctx.Done(): - cancel() - b.Fatalf("command timeout after %v", defaultCmdTimeout) - } - cancel() + executeCmd([]string{"scw", "rdb", "backup", "list", "instance-id=" + instance.ID}) + stats.record(time.Since(start)) } b.StopTimer() @@ -448,22 +396,8 @@ func BenchmarkDatabaseList(b *testing.B) { for range b.N { start := time.Now() - - ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout) - done := make(chan any, 1) - - go func() { - done <- executeCmd([]string{"scw", "rdb", "database", "list", "instance-id=" + instance.ID}) - }() - - select { - case <-done: - stats.record(time.Since(start)) - case <-ctx.Done(): - cancel() - b.Fatalf("command timeout after %v", defaultCmdTimeout) - } - cancel() + executeCmd([]string{"scw", "rdb", "database", "list", "instance-id=" + instance.ID}) + stats.record(time.Since(start)) } b.StopTimer() From 1924639df9f8802d29968187ab54db30acceb141 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Mon, 24 Nov 2025 06:25:39 +0100 Subject: [PATCH 13/20] feat(benchstat): implement --update flag to regenerate baselines --- cmd/scw-benchstat/main.go | 50 ++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 5a7cb79d3e..8b48953f2b 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -19,21 +19,22 @@ const ( ) type config struct { - bench string - benchtime string - count int - benchmem bool - failMetrics []string - threshold float64 - installTool bool - targetDirs []string - verbose bool + bench string + benchtime string + count int + benchmem bool + failMetrics []string + threshold float64 + installTool bool + targetDirs []string + verbose bool + update bool } type benchResult struct { - name string - timePerOp float64 - bytesPerOp int64 + name string + timePerOp float64 + bytesPerOp int64 allocsPerOp int64 } @@ -58,12 +59,17 @@ func main() { log.Fatal("no benchmark directories found") } + var hadError bool for _, dir := range cfg.targetDirs { if err := runBenchmarksForDir(cfg, dir); err != nil { - log.Printf("failed to run benchmarks for %s: %v", dir, err) - os.Exit(1) + log.Printf("❌ failed to run benchmarks for %s: %v", dir, err) + hadError = true } } + + if hadError { + os.Exit(1) + } } func parseFlags() config { @@ -76,9 +82,10 @@ func parseFlags() config { flag.Float64Var(&cfg.threshold, "threshold", 1.5, "performance regression threshold (e.g., 1.5 = 50% slower)") flag.BoolVar(&cfg.installTool, "install-benchstat", false, "install benchstat tool if not found") flag.BoolVar(&cfg.verbose, "verbose", false, "verbose output") + flag.BoolVar(&cfg.update, "update", false, "update baseline files instead of comparing") var failMetricsStr string - flag.StringVar(&failMetricsStr, "fail-metrics", "", "comma-separated list of metrics to check for regressions (time/op,B/op,allocs/op)") + flag.StringVar(&failMetricsStr, "fail-metrics", "", "comma-separated list of metrics to check for regressions (default: time/op)") var targetDirsStr string flag.StringVar(&targetDirsStr, "target-dirs", "", "comma-separated list of directories to benchmark") @@ -87,6 +94,8 @@ func parseFlags() config { if failMetricsStr != "" { cfg.failMetrics = strings.Split(failMetricsStr, ",") + } else { + cfg.failMetrics = []string{"time/op"} } if targetDirsStr != "" { @@ -147,6 +156,15 @@ func runBenchmarksForDir(cfg config, dir string) error { return fmt.Errorf("failed to run benchmarks: %w", err) } + // Update mode: always overwrite baseline + if cfg.update { + if err := saveBaseline(baselineFile, newResults); err != nil { + return fmt.Errorf("failed to update baseline: %w", err) + } + fmt.Printf("✅ Baseline updated: %s\n", baselineFile) + return nil + } + // Check if baseline exists if _, err := os.Stat(baselineFile); os.IsNotExist(err) { fmt.Printf("No baseline found at %s. Creating new baseline.\n", baselineFile) @@ -163,7 +181,7 @@ func runBenchmarksForDir(cfg config, dir string) error { func runBenchmarks(cfg config, dir string) (string, error) { args := []string{"test", "-bench=" + cfg.bench, "-benchtime=" + cfg.benchtime, "-count=" + strconv.Itoa(cfg.count)} - + if cfg.benchmem { args = append(args, "-benchmem") } From 80e7f4a5a62806c60226cddb31c83bf129fb9856 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Mon, 24 Nov 2025 06:25:47 +0100 Subject: [PATCH 14/20] feat(env): add BenchCmdTimeout constant for configurable benchmark timeout --- internal/env/env.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/env/env.go b/internal/env/env.go index fca4e69cf4..ccb8032aa1 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -8,6 +8,9 @@ const ( // BenchTrace if set to "true" will enable detailed benchmark statistics (min, median, mean, p95, max) BenchTrace = "CLI_BENCH_TRACE" + // BenchCmdTimeout sets the command timeout for benchmarks (e.g., "30s", "1m"). Default: 30s + BenchCmdTimeout = "CLI_BENCH_CMD_TIMEOUT" + // UpdateCassettes if set to "true" will trigger the cassettes to be recorded UpdateCassettes = "CLI_UPDATE_CASSETTES" From 944757cfea6afd405f2e0487706f8e3fbfc5f585 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Mon, 24 Nov 2025 07:16:56 +0100 Subject: [PATCH 15/20] fix(benchstat): use CombinedOutput for better error reporting --- cmd/scw-benchstat/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 8b48953f2b..520ab977f1 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -231,9 +231,9 @@ func compareWithBaseline(cfg config, baselineFile, newResults string) error { // Run benchstat comparison cmd := exec.Command("benchstat", "-format=csv", baselineFile, tmpFile.Name()) - output, err := cmd.Output() + output, err := cmd.CombinedOutput() if err != nil { - return fmt.Errorf("failed to compare with benchstat for %s: %w", filepath.Dir(baselineFile), err) + return fmt.Errorf("failed to compare with benchstat for %s: %w\nOutput: %s", filepath.Dir(baselineFile), err, output) } // Parse CSV output and check for regressions From 316963938fe364f12901d459aa5567b3e947675a Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Mon, 24 Nov 2025 11:38:19 +0100 Subject: [PATCH 16/20] fix(lint): resolve all golangci-lint issues --- cmd/scw-benchstat/main.go | 121 ++++++++++++------ .../rdb/v1/custom_benchmark_test.go | 46 ++++--- internal/namespaces/rdb/v1/helper_test.go | 3 +- internal/testhelpers/helpers_benchmark.go | 5 +- 4 files changed, 119 insertions(+), 56 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 520ab977f1..3f9e8272b5 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/csv" + "errors" "flag" "fmt" "log" @@ -31,13 +32,6 @@ type config struct { update bool } -type benchResult struct { - name string - timePerOp float64 - bytesPerOp int64 - allocsPerOp int64 -} - func main() { cfg := parseFlags() @@ -48,7 +42,10 @@ func main() { } if !isBenchstatAvailable() { - log.Fatalf("benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat@%s in your environment or run with --install-benchstat", benchstatVersion) + log.Fatalf( + "benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat@%s in your environment or run with --install-benchstat", + benchstatVersion, + ) } if len(cfg.targetDirs) == 0 { @@ -79,16 +76,36 @@ func parseFlags() config { flag.StringVar(&cfg.benchtime, "benchtime", "1s", "benchmark time") flag.IntVar(&cfg.count, "count", 5, "number of benchmark runs") flag.BoolVar(&cfg.benchmem, "benchmem", false, "include memory allocation stats") - flag.Float64Var(&cfg.threshold, "threshold", 1.5, "performance regression threshold (e.g., 1.5 = 50% slower)") - flag.BoolVar(&cfg.installTool, "install-benchstat", false, "install benchstat tool if not found") + flag.Float64Var( + &cfg.threshold, + "threshold", + 1.5, + "performance regression threshold (e.g., 1.5 = 50% slower)", + ) + flag.BoolVar( + &cfg.installTool, + "install-benchstat", + false, + "install benchstat tool if not found", + ) flag.BoolVar(&cfg.verbose, "verbose", false, "verbose output") flag.BoolVar(&cfg.update, "update", false, "update baseline files instead of comparing") var failMetricsStr string - flag.StringVar(&failMetricsStr, "fail-metrics", "", "comma-separated list of metrics to check for regressions (default: time/op)") + flag.StringVar( + &failMetricsStr, + "fail-metrics", + "", + "comma-separated list of metrics to check for regressions (default: time/op)", + ) var targetDirsStr string - flag.StringVar(&targetDirsStr, "target-dirs", "", "comma-separated list of directories to benchmark") + flag.StringVar( + &targetDirsStr, + "target-dirs", + "", + "comma-separated list of directories to benchmark", + ) flag.Parse() @@ -107,37 +124,41 @@ func parseFlags() config { func installBenchstat() error { fmt.Printf("Installing benchstat@%s...\n", benchstatVersion) - cmd := exec.Command("go", "install", fmt.Sprintf("golang.org/x/perf/cmd/benchstat@%s", benchstatVersion)) + cmd := exec.Command("go", "install", "golang.org/x/perf/cmd/benchstat@"+benchstatVersion) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr + return cmd.Run() } func isBenchstatAvailable() bool { _, err := exec.LookPath("benchstat") + return err == nil } func findBenchmarkDirs() []string { var dirs []string - err := filepath.WalkDir("internal/namespaces", func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - if d.IsDir() { - return nil - } + err := filepath.WalkDir( + "internal/namespaces", + func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } - if strings.HasSuffix(d.Name(), "_benchmark_test.go") { - dir := filepath.Dir(path) - dirs = append(dirs, dir) - } + if d.IsDir() { + return nil + } - return nil - }) + if strings.HasSuffix(d.Name(), "_benchmark_test.go") { + dir := filepath.Dir(path) + dirs = append(dirs, dir) + } + return nil + }, + ) if err != nil { log.Printf("error scanning for benchmark directories: %v", err) } @@ -162,6 +183,7 @@ func runBenchmarksForDir(cfg config, dir string) error { return fmt.Errorf("failed to update baseline: %w", err) } fmt.Printf("✅ Baseline updated: %s\n", baselineFile) + return nil } @@ -172,6 +194,7 @@ func runBenchmarksForDir(cfg config, dir string) error { return fmt.Errorf("failed to save baseline: %w", err) } fmt.Printf("Baseline saved to %s\n", baselineFile) + return nil } @@ -180,7 +203,12 @@ func runBenchmarksForDir(cfg config, dir string) error { } func runBenchmarks(cfg config, dir string) (string, error) { - args := []string{"test", "-bench=" + cfg.bench, "-benchtime=" + cfg.benchtime, "-count=" + strconv.Itoa(cfg.count)} + args := []string{ + "test", + "-bench=" + cfg.bench, + "-benchtime=" + cfg.benchtime, + "-count=" + strconv.Itoa(cfg.count), + } if cfg.benchmem { args = append(args, "-benchmem") @@ -208,11 +236,11 @@ func runBenchmarks(cfg config, dir string) (string, error) { func saveBaseline(filename, content string) error { dir := filepath.Dir(filename) - if err := os.MkdirAll(dir, 0755); err != nil { + if err := os.MkdirAll(dir, 0o755); err != nil { return err } - return os.WriteFile(filename, []byte(content), 0644) + return os.WriteFile(filename, []byte(content), 0o644) } func compareWithBaseline(cfg config, baselineFile, newResults string) error { @@ -233,7 +261,12 @@ func compareWithBaseline(cfg config, baselineFile, newResults string) error { cmd := exec.Command("benchstat", "-format=csv", baselineFile, tmpFile.Name()) output, err := cmd.CombinedOutput() if err != nil { - return fmt.Errorf("failed to compare with benchstat for %s: %w\nOutput: %s", filepath.Dir(baselineFile), err, output) + return fmt.Errorf( + "failed to compare with benchstat for %s: %w\nOutput: %s", + filepath.Dir(baselineFile), + err, + output, + ) } // Parse CSV output and check for regressions @@ -249,6 +282,7 @@ func checkForRegressions(cfg config, csvOutput string) error { if len(records) < 2 { fmt.Println("No benchmark comparisons found") + return nil } @@ -263,7 +297,7 @@ func checkForRegressions(cfg config, csvOutput string) error { newAllocsIdx := findColumnIndex(header, "new allocs/op") if nameIdx == -1 { - return fmt.Errorf("could not find 'name' column in benchstat output") + return errors.New("could not find 'name' column in benchstat output") } var regressions []string @@ -278,7 +312,10 @@ func checkForRegressions(cfg config, csvOutput string) error { // Check time/op regression if contains(cfg.failMetrics, "time/op") && oldTimeIdx != -1 && newTimeIdx != -1 { if regression := checkMetricRegression(record, oldTimeIdx, newTimeIdx, cfg.threshold); regression != "" { - regressions = append(regressions, fmt.Sprintf("%s: time/op %s", benchName, regression)) + regressions = append( + regressions, + fmt.Sprintf("%s: time/op %s", benchName, regression), + ) } } @@ -292,7 +329,10 @@ func checkForRegressions(cfg config, csvOutput string) error { // Check allocs/op regression if contains(cfg.failMetrics, "allocs/op") && oldAllocsIdx != -1 && newAllocsIdx != -1 { if regression := checkMetricRegression(record, oldAllocsIdx, newAllocsIdx, cfg.threshold); regression != "" { - regressions = append(regressions, fmt.Sprintf("%s: allocs/op %s", benchName, regression)) + regressions = append( + regressions, + fmt.Sprintf("%s: allocs/op %s", benchName, regression), + ) } } @@ -310,10 +350,15 @@ func checkForRegressions(cfg config, csvOutput string) error { for _, regression := range regressions { fmt.Printf(" - %s\n", regression) } - return fmt.Errorf("performance regressions detected") + + return errors.New("performance regressions detected") } - fmt.Printf("✅ No significant performance regressions detected (threshold: %.1fx)\n", cfg.threshold) + fmt.Printf( + "✅ No significant performance regressions detected (threshold: %.1fx)\n", + cfg.threshold, + ) + return nil } @@ -323,6 +368,7 @@ func findColumnIndex(header []string, columnName string) int { return i } } + return -1 } @@ -354,7 +400,7 @@ func parseMetricValue(s string) (float64, error) { s = strings.TrimSpace(s) if s == "" || s == "-" { - return 0, fmt.Errorf("empty value") + return 0, errors.New("empty value") } return strconv.ParseFloat(s, 64) @@ -366,5 +412,6 @@ func contains(slice []string, item string) bool { return true } } + return false } diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index df8c6c65b1..ea6da184b7 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -40,14 +40,7 @@ import ( // // go run ./cmd/scw-benchstat --install-benchstat --bench=. --count=10 -const ( - defaultCmdTimeout = 30 * time.Second - instanceReadyTimeout = 3 * time.Minute -) - -var ( - sharedInstance *rdbSDK.Instance -) +var sharedInstance *rdbSDK.Instance // TestMain ensures shared instance cleanup func TestMain(m *testing.M) { @@ -58,6 +51,7 @@ func TestMain(m *testing.M) { func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) { b.Helper() + return testhelpers.SetupBenchmark(b, rdb.GetCommands()) } @@ -65,7 +59,7 @@ func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn fu b.Helper() maxRetries := 5 - for i := 0; i < maxRetries; i++ { + for i := range maxRetries { err := cleanupFn() if err == nil { return @@ -78,17 +72,28 @@ func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn fu // Fallback: check error message for transient state keywords if !isConflict { errMsg := err.Error() - isConflict = strings.Contains(errMsg, "transient state") || strings.Contains(errMsg, "backuping") + isConflict = strings.Contains(errMsg, "transient state") || + strings.Contains(errMsg, "backuping") } if isConflict && i < maxRetries-1 { waitTime := time.Duration(2*(i+1)) * time.Second - b.Logf("cleanup conflict for %s=%s (attempt %d/%d), waiting %v: %v", name, resourceID, i+1, maxRetries, waitTime, err) + b.Logf( + "cleanup conflict for %s=%s (attempt %d/%d), waiting %v: %v", + name, + resourceID, + i+1, + maxRetries, + waitTime, + err, + ) time.Sleep(waitTime) + continue } b.Errorf("cleanup failure (%s=%s) after %d attempts: %v", name, resourceID, i+1, err) + return } } @@ -143,11 +148,17 @@ func (s *benchmarkStats) report(b *testing.B) { len(s.timings), minVal, median, mean, p95, maxVal) } -func getOrCreateSharedInstance(b *testing.B, client *scw.Client, executeCmd func([]string) any, meta core.TestMetadata) *rdbSDK.Instance { +func getOrCreateSharedInstance( + b *testing.B, + client *scw.Client, + executeCmd func([]string) any, + meta core.TestMetadata, +) *rdbSDK.Instance { b.Helper() if sharedInstance != nil { b.Log("Reusing existing shared RDB instance") + return sharedInstance } @@ -167,11 +178,12 @@ func getOrCreateSharedInstance(b *testing.B, client *scw.Client, executeCmd func b.Logf("Shared RDB instance created: %s", instance.ID) - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { b.Fatalf("Shared instance not ready: %v", err) } b.Log("Shared instance is ready") + return sharedInstance } @@ -189,6 +201,7 @@ func cleanupSharedInstance() { ) if err != nil { fmt.Printf("Error creating client for cleanup: %v\n", err) + return } @@ -222,6 +235,7 @@ func cleanupSharedInstance() { OverrideEnv: map[string]string{}, Ctx: context.Background(), }) + return result } @@ -284,7 +298,7 @@ func BenchmarkBackupGet(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { b.Fatalf("Instance not ready before backup: %v", err) } @@ -335,7 +349,7 @@ func BenchmarkBackupList(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { b.Fatalf("Instance not ready before backup 1: %v", err) } @@ -343,7 +357,7 @@ func BenchmarkBackupList(b *testing.B) { b.Fatalf("Failed to create backup 1: %v", err) } - if err := waitForInstanceReady(executeCmd, instance.ID, instanceReadyTimeout); err != nil { + if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { b.Fatalf("Instance not ready before backup 2: %v", err) } diff --git a/internal/namespaces/rdb/v1/helper_test.go b/internal/namespaces/rdb/v1/helper_test.go index 6296726557..b7801fc64b 100644 --- a/internal/namespaces/rdb/v1/helper_test.go +++ b/internal/namespaces/rdb/v1/helper_test.go @@ -180,9 +180,8 @@ func deleteInstanceDirect() core.AfterFunc { func waitForInstanceReady( executeCmd func([]string) any, instanceID string, - maxWait time.Duration, ) error { - ctx, cancel := context.WithTimeout(context.Background(), maxWait) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() backoff := time.Second diff --git a/internal/testhelpers/helpers_benchmark.go b/internal/testhelpers/helpers_benchmark.go index fe5e4eb7fa..084f29c275 100644 --- a/internal/testhelpers/helpers_benchmark.go +++ b/internal/testhelpers/helpers_benchmark.go @@ -11,7 +11,10 @@ import ( // SetupBenchmark initializes a Scaleway client and test metadata for benchmarks. // It loads credentials from the active profile and environment variables. -func SetupBenchmark(b *testing.B, commands *core.Commands) (*scw.Client, core.TestMetadata, func(args []string) any) { +func SetupBenchmark( + b *testing.B, + commands *core.Commands, +) (*scw.Client, core.TestMetadata, func(args []string) any) { b.Helper() clientOpts := []scw.ClientOption{ From e2c5e28ad7d8f85e72b2524b2c87b6c2dc5bc037 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Fri, 3 Apr 2026 14:39:49 +0200 Subject: [PATCH 17/20] fix(lint): address golines and modernize on benchstat and RDB benchmarks. --- cmd/scw-benchstat/main.go | 26 +++++++------------ .../rdb/v1/custom_benchmark_test.go | 6 ++--- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 3f9e8272b5..63427d9a7d 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strconv" "strings" "time" @@ -310,8 +311,9 @@ func checkForRegressions(cfg config, csvOutput string) error { benchName := record[nameIdx] // Check time/op regression - if contains(cfg.failMetrics, "time/op") && oldTimeIdx != -1 && newTimeIdx != -1 { - if regression := checkMetricRegression(record, oldTimeIdx, newTimeIdx, cfg.threshold); regression != "" { + if slices.Contains(cfg.failMetrics, "time/op") && oldTimeIdx != -1 && newTimeIdx != -1 { + regression := checkMetricRegression(record, oldTimeIdx, newTimeIdx, cfg.threshold) + if regression != "" { regressions = append( regressions, fmt.Sprintf("%s: time/op %s", benchName, regression), @@ -320,15 +322,17 @@ func checkForRegressions(cfg config, csvOutput string) error { } // Check B/op regression - if contains(cfg.failMetrics, "B/op") && oldBytesIdx != -1 && newBytesIdx != -1 { - if regression := checkMetricRegression(record, oldBytesIdx, newBytesIdx, cfg.threshold); regression != "" { + if slices.Contains(cfg.failMetrics, "B/op") && oldBytesIdx != -1 && newBytesIdx != -1 { + regression := checkMetricRegression(record, oldBytesIdx, newBytesIdx, cfg.threshold) + if regression != "" { regressions = append(regressions, fmt.Sprintf("%s: B/op %s", benchName, regression)) } } // Check allocs/op regression - if contains(cfg.failMetrics, "allocs/op") && oldAllocsIdx != -1 && newAllocsIdx != -1 { - if regression := checkMetricRegression(record, oldAllocsIdx, newAllocsIdx, cfg.threshold); regression != "" { + if slices.Contains(cfg.failMetrics, "allocs/op") && oldAllocsIdx != -1 && newAllocsIdx != -1 { + regression := checkMetricRegression(record, oldAllocsIdx, newAllocsIdx, cfg.threshold) + if regression != "" { regressions = append( regressions, fmt.Sprintf("%s: allocs/op %s", benchName, regression), @@ -405,13 +409,3 @@ func parseMetricValue(s string) (float64, error) { return strconv.ParseFloat(s, 64) } - -func contains(slice []string, item string) bool { - for _, s := range slice { - if s == item { - return true - } - } - - return false -} diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index ea6da184b7..c2982d2949 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -7,7 +7,7 @@ import ( "fmt" "net/http" "os" - "sort" + "slices" "strings" "testing" "time" @@ -134,9 +134,7 @@ func (s *benchmarkStats) report(b *testing.B) { return } - sort.Slice(s.timings, func(i, j int) bool { - return s.timings[i] < s.timings[j] - }) + slices.Sort(s.timings) minVal := s.timings[0] maxVal := s.timings[len(s.timings)-1] From 44846137b1d9383fc08bb9e1d6df3870efc04c73 Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Tue, 7 Apr 2026 06:07:49 +0200 Subject: [PATCH 18/20] chore(rdb): document benchstat setup, inline testhelpers.SetupBenchmark, and fix golines in scw-benchstat. --- cmd/scw-benchstat/main.go | 3 +- docs/BENCHMARKS.md | 9 ++++-- .../rdb/v1/custom_benchmark_test.go | 29 +++++++------------ 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 63427d9a7d..3a4f94b618 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -330,7 +330,8 @@ func checkForRegressions(cfg config, csvOutput string) error { } // Check allocs/op regression - if slices.Contains(cfg.failMetrics, "allocs/op") && oldAllocsIdx != -1 && newAllocsIdx != -1 { + if slices.Contains(cfg.failMetrics, "allocs/op") && + oldAllocsIdx != -1 && newAllocsIdx != -1 { regression := checkMetricRegression(record, oldAllocsIdx, newAllocsIdx, cfg.threshold) if regression != "" { regressions = append( diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 5e9a7a4c07..d9efd2b217 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -209,12 +209,15 @@ The tool fails if any metric exceeds the threshold: Create a file named `custom_benchmark_test.go` in your namespace: ```go -package mynamespace_test +package rdb_test import ( "os" "testing" "time" + + "github.com/scaleway/scaleway-cli/v2/internal/testhelpers" + "github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1" ) func BenchmarkMyCommand(b *testing.B) { @@ -223,8 +226,8 @@ func BenchmarkMyCommand(b *testing.B) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } - // Setup: create resources, clients, etc. - client, meta, executeCmd := setupBenchmark(b) + // Setup: client + executeCmd from shared helper (see internal/testhelpers/helpers_benchmark.go) + client, meta, executeCmd := testhelpers.SetupBenchmark(b, rdb.GetCommands()) // Reset timer to exclude setup time b.ResetTimer() diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index c2982d2949..809f11cc39 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -22,21 +22,20 @@ import ( // Benchmarks for RDB commands. // -// Baseline stored in testdata/benchmark.baseline (like golden files). +// Baseline: testdata/benchmark.baseline (committed like golden files). Full workflow: docs/BENCHMARKS.md. // -// Install benchstat (required for comparison): +// CLI setup uses testhelpers.SetupBenchmark(b, rdb.GetCommands()) — same pattern as other namespace benchmarks. // -// go install golang.org/x/perf/cmd/benchstat@latest +// Comparing locally with upstream benchstat (install once, then compare): // -// To compare performance: +// 1. Install: go install golang.org/x/perf/cmd/benchstat@latest +// 2. Compare: benchstat testdata/benchmark.baseline <(CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x .) // -// benchstat testdata/benchmark.baseline <(CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x .) -// -// To update baseline: +// Update baseline: // // CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x . > testdata/benchmark.baseline // -// Or use the automated tool (installs benchstat automatically): +// Or use scw-benchstat (can install benchstat for you: --install-benchstat): // // go run ./cmd/scw-benchstat --install-benchstat --bench=. --count=10 @@ -49,12 +48,6 @@ func TestMain(m *testing.M) { os.Exit(code) } -func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) { - b.Helper() - - return testhelpers.SetupBenchmark(b, rdb.GetCommands()) -} - func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn func() error) { b.Helper() @@ -263,7 +256,7 @@ func BenchmarkInstanceGet(b *testing.B) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } - client, meta, executeCmd := setupBenchmark(b) + client, meta, executeCmd := testhelpers.SetupBenchmark(b, rdb.GetCommands()) instance := getOrCreateSharedInstance(b, client, executeCmd, meta) stats := newBenchmarkStats() @@ -285,7 +278,7 @@ func BenchmarkBackupGet(b *testing.B) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } - client, meta, executeCmd := setupBenchmark(b) + client, meta, executeCmd := testhelpers.SetupBenchmark(b, rdb.GetCommands()) instance := getOrCreateSharedInstance(b, client, executeCmd, meta) ctx := &core.BeforeFuncCtx{ @@ -336,7 +329,7 @@ func BenchmarkBackupList(b *testing.B) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } - client, meta, executeCmd := setupBenchmark(b) + client, meta, executeCmd := testhelpers.SetupBenchmark(b, rdb.GetCommands()) instance := getOrCreateSharedInstance(b, client, executeCmd, meta) ctx := &core.BeforeFuncCtx{ @@ -399,7 +392,7 @@ func BenchmarkDatabaseList(b *testing.B) { b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.") } - client, meta, executeCmd := setupBenchmark(b) + client, meta, executeCmd := testhelpers.SetupBenchmark(b, rdb.GetCommands()) instance := getOrCreateSharedInstance(b, client, executeCmd, meta) stats := newBenchmarkStats() From 59bdaed69f6c5a127cc0c30d531cfc04fb0151ff Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Tue, 7 Apr 2026 15:40:45 +0200 Subject: [PATCH 19/20] refactor(rdb): remove benchstat auto-install and rely on SDK waiter for instance readiness. --- cmd/scw-benchstat/main.go | 31 +---------- docs/BENCHMARKS.md | 6 -- .../rdb/v1/custom_benchmark_test.go | 12 ++-- internal/namespaces/rdb/v1/helper_test.go | 55 +++++-------------- 4 files changed, 20 insertions(+), 84 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 3a4f94b618..800041db24 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -16,10 +16,6 @@ import ( "time" ) -const ( - benchstatVersion = "v0.0.0-20251023143056-3684bd442cc8" -) - type config struct { bench string benchtime string @@ -27,7 +23,6 @@ type config struct { benchmem bool failMetrics []string threshold float64 - installTool bool targetDirs []string verbose bool update bool @@ -36,17 +31,8 @@ type config struct { func main() { cfg := parseFlags() - if cfg.installTool { - if err := installBenchstat(); err != nil { - log.Fatalf("failed to install benchstat: %v", err) - } - } - if !isBenchstatAvailable() { - log.Fatalf( - "benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat@%s in your environment or run with --install-benchstat", - benchstatVersion, - ) + log.Fatal("benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat in your environment") } if len(cfg.targetDirs) == 0 { @@ -83,12 +69,6 @@ func parseFlags() config { 1.5, "performance regression threshold (e.g., 1.5 = 50% slower)", ) - flag.BoolVar( - &cfg.installTool, - "install-benchstat", - false, - "install benchstat tool if not found", - ) flag.BoolVar(&cfg.verbose, "verbose", false, "verbose output") flag.BoolVar(&cfg.update, "update", false, "update baseline files instead of comparing") @@ -123,15 +103,6 @@ func parseFlags() config { return cfg } -func installBenchstat() error { - fmt.Printf("Installing benchstat@%s...\n", benchstatVersion) - cmd := exec.Command("go", "install", "golang.org/x/perf/cmd/benchstat@"+benchstatVersion) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - return cmd.Run() -} - func isBenchstatAvailable() bool { _, err := exec.LookPath("benchstat") diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index d9efd2b217..cb3bb98b5a 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -112,7 +112,6 @@ go run ./cmd/scw-benchstat [OPTIONS] | `--benchmem` | `false` | Measure memory allocations | | `--fail-metrics` | - | Comma-separated metrics to check: `time/op`, `B/op`, `allocs/op` | | `--threshold` | `1.5` | Regression threshold (1.5 = fail if 50% slower) | -| `--install-benchstat` | `false` | Auto-install benchstat if not found | | `--target-dirs` | (all) | Comma-separated directories to benchmark | | `--verbose` | `false` | Enable verbose output | @@ -373,11 +372,6 @@ Install benchstat: go install golang.org/x/perf/cmd/benchstat@latest ``` -Or use auto-install: -```bash -go run ./cmd/scw-benchstat --install-benchstat -``` - ### "signal: killed" The benchmark process was killed (timeout or OOM). Try: diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go index 809f11cc39..511e15326d 100644 --- a/internal/namespaces/rdb/v1/custom_benchmark_test.go +++ b/internal/namespaces/rdb/v1/custom_benchmark_test.go @@ -35,9 +35,9 @@ import ( // // CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x . > testdata/benchmark.baseline // -// Or use scw-benchstat (can install benchstat for you: --install-benchstat): +// Or use scw-benchstat: // -// go run ./cmd/scw-benchstat --install-benchstat --bench=. --count=10 +// go run ./cmd/scw-benchstat --bench=. --count=10 var sharedInstance *rdbSDK.Instance @@ -169,7 +169,7 @@ func getOrCreateSharedInstance( b.Logf("Shared RDB instance created: %s", instance.ID) - if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { + if err := waitForInstanceReady(client, instance.ID); err != nil { b.Fatalf("Shared instance not ready: %v", err) } @@ -289,7 +289,7 @@ func BenchmarkBackupGet(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { + if err := waitForInstanceReady(client, instance.ID); err != nil { b.Fatalf("Instance not ready before backup: %v", err) } @@ -340,7 +340,7 @@ func BenchmarkBackupList(b *testing.B) { meta["Instance"] = rdb.CreateInstanceResult{Instance: instance} - if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { + if err := waitForInstanceReady(client, instance.ID); err != nil { b.Fatalf("Instance not ready before backup 1: %v", err) } @@ -348,7 +348,7 @@ func BenchmarkBackupList(b *testing.B) { b.Fatalf("Failed to create backup 1: %v", err) } - if err := waitForInstanceReady(executeCmd, instance.ID); err != nil { + if err := waitForInstanceReady(client, instance.ID); err != nil { b.Fatalf("Instance not ready before backup 2: %v", err) } diff --git a/internal/namespaces/rdb/v1/helper_test.go b/internal/namespaces/rdb/v1/helper_test.go index 360090801b..f2bfb9521b 100644 --- a/internal/namespaces/rdb/v1/helper_test.go +++ b/internal/namespaces/rdb/v1/helper_test.go @@ -1,7 +1,6 @@ package rdb_test import ( - "context" "errors" "fmt" "time" @@ -176,47 +175,19 @@ func deleteInstanceDirect() core.AfterFunc { } } -func waitForInstanceReady( - executeCmd func([]string) any, - instanceID string, -) error { - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) - defer cancel() - - backoff := time.Second - for { - select { - case <-ctx.Done(): - return fmt.Errorf( - "timeout waiting for instance %s to be ready for operations", - instanceID, - ) - default: - result := executeCmd([]string{"scw", "rdb", "instance", "get", instanceID}) - - // Try direct type assertion first - if instance, ok := result.(*rdbSDK.Instance); ok { - if instance.Status == rdbSDK.InstanceStatusReady { - time.Sleep(5 * time.Second) - - return nil - } - } else { - v := result.(struct { - *rdbSDK.Instance - ACLs []*rdbSDK.ACLRule `json:"acls"` - }) - if v.Instance != nil && v.Instance.Status == rdbSDK.InstanceStatusReady { - time.Sleep(5 * time.Second) - - return nil - } - } +func waitForInstanceReady(client *scw.Client, instanceID string) error { + api := rdbSDK.NewAPI(client) - time.Sleep(backoff) - if backoff < 10*time.Second { - backoff *= 2 - } - } + _, err := api.WaitForInstance(&rdbSDK.WaitForInstanceRequest{ + InstanceID: instanceID, + Timeout: scw.TimeDurationPtr(3 * time.Minute), + }) + if err != nil { + return fmt.Errorf("timeout waiting for instance %s to be ready for operations: %w", instanceID, err) } + + // Give control plane a short settling window before dependent operations. + time.Sleep(5 * time.Second) + + return nil } From 167b43581fc04c925b99a12a29b364376941ccbc Mon Sep 17 00:00:00 2001 From: Jonathan Remy Date: Wed, 8 Apr 2026 15:34:08 +0200 Subject: [PATCH 20/20] fix(lint): wrap long lines flagged by golines in benchstat and rdb helper tests. --- cmd/scw-benchstat/main.go | 4 +++- internal/namespaces/rdb/v1/helper_test.go | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/scw-benchstat/main.go b/cmd/scw-benchstat/main.go index 800041db24..3184f8cf77 100644 --- a/cmd/scw-benchstat/main.go +++ b/cmd/scw-benchstat/main.go @@ -32,7 +32,9 @@ func main() { cfg := parseFlags() if !isBenchstatAvailable() { - log.Fatal("benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat in your environment") + log.Fatal( + "benchstat not found in PATH; install golang.org/x/perf/cmd/benchstat in your environment", + ) } if len(cfg.targetDirs) == 0 { diff --git a/internal/namespaces/rdb/v1/helper_test.go b/internal/namespaces/rdb/v1/helper_test.go index f2bfb9521b..c40c38b6c3 100644 --- a/internal/namespaces/rdb/v1/helper_test.go +++ b/internal/namespaces/rdb/v1/helper_test.go @@ -183,7 +183,11 @@ func waitForInstanceReady(client *scw.Client, instanceID string) error { Timeout: scw.TimeDurationPtr(3 * time.Minute), }) if err != nil { - return fmt.Errorf("timeout waiting for instance %s to be ready for operations: %w", instanceID, err) + return fmt.Errorf( + "timeout waiting for instance %s to be ready for operations: %w", + instanceID, + err, + ) } // Give control plane a short settling window before dependent operations.