Open fabriziopandini opened 2 years ago
/triage accepted
Agree that we can host it for now in the CAPI repo. I would suggest to bring up the idea already in parallel in the kube-state-metrics repository. We can then decide if we merge it first into CAPI or kube-state-metrics depending on feedback.
Given that kube-state-metrics is also a Kubernetes project it could also be possible to add this to controller-gen. Although ideally it would land in kube-state-metrics given that this could be interesting for all users of kube-state-metrics and not only folks who are writing controllers with controller-runtime.
I'll try to do a first POC and open up a issue in kube-state-metrics repo to start discussion about it. I also see it as target in kube-state-metrics, and maybe a section here in the docs for provider guidance.
/assign
First proposal about how to model metrics with golang markers (thanks to @sbueringer and @chrischdi for helping in defining this)
Note: all the markers must have a prefix yet TBD, using Metrics
for now
Note: all the markers must comply with https://book.kubebuilder.io/reference/markers.html#marker-syntax
Note: in a follow-up iteration we might consider introducing additional markers for defining metrics once for many types
// +Metrics:namePrefix=<string> on struct
Defines the metricNamePrefix for all the metrics derived from the struct the markers apply to. e.g.
// +Metrics:namePrefix=capi_cluster
type Cluster struct { ... }
// +Metrics:labelsFromPath:name=<string>,JSONPath=<string> on struct
Defines the label that applies to all the metrics derived from the struct the markers apply to. e.g.
// +Metrics:labelsFromPath:name=name,JSONPath=".metadata.name"
// +Metrics:labelsFromPath:name=namespace,JSONPath=".metadata.namespace"
// +Metrics:labelsFromPath:name=uid,JSONPath=".metadata.uid"
type Cluster struct { ... }
// + Metrics:gauge:name=<string>,help=<string>,nilIsZero=<bool>,JSONPath:<string>,labelFromPath={name: <string>,JSONPath: <string>} on field
If applied to a field, it creates a metric of type gauge for the field the markers apply to.
name=<string>
the name of the metrichelp=<string>
the help of the metricnilIsZero=<bool>
optional; force the metric to count nil values as zeroJSONPath:<string>
optional; in case the field is a complex type, this allows creating metrics for nested fields given their pathlabelFromPath={name: <string>, JSONPath: <string>}
optional; can be repeated many times; allows adding labels whose values are read from the given path; JSONPath can be omitted, and in this case the label will be set to the value of the field the marker applies to [@chrischdi I have added this for consistency with other metrics types, not sure if it makes sense]
eg.
// +Metrics:gauge:name="spec_paused",help="Whether the cluster is paused and any of its resources will not be processed by the controllers.",nilIsZero=true
Paused bool `json:"paused,omitempty"`
// +Metrics:gauge:name="created",JSONPath='.creationTimestamp",help="Unix creation timestamp."
metav1.ObjectMeta `json:"metadata,omitempty"`
// + Metrics:stateset:name=<string>,help=<string>,labelName=<string>,labelName=<string>,list=[]<string>,JSONPath:<string>,labelFromPath={name: <string>,JSONPath: <string>} on field
Creates a metric of type stateset for the field the markers apply to.
name=<string>
the name of the metrichelp=<string>
the help of the metriclabelName=<string>
list=[]<string>
the list of values for the statesetJSONPath:<string>
optional; in case the field is a complex type, this allows creating metrics for nested fields given their pathlabelFromPath={name: <string>, JSONPath: <string>}
optional; can be repeated many times; allows adding labels whose values are read from the given path; JSONPath can be omitted, and in this case the label will be set to the value of the field the marker applies to
eg.
// +Metrics:stateset:name="status_phase",help="The clusters current phase.",labelName=phase,list{"Pending", "Provisioning", "Provisioned", "Deleting", "Failed", "Unknown"}
Phase string `json:"phase,omitempty"`
// +Metrics:stateset:name="status_condition",help="The condition of a cluster.",labelName="status",JSONPath: ".status",list{"True", "False", "Unknown"},labelFromPath={name: type,JSONPath: ".type"}
Conditions Conditions `json:"conditions,omitempty"`
// + Metrics:info:name=<string>,help=<string>,JSONPath:<string>,labelFromPath={name: <string>,JSONPath: <string>} on field or struct
Creates metric of type info for the field the markers apply to or for the struct the markers applies to
name=<string>
the name of the metrichelp=<string>
the help of the metricJSONPath:<string>
optional when the marker is applied to a field, required when the marker applies to a struct; this allows creating metrics for nested fields given their pathlabelFromPath={name: <string>, JSONPath: <string>}
optional; can be repeated many times; allows adding labels whose values are read from the given path; JSONPath can be omitted, and in this case the label will be set to the value of the field the marker applies to
eg.
// +Metrics:info:name="info",help="Information about a cluster.",labelFromPath={name: topology_version,JSONPath: ".spec.topology.version"},labelFromPath={name: topology_class,JSONPath: ".spec.topology.class"}
// Cluster is the Schema for the clusters API.
type Cluster struct {
// +Metrics:info:name="annotation_paused",JSONPath='.annotations.['cluster\\.x-k8s.io/paused']",help="Whether the cluster is paused and any of its resources will not be processed by the controllers.",labelFromPath={name: paused_value}
metav1.ObjectMeta `json:"metadata,omitempty"`
}
Sounds good to me.
I didn't do a very detailed review, but in general it looks good and we can discuss some minor details on PRs when necessary.
One small addition:
Instead of labelFromPath={name: <string>, JSONPath: <string>},labelFromPath={name: <string>, JSONPath: <string>}
We could define it as: labelsFromPath={<name>: <JSONPath>}
(while <name>
and <JSONPath>
would both be strings.
Following example:
labelFromPath={name: "foo", JSONPath: ".foo"},labelFromPath={name: "bar", JSONPath: ".bar"}
would turn to:
labelsFromPath={"foo": ".foo", "bar": ".bar"}
This issue has not been updated in over 1 year, and should be re-triaged.
You can:
/triage accepted
(org members only)/close
For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/
/remove-triage accepted
/triage accepted
Getting closer to either getting it merged in kube-state-metrics or otherwise I think we will add it to CAPI..
I agree, I basically asked this question at https://github.com/kubernetes/kube-state-metrics/pull/2014
Yup, my comment was based on what you wrote there :D
/priority important-longterm
User Story
As a developer, I would like the kube-state-metrics configuration to be automatically generated from the API
Detailed Description
We currently introduced the first kube-state-metrics configuration for Cluster API objects, but in the current state this file must be updated every time there is an API change.
We would like to automate the generation of the kube-state-metrics configuration, so changes in the API will be automatically reflected.
Anything else you would like to add:
A similar tool can be really usefull to allow impementation of kube-state-metrics in providers; ideally, this tool should be hosted in kube-state-metrics project, but IMO it is ok to host this temporarily in CAPI if this speed up the process.
/kind feature