From fcdd55b6c90718252f4c9b45e60ac09b8e568fcb Mon Sep 17 00:00:00 2001 From: Ashwanth Date: Fri, 29 Sep 2023 10:53:48 +0530 Subject: [PATCH] embedded cache: update metric namespace and remove duplicate metrics (#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 # **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](https://github.com/grafana/loki/commit/d10549e3ece02120974929894ee333d07755d213) --- CHANGELOG.md | 1 + docs/sources/setup/upgrade/_index.md | 15 +++++ integration/loki_micro_services_test.go | 15 +++-- pkg/storage/chunk/cache/embeddedcache.go | 59 +++---------------- pkg/storage/chunk/cache/embeddedcache_test.go | 21 ------- 5 files changed, 36 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9a7c8af187d5..09dc62b4cef16 100644 --- 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 index 9588aaebc1a7a..9b0fd3038eeda 100644 --- 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 index 00df38279ec62..33323e2af0696 100644 --- 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 index fb9c8abc148ea..2536fed0cc864 100644 --- 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 index 08f71353fd2d0..33b933456a332 100644 --- 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,15 +182,12 @@ 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. @@ -213,15 +195,12 @@ func TestEmbeddedCacheExpiry(t *testing.T) { _, 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()