New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
embedded cache: update metric namespace and remove duplicate metrics #10693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[docs team] LGTM. Just one suggestion (but only if it makes sense given the change).
- 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` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are a one-to-one replacement, to avoid confusion I'd list them next to the parameter they're replacing. (If they aren't a one-to-one replacement, then ignore this suggestion!)
- 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 removed. Instead use the new metrics, which instrument requests made to the configured cache (`embeddedcache`, `memcached` or `redis`). | |
- `querier_cache_added_total` instead use `loki_cache_fetched_keys` | |
- `querier_cache_gets_total` instead use `loki_cache_hits` | |
- `querier_cache_misses_total` instead use `loki_cache_request_duration_seconds` |
…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)
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: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 toloki_embeddedcache_added_new_total
querier_cache_evicted_total
is renamed toloki_embeddedcache_evicted_total
querier_cache_entries
is renamed toloki_embeddedcache_entries
querier_cache_memory_bytes
is renamed toloki_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
CONTRIBUTING.md
guide (required)CHANGELOG.md
updatedadd-to-release-notes
labeldocs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PR