Split cache set/get and add metrics

1 files changed, 50 insertions, 21 deletions

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