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
release-22.2.0: metrics: add tsdb persistence to AggHistogram #91410
release-22.2.0: metrics: add tsdb persistence to AggHistogram #91410
Conversation
Thanks for opening a backport. Please check the backport criteria before merging:
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
Add a brief release justification to the body of your PR to justify this backport. Some other things to consider:
|
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @rafiss)
340a937
to
58204f0
Compare
Added port of test fixes from #88076 and added a note to that regard to PR+commit. |
Previously, `AggHistogram` instances would not persist their quantiles to tsdb due to a missing interface implementation of `metrics.WindowedHistogram`. This PR adds a trivial implementation that delegates to the aggregate histogram instance within the struct. This is relatively safe to do even though an `AggHistogram` could contain many children because we are only exporting a single set of aggregate quantiles per-`AggHistogram`. The children are only iterated over via the `PrometheusIterable` interface which is used by the prometheus exporter, but not by the metrics recorder. NOTE: This backported commit includes a test fix to the `kv` package that was lifted from cockroachdb#88076. Release note (bug fix, ops change): Previously, certain aggregate histograms would appear in `_status/vars` but not be available for graphing in the DB Console. These are now made available. They include changefeed-related histograms, and row-level-TTL histograms. Epic: None
58204f0
to
1cc9428
Compare
Backport 1/1 commits from #90769.
/cc @cockroachdb/release
Previously,
AggHistogram
instances would not persist their quantiles to tsdb due to a missing interface implementation ofmetrics.WindowedHistogram
. This PR adds a trivial implementation that delegates to the aggregate histogram instance within the struct.This is relatively safe to do even though an
AggHistogram
could contain many children because we are only exporting a single set of aggregate quantiles per-AggHistogram
. The children are only iterated over via thePrometheusIterable
interface which is used by the prometheus exporter, but not by the metrics recorder.NOTE: This backported commit includes a test fix to the
kv
package that waslifted from #88076.
Release note (bug fix, ops change): Previously, certain aggregate histograms would appear in
_status/vars
but not be available for graphing in the DB Console. These are now made available. They include changefeed-related histograms, and row-level-TTL histograms.Epic: None
Release Justification: Category 3: Fixes for high-priority or high-severity bugs in existing functionality