stats: Move wants refs from GET to POST

When doing a POST to git-upload-pack(1), then we send along a set of
"want"s which tell the remote side which references we want to receive.
So while this depends on us knowing which references exist in the first
place, it still is inarguably a property of the POST and not of the GET
which discovers available refs.

Let's make the relation more explicit by 1. moving wants into the `Post`
structure and 2. accepting announced references as parameter when doing
the post.

5 files changed, 21 insertions, 24 deletions

diff --git a/.golangci.yml b/.golangci.yml
index 9ed865be8..d6475eb0e 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -290,10 +290,6 @@ issues:
     - linters:
         - golint
       path: "internal/git/stats/analyzehttp.go"
-      text: "exported method `Clone.RefsWanted` should have comment or be unexported"
-    - linters:
-        - golint
-      path: "internal/git/stats/analyzehttp.go"
       text: "exported type `Get` should have comment or be unexported"
     - linters:
         - golint
diff --git a/cmd/gitaly-debug/analyzehttp.go b/cmd/gitaly-debug/analyzehttp.go
index a4d513a98..a61da43f5 100644
--- a/cmd/gitaly-debug/analyzehttp.go
+++ b/cmd/gitaly-debug/analyzehttp.go
@@ -23,7 +23,6 @@ func analyzeHTTPClone(cloneURL string) {
 		{"payload size", st.Get.PayloadSize()},
 		{"Git packets received", st.Get.Packets()},
 		{"refs advertised", len(st.Get.Refs())},
-		{"wanted refs", st.RefsWanted()},
 	} {
 		entry.print()
 	}
@@ -35,6 +34,7 @@ func analyzeHTTPClone(cloneURL string) {
 		{"response body time", st.Post.ResponseBody()},
 		{"largest single Git packet", st.Post.LargestPacketSize()},
 		{"Git packets received", st.Post.Packets()},
+		{"wanted refs", st.Post.RefsWanted()},
 	} {
 		entry.print()
 	}
diff --git a/internal/blackbox/blackbox.go b/internal/blackbox/blackbox.go
index 1563bec6d..1147b2c15 100644
--- a/internal/blackbox/blackbox.go
+++ b/internal/blackbox/blackbox.go
@@ -125,7 +125,7 @@ func (b Blackbox) doProbe(probe Probe) {
 	setGauge(b.getFirstPacket, clone.Get.FirstGitPacket().Seconds())
 	setGauge(b.getTotalTime, clone.Get.ResponseBody().Seconds())
 	setGauge(b.getAdvertisedRefs, float64(len(clone.Get.Refs())))
-	setGauge(b.wantedRefs, float64(clone.RefsWanted()))
+	setGauge(b.wantedRefs, float64(clone.Post.RefsWanted()))
 	setGauge(b.postTotalTime, clone.Post.ResponseBody().Seconds())
 	setGauge(b.postFirstProgressPacket, clone.Post.BandFirstPacket("progress").Seconds())
 	setGauge(b.postFirstPackPacket, clone.Post.BandFirstPacket("pack").Seconds())
diff --git a/internal/git/stats/analyzehttp.go b/internal/git/stats/analyzehttp.go
index f45444a78..c57cc41a2 100644
--- a/internal/git/stats/analyzehttp.go
+++ b/internal/git/stats/analyzehttp.go
@@ -20,20 +20,17 @@ type Clone struct {
 	User        string
 	Password    string
 
-	wants []string // all branch and tag pointers
-	Get   Get
-	Post  Post
+	Get  Get
+	Post Post
 }
 
-func (cl *Clone) RefsWanted() int { return len(cl.wants) }
-
 // Perform does a Git HTTP clone, discarding cloned data to /dev/null.
 func (cl *Clone) Perform(ctx context.Context) error {
 	if err := cl.doGet(ctx); err != nil {
 		return ctxErr(ctx, err)
 	}
 
-	if err := cl.doPost(ctx); err != nil {
+	if err := cl.doPost(ctx, cl.Get.Refs()); err != nil {
 		return ctxErr(ctx, err)
 	}
 
@@ -126,12 +123,6 @@ func (cl *Clone) doGet(ctx context.Context) error {
 		return err
 	}
 
-	for _, ref := range cl.Get.Refs() {
-		if strings.HasPrefix(ref.Name, "refs/heads/") || strings.HasPrefix(ref.Name, "refs/tags/") {
-			cl.wants = append(cl.wants, ref.Oid)
-		}
-	}
-
 	return nil
 }
 
@@ -140,6 +131,7 @@ type Post struct {
 	responseHeader time.Duration
 	httpStatus     int
 	stats          FetchPack
+	wantedRefs     []string
 }
 
 func (p *Post) ResponseHeader() time.Duration { return p.responseHeader }
@@ -149,6 +141,9 @@ func (p *Post) ResponseBody() time.Duration   { return p.stats.responseBody.Sub(
 func (p *Post) Packets() int                  { return p.stats.packets }
 func (p *Post) LargestPacketSize() int        { return p.stats.largestPacketSize }
 
+// RefsWanted returns the number of references sent to the remote repository as "want"s.
+func (p *Post) RefsWanted() int { return len(p.wantedRefs) }
+
 func (p *Post) BandPackets(b string) int       { return p.stats.multiband[b].packets }
 func (p *Post) BandPayloadSize(b string) int64 { return p.stats.multiband[b].size }
 func (p *Post) BandFirstPacket(b string) time.Duration {
@@ -158,10 +153,16 @@ func (p *Post) BandFirstPacket(b string) time.Duration {
 // See
 // https://github.com/git/git/blob/v2.25.0/Documentation/technical/http-protocol.txt#L351
 // for background information.
-func (cl *Clone) buildPost(ctx context.Context) (*http.Request, error) {
+func (cl *Clone) buildPost(ctx context.Context, announcedRefs []Reference) (*http.Request, error) {
+	for _, ref := range announcedRefs {
+		if strings.HasPrefix(ref.Name, "refs/heads/") || strings.HasPrefix(ref.Name, "refs/tags/") {
+			cl.Post.wantedRefs = append(cl.Post.wantedRefs, ref.Oid)
+		}
+	}
+
 	reqBodyRaw := &bytes.Buffer{}
 	reqBodyGzip := gzip.NewWriter(reqBodyRaw)
-	for i, oid := range cl.wants {
+	for i, oid := range cl.Post.wantedRefs {
 		if i == 0 {
 			oid += " multi_ack_detailed no-done side-band-64k thin-pack ofs-delta deepen-since deepen-not agent=git/2.21.0"
 		}
@@ -201,8 +202,8 @@ func (cl *Clone) buildPost(ctx context.Context) (*http.Request, error) {
 	return req, nil
 }
 
-func (cl *Clone) doPost(ctx context.Context) error {
-	req, err := cl.buildPost(ctx)
+func (cl *Clone) doPost(ctx context.Context, announcedRefs []Reference) error {
+	req, err := cl.buildPost(ctx, announcedRefs)
 	if err != nil {
 		return err
 	}
diff --git a/internal/git/stats/analyzehttp_test.go b/internal/git/stats/analyzehttp_test.go
index 84b8eca38..e0cde04d4 100644
--- a/internal/git/stats/analyzehttp_test.go
+++ b/internal/git/stats/analyzehttp_test.go
@@ -27,8 +27,8 @@ func TestClone(t *testing.T) {
 	clone := Clone{URL: fmt.Sprintf("http://localhost:%d/%s", serverPort, filepath.Base(repoPath))}
 	require.NoError(t, clone.Perform(ctx), "perform analysis clone")
 
-	const expectedWants = 90 // based on contents of _support/gitlab-test.git-packed-refs
-	require.Greater(t, clone.RefsWanted(), expectedWants, "number of wanted refs")
+	const expectedRequests = 90 // based on contents of _support/gitlab-test.git-packed-refs
+	require.Greater(t, clone.Post.RefsWanted(), expectedRequests, "number of wanted refs")
 
 	require.Equal(t, 200, clone.Get.HTTPStatus(), "get status")
 	require.Greater(t, clone.Get.Packets(), 0, "number of get packets")