Merge pull request #880 from fluxcd/cache-custom-metric-labels

Add option for custom event namespace label in cache metrics

Author: matheuscscp

auth/go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.2
 	github.com/bradleyfalzon/ghinstallation/v2 v2.14.0
-	github.com/fluxcd/pkg/cache v0.5.0
+	github.com/fluxcd/pkg/cache v0.6.0
 	github.com/fluxcd/pkg/ssh v0.17.0
 	github.com/onsi/gomega v1.36.2
 	golang.org/x/net v0.35.0

cache/cache.go

@@ -85,7 +85,7 @@ func New[T any](capacity int, opts ...Options) (*Cache[T], error) {
 	}
 
 	if opt.registerer != nil {
-		c.metrics = newCacheMetrics(opt.metricsPrefix, opt.registerer)
+		c.metrics = newCacheMetrics(opt.metricsPrefix, opt.registerer, opts...)
 	}
 
 	C := &Cache[T]{cache: c}

cache/lru.go

@@ -97,7 +97,7 @@ func NewLRU[T any](capacity int, opts ...Options) (*LRU[T], error) {
 	}
 
 	if opt.registerer != nil {
-		lru.metrics = newCacheMetrics(opt.metricsPrefix, opt.registerer)
+		lru.metrics = newCacheMetrics(opt.metricsPrefix, opt.registerer, opts...)
 	}
 
 	return lru, nil
@@ -139,7 +139,7 @@ func (c *LRU[T]) GetIfOrSet(ctx context.Context,
 	opts ...Options,
 ) (value T, ok bool, err error) {
 
-	var evicted bool
+	var existed, evicted bool
 
 	c.mu.Lock()
 	defer func() {
@@ -150,21 +150,22 @@ func (c *LRU[T]) GetIfOrSet(ctx context.Context,
 
 		// Record metrics.
 		status := StatusSuccess
-		event := CacheEventTypeMiss
-		switch {
-		case ok:
-			event = CacheEventTypeHit
-		case evicted:
-			recordEviction(c.metrics)
-		case err == nil:
-			recordItemIncrement(c.metrics)
-		default:
+		if err != nil {
 			status = StatusFailure
 		}
 		recordRequest(c.metrics, status)
+		event := CacheEventTypeMiss
+		if ok {
+			event = CacheEventTypeHit
+		}
 		if obj := o.involvedObject; obj != nil {
 			c.RecordCacheEvent(event, obj.Kind, obj.Name, obj.Namespace)
 		}
+		if evicted {
+			recordEviction(c.metrics)
+		} else if !existed && err == nil {
+			recordItemIncrement(c.metrics)
+		}
 
 		// Print debug logs. The involved object should already be set in the context logger.
 		switch l := logr.FromContextOrDiscard(ctx).V(1).WithValues("key", key); {
@@ -180,16 +181,15 @@ func (c *LRU[T]) GetIfOrSet(ctx context.Context,
 	}()
 
 	var curNode *node[T]
-	curNode, ok = c.cache[key]
+	curNode, existed = c.cache[key]
 
-	if ok {
+	if existed {
 		c.delete(curNode)
 		if condition(curNode.value) {
 			_ = c.add(curNode)
-			value = curNode.value
+			value, ok = curNode.value, true
 			return
 		}
-		ok = false
 	}
 
 	value, err = fetch(ctx)

cache/metrics.go

@@ -43,8 +43,11 @@ type cacheMetrics struct {
 }
 
 // newcacheMetrics returns a new cacheMetrics.
-func newCacheMetrics(prefix string, reg prometheus.Registerer) *cacheMetrics {
-	labels := []string{"event_type", "kind", "name", "namespace"}
+func newCacheMetrics(prefix string, reg prometheus.Registerer, opts ...Options) *cacheMetrics {
+	o := storeOptions{eventNamespaceLabel: "namespace"}
+	o.apply(opts...)
+
+	labels := []string{"event_type", "kind", "name", o.eventNamespaceLabel}
 	return &cacheMetrics{
 		cacheEventsCounter: promauto.With(reg).NewCounterVec(
 			prometheus.CounterOpts{

cache/metrics_test.go

@@ -18,6 +18,7 @@ package cache
 
 import (
 	"bytes"
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -26,22 +27,39 @@ import (
 )
 
 func TestCacheMetrics(t *testing.T) {
-	g := NewWithT(t)
-	reg := prometheus.NewPedanticRegistry()
-	m := newCacheMetrics("gotk_", reg)
-	g.Expect(m).ToNot(BeNil())
+	for _, tt := range []struct {
+		name        string
+		opts        []Options
+		eventLabels string
+	}{
+		{
+			name:        "default event namespace label",
+			eventLabels: `kind="TestObject",name="test",namespace="test-ns"`,
+		},
+		{
+			name:        "custom event namespace label",
+			opts:        []Options{WithEventNamespaceLabel("exported_namespace")},
+			eventLabels: `exported_namespace="test-ns",kind="TestObject",name="test"`,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			reg := prometheus.NewPedanticRegistry()
+			m := newCacheMetrics("gotk_", reg, tt.opts...)
+			g.Expect(m).ToNot(BeNil())
 
-	// CounterVec is a collection of counters and is not exported until it has counters in it.
-	m.incCacheEvents(CacheEventTypeHit, []string{"TestObject", "test", "test-ns"}...)
-	m.incCacheEvents(CacheEventTypeMiss, []string{"TestObject", "test", "test-ns"}...)
-	m.incCacheRequests("success")
-	m.incCacheRequests("failure")
+			// CounterVec is a collection of counters and is not exported until it has counters in it.
+			m.incCacheEvents(CacheEventTypeHit, []string{"TestObject", "test", "test-ns"}...)
+			m.incCacheEvents(CacheEventTypeMiss, []string{"TestObject", "test", "test-ns"}...)
+			m.incCacheRequests("success")
+			m.incCacheRequests("failure")
 
-	validateMetrics(reg, `
+			validateMetrics(reg, fmt.Sprintf(`
 		# HELP gotk_cache_events_total Total number of cache retrieval events for a Gitops Toolkit resource reconciliation.
 		# TYPE gotk_cache_events_total counter
-		gotk_cache_events_total{event_type="cache_hit",kind="TestObject",name="test",namespace="test-ns"} 1
-		gotk_cache_events_total{event_type="cache_miss",kind="TestObject",name="test",namespace="test-ns"} 1
+		gotk_cache_events_total{event_type="cache_hit",%[1]s} 1
+		gotk_cache_events_total{event_type="cache_miss",%[1]s} 1
 		# HELP gotk_cache_evictions_total Total number of cache evictions.
 		# TYPE gotk_cache_evictions_total counter
 		gotk_cache_evictions_total 0
@@ -52,11 +70,13 @@ func TestCacheMetrics(t *testing.T) {
 		# HELP gotk_cached_items Total number of items in the cache.
 		# TYPE gotk_cached_items gauge
 		gotk_cached_items 0
-	`, t)
+	`, tt.eventLabels), t)
 
-	res, err := testutil.GatherAndLint(reg)
-	g.Expect(err).ToNot(HaveOccurred())
-	g.Expect(res).To(BeEmpty())
+			res, err := testutil.GatherAndLint(reg)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(res).To(BeEmpty())
+		})
+	}
 }
 
 func validateMetrics(reg prometheus.Gatherer, expected string, t *testing.T) {

cache/store.go

@@ -44,13 +44,14 @@ type Expirable[T any] interface {
 }
 
 type storeOptions struct {
-	interval       time.Duration
-	registerer     prometheus.Registerer
-	metricsPrefix  string
-	maxDuration    time.Duration
-	involvedObject *InvolvedObject
-	debugKey       string
-	debugValueFunc func(any) any
+	interval            time.Duration
+	registerer          prometheus.Registerer
+	metricsPrefix       string
+	maxDuration         time.Duration
+	involvedObject      *InvolvedObject
+	debugKey            string
+	debugValueFunc      func(any) any
+	eventNamespaceLabel string
 }
 
 func (o *storeOptions) apply(opts ...Options) error {
@@ -108,3 +109,11 @@ func WithInvolvedObject(kind, name, namespace string) Options {
 		return nil
 	}
 }
+
+// WithEventNamespaceLabel sets the namespace label for the cache events.
+func WithEventNamespaceLabel(label string) Options {
+	return func(o *storeOptions) error {
+		o.eventNamespaceLabel = label
+		return nil
+	}
+}

cache/token.go

@@ -103,7 +103,7 @@ func (c *TokenCache) GetOrSet(ctx context.Context,
 		so.debugKey = "token"
 		so.debugValueFunc = func(v any) any {
 			return map[string]any{
-				"duration": v.(*tokenItem).token.GetDuration(),
+				"duration": v.(*tokenItem).token.GetDuration().String(),
 			}
 		}
 		return nil

git/go.mod

@@ -25,7 +25,7 @@ require (
 	github.com/bradleyfalzon/ghinstallation/v2 v2.14.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cloudflare/circl v1.5.0 // indirect
-	github.com/fluxcd/pkg/cache v0.5.0 // indirect
+	github.com/fluxcd/pkg/cache v0.6.0 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
 	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect

git/gogit/go.mod

@@ -41,7 +41,7 @@ require (
 	github.com/cloudflare/circl v1.5.0 // indirect
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
-	github.com/fluxcd/pkg/cache v0.5.0 // indirect
+	github.com/fluxcd/pkg/cache v0.6.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/gofrs/uuid v4.4.0+incompatible // indirect

git/internal/e2e/go.mod

@@ -42,7 +42,7 @@ require (
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/fluxcd/gitkit v0.6.0 // indirect
-	github.com/fluxcd/pkg/cache v0.5.0 // indirect
+	github.com/fluxcd/pkg/cache v0.6.0 // indirect
 	github.com/fluxcd/pkg/version v0.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect