embedded cache: update metric namespace and remove duplicate metrics (g…

…rafana#10693) **What this PR does / why we need it**: Removes the following embedded cache metrics since `loki_cache_request_duration_seconds` (which instruments requests made to all the caches) [already covers them](https://github.com/grafana/loki/blob/6a07988491124c91d4acb1cbbf2ee2a4d7dc8900/pkg/storage/chunk/cache/cache.go#L110): - `querier_cache_added_total` - `querier_cache_gets_total` - `querier_cache_misses_total` Updates the namespace and subsystem for the following metrics - `querier_cache_added_new_total` is renamed to `loki_embeddedcache_added_new_total` - `querier_cache_evicted_total` is renamed to `loki_embeddedcache_evicted_total` - `querier_cache_entries` is renamed to `loki_embeddedcache_entries` - `querier_cache_memory_bytes` is renamed to `loki_embeddedcache_memory_bytes` Removes already deprecated metric `querier_cache_stale_gets_total` **Which issue(s) this PR fixes**: Fixes #<issue number> **Special notes for your reviewer**: **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [x] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](grafana@d10549e)
JStickler · Sep 29, 2023 · fcdd55b · fcdd55b
1 parent 7db7a96
commit fcdd55b
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 * [10534](https://github.com/grafana/loki/pull/10534) **chaudum** Remove configuration `use_boltdb_shipper_as_backup`
 * [10620](https://github.com/grafana/loki/pull/10620) **ashwanthgoli** Enable embedded cache if no other cache is explicitly enabled.
 * [10655](https://github.com/grafana/loki/pull/10655) **chaudum** Remove legacy ingester shutdown handler `/ingester/flush_shutdown`.
+* [10693](https://github.com/grafana/loki/pull/10693) **ashwanthgoli** Embedded cache: Updates the metric prefix from `querier_cache_` to `loki_embeddedcache_` and removes duplicate metrics.
 
 ##### Fixes
 

diff --git a/docs/sources/setup/upgrade/_index.md b/docs/sources/setup/upgrade/_index.md
@@ -69,6 +69,21 @@ This new metric will provide a more clear signal that there is an issue with ing
 
 1. `frontend.embedded-cache.max-size-mb` Embedded results cache size now defaults to 100MB.
 
+#### Embedded cache metric changes
+
+- The following embedded cache metrics are removed. Instead use `loki_cache_fetched_keys`, `loki_cache_hits`, `loki_cache_request_duration_seconds` which instruments requests made to the configured cache (`embeddedcache`, `memcached` or `redis`).
+  - `querier_cache_added_total`
+  - `querier_cache_gets_total`
+  - `querier_cache_misses_total`
+
+- The following embedded cache metrics are renamed:
+  - `querier_cache_added_new_total` is renamed to `loki_embeddedcache_added_new_total`
+  - `querier_cache_evicted_total` is renamed to `loki_embeddedcache_evicted_total`
+  - `querier_cache_entries` is renamed to `loki_embeddedcache_entries`
+  - `querier_cache_memory_bytes` is renamed to `loki_embeddedcache_memory_bytes`
+
+- Already deprecated metric `querier_cache_stale_gets_total` is now removed.
+
 ## 2.9.0
 
 ### Loki

diff --git a/integration/loki_micro_services_test.go b/integration/loki_micro_services_test.go
@@ -734,17 +734,24 @@ func assertCacheState(t *testing.T, metrics string, e *expectedCacheState) {
 		},
 	}
 
-	mf, found := mfs["querier_cache_added_new_total"]
+	mf, found := mfs["loki_embeddedcache_added_new_total"]
 	require.True(t, found)
 	require.Equal(t, e.added, getValueFromMF(mf, lbs))
 
-	mf, found = mfs["querier_cache_gets_total"]
+	lbs = []*dto.LabelPair{
+		{
+			Name:  proto.String("name"),
+			Value: proto.String(e.cacheName),
+		},
+	}
+
+	mf, found = mfs["loki_cache_fetched_keys"]
 	require.True(t, found)
 	require.Equal(t, e.gets, getValueFromMF(mf, lbs))
 
-	mf, found = mfs["querier_cache_misses_total"]
+	mf, found = mfs["loki_cache_hits"]
 	require.True(t, found)
-	require.Equal(t, e.misses, getValueFromMF(mf, lbs))
+	require.Equal(t, e.gets-e.misses, getValueFromMF(mf, lbs))
 }
 
 type expectedCacheState struct {

diff --git a/pkg/storage/chunk/cache/embeddedcache.go b/pkg/storage/chunk/cache/embeddedcache.go
@@ -47,13 +47,9 @@ type EmbeddedCache struct {
 
 	done chan struct{}
 
-	entriesAdded    prometheus.Counter
 	entriesAddedNew prometheus.Counter
 	entriesEvicted  *prometheus.CounterVec
 	entriesCurrent  prometheus.Gauge
-	totalGets       prometheus.Counter
-	totalMisses     prometheus.Counter
-	staleGets       prometheus.Counter
 	memoryBytes     prometheus.Gauge
 }
 
@@ -110,65 +106,33 @@ func NewEmbeddedCache(name string, cfg EmbeddedCacheConfig, reg prometheus.Regis
 
 		done: make(chan struct{}),
 
-		entriesAdded: promauto.With(reg).NewCounter(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
-			Name:        "added_total",
-			Help:        "The total number of Put calls on the cache",
-			ConstLabels: prometheus.Labels{"cache": name},
-		}),
-
 		entriesAddedNew: promauto.With(reg).NewCounter(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
+			Namespace:   "loki",
+			Subsystem:   "embeddedcache",
 			Name:        "added_new_total",
 			Help:        "The total number of new entries added to the cache",
 			ConstLabels: prometheus.Labels{"cache": name},
 		}),
 
 		entriesEvicted: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
+			Namespace:   "loki",
+			Subsystem:   "embeddedcache",
 			Name:        "evicted_total",
 			Help:        "The total number of evicted entries",
 			ConstLabels: prometheus.Labels{"cache": name},
 		}, []string{"reason"}),
 
 		entriesCurrent: promauto.With(reg).NewGauge(prometheus.GaugeOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
+			Namespace:   "loki",
+			Subsystem:   "embeddedcache",
 			Name:        "entries",
-			Help:        "The total number of entries",
-			ConstLabels: prometheus.Labels{"cache": name},
-		}),
-
-		totalGets: promauto.With(reg).NewCounter(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
-			Name:        "gets_total",
-			Help:        "The total number of Get calls",
-			ConstLabels: prometheus.Labels{"cache": name},
-		}),
-
-		totalMisses: promauto.With(reg).NewCounter(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
-			Name:        "misses_total",
-			Help:        "The total number of Get calls that had no valid entry",
-			ConstLabels: prometheus.Labels{"cache": name},
-		}),
-
-		staleGets: promauto.With(reg).NewCounter(prometheus.CounterOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
-			Name:        "stale_gets_total",
-			Help:        "The total number of Get calls that had an entry which expired (deprecated)",
+			Help:        "Current number of entries in the cache",
 			ConstLabels: prometheus.Labels{"cache": name},
 		}),
 
 		memoryBytes: promauto.With(reg).NewGauge(prometheus.GaugeOpts{
-			Namespace:   "querier",
-			Subsystem:   "cache",
+			Namespace:   "loki",
+			Subsystem:   "embeddedcache",
 			Name:        "memory_bytes",
 			Help:        "The current cache size in bytes",
 			ConstLabels: prometheus.Labels{"cache": name},
@@ -231,8 +195,6 @@ func (c *EmbeddedCache) Fetch(ctx context.Context, keys []string) (found []strin
 
 // Store implements Cache.
 func (c *EmbeddedCache) Store(_ context.Context, keys []string, values [][]byte) error {
-	c.entriesAdded.Inc()
-
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -314,8 +276,6 @@ func (c *EmbeddedCache) put(key string, value []byte) {
 
 // Get returns the stored value against the key and when the key was last updated.
 func (c *EmbeddedCache) Get(_ context.Context, key string) ([]byte, bool) {
-	c.totalGets.Inc()
-
 	c.lock.RLock()
 	defer c.lock.RUnlock()
 
@@ -325,7 +285,6 @@ func (c *EmbeddedCache) Get(_ context.Context, key string) ([]byte, bool) {
 		return entry.value, true
 	}
 
-	c.totalMisses.Inc()
 	return nil, false
 }
 

diff --git a/pkg/storage/chunk/cache/embeddedcache_test.go b/pkg/storage/chunk/cache/embeddedcache_test.go
@@ -64,14 +64,11 @@ func TestEmbeddedCacheEviction(t *testing.T) {
 
 		reason := fullReason
 
-		assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(1))
 		assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(0))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries)))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len()))
-		assert.Equal(t, testutil.ToFloat64(c.totalGets), float64(0))
-		assert.Equal(t, testutil.ToFloat64(c.totalMisses), float64(0))
 		assert.Equal(t, testutil.ToFloat64(c.memoryBytes), float64(cnt*sizeOf(itemTemplate)))
 
 		for i := 0; i < cnt; i++ {
@@ -81,14 +78,11 @@ func TestEmbeddedCacheEviction(t *testing.T) {
 			require.Equal(t, []byte(key), value)
 		}
 
-		assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(1))
 		assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesEvicted), float64(0))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries)))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len()))
-		assert.Equal(t, testutil.ToFloat64(c.totalGets), float64(cnt))
-		assert.Equal(t, testutil.ToFloat64(c.totalMisses), float64(0))
 		assert.Equal(t, testutil.ToFloat64(c.memoryBytes), float64(cnt*sizeOf(itemTemplate)))
 
 		// Check evictions
@@ -105,14 +99,11 @@ func TestEmbeddedCacheEviction(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, c.entries, cnt)
 
-		assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(2))
 		assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries)))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len()))
-		assert.Equal(t, testutil.ToFloat64(c.totalGets), float64(cnt))
-		assert.Equal(t, testutil.ToFloat64(c.totalMisses), float64(0))
 		assert.Equal(t, testutil.ToFloat64(c.memoryBytes), float64(cnt*sizeOf(itemTemplate)))
 
 		for i := 0; i < cnt-evicted; i++ {
@@ -126,14 +117,11 @@ func TestEmbeddedCacheEviction(t *testing.T) {
 			require.Equal(t, []byte(key), value)
 		}
 
-		assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(2))
 		assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries)))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len()))
-		assert.Equal(t, testutil.ToFloat64(c.totalGets), float64(cnt*2+evicted))
-		assert.Equal(t, testutil.ToFloat64(c.totalMisses), float64(cnt-evicted))
 		assert.Equal(t, testutil.ToFloat64(c.memoryBytes), float64(cnt*sizeOf(itemTemplate)))
 
 		// Check updates work
@@ -156,14 +144,11 @@ func TestEmbeddedCacheEviction(t *testing.T) {
 			require.Equal(t, []byte(fmt.Sprintf("%02d", i*2)), value)
 		}
 
-		assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(3))
 		assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries)))
 		assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len()))
-		assert.Equal(t, testutil.ToFloat64(c.totalGets), float64(cnt*2+evicted*2))
-		assert.Equal(t, testutil.ToFloat64(c.totalMisses), float64(cnt-evicted))
 		assert.Equal(t, testutil.ToFloat64(c.memoryBytes), float64(cnt*sizeOf(itemTemplate)))
 
 		c.Stop()
@@ -197,31 +182,25 @@ func TestEmbeddedCacheExpiry(t *testing.T) {
 	_, ok = c.Get(ctx, key1)
 	require.False(t, ok)
 
-	assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesAdded))
 	assert.Equal(t, float64(4), testutil.ToFloat64(c.entriesAddedNew))
 	assert.Equal(t, float64(0), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(expiredReason)))
 	assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(fullReason)))
 	assert.Equal(t, float64(3), testutil.ToFloat64(c.entriesCurrent))
 	assert.Equal(t, float64(len(c.entries)), testutil.ToFloat64(c.entriesCurrent))
 	assert.Equal(t, float64(c.lru.Len()), testutil.ToFloat64(c.entriesCurrent))
-	assert.Equal(t, float64(2), testutil.ToFloat64(c.totalGets))
-	assert.Equal(t, float64(1), testutil.ToFloat64(c.totalMisses))
 	assert.Equal(t, float64(memorySz), testutil.ToFloat64(c.memoryBytes))
 
 	// Expire the item.
 	time.Sleep(2 * cfg.TTL)
 	_, ok = c.Get(ctx, key4)
 	require.False(t, ok)
 
-	assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesAdded))
 	assert.Equal(t, float64(4), testutil.ToFloat64(c.entriesAddedNew))
 	assert.Equal(t, float64(3), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(expiredReason)))
 	assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(fullReason)))
 	assert.Equal(t, float64(0), testutil.ToFloat64(c.entriesCurrent))
 	assert.Equal(t, float64(len(c.entries)), testutil.ToFloat64(c.entriesCurrent))
 	assert.Equal(t, float64(c.lru.Len()), testutil.ToFloat64(c.entriesCurrent))
-	assert.Equal(t, float64(3), testutil.ToFloat64(c.totalGets))
-	assert.Equal(t, float64(2), testutil.ToFloat64(c.totalMisses))
 	assert.Equal(t, float64(memorySz), testutil.ToFloat64(c.memoryBytes))
 
 	c.Stop()