roachprod-microbench: post GitHub issues for performance regressions

build/teamcity/cockroach/nightlies/microbenchmark_weekly.sh

@@ -207,6 +207,7 @@ if [ -d "$output_dir/experiment" ] && [ "$(ls -A "$output_dir/experiment")" ] \
     --sheet-desc="$sheet_description" \
     ${influx_token:+--influx-token="$influx_token"} \
     ${influx_host:+--influx-host="$influx_host"} \
+    ${TRIGGERED_BUILD:+--post-issues} \
     2>&1 | tee "$output_dir/sheets.txt"
 else
   echo "No microbenchmarks were run. Skipping comparison."

pkg/cmd/roachprod-microbench/compare.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod-microbench/google"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod-microbench/model"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod-microbench/util"
+	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/oserror"
@@ -37,6 +38,7 @@ type compareConfig struct {
 	baselineDir   string
 	sheetDesc     string
 	threshold     float64
+	postIssues    bool
 }
 
 type slackConfig struct {
@@ -349,6 +351,53 @@ func (c *compare) compareUsingThreshold(comparisonResultsMap model.ComparisonRes
 	return nil
 }
 
+func (c *compare) postRegressionIssues(
+	links map[string]string, comparisonResultsMap model.ComparisonResultsMap,
+) error {
+	loggerCfg := logger.Config{Stdout: os.Stdout, Stderr: os.Stderr}
+	l, err := loggerCfg.NewLogger("")
+	if err != nil {
+		return errors.Wrap(err, "failed to create logger for posting issues")
+	}
+
+	for pkgName, comparisonResults := range comparisonResultsMap {
+		var regressions []regressionInfo
+
+		for _, result := range comparisonResults {
+			for _, detail := range result.Comparisons {
+				comparison := detail.Comparison
+				metric := result.Metric
+
+				// Check if this is a regression
+				isRegression := (comparison.Delta < 0 && metric.Better > 0) ||
+					(comparison.Delta > 0 && metric.Better < 0)
+
+				if isRegression && math.Abs(comparison.Delta) >= slackPercentageThreshold {
+					regressions = append(regressions, regressionInfo{
+						benchmarkName:  detail.BenchmarkName,
+						metricUnit:     result.Metric.Unit,
+						percentChange:  math.Abs(comparison.Delta),
+						formattedDelta: comparison.FormattedDelta,
+					})
+				}
+			}
+		}
+
+		if len(regressions) > 0 {
+			sheetLink := links[pkgName]
+			formatter, req := createRegressionPostRequest(pkgName, regressions, sheetLink, c.sheetDesc)
+			err := postBenchmarkIssue(c.ctx, l, formatter, req)
+			if err != nil {
+				log.Printf("failed to post regression issue for package %s: %v", pkgName, err)
+				continue
+			}
+			log.Printf("Posted regression issue for package: %s with %d regression(s)", pkgName, len(regressions))
+		}
+	}
+
+	return nil
+}
+
 func (c *compare) pushToInfluxDB(comparisonResultsMap model.ComparisonResultsMap) error {
 	client := influxdb2.NewClient(c.influxConfig.host, c.influxConfig.token)
 	defer client.Close()

pkg/cmd/roachprod-microbench/github.go

@@ -43,6 +43,48 @@ func createBenchmarkPostRequest(
 	return formatter, req
 }
 
+// regressionInfo holds information about a single benchmark regression.
+type regressionInfo struct {
+	benchmarkName  string
+	metricUnit     string
+	percentChange  float64
+	formattedDelta string
+}
+
+// createRegressionPostRequest creates a post request for benchmark performance regressions.
+func createRegressionPostRequest(
+	pkgName string, regressions []regressionInfo, sheetLink, description string,
+) (issues.IssueFormatter, issues.PostRequest) {
+	// Build the regression summary message
+	var sb strings.Builder
+	sb.WriteString(fmt.Sprintf("Performance regressions detected in package %s\n\n", pkgName))
+	sb.WriteString(fmt.Sprintf("Comparison: %s\n\n", description))
+	sb.WriteString(fmt.Sprintf("Found %d benchmark(s) with regressions ≥20%%:\n\n", len(regressions)))
+
+	for i, reg := range regressions {
+		if i >= 10 { // Limit to 10 regressions in the summary
+			sb.WriteString(fmt.Sprintf("... and %d more regression(s)\n", len(regressions)-i))
+			break
+		}
+		sb.WriteString(fmt.Sprintf("• %s (%s): %s (%.1f%%)\n",
+			reg.benchmarkName, reg.metricUnit, reg.formattedDelta, reg.percentChange))
+	}
+
+	sb.WriteString(fmt.Sprintf("\nDetailed comparison: %s\n", sheetLink))
+
+	// Create a failure object for the package with all regressions
+	title := fmt.Sprintf("%s: performance regression", pkgName)
+	f := githubpost.MicrobenchmarkFailure(
+		pkgName,
+		title,
+		sb.String(),
+	)
+
+	formatter, req := githubpost.DefaultFormatter(context.Background(), f)
+	req.Labels = append(req.Labels, "O-microbench", "C-performance")
+	return formatter, req
+}
+
 // postBenchmarkIssue posts a benchmark issue to github.
 func postBenchmarkIssue(
 	ctx context.Context, l *logger.Logger, formatter issues.IssueFormatter, req issues.PostRequest,

pkg/cmd/roachprod-microbench/github_test.go

@@ -55,6 +55,232 @@ func TestCreatePostRequest(t *testing.T) {
 }
 
 // formatPostRequest emulates the behavior of the githubpost package.
+func TestCreateRegressionPostRequestBasic(t *testing.T) {
+	regressions := []regressionInfo{
+		{
+			benchmarkName:  "BenchmarkScan",
+			metricUnit:     "ns/op",
+			percentChange:  25.5,
+			formattedDelta: "+1.2ms",
+		},
+		{
+			benchmarkName:  "BenchmarkInsert",
+			metricUnit:     "ns/op",
+			percentChange:  30.0,
+			formattedDelta: "+500µs",
+		},
+	}
+
+	formatter, req := createRegressionPostRequest(
+		"pkg/sql",
+		regressions,
+		"https://docs.google.com/spreadsheets/d/test123",
+		"master -> release-24.1",
+	)
+
+	// Verify labels
+	expectedLabels := []string{"O-microbench", "C-performance"}
+	for _, label := range expectedLabels {
+		found := false
+		for _, l := range req.Labels {
+			if l == label {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("Expected label %q not found in %v", label, req.Labels)
+		}
+	}
+
+	// Verify title contains package name
+	data := issues.TemplateData{
+		PostRequest:      req,
+		Parameters:       req.ExtraParams,
+		CondensedMessage: issues.CondensedMessage(req.Message),
+		PackageNameShort: req.PackageName,
+	}
+	title := formatter.Title(data)
+	if !strings.Contains(title, "pkg/sql") {
+		t.Errorf("Title should contain package name: %s", title)
+	}
+	if !strings.Contains(title, "performance regression") {
+		t.Errorf("Title should contain 'performance regression': %s", title)
+	}
+
+	// Verify message contains key information
+	if !strings.Contains(req.Message, "pkg/sql") {
+		t.Error("Message should contain package name")
+	}
+	if !strings.Contains(req.Message, "BenchmarkScan") {
+		t.Error("Message should contain benchmark name")
+	}
+	if !strings.Contains(req.Message, "25.5%") {
+		t.Error("Message should contain percentage")
+	}
+	if !strings.Contains(req.Message, "https://docs.google.com/spreadsheets/d/test123") {
+		t.Error("Message should contain sheet link")
+	}
+	if !strings.Contains(req.Message, "master -> release-24.1") {
+		t.Error("Message should contain comparison description")
+	}
+}
+
+func TestCreateRegressionPostRequestMultiple(t *testing.T) {
+	// Test with multiple regressions
+	regressions := make([]regressionInfo, 15)
+	for i := 0; i < 15; i++ {
+		regressions[i] = regressionInfo{
+			benchmarkName:  fmt.Sprintf("Benchmark%d", i),
+			metricUnit:     "ns/op",
+			percentChange:  20.0 + float64(i),
+			formattedDelta: fmt.Sprintf("+%dµs", i*100),
+		}
+	}
+
+	_, req := createRegressionPostRequest(
+		"pkg/kv",
+		regressions,
+		"https://docs.google.com/spreadsheets/d/test456",
+		"baseline -> experiment",
+	)
+
+	// Should mention total count
+	if !strings.Contains(req.Message, "15 benchmark(s)") {
+		t.Error("Message should contain total regression count")
+	}
+
+	// Should truncate to 10 and show "... and 5 more"
+	if !strings.Contains(req.Message, "5 more regression(s)") {
+		t.Errorf("Message should indicate truncation. Got: %s", req.Message)
+	}
+
+	// First regression should be present
+	if !strings.Contains(req.Message, "Benchmark0") {
+		t.Error("Message should contain first benchmark")
+	}
+
+	// 11th regression should not be listed individually
+	if strings.Contains(req.Message, "Benchmark10") {
+		t.Error("Message should not list 11th benchmark individually")
+	}
+}
+
+func TestCreateRegressionPostRequestTruncation(t *testing.T) {
+	// Test the exact boundary at 10 regressions
+	regressions := make([]regressionInfo, 10)
+	for i := 0; i < 10; i++ {
+		regressions[i] = regressionInfo{
+			benchmarkName:  fmt.Sprintf("Benchmark%d", i),
+			metricUnit:     "ns/op",
+			percentChange:  20.0,
+			formattedDelta: "+100µs",
+		}
+	}
+
+	_, req := createRegressionPostRequest(
+		"pkg/storage",
+		regressions,
+		"https://docs.google.com/spreadsheets/d/test",
+		"test comparison",
+	)
+
+	// All 10 should be listed
+	for i := 0; i < 10; i++ {
+		if !strings.Contains(req.Message, fmt.Sprintf("Benchmark%d", i)) {
+			t.Errorf("Message should contain Benchmark%d", i)
+		}
+	}
+
+	// Should not show truncation message for exactly 10
+	if strings.Contains(req.Message, "more regression(s)") {
+		t.Error("Should not show truncation message for exactly 10 regressions")
+	}
+}
+
+func TestCreateRegressionPostRequestFormat(t *testing.T) {
+	// Test the full formatted output of a regression issue
+	regressions := []regressionInfo{
+		{
+			benchmarkName:  "BenchmarkScan/rows=1000",
+			metricUnit:     "ns/op",
+			percentChange:  25.5,
+			formattedDelta: "+1.2ms",
+		},
+		{
+			benchmarkName:  "BenchmarkInsert/batch=100",
+			metricUnit:     "B/op",
+			percentChange:  30.0,
+			formattedDelta: "+5.0KB",
+		},
+	}
+
+	formatter, req := createRegressionPostRequest(
+		"pkg/sql/exec",
+		regressions,
+		"https://docs.google.com/spreadsheets/d/example123",
+		"v24.1.0 -> v24.2.0",
+	)
+
+	// Verify the full formatted message structure
+	output, err := formatPostRequest(formatter, req)
+	if err != nil {
+		t.Fatalf("Failed to format post request: %v", err)
+	}
+
+	// Check title format
+	data := issues.TemplateData{
+		PostRequest:      req,
+		Parameters:       req.ExtraParams,
+		CondensedMessage: issues.CondensedMessage(req.Message),
+		PackageNameShort: req.PackageName,
+	}
+	title := formatter.Title(data)
+
+	// Verify title structure
+	if !strings.Contains(title, "pkg/sql/exec") {
+		t.Errorf("Title should contain package name, got: %s", title)
+	}
+	if !strings.Contains(title, "performance regression") {
+		t.Errorf("Title should contain 'performance regression', got: %s", title)
+	}
+
+	// Verify formatted output contains all key elements
+	expectedElements := []string{
+		"Performance regressions detected in package pkg/sql/exec",
+		"Comparison: v24.1.0 -> v24.2.0",
+		"Found 2 benchmark(s) with regressions ≥20%",
+		"BenchmarkScan/rows=1000",
+		"(ns/op): +1.2ms (25.5%)",
+		"BenchmarkInsert/batch=100",
+		"(B/op): +5.0KB (30.0%)",
+		"Detailed comparison: https://docs.google.com/spreadsheets/d/example123",
+	}
+
+	for _, expected := range expectedElements {
+		if !strings.Contains(output, expected) {
+			t.Errorf("Formatted output should contain %q\nGot:\n%s", expected, output)
+		}
+	}
+
+	// Verify labels are correct
+	if !containsLabel(req.Labels, "O-microbench") {
+		t.Errorf("Should have O-microbench label, got: %v", req.Labels)
+	}
+	if !containsLabel(req.Labels, "C-performance") {
+		t.Errorf("Should have C-performance label, got: %v", req.Labels)
+	}
+}
+
+func containsLabel(labels []string, label string) bool {
+	for _, l := range labels {
+		if l == label {
+			return true
+		}
+	}
+	return false
+}
+
 func formatPostRequest(formatter issues.IssueFormatter, req issues.PostRequest) (string, error) {
 	// These fields can vary based on the test env so we set them to arbitrary
 	// values here.

pkg/cmd/roachprod-microbench/main.go

@@ -173,6 +173,12 @@ func makeCompareCommand() *cobra.Command {
 					return err
 				}
 			}
+			if c.postIssues {
+				err = c.postRegressionIssues(links, comparisonResult)
+				if err != nil {
+					return err
+				}
+			}
 		}
 
 		if c.influxConfig.token != "" {
@@ -207,6 +213,7 @@ func makeCompareCommand() *cobra.Command {
 	cmd.Flags().StringVar(&config.influxConfig.token, "influx-token", config.influxConfig.token, "pass an InfluxDB auth token to push the results to InfluxDB")
 	cmd.Flags().StringToStringVar(&config.influxConfig.metadata, "influx-metadata", config.influxConfig.metadata, "pass metadata to add to the InfluxDB measurement")
 	cmd.Flags().Float64Var(&config.threshold, "threshold", config.threshold, "threshold in percentage value for detecting perf regression ")
+	cmd.Flags().BoolVar(&config.postIssues, "post-issues", config.postIssues, "post GitHub issues for performance regressions (requires env vars for github issues to be set)")
 	return cmd
 }