fix(customresourcestate): handle nil path gracefully in StateSet metrics#2884
fix(customresourcestate): handle nil path gracefully in StateSet metrics#2884Br1an67 wants to merge 1 commit intokubernetes:mainfrom
Conversation
When CustomResourceDefinition status fields don't exist at resource creation time, StateSet metrics would previously log an error for each resource instance. This caused error spam for CRDs with many resources. Now, when the path resolves to nil (field doesn't exist), StateSet returns empty results instead of an error, consistent with how Gauge handles nil values when NilIsZero is false.
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Br1an67 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 |
k8s-ci-robot
added
needs-triage
|
This issue is currently awaiting triage. If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the The 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. |
k8s-ci-robot
added
the
size/S
|
@Br1an67 can you sign the CLA? |
Br1an67
force-pushed
the
fix/issue-2482-crd-status-nil-handling
branch
from
17ed36a to
24da992
Compare
k8s-ci-robot
added
cncf-cla: yes
There was a problem hiding this comment.
Pull request overview
Adjusts custom resource StateSet metric evaluation to avoid error spam when a configured path resolves to nil (common for CRD status fields during resource creation).
Changes:
- Update
compiledStateSet.values()to return no results/no error when the resolved value isnil. - Add a unit test covering the “StateSet nil path” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/customresourcestate/registry_factory.go | Treat nil resolved StateSet values as “no data” instead of an error. |
| pkg/customresourcestate/registry_factory_test.go | Add regression test to ensure missing StateSet paths don’t produce errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if comparable == nil { | ||
| return []eachValue{}, nil | ||
| } | ||
| return []eachValue{}, []error{fmt.Errorf("%s: expected value for path to be string, got %T", c.path, comparable)} |
| {name: "stateset nil path", each: &compiledStateSet{ | ||
| compiledCommon: compiledCommon{ | ||
| path: mustCompilePath(t, "does", "not", "exist"), | ||
| }, | ||
| LabelName: "phase", | ||
| List: []string{"foo", "bar"}, | ||
| }, wantResult: []eachValue{}, wantErrors: nil}, | ||
| {name: "stateset non-string value", each: &compiledStateSet{ | ||
| compiledCommon: compiledCommon{ | ||
| path: mustCompilePath(t, "spec", "replicas"), | ||
| }, | ||
| LabelName: "phase", | ||
| List: []string{"1", "2"}, | ||
| }, wantResult: []eachValue{}, wantErrors: []error{ | ||
| errors.New("[spec,replicas]: expected value for path to be string, got float64"), | ||
| }}, |
Fixes #2482
What this PR does / why we need it:
When CustomResourceDefinition status fields don't exist at resource creation time, StateSet metrics would previously log an error for each resource instance. This caused error spam like:
Status fields are not guaranteed to exist at resource creation, so this behavior was inconsistent with known types where nil values are handled gracefully (e.g., Gauge with
NilIsZero).This PR modifies
compiledStateSet.values()to return empty results instead of an error when the path resolves to nil, consistent with how Gauge handles nil values.How does this change affect the cardinality of KSM: Does not change cardinality
Which issue(s) this PR fixes:
Fixes #2482