Move metrics from git-http.go to response-writer.go

internal/git/git-http.go

@@ -16,35 +16,10 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/prometheus/client_golang/prometheus"
-
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
 )
 
-var (
-	gitHTTPRequests = prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "gitlab_workhorse_git_http_requests",
-			Help: "How many Git HTTP requests have been processed by gitlab-workhorse, partitioned by request type and agent.",
-		},
-		[]string{"request_type", "agent"},
-	)
-
-	gitHTTPBytes = prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "gitlab_workhorse_git_http_bytes",
-			Help: "How many Git HTTP bytes have been sent by gitlab-workhorse, partitioned by request type, agent and direction.",
-		},
-		[]string{"request_type", "agent", "direction"},
-	)
-)
-
-func init() {
-	prometheus.MustRegister(gitHTTPRequests)
-	prometheus.MustRegister(gitHTTPBytes)
-}
-
 func GetInfoRefs(a *api.API) http.Handler {
 	return repoPreAuthorizeHandler(a, handleGetInfoRefs)
 }
@@ -63,17 +38,6 @@ func looksLikeRepo(p string) bool {
 	return true
 }
 
-func getRequestAgent(r *http.Request) string {
-	u, _, ok := r.BasicAuth()
-	if ok && u == "gitlab-ci-token" {
-		return "gitlab-ci"
-	} else if ok {
-		return "logged"
-	} else {
-		return "anonymous"
-	}
-}
-
 func repoPreAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		if a.RepoPath == "" {
@@ -90,21 +54,18 @@ func repoPreAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Han
 	}, "")
 }
 
-func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response) {
-	rpc := r.URL.Query().Get("service")
+func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response) {
+	w := NewGitHttpResponseWriter(rw)
+	// Log 0 bytes in because we ignore the request body (and there usually is none anyway).
+	defer w.Log(r, 0)
+
+	rpc := getService(r)
 	if !(rpc == "git-upload-pack" || rpc == "git-receive-pack") {
 		// The 'dumb' Git HTTP protocol is not supported
 		http.Error(w, "Not Found", 404)
 		return
 	}
 
-	requestType := "get-info-refs"
-	gitHTTPRequests.WithLabelValues(requestType, getRequestAgent(r)).Inc()
-	var writtenOut int64
-	defer func() {
-		gitHTTPBytes.WithLabelValues(requestType, getRequestAgent(r), "out").Add(float64(writtenOut))
-	}()
-
 	// Prepare our Git subprocess
 	cmd := gitCommand(a.GL_ID, "git", subCommand(rpc), "--stateless-rpc", "--advertise-refs", a.RepoPath)
 	stdout, err := cmd.StdoutPipe()
@@ -131,7 +92,7 @@ func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response)
 		helper.LogError(r, fmt.Errorf("handleGetInfoRefs: pktFlush: %v", err))
 		return
 	}
-	if writtenOut, err = io.Copy(w, stdout); err != nil {
+	if _, err := io.Copy(w, stdout); err != nil {
 		helper.LogError(
 			r,
 			&copyError{fmt.Errorf("handleGetInfoRefs: copy output of %v: %v", cmd.Args, err)},
@@ -144,27 +105,24 @@ func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response)
 	}
 }
 
-func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
+func handlePostRPC(rw http.ResponseWriter, r *http.Request, a *api.Response) {
 	var err error
 	var body io.Reader
 	var isShallowClone bool
+	var writtenIn int64
+
+	w := NewGitHttpResponseWriter(rw)
+	defer func() {
+		w.Log(r, writtenIn)
+	}()
 
-	// Get Git action from URL
-	action := filepath.Base(r.URL.Path)
+	action := getService(r)
 	if !(action == "git-upload-pack" || action == "git-receive-pack") {
 		// The 'dumb' Git HTTP protocol is not supported
 		helper.Fail500(w, r, fmt.Errorf("handlePostRPC: unsupported action: %s", r.URL.Path))
 		return
 	}
 
-	requestType := "post-" + action
-	gitHTTPRequests.WithLabelValues(requestType, getRequestAgent(r)).Inc()
-	var writtenIn, writtenOut int64
-	defer func() {
-		gitHTTPBytes.WithLabelValues(requestType, getRequestAgent(r), "in").Add(float64(writtenIn))
-		gitHTTPBytes.WithLabelValues(requestType, getRequestAgent(r), "out").Add(float64(writtenOut))
-	}()
-
 	if action == "git-upload-pack" {
 		buffer := &bytes.Buffer{}
 		// Only sniff on the first 4096 bytes: we assume that if we find no
@@ -220,7 +178,7 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return
 
 	// This io.Copy may take a long time, both for Git push and pull.
-	if writtenOut, err = io.Copy(w, stdout); err != nil {
+	if _, err := io.Copy(w, stdout); err != nil {
 		helper.LogError(
 			r,
 			&copyError{fmt.Errorf("handlePostRPC: copy output of %v: %v", cmd.Args, err)},
@@ -233,6 +191,13 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	}
 }
 
+func getService(r *http.Request) string {
+	if r.Method == "GET" {
+		return r.URL.Query().Get("service")
+	}
+	return filepath.Base(r.URL.Path)
+}
+
 func isExitError(err error) bool {
 	_, ok := err.(*exec.ExitError)
 	return ok

internal/git/response-writter.go

+package git
+
+import (
+	"net/http"
+	"strconv"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+const (
+	directionIn  = "in"
+	directionOut = "out"
+)
+
+var (
+	gitHTTPSessionsActive = prometheus.NewGauge(prometheus.GaugeOpts{
+		Name: "gitlab_workhorse_git_http_sessions_active",
+		Help: "Number of Git HTTP request-response cycles currently being handled by gitlab-workhorse.",
+	})
+
+	gitHTTPRequests = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitlab_workhorse_git_http_requests",
+			Help: "How many Git HTTP requests have been processed by gitlab-workhorse, partitioned by request type and agent.",
+		},
+		[]string{"method", "code", "service", "agent"},
+	)
+
+	gitHTTPBytes = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitlab_workhorse_git_http_bytes",
+			Help: "How many Git HTTP bytes have been sent by gitlab-workhorse, partitioned by request type, agent and direction.",
+		},
+		[]string{"method", "code", "service", "agent", "direction"},
+	)
+)
+
+func init() {
+	prometheus.MustRegister(gitHTTPSessionsActive)
+	prometheus.MustRegister(gitHTTPRequests)
+	prometheus.MustRegister(gitHTTPBytes)
+}
+
+type GitHttpResponseWriter struct {
+	rw      http.ResponseWriter
+	status  int
+	written int64
+}
+
+func NewGitHttpResponseWriter(rw http.ResponseWriter) *GitHttpResponseWriter {
+	gitHTTPSessionsActive.Inc()
+	return &GitHttpResponseWriter{
+		rw: rw,
+	}
+}
+
+func (w *GitHttpResponseWriter) Header() http.Header {
+	return w.rw.Header()
+}
+
+func (w *GitHttpResponseWriter) Write(data []byte) (n int, err error) {
+	if w.status == 0 {
+		w.WriteHeader(http.StatusOK)
+	}
+
+	n, err = w.rw.Write(data)
+	w.written += int64(n)
+	return n, err
+}
+
+func (w *GitHttpResponseWriter) WriteHeader(status int) {
+	if w.status != 0 {
+		return
+	}
+
+	w.status = status
+	w.rw.WriteHeader(status)
+}
+
+func (w *GitHttpResponseWriter) Log(r *http.Request, writtenIn int64) {
+	service := getService(r)
+	agent := getRequestAgent(r)
+
+	gitHTTPSessionsActive.Dec()
+	gitHTTPRequests.WithLabelValues(r.Method, strconv.Itoa(w.status), service, agent).Inc()
+	gitHTTPBytes.WithLabelValues(r.Method, strconv.Itoa(w.status), service, agent, directionIn).
+		Add(float64(writtenIn))
+	gitHTTPBytes.WithLabelValues(r.Method, strconv.Itoa(w.status), service, agent, directionOut).
+		Add(float64(w.written))
+}
+
+func getRequestAgent(r *http.Request) string {
+	u, _, ok := r.BasicAuth()
+	if !ok {
+		return "anonymous"
+	}
+
+	if u == "gitlab-ci-token" {
+		return "gitlab-ci"
+	}
+
+	return "logged"
+}

internal/helper/logging.go

@@ -68,7 +68,7 @@ func (l *LoggingResponseWriter) Write(data []byte) (n int, err error) {
 	}
 	n, err = l.rw.Write(data)
 	l.written += int64(n)
-	return
+	return n, err
 }
 
 func (l *LoggingResponseWriter) WriteHeader(status int) {

internal/sendfile/sendfile.go

@@ -34,6 +34,8 @@ var (
 		},
 		[]string{"type"},
 	)
+
+	artifactsSendFile = regexp.MustCompile("builds/[0-9]+/artifacts")
 )
 
 type sendFileResponseWriter struct {
@@ -118,7 +120,7 @@ func sendFileFromDisk(w http.ResponseWriter, r *http.Request, file string) {
 func countSendFileMetrics(size int64, r *http.Request) {
 	var requestType string
 	switch {
-	case regexp.MustCompile("builds/[0-9]+/artifacts").MatchString(r.RequestURI):
+	case artifactsSendFile.MatchString(r.RequestURI):
 		requestType = "artifacts"
 	default:
 		requestType = "other"