Skip to content

Trt 1989 benchmarking #3557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
neisw wants to merge 3 commits into
base: main
Choose a base branch
from
+139 −16
Open

Trt 1989 benchmarking #3557

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4db054c
TRT-1989: additional benchmarking
neisw May 21, 2026
9681e27
TRT-1989: write benchmarking file
neisw May 23, 2026
117d9e3
TRT-1989: lint update
neisw May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 139 additions & 16 deletions pkg/flags/postgres_benchmarking_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ package flags
import (
"fmt"
"os"
"path/filepath"
"sort"
"strings"
"testing"
"time"

"github.com/openshift/sippy/pkg/api"
apitype "github.com/openshift/sippy/pkg/apis/api"
"github.com/openshift/sippy/pkg/db"
"github.com/openshift/sippy/pkg/db/models"
"github.com/openshift/sippy/pkg/db/query"
"github.com/openshift/sippy/pkg/filter"
"github.com/openshift/sippy/pkg/util"
log "github.com/sirupsen/logrus"
)

Expand All @@ -33,7 +37,19 @@ type benchmarkResult struct {
max time.Duration
}

func printSummaryTable(results []benchmarkResult) {
func extractConnectionName(dsn string) string {
atIdx := strings.Index(dsn, "@")
if atIdx < 0 {
return ""
}
host := dsn[atIdx+1:]
if dotIdx := strings.Index(host, "."); dotIdx > 0 {
return host[:dotIdx]
}
return ""
}
Comment on lines +40 to +50
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Connection-name parsing drops common DSNs and can generate invalid filename prefixes.

extractConnectionName returns "" for hosts without a dot (e.g. localhost), so file reporting is silently skipped. It can also accidentally include :port/ when a dot appears later in the DB path.

Proposed fix 
 func extractConnectionName(dsn string) string {
-	atIdx := strings.Index(dsn, "@")
+	atIdx := strings.LastIndex(dsn, "@")
 	if atIdx < 0 {
 		return ""
 	}
-	host := dsn[atIdx+1:]
-	if dotIdx := strings.Index(host, "."); dotIdx > 0 {
-		return host[:dotIdx]
+
+	hostPort := dsn[atIdx+1:]
+	if slashIdx := strings.Index(hostPort, "/"); slashIdx >= 0 {
+		hostPort = hostPort[:slashIdx]
+	}
+
+	host := hostPort
+	if colonIdx := strings.Index(host, ":"); colonIdx >= 0 {
+		host = host[:colonIdx]
+	}
+	if host == "" {
+		return ""
+	}
+
+	if dotIdx := strings.Index(host, "."); dotIdx > 0 {
+		return host[:dotIdx]
 	}
-	return ""
+	return host
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func extractConnectionName(dsn string) string {
atIdx := strings.Index(dsn, "@")
if atIdx < 0 {
return ""
}
host := dsn[atIdx+1:]
if dotIdx := strings.Index(host, "."); dotIdx > 0 {
return host[:dotIdx]
}
return ""
}
func extractConnectionName(dsn string) string {
atIdx := strings.LastIndex(dsn, "@")
if atIdx < 0 {
return ""
}
hostPort := dsn[atIdx+1:]
if slashIdx := strings.Index(hostPort, "/"); slashIdx >= 0 {
hostPort = hostPort[:slashIdx]
}
host := hostPort
if colonIdx := strings.Index(host, ":"); colonIdx >= 0 {
host = host[:colonIdx]
}
if host == "" {
return ""
}
if dotIdx := strings.Index(host, "."); dotIdx > 0 {
return host[:dotIdx]
}
return host
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/flags/postgres_benchmarking_test.go` around lines 39 - 49, The
extractConnectionName function currently slices the DSN after "@" but can
include path or port and returns empty for hosts without dots; update it to
first isolate the authority portion (stop at the first "/" or "?" after the
"@"), then strip any ":port" if present, and finally return the host portion —
if a dot exists in that host return the substring before the first dot,
otherwise return the full host (so "localhost" yields "localhost" rather than
""). Modify the function extractConnectionName to implement this parsing order
to avoid picking up dots from the DB path and to preserve common hosts without
dots.


func printSummaryTable(t *testing.T, results []benchmarkResult, connName string) {
nameWidth := 4
for _, r := range results {
if len(r.name) > nameWidth {
Expand All @@ -45,16 +61,32 @@ func printSummaryTable(results []benchmarkResult) {
return results[i].avg > results[j].avg
})

var sb strings.Builder
header := fmt.Sprintf(" %-*s %5s %12s %12s %12s %12s",
nameWidth, "Name", "Iters", "Total", "Avg", "Min", "Max")
fmt.Println()
fmt.Println(header)
fmt.Println(" " + strings.Repeat("-", len(header)-2))
sb.WriteString("\n")
sb.WriteString(header + "\n")
sb.WriteString(" " + strings.Repeat("-", len(header)-2) + "\n")
for _, r := range results {
fmt.Printf(" %-*s %5d %12s %12s %12s %12s\n",
fmt.Fprintf(&sb, " %-*s %5d %12s %12s %12s %12s\n",
nameWidth, r.name, r.iterations, r.total, r.avg, r.min, r.max)
}
fmt.Println()
sb.WriteString("\n")
fmt.Print(sb.String())

// optional helper to track results
benchmarkFilePath := os.Getenv("benchmarking_file_path")
if connName != "" && len(benchmarkFilePath) > 0 {
ts := time.Now().UTC().Format("2006-01-02T15-04-05")
filename := fmt.Sprintf("benchmark-%s-%s.txt", connName, ts)
fullPath := filepath.Join(benchmarkFilePath, filename)
fullPath = filepath.Clean(fullPath)
if err := os.WriteFile(fullPath, []byte(sb.String()), 0600); err != nil { // #nosec G703
t.Logf("failed to write benchmark report to %s: %v", fullPath, err)
} else {
t.Logf("benchmark report written to %s", fullPath)
}
}
}

func runBenchmarkCase(t *testing.T, dbc *db.DB, bc benchmarkCase, iterations int) benchmarkResult {
Expand Down Expand Up @@ -274,6 +306,97 @@ func getBenchmarkCases(asOf time.Time) []benchmarkCase {
return nil
},
},
{
name: "VariantReports",
fn: func(dbc *db.DB) error {
start, boundary, end := util.PeriodToDates("default", asOf)
results, err := query.VariantReports(dbc, benchmarkRelease, start, boundary, end)
if err == nil {
log.Printf("VariantReports: %d variants", len(results))
}
return err
},
},
{
name: "JobReports",
fn: func(dbc *db.DB) error {
start, boundary, end := util.PeriodToDates("default", asOf)
results, err := query.JobReports(dbc, &filter.FilterOptions{Filter: &filter.Filter{}}, benchmarkRelease, start, boundary, end)
if err == nil {
log.Printf("JobReports: %d jobs", len(results))
}
return err
},
},
{
name: "BuildClusterHealth",
fn: func(dbc *db.DB) error {
start, boundary, end := util.PeriodToDates("default", asOf)
results, err := query.BuildClusterHealth(dbc, start, boundary, end)
if err == nil {
log.Printf("BuildClusterHealth: %d clusters", len(results))
}
return err
},
},
{
name: "RecentTestFailures",
fn: func(dbc *db.DB) error {
period := 7 * 24 * time.Hour
previousPeriod := 7 * 24 * time.Hour
pagination := &apitype.Pagination{PerPage: 20, Page: 0}
result, err := api.GetRecentTestFailures(dbc, benchmarkRelease, period, &previousPeriod, false, &filter.FilterOptions{Filter: &filter.Filter{}}, pagination, asOf)
if err == nil {
log.Printf("RecentTestFailures: %d rows", result.TotalRows)
}
return err
},
},
{
name: "PullRequestReport",
fn: func(dbc *db.DB) error {
results, err := query.PullRequestReport(dbc, &filter.FilterOptions{Filter: &filter.Filter{}}, benchmarkRelease)
if err == nil {
log.Printf("PullRequestReport: %d PRs", len(results))
}
return err
},
},
{
name: "RepositoryReport",
fn: func(dbc *db.DB) error {
results, err := query.RepositoryReport(dbc, &filter.FilterOptions{Filter: &filter.Filter{}}, benchmarkRelease, asOf)
if err == nil {
log.Printf("RepositoryReport: %d repos", len(results))
}
return err
},
},
{
name: "JobsRunsReport",
fn: func(dbc *db.DB) error {
pagination := &apitype.Pagination{PerPage: 20, Page: 0}
result, err := api.JobsRunsReportFromDB(dbc, &filter.FilterOptions{Filter: &filter.Filter{}}, benchmarkRelease, pagination, asOf)
if err == nil {
log.Printf("JobsRunsReport: %d rows", result.TotalRows)
}
return err
},
},
{
name: "ProwJobHistoricalTestCounts",
fn: func(dbc *db.DB) error {
var prowJob models.ProwJob
if err := dbc.DB.Where("name = ? AND release = ?", benchmarkJobName, benchmarkRelease).First(&prowJob).Error; err != nil {
return err
}
count, err := query.ProwJobHistoricalTestCounts(dbc, prowJob.ID)
if err == nil {
log.Printf("ProwJobHistoricalTestCounts for %s: %d", benchmarkJobName, count)
}
return err
},
},
{
name: "TestAnalysisPassRate",
fn: func(dbc *db.DB) error {
Expand All @@ -298,7 +421,7 @@ func getBenchmarkCases(asOf time.Time) []benchmarkCase {
}
}

func getBenchmarkDBClient(t *testing.T) *db.DB {
func getBenchmarkDBClient(t *testing.T) (*db.DB, string) {
t.Helper()
dsn := os.Getenv("db_benchmarking_dsn")
if dsn == "" {
Expand All @@ -323,11 +446,11 @@ func getBenchmarkDBClient(t *testing.T) *db.DB {
t.Logf("failed to close DB client: %v", err)
}
})
return dbc
return dbc, extractConnectionName(dsn)
}

func Test_BenchmarkIndividual(t *testing.T) {
dbc := getBenchmarkDBClient(t)
dbc, connName := getBenchmarkDBClient(t)
asOf := time.Now().UTC()
iterations := 3
cases := getBenchmarkCases(asOf)
Expand All @@ -339,23 +462,23 @@ func Test_BenchmarkIndividual(t *testing.T) {
results = append(results, r)
})
}
printSummaryTable(results)
printSummaryTable(t, results, connName)
}

func Test_BenchmarkFindTestsByRelease(t *testing.T) {
dbc := getBenchmarkDBClient(t)
dbc, connName := getBenchmarkDBClient(t)
iterations := 1
bc, ok := getIndividualBenchmarkCases()["FindTestsByRelease"]
if !ok {
t.Fatal("benchmark case \"FindTestsByRelease\" not found")
}

r := runBenchmarkCase(t, dbc, bc, iterations)
printSummaryTable([]benchmarkResult{r})
printSummaryTable(t, []benchmarkResult{r}, connName)
}

func Test_BenchmarkCombined(t *testing.T) {
dbc := getBenchmarkDBClient(t)
dbc, connName := getBenchmarkDBClient(t)
asOf := time.Now().UTC()
iterations := 3

Expand All @@ -372,11 +495,11 @@ func Test_BenchmarkCombined(t *testing.T) {
results = append(results, r)
})
}
printSummaryTable(results)
printSummaryTable(t, results, connName)
}

func Test_BenchmarkGroup(t *testing.T) {
dbc := getBenchmarkDBClient(t)
dbc, connName := getBenchmarkDBClient(t)
asOf := time.Now().UTC()
iterations := 1
cases := getBenchmarkCases(asOf)
Expand All @@ -402,5 +525,5 @@ func Test_BenchmarkGroup(t *testing.T) {
fmt.Printf(" group iteration %d: %s\n", i+1, elapsed)
}
group.avg = group.total / time.Duration(iterations)
printSummaryTable([]benchmarkResult{group})
printSummaryTable(t, []benchmarkResult{group}, connName)
}