Split cache set/get and add metrics

3 files changed, 76 insertions, 22 deletions

diff --git a/acceptance_test.go b/acceptance_test.go
index 83f09cdf..1df6abfb 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -459,7 +459,8 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) {
 	defer teardown()
 
 	// need to call an actual resource to populate certain metrics e.g. gitlab_pages_domains_source_api_requests_total
-	res, err := GetPageFromListener(t, httpListener, "zip.gitlab.io", "/index.html/")
+	res, err := GetPageFromListener(t, httpListener, "zip.gitlab.io",
+		"/symlink.html")
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, res.StatusCode)
 
@@ -496,6 +497,8 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) {
 	// zip archives
 	require.Contains(t, string(body), "gitlab_pages_zip_opened")
 	require.Contains(t, string(body), "gitlab_pages_zip_cache_requests")
+	require.Contains(t, string(body), "gitlab_pages_zip_data_offset_cache_requests")
+	require.Contains(t, string(body), "gitlab_pages_zip_readlink_cache_requests")
 	require.Contains(t, string(body), "gitlab_pages_zip_cached_archives")
 	require.Contains(t, string(body), "gitlab_pages_zip_archive_entries_cached")
 	require.Contains(t, string(body), "gitlab_pages_zip_opened_entries_count")
diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go
index 39b624da..f602d750 100644
--- a/internal/vfs/zip/archive.go
+++ b/internal/vfs/zip/archive.go
@@ -54,7 +54,6 @@ type zipArchive struct {
 	archive  *zip.Reader
 	err      error
 
-	// TODO: add metrics https://gitlab.com/gitlab-org/gitlab-pages/-/issues/423
 	files map[string]*zip.File
 }
 
@@ -168,15 +167,11 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) {
 		return nil, os.ErrNotExist
 	}
 
-	item, err := a.fs.dataOffsetCache.Fetch(a.cacheKey+name, DataOffsetCacheInterval, func() (interface{}, error) {
-		return file.DataOffset()
-	})
+	dataOffset, err := a.getDataOffset(name, file)
 	if err != nil {
 		return nil, err
 	}
 
-	dataOffset := item.Value().(int64)
-
 	// only read from dataOffset up to the size of the compressed file
 	reader := a.reader.SectionReader(ctx, dataOffset, int64(file.CompressedSize64))
 
@@ -190,6 +185,28 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) {
 	}
 }
 
+func (a *zipArchive) getDataOffset(name string, file *zip.File) (int64, error) {
+	var dataOffset int64
+	item := a.fs.dataOffsetCache.Get(a.cacheKey + name)
+	if item == nil || item.Expired() {
+		var err error
+
+		dataOffset, err = file.DataOffset()
+		if err != nil {
+			return 0, err
+		}
+
+		a.fs.dataOffsetCache.Set(a.cacheKey+name, dataOffset, DataOffsetCacheInterval)
+
+		metrics.ZipServingArchiveDataOffsetCache.WithLabelValues("miss").Inc()
+	} else {
+		dataOffset = item.Value().(int64)
+		metrics.ZipServingArchiveDataOffsetCache.WithLabelValues("hit").Inc()
+	}
+
+	return dataOffset, nil
+}
+
 // Lstat finds the file by name inside the zipArchive and returns its FileInfo
 func (a *zipArchive) Lstat(ctx context.Context, name string) (os.FileInfo, error) {
 	file := a.findFile(name)
@@ -210,35 +227,47 @@ func (a *zipArchive) Readlink(ctx context.Context, name string) (string, error)
 	if file.FileInfo().Mode()&os.ModeSymlink != os.ModeSymlink {
 		return "", errNotSymlink
 	}
+	symlink, err := a.getSymlink(name, file)
+	if err != nil {
+		return "", err
+	}
 
-	item, err := a.fs.readlinkCache.Fetch(a.cacheKey+name, ReadLinkCacheInterval, func() (interface{}, error) {
+	// return errSymlinkSize if the number of bytes read from the link is too big
+	if len(symlink) > maxSymlinkSize {
+		return "", errSymlinkSize
+	}
+
+	return symlink, nil
+}
+
+func (a *zipArchive) getSymlink(name string, file *zip.File) (string, error) {
+	var symlink string
+
+	item := a.fs.readlinkCache.Get(a.cacheKey + name)
+	if item == nil || item.Expired() {
 		rc, err := file.Open()
 		if err != nil {
-			return nil, err
+			return "", err
 		}
 		defer rc.Close()
 
-		var symlink [maxSymlinkSize + 1]byte
+		var link [maxSymlinkSize + 1]byte
 
 		// read up to len(symlink) bytes from the link file
-		n, err := io.ReadFull(rc, symlink[:])
+		n, err := io.ReadFull(rc, link[:])
 		if err != nil && err != io.ErrUnexpectedEOF {
 			// if err == io.ErrUnexpectedEOF the link is smaller than len(symlink) so it's OK to not return it
-			return nil, err
+			return "", err
 		}
 
+		symlink = string(link[:n])
 		// cache symlink up to desired size
-		return string(symlink[:n]), nil
-	})
-	if err != nil {
-		return "", err
-	}
-
-	symlink := item.Value().(string)
+		a.fs.readlinkCache.Set(a.cacheKey+name, symlink, ReadLinkCacheInterval)
 
-	// return errSymlinkSize if the number of bytes read from the link is too big
-	if len(symlink) > maxSymlinkSize {
-		return "", errSymlinkSize
+		metrics.ZipServingArchiveReadlinkCache.WithLabelValues("miss").Inc()
+	} else {
+		symlink = item.Value().(string)
+		metrics.ZipServingArchiveReadlinkCache.WithLabelValues("hit").Inc()
 	}
 
 	return symlink, nil
diff --git a/metrics/metrics.go b/metrics/metrics.go
index d6dc8ce1..d2243fae 100644
--- a/metrics/metrics.go
+++ b/metrics/metrics.go
@@ -173,6 +173,26 @@ var (
 		[]string{"cache"},
 	)
 
+	// ZipServingArchiveDataOffsetCache is the number of zip archive
+	// data offset cache hits/misses
+	ZipServingArchiveDataOffsetCache = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitlab_pages_zip_data_offset_cache_requests",
+			Help: "The number of zip archive data offset cache hits/misses",
+		},
+		[]string{"cache"},
+	)
+
+	// ZipServingArchiveReadlinkCache is the number of zip archive Readlink
+	// cache hits/misses
+	ZipServingArchiveReadlinkCache = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitlab_pages_zip_readlink_cache_requests",
+			Help: "The number of zip archives Readlink cache hits/misses",
+		},
+		[]string{"cache"},
+	)
+
 	// ZipCachedArchives is the number of zip archives currently in the cache
 	ZipCachedArchives = prometheus.NewGauge(
 		prometheus.GaugeOpts{
@@ -226,6 +246,8 @@ func MustRegister() {
 		ZipOpened,
 		ZipOpenedEntriesCount,
 		ZipServingArchiveCache,
+		ZipServingArchiveDataOffsetCache,
+		ZipServingArchiveReadlinkCache,
 		ZipArchiveEntriesCached,
 		ZipCachedArchives,
 	)