Rehearsal metrics: Find and count stale rehearsal occurences

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
 }

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()
+}

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))
+			}
+		})
+	}
+}