Add Prometheus Native Histogram defaults #129406
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hi @SuperQ. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/sig instrumentation |
cb8750e to
3b96014
Compare
There was a problem hiding this comment.
Just a quick comment. The default values look good in general. The reason why there aren't any in client_golang yet is that we still have to see in practice what good default values are. (I'll be back from vacations only 2025-01-07. Happy to discuss things in more detail then.)
|
/triage accepted |
|
Specifying these defaults in the Kubernetes codebase means we'd be taking on the responsibility of maintaining them. Given the wide range of ways users might want their metrics to look – which can vary significantly – these defaults might not be appropriate for everyone, esp. as was called out here? Also, since this feature is still declared experimental by Prometheus, baking in defaults could create maintenance challenges down the line as the feature evolves. Wouldn't it be more flexible and user-friendly to leave this as a configurable option only? |
Add some new default values to the metrics package for the Prometheus Native Histogram support. * Use the upstream documented recommendation for a 10% bucket factor. * Set an arbitrary limit of 160 buckets to avoid unbounded memory growth. * Set a 1 hour minimum reset interval to avoid frequent resets which would affect efficiency of the 120 sample per hour Prometheus chunks given a 30s scrape interval. See: kubernetes#128842 Signed-off-by: SuperQ <superq@gmail.com>
3b96014 to
4aa7a70
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SuperQ The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@richabanker We already set a number of defaults for other metrics, we already have to maintain them. The point of this change is to discuss and come to some consensus on these defaults in the context of Kubernetes. Having these be configuration options would be great, but we still need to set some sensible defaults for the configurations. |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
Not stale /remove-lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
This would be really nice to move forward with. Getting native histograms started in Kubernetes would help a lot of users reduce the load on their metircs collection as it reduces the cardinality and improves the data quality of Kubernetes API metrics. /remove-lifecycle rotten |
|
Hello, apologies for the delay here. The SIG has triaged this effort and identified that there are certain prerequisites that need to be met before we can move forward in this direction, the first of which is a tracking KEP. The scope and details that the KEP will entail have already been discussed and will soon be driven by one of the SIG members. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add some new default values to the metrics package for the Prometheus Native Histogram support.
Which issue(s) this PR fixes:
See: #128842
Special notes for your reviewer:
Does this PR introduce a user-facing change?
NONE
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: