Create k8s-metrics-reviewer.md for SIG-Instrumentation - #9035
Create k8s-metrics-reviewer.md for SIG-Instrumentation#9035CatherineF-dev wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: CatherineF-dev The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@CatherineF-dev: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: CatherineF-dev The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| @@ -0,0 +1,34 @@ | |||
| # SIG-Instrumentation Agent Skills | |||
There was a problem hiding this comment.
This README seems specific to only metrics but contributors/devel/sig-instrumentation contains logging, events and traces style guides too
|
/lgtm |
| - Buckets should match the realistic value range and be powered/spaced sensibly (often | ||
| `prometheus.ExponentialBuckets` / `DefBuckets`). Flag too-few buckets, buckets in the wrong unit, | ||
| or buckets that don't cover the expected range. | ||
| - **Limit the number of buckets:** A classic histogram should have a reasonable number of buckets (typically ≤ 30-50 buckets). If a metric defines too many manual buckets, flag it as a **BLOCKER/QUESTION** to protect against memory explosion. |
There was a problem hiding this comment.
Typical is between 10 and 15. 30-50 is a lot
| - Guarantees to enforce in review: | ||
| - `ALPHA` / `INTERNAL`: no guarantees, may change/delete anytime. | ||
| - `BETA`: labels may be **added** but **not removed**; ~1 release / 4 months min lifetime. | ||
| - `STABLE`: name, type, labels, buckets are **frozen** — no labels added or removed; only change |
There was a problem hiding this comment.
Buckets being frozen is correct since modifying them is as much of a breaking change as modifying labels. That said, systems can evolve and we might allow it very rarely if the change actually make sense. But that might not be needed anymore with native histograms in the picture.
Add SIG-Instrumentation Metrics Review guidelines for reviewing Kubernetes metrics changes, including stability framework and naming conventions.
Which issue(s) this PR fixes:
Fixes #9036