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
Combine hit and miss cache metrics into single tagged metric #1921
Conversation
Generate changelog in
|
stats.disabled: | ||
type: meter | ||
tags: [cache] | ||
docs: | | ||
Registered cache does not have stats recording enabled, stats will always be zero. | ||
To enable cache metrics, stats recording must be enabled when constructing the cache: | ||
Caffeine.newBuilder().recordStats() |
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.
This is unused
2bf5d89
to
a5bd94f
Compare
a5bd94f
to
7a827fb
Compare
Agree that hit/miss tag makes metric easier to consume, will just need to update some dashboards anyway |
Invalidated by push of 92a0ac1
@carterkozak @schlosna can I get another +1? Looks like the changelog commit invalidated the approval. |
Released 0.87.0 |
👍 We should figure out if policybot should be invalidating on changelog |
I suspect we do this for our open source repos because commits may be authored by non-Palantir contributors. |
We already broke the
cache.hit
andcache.miss
metrics when we changed them to meters in #1897. We can take this opportunity to improve these metrics by combining them into a singlecache.request
metric tagged byresult
.This makes it easier for consumers to make queries for things like the total number of requests or the hit rate, without having to know/assume that "requests = hits + misses".