Merge branch 'fix/artifacts-uploads-metrics' into 'master'

Replace 'gitlab_workhorse_artifacts_upload_*' with labeled version of 'gitlab_workhorse_multipart_upload_*'

Closes #88

See merge request !106

internal/artifacts/artifacts_upload.go

@@ -9,35 +9,12 @@ import (
 	"os/exec"
 	"syscall"
 
-	"github.com/prometheus/client_golang/prometheus"
-
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/upload"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/zipartifacts"
 )
 
-var (
-	artifactsUploadRequests = prometheus.NewCounter(
-		prometheus.CounterOpts{
-			Name: "gitlab_workhorse_artifacts_upload_requests",
-			Help: "How many artifacts upload requests have been processed by gitlab-workhorse.",
-		},
-	)
-
-	artifactsUploadBytes = prometheus.NewCounter(
-		prometheus.CounterOpts{
-			Name: "gitlab_workhorse_artifacts_upload_bytes",
-			Help: "How many artifacts upload bytes have been sent by gitlab-workhorse.",
-		},
-	)
-)
-
-func init() {
-	prometheus.MustRegister(artifactsUploadRequests)
-	prometheus.MustRegister(artifactsUploadBytes)
-}
-
 type artifactsUploadProcessor struct {
 	TempPath     string
 	metadataFile string
@@ -92,6 +69,10 @@ func (a *artifactsUploadProcessor) Finalize() error {
 	return nil
 }
 
+func (a *artifactsUploadProcessor) Name() string {
+	return "artifacts"
+}
+
 func (a *artifactsUploadProcessor) Cleanup() {
 	if a.metadataFile != "" {
 		os.Remove(a.metadataFile)
@@ -100,11 +81,6 @@ func (a *artifactsUploadProcessor) Cleanup() {
 
 func UploadArtifacts(myAPI *api.API, h http.Handler) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
-		artifactsUploadRequests.Inc()
-		defer func() {
-			artifactsUploadBytes.Add(float64(r.ContentLength))
-		}()
-
 		if a.TempPath == "" {
 			helper.Fail500(w, r, fmt.Errorf("UploadArtifacts: TempPath is empty"))
 			return

internal/upload/accelerate.go

@@ -55,3 +55,7 @@ func (s *savedFileTracker) Finalize() error {
 	s.request.Header.Set(RewrittenFieldsHeader, tokenString)
 	return nil
 }
+
+func (a *savedFileTracker) Name() string {
+	return "accelerate"
+}

internal/upload/rewrite.go

@@ -14,26 +14,29 @@ import (
 )
 
 var (
-	multipartUploadRequests = prometheus.NewCounter(
+	multipartUploadRequests = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 
 			Name: "gitlab_workhorse_multipart_upload_requests",
-			Help: "How many multipart upload requests have been processed by gitlab-workhorse.",
+			Help: "How many multipart upload requests have been processed by gitlab-workhorse. Partitioned by type.",
 		},
+		[]string{"type"},
 	)
 
-	multipartFileUploadBytes = prometheus.NewCounter(
+	multipartFileUploadBytes = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "gitlab_workhorse_multipart_upload_bytes",
-			Help: "How many disk bytes of multipart file parts have been succesfully written by gitlab-workhorse.",
+			Help: "How many disk bytes of multipart file parts have been succesfully written by gitlab-workhorse. Partitioned by type.",
 		},
+		[]string{"type"},
 	)
 
-	multipartFiles = prometheus.NewCounter(
+	multipartFiles = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "gitlab_workhorse_multipart_upload_files",
-			Help: "How many multipart file parts have been processed by gitlab-workhorse.",
+			Help: "How many multipart file parts have been processed by gitlab-workhorse. Partitioned by type.",
 		},
+		[]string{"type"},
 	)
 )
 
@@ -61,7 +64,7 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, te
 		return nil, fmt.Errorf("get multipart reader: %v", err)
 	}
 
-	multipartUploadRequests.Inc()
+	multipartUploadRequests.WithLabelValues(filter.Name()).Inc()
 
 	rew := &rewriter{
 		writer:   writer,
@@ -109,7 +112,8 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, te
 }
 
 func (rew *rewriter) handleFilePart(name string, p *multipart.Part) error {
-	multipartFiles.Inc()
+	multipartFiles.WithLabelValues(rew.filter.Name()).Inc()
+
 	filename := p.FileName()
 
 	if strings.Contains(filename, "/") || filename == "." || filename == ".." {
@@ -141,7 +145,7 @@ func (rew *rewriter) handleFilePart(name string, p *multipart.Part) error {
 	if err != nil {
 		return fmt.Errorf("copy from multipart to tempfile: %v", err)
 	}
-	multipartFileUploadBytes.Add(float64(written))
+	multipartFileUploadBytes.WithLabelValues(rew.filter.Name()).Add(float64(written))
 
 	file.Close()
 

internal/upload/uploads.go

@@ -15,6 +15,7 @@ type MultipartFormProcessor interface {
 	ProcessFile(formName, fileName string, writer *multipart.Writer) error
 	ProcessField(formName string, writer *multipart.Writer) error
 	Finalize() error
+	Name() string
 }
 
 func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, tempPath string, filter MultipartFormProcessor) {

internal/upload/uploads_test.go

@@ -42,6 +42,10 @@ func (a *testFormProcessor) Finalize() error {
 	return nil
 }
 
+func (a *testFormProcessor) Name() string {
+	return ""
+}
+
 func TestUploadTempPathRequirement(t *testing.T) {
 	response := httptest.NewRecorder()
 	request, err := http.NewRequest("", "", nil)