Add Name to the VFS interface to use with metrics

12 files changed, 47 insertions, 31 deletions

diff --git a/internal/serving/disk/local/serving.go b/internal/serving/disk/local/serving.go
index d4a882bc..2882a301 100644
--- a/internal/serving/disk/local/serving.go
+++ b/internal/serving/disk/local/serving.go
@@ -8,7 +8,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
-var instance = disk.New(vfs.Instrumented(&local.VFS{}, "local"), metrics.DiskServingFileSize)
+var instance = disk.New(vfs.Instrumented(local.New("local")), metrics.VFSServingFileSize)
 
 // Instance returns a serving instance that is capable of reading files
 // from the disk
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index 5b34c556..559d332c 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -20,7 +20,7 @@ import (
 
 // Reader is a disk access driver
 type Reader struct {
-	fileSizeMetric prometheus.Histogram
+	fileSizeMetric *prometheus.HistogramVec
 	vfs            vfs.VFS
 }
 
@@ -196,7 +196,7 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h
 
 	w.Header().Set("Content-Type", contentType)
 
-	reader.fileSizeMetric.Observe(float64(fi.Size()))
+	reader.fileSizeMetric.WithLabelValues(reader.vfs.Name()).Observe(float64(fi.Size()))
 
 	// Support vfs.SeekableFile if available (uncompressed files)
 	if rs, ok := file.(vfs.SeekableFile); ok {
@@ -231,7 +231,7 @@ func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter
 		return err
 	}
 
-	reader.fileSizeMetric.Observe(float64(fi.Size()))
+	reader.fileSizeMetric.WithLabelValues(reader.vfs.Name()).Observe(float64(fi.Size()))
 
 	w.Header().Set("Content-Type", contentType)
 	w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10))
diff --git a/internal/serving/disk/serving.go b/internal/serving/disk/serving.go
index 94ea3f23..312ed093 100644
--- a/internal/serving/disk/serving.go
+++ b/internal/serving/disk/serving.go
@@ -43,7 +43,7 @@ func (s *Disk) ServeNotFoundHTTP(h serving.Handler) {
 
 // New returns a serving instance that is capable of reading files
 // from the VFS
-func New(vfs vfs.VFS, fileSizeMetric prometheus.Histogram) serving.Serving {
+func New(vfs vfs.VFS, fileSizeMetric *prometheus.HistogramVec) serving.Serving {
 	return &Disk{
 		reader: Reader{
 			fileSizeMetric: fileSizeMetric,
diff --git a/internal/serving/disk/symlink/path_test.go b/internal/serving/disk/symlink/path_test.go
index 8c1d19b3..5e7ea16d 100644
--- a/internal/serving/disk/symlink/path_test.go
+++ b/internal/serving/disk/symlink/path_test.go
@@ -21,7 +21,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs/local"
 )
 
-var fs = vfs.Instrumented(&local.VFS{}, "local")
+var fs = vfs.Instrumented(local.New("local"))
 
 type EvalSymlinksTest struct {
 	// If dest is empty, the path is created; otherwise the dest is symlinked to the path.
diff --git a/internal/serving/disk/zip/serving.go b/internal/serving/disk/zip/serving.go
index 42cc8f0c..63578379 100644
--- a/internal/serving/disk/zip/serving.go
+++ b/internal/serving/disk/zip/serving.go
@@ -8,7 +8,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
-var instance = disk.New(vfs.Instrumented(zip.New(), "zip"), metrics.ZipServingFileSize)
+var instance = disk.New(vfs.Instrumented(zip.New("zip")), metrics.VFSServingFileSize)
 
 // Instance returns a serving instance that is capable of reading files
 // from a zip archives opened from a URL, most likely stored in object storage
diff --git a/internal/testhelpers/tmpdir.go b/internal/testhelpers/tmpdir.go
index 85201238..89223621 100644
--- a/internal/testhelpers/tmpdir.go
+++ b/internal/testhelpers/tmpdir.go
@@ -13,7 +13,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs/local"
 )
 
-var fs = vfs.Instrumented(&local.VFS{}, "local")
+var fs = vfs.Instrumented(local.New("local"))
 
 func TmpDir(t *testing.T, pattern string) (vfs.Root, string, func()) {
 	tmpDir, err := ioutil.TempDir("", pattern)
diff --git a/internal/vfs/local/vfs.go b/internal/vfs/local/vfs.go
index a2eb14f7..0ece253d 100644
--- a/internal/vfs/local/vfs.go
+++ b/internal/vfs/local/vfs.go
@@ -11,7 +11,16 @@ import (
 
 var errNotDirectory = errors.New("path needs to be a directory")
 
-type VFS struct{}
+type VFS struct {
+	name string
+}
+
+// New instance of VFS
+func New(name string) *VFS {
+	return &VFS{
+		name: name,
+	}
+}
 
 func (fs VFS) Root(ctx context.Context, path string) (vfs.Root, error) {
 	rootPath, err := filepath.Abs(path)
@@ -35,3 +44,7 @@ func (fs VFS) Root(ctx context.Context, path string) (vfs.Root, error) {
 
 	return &Root{rootPath: rootPath}, nil
 }
+
+func (fs *VFS) Name() string {
+	return fs.name
+}
diff --git a/internal/vfs/local/vfs_test.go b/internal/vfs/local/vfs_test.go
index ec67d595..0c56cbbc 100644
--- a/internal/vfs/local/vfs_test.go
+++ b/internal/vfs/local/vfs_test.go
@@ -11,7 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var localVFS = &VFS{}
+var localVFS = New("local_test")
 
 func tmpDir(t *testing.T) (string, func()) {
 	tmpDir, err := ioutil.TempDir("", "vfs")
diff --git a/internal/vfs/vfs.go b/internal/vfs/vfs.go
index 9d9551a1..bd87dc7c 100644
--- a/internal/vfs/vfs.go
+++ b/internal/vfs/vfs.go
@@ -12,10 +12,11 @@ import (
 // VFS abstracts the things Pages needs to serve a static site from disk.
 type VFS interface {
 	Root(ctx context.Context, path string) (Root, error)
+	Name() string
 }
 
-func Instrumented(fs VFS, name string) VFS {
-	return &instrumentedVFS{fs: fs, name: name}
+func Instrumented(fs VFS) VFS {
+	return &instrumentedVFS{fs: fs, name: fs.Name()}
 }
 
 type instrumentedVFS struct {
@@ -46,3 +47,7 @@ func (i *instrumentedVFS) Root(ctx context.Context, path string) (Root, error) {
 
 	return &instrumentedRoot{root: root, name: i.name, rootPath: path}, nil
 }
+
+func (i *instrumentedVFS) Name() string {
+	return i.name
+}
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index dfa2f38e..b3d4429b 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -23,9 +23,19 @@ var (
 
 // zipVFS is a simple cached implementation of the vfs.VFS interface
 type zipVFS struct {
+	name  string
 	cache *cache.Cache
 }
 
+// New creates a zipVFS instance that can be used by a serving request
+func New(name string) vfs.VFS {
+	return &zipVFS{
+		name: name,
+		// TODO: add cache operation callbacks https://gitlab.com/gitlab-org/gitlab-pages/-/issues/465
+		cache: cache.New(cacheExpirationInterval, cacheRefreshInterval),
+	}
+}
+
 // Root opens an archive given a URL path and returns an instance of zipArchive
 // that implements the vfs.VFS interface.
 // To avoid using locks, the findOrOpenArchive function runs inside of a for
@@ -45,12 +55,8 @@ func (fs *zipVFS) Root(ctx context.Context, path string) (vfs.Root, error) {
 	}
 }
 
-// New creates a zipVFS instance that can be used by a serving request
-func New() vfs.VFS {
-	return &zipVFS{
-		// TODO: add cache operation callbacks https://gitlab.com/gitlab-org/gitlab-pages/-/issues/465
-		cache: cache.New(cacheExpirationInterval, cacheRefreshInterval),
-	}
+func (fs *zipVFS) Name() string {
+	return fs.name
 }
 
 // findOrOpenArchive if found in fs.cache refresh if needed and return it.
diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go
index 5e1e8b65..7b27b632 100644
--- a/internal/vfs/zip/vfs_test.go
+++ b/internal/vfs/zip/vfs_test.go
@@ -25,7 +25,7 @@ func TestVFSRoot(t *testing.T) {
 		},
 	}
 
-	testZipVFS := New()
+	testZipVFS := New("zip_test")
 
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
diff --git a/metrics/metrics.go b/metrics/metrics.go
index 0f532144..4754bfb2 100644
--- a/metrics/metrics.go
+++ b/metrics/metrics.go
@@ -78,21 +78,13 @@ var (
 		Help: "The time (in seconds) it takes to get a response from the GitLab domains API",
 	}, []string{"status_code"})
 
-	// DiskServingFileSize metric for file size serving.
-	DiskServingFileSize = prometheus.NewHistogram(prometheus.HistogramOpts{
+	// VFSServingFileSize metric for file size serving. Includes a vfs_name (local or zip).
+	VFSServingFileSize = prometheus.NewHistogramVec(prometheus.HistogramOpts{
 		Name: "gitlab_pages_disk_serving_file_size_bytes",
 		Help: "The size in bytes for each file that has been served",
 		// From 1B to 100MB in *10 increments (1 10 100 1,000 10,000 100,000 1'000,000 10'000,000 100'000,000)
 		Buckets: prometheus.ExponentialBuckets(1.0, 10.0, 9),
-	})
-
-	// ZipServingFileSize metric for file size serving.
-	ZipServingFileSize = prometheus.NewHistogram(prometheus.HistogramOpts{
-		Name: "gitlab_pages_zip_serving_file_size_bytes",
-		Help: "The size in bytes for each file that has been served",
-		// From 1B to 100MB in *10 increments (1 10 100 1,000 10,000 100,000 1'000,000 10'000,000 100'000,000)
-		Buckets: prometheus.ExponentialBuckets(1.0, 10.0, 9),
-	})
+	}, []string{"vfs_name"})
 
 	// ServingTime metric for time taken to find a file serving it or not found.
 	ServingTime = prometheus.NewHistogram(prometheus.HistogramOpts{
@@ -137,7 +129,7 @@ func MustRegister() {
 		DomainsSourceFailures,
 		ServerlessRequests,
 		ServerlessLatency,
-		DiskServingFileSize,
+		VFSServingFileSize,
 		ServingTime,
 		VFSOperations,
 		ObjectStorageBackendReqTotal,