diff --git a/cmd/pj-rehearse-metrics/pj-rehearse-metrics.go b/cmd/pj-rehearse-metrics/pj-rehearse-metrics.go index a7ecaedc..2b398ef0 100644 --- a/cmd/pj-rehearse-metrics/pj-rehearse-metrics.go +++ b/cmd/pj-rehearse-metrics/pj-rehearse-metrics.go @@ -237,6 +237,10 @@ func run() error { overLimit := rehearse.NewMetricsCounter("PRs hitting the limit of rehearsed jobs", func(m *rehearse.Metrics) bool { return len(m.Actual) > 0 && m.Execution == nil }) + allBuilds := rehearse.AllBuilds{Pulls: map[int][]*rehearse.Metrics{}} + staleJobs := &rehearse.StaleStatusCounter{Builds: &allBuilds} + + counters := []rehearse.MetricsCounter{overLimit, staleJobs} if err := filepath.Walk(o.cacheDir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -254,14 +258,18 @@ func run() error { metrics.JobSpec.BuildID = filepath.Base(path) } - overLimit.Process(metrics) + for _, counter := range counters { + counter.Process(metrics) + } return nil }); err != nil { return fmt.Errorf("failed to iterate over scraped metrics: %v", err) } - fmt.Printf("%s", overLimit.Report()) + for _, counter := range counters { + fmt.Printf("\n%s", counter.Report()) + } return nil } diff --git a/pkg/rehearse/metrics.go b/pkg/rehearse/metrics.go index 3a4da7b0..b2149671 100644 --- a/pkg/rehearse/metrics.go +++ b/pkg/rehearse/metrics.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "sort" "strings" "github.com/sirupsen/logrus" @@ -199,3 +200,136 @@ PR links: pctBuilds := float64(m.matchingBuilds) / float64(m.totalBuilds) * 100.0 return fmt.Sprintf(template, m.purpose, prCount, len(m.seenPrs), pctPrs, m.matchingBuilds, m.totalBuilds, pctBuilds, strings.Join(links, "\n")) } + +type AllBuilds struct { + Pulls map[int][]*Metrics +} + +func (b *AllBuilds) Process(build *Metrics) { + pr := build.JobSpec.Refs.Pulls[0].Number + if len(b.Pulls[pr]) == 0 { + b.Pulls[pr] = []*Metrics{} + } + b.Pulls[pr] = append(b.Pulls[pr], build) +} + +func (b *AllBuilds) Sort() { + for pr := range b.Pulls { + sort.Slice(b.Pulls[pr], func(i, j int) bool { + return b.Pulls[pr][i].JobSpec.BuildID < b.Pulls[pr][j].JobSpec.BuildID + }) + } +} + +func (b *AllBuilds) PrTotal() int { + return len(b.Pulls) +} + +func (b *AllBuilds) BuildsTotal() int { + total := 0 + for _, builds := range b.Pulls { + total += len(builds) + } + + return total +} + +type staleStatusOcc struct { + pr int + sha, oldBuild, newBuild string + jobs []string +} + +func (s *staleStatusOcc) message() string { + template := "- https://github.com/openshift/release/pull/%d got stale statuses on SHA %s (old build=%s new build=%s):\n - %s" + return fmt.Sprintf(template, s.pr, s.sha, s.oldBuild, s.newBuild, strings.Join(s.jobs, "\n - ")) +} + +type staleStatusStats struct { + prHit, prTotal, prPct int + buildsHit, buildsTotal, buildsPct int + occurrences []staleStatusOcc +} + +func (s *staleStatusStats) messages() string { + if s.buildsHit > 0 { + messages := make([]string, 0, len(s.occurrences)) + for _, occ := range s.occurrences { + messages = append(messages, occ.message()) + } + return strings.Join(messages, "\n") + } + + return "No occurrences" +} + +func (s *staleStatusStats) createReport() string { + template := `# Stale job statuses in PRs + +PR statistics: %d/%d (%d%%) +Build statistics: %d/%d (%d%%) + +PR links: +%s +` + return fmt.Sprintf(template, s.prHit, s.prTotal, s.prPct, s.buildsHit, s.buildsTotal, s.buildsPct, s.messages()) +} + +type StaleStatusCounter struct { + Builds *AllBuilds +} + +func (s *StaleStatusCounter) Process(build *Metrics) { + s.Builds.Process(build) +} + +func (s *StaleStatusCounter) computeStats() *staleStatusStats { + s.Builds.Sort() + + stats := &staleStatusStats{} + + for pr, builds := range s.Builds.Pulls { + prWasHit := false + for i := 1; i < len(builds); i++ { + old := builds[i-1] + oldSHA := old.JobSpec.Refs.Pulls[0].SHA + new := builds[i] + newSHA := new.JobSpec.Refs.Pulls[0].SHA + if newSHA == "" || oldSHA != newSHA { + continue + } + staleJobs := []string{} + for job := range old.Opportunities { + if _, hasJob := new.Opportunities[job]; !hasJob { + staleJobs = append(staleJobs, job) + } + } + if len(staleJobs) > 0 { + prWasHit = true + stats.buildsHit += len(staleJobs) + occurrence := staleStatusOcc{ + pr: pr, + sha: newSHA, + oldBuild: old.JobSpec.BuildID, + newBuild: new.JobSpec.BuildID, + jobs: staleJobs, + } + stats.occurrences = append(stats.occurrences, occurrence) + } + } + if prWasHit { + stats.prHit++ + } + } + + stats.prTotal = s.Builds.PrTotal() + stats.buildsTotal = s.Builds.BuildsTotal() + stats.prPct = int(float64(stats.prHit) / float64(stats.prTotal) * 100.0) + stats.buildsPct = int(float64(stats.buildsHit) / float64(stats.buildsTotal) * 100.0) + + return stats +} + +func (s *StaleStatusCounter) Report() string { + return s.computeStats().createReport() +} diff --git a/pkg/rehearse/metrics_test.go b/pkg/rehearse/metrics_test.go index ad6a546c..185ab984 100644 --- a/pkg/rehearse/metrics_test.go +++ b/pkg/rehearse/metrics_test.go @@ -248,3 +248,65 @@ PR links: t.Errorf("Report differs from expected:\n%s", diff.StringDiff(expected, actual)) } } + +func TestStaleStatusCounter(t *testing.T) { + makeTestBuild := func(pr int, id, sha string, jobs []string) *Metrics { + opps := map[string][]string{} + for _, job := range jobs { + opps[job] = []string{"some reason to rehearse"} + } + return &Metrics{ + JobSpec: &downwardapi.JobSpec{ + BuildID: id, + Refs: &v1.Refs{ + Pulls: []v1.Pull{{Number: pr, SHA: sha}}, + }, + }, + Opportunities: opps, + } + } + testCases := []struct { + description string + builds []*Metrics + expected *staleStatusStats + }{{ + description: "job rehearsed in two subsequent builds over same sha is not stale", + builds: []*Metrics{ + makeTestBuild(1, "build-1", "SHA", []string{"rehearsed-job"}), + makeTestBuild(1, "build-2", "SHA", []string{"rehearsed-job"}), + }, + expected: &staleStatusStats{prHit: 0, prTotal: 1, prPct: 0, buildsHit: 0, buildsTotal: 2, buildsPct: 0}, + }, { + description: "job rehearsed in old build but not in new one is not stale when SHA differs", + builds: []*Metrics{ + makeTestBuild(1, "build-1", "SHA", []string{"rehearsed-job"}), + makeTestBuild(1, "build-2", "D11FFE2E47SHA", []string{"another-rehearsed-job"}), + }, + expected: &staleStatusStats{prHit: 0, prTotal: 1, prPct: 0, buildsHit: 0, buildsTotal: 2, buildsPct: 0}, + }, { + description: "job rehearsed in old build but not in new one over same SHA is stale", + builds: []*Metrics{ + makeTestBuild(1, "build-1", "SHA", []string{"stale-job", "rehearsed-job"}), + makeTestBuild(1, "build-2", "SHA", []string{"rehearsed-job"}), + }, + expected: &staleStatusStats{ + prHit: 1, prTotal: 1, prPct: 100, buildsHit: 1, buildsTotal: 2, buildsPct: 50, + occurrences: []staleStatusOcc{{pr: 1, sha: "SHA", oldBuild: "build-1", newBuild: "build-2", jobs: []string{"stale-job"}}}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + allBuilds := AllBuilds{Pulls: map[int][]*Metrics{}} + counter := StaleStatusCounter{&allBuilds} + for _, build := range tc.builds { + counter.Process(build) + } + stats := counter.computeStats() + if !reflect.DeepEqual(tc.expected, stats) { + t.Errorf("Stats differ from expected:\n%s", diff.ObjectReflectDiff(tc.expected, stats)) + } + }) + } +}